Merge "Update local_id limit to 255 characters"

7 files changed, 107 insertions, 3 deletions

diff --git a/keystone/common/sql/contract_repo/versions/079_contract_update_local_id_limit.py b/keystone/common/sql/contract_repo/versions/079_contract_update_local_id_limit.py
new file mode 100644
index 000000000..2b09cbc99
--- /dev/null
+++ b/keystone/common/sql/contract_repo/versions/079_contract_update_local_id_limit.py
@@ -0,0 +1,18 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+# This is a placeholder for Ussuri backports. Do not use this number for new
+# Victoria work. New Victoria work starts after all the placeholders.
+
+
+def upgrade(migrate_engine):
+    pass
diff --git a/keystone/common/sql/data_migration_repo/versions/079_migrate_update_local_id_limit.py b/keystone/common/sql/data_migration_repo/versions/079_migrate_update_local_id_limit.py
new file mode 100644
index 000000000..2b09cbc99
--- /dev/null
+++ b/keystone/common/sql/data_migration_repo/versions/079_migrate_update_local_id_limit.py
@@ -0,0 +1,18 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+# This is a placeholder for Ussuri backports. Do not use this number for new
+# Victoria work. New Victoria work starts after all the placeholders.
+
+
+def upgrade(migrate_engine):
+    pass
diff --git a/keystone/common/sql/expand_repo/versions/079_expand_update_local_id_limit.py b/keystone/common/sql/expand_repo/versions/079_expand_update_local_id_limit.py
new file mode 100644
index 000000000..20db83851
--- /dev/null
+++ b/keystone/common/sql/expand_repo/versions/079_expand_update_local_id_limit.py
@@ -0,0 +1,24 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import sqlalchemy as sql
+
+
+def upgrade(migrate_engine):
+
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+
+    id_mapping_table = sql.Table(
+        'id_mapping', meta, autoload=True
+    )
+    id_mapping_table.c.local_id.alter(type=sql.String(255))
diff --git a/keystone/identity/mapping_backends/sql.py b/keystone/identity/mapping_backends/sql.py
index 676d14492..6fadd6a0b 100644
--- a/keystone/identity/mapping_backends/sql.py
+++ b/keystone/identity/mapping_backends/sql.py
@@ -21,7 +21,7 @@ class IDMapping(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'id_mapping'
     public_id = sql.Column(sql.String(64), primary_key=True)
     domain_id = sql.Column(sql.String(64), nullable=False)
-    local_id = sql.Column(sql.String(64), nullable=False)
+    local_id = sql.Column(sql.String(255), nullable=False)
     # NOTE(henry-nash): Postgres requires a name to be defined for an Enum
     entity_type = sql.Column(
         sql.Enum(identity_mapping.EntityType.USER,
diff --git a/keystone/tests/unit/test_backend_id_mapping_sql.py b/keystone/tests/unit/test_backend_id_mapping_sql.py
index baee34e99..7729d53c6 100644
--- a/keystone/tests/unit/test_backend_id_mapping_sql.py
+++ b/keystone/tests/unit/test_backend_id_mapping_sql.py
@@ -33,7 +33,7 @@ class SqlIDMappingTable(test_backend_sql.SqlModels):
     def test_id_mapping(self):
         cols = (('public_id', sql.String, 64),
                 ('domain_id', sql.String, 64),
-                ('local_id', sql.String, 64),
+                ('local_id', sql.String, 255),
                 ('entity_type', sql.Enum, None))
         self.assertExpectedSchema('id_mapping', cols)
 
@@ -169,6 +169,26 @@ class SqlIDMapping(test_backend_sql.SqlTests):
         self.assertEqual(
             public_id, PROVIDERS.id_mapping_api.get_public_id(local_entity))
 
+    def test_id_mapping_handles_ids_greater_than_64_characters(self):
+        initial_mappings = len(mapping_sql.list_id_mappings())
+        local_id = 'Aa' * 100
+        local_entity = {'domain_id': self.domainA['id'],
+                        'local_id': local_id,
+                        'entity_type': mapping.EntityType.GROUP}
+
+        # Check no mappings for the new local entity
+        self.assertIsNone(PROVIDERS.id_mapping_api.get_public_id(local_entity))
+
+        # Create the new mapping and then read it back
+        public_id = PROVIDERS.id_mapping_api.create_id_mapping(local_entity)
+        self.assertThat(mapping_sql.list_id_mappings(),
+                        matchers.HasLength(initial_mappings + 1))
+        self.assertEqual(
+            public_id, PROVIDERS.id_mapping_api.get_public_id(local_entity))
+        self.assertEqual(
+            local_id,
+            PROVIDERS.id_mapping_api.get_id_mapping(public_id)['local_id'])
+
     def test_delete_public_id_is_silent(self):
         # Test that deleting an invalid public key is silent
         PROVIDERS.id_mapping_api.delete_id_mapping(uuid.uuid4().hex)
diff --git a/keystone/tests/unit/test_sql_banned_operations.py b/keystone/tests/unit/test_sql_banned_operations.py
index 9916657da..220714075 100644
--- a/keystone/tests/unit/test_sql_banned_operations.py
+++ b/keystone/tests/unit/test_sql_banned_operations.py
@@ -263,7 +263,12 @@ class TestKeystoneExpandSchemaMigrations(
         # timestamp to datetime and updates the initial value in the contract
         # phase. Adding an exception here to pass expand banned tests,
         # otherwise fails.
-        4
+        4,
+
+        # Migration 79 changes a varchar column length, doesn't
+        # convert the data within that column/table and doesn't rebuild
+        # indexes.
+        79
     ]
 
     def setUp(self):
diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py
index 50c28707a..502032f66 100644
--- a/keystone/tests/unit/test_sql_upgrade.py
+++ b/keystone/tests/unit/test_sql_upgrade.py
@@ -3499,6 +3499,25 @@ class FullMigration(SqlMigrateBase, unit.TestCase):
             ['id', 'domain_id', 'enabled', 'description',
              'authorization_ttl'])
 
+    def test_migration_079_expand_update_local_id_limit(self):
+        self.expand(78)
+        self.migrate(78)
+        self.contract(78)
+
+        id_mapping_table = sqlalchemy.Table('id_mapping',
+                                            self.metadata, autoload=True)
+        # assert local_id column is a string of 64 characters (before)
+        self.assertEqual('VARCHAR(64)', str(id_mapping_table.c.local_id.type))
+
+        self.expand(79)
+        self.migrate(79)
+        self.contract(79)
+
+        id_mapping_table = sqlalchemy.Table('id_mapping',
+                                            self.metadata, autoload=True)
+        # assert local_id column is a string of 255 characters (after)
+        self.assertEqual('VARCHAR(255)', str(id_mapping_table.c.local_id.type))
+
 
 class MySQLOpportunisticFullMigration(FullMigration):
     FIXTURE = db_fixtures.MySQLOpportunisticFixture