Update local_id limit to 255 characters

This avoids the "String length exceeded." error, when using LDAP
domain specific backend in case the user uses a user id
attribute, which can exceed the previous constraint of 64 chars.

Change-Id: I923a2a2a5e79c8f265ff436e96258288dddb867b
Closes-Bug: #1929066
Resolves: rhbz#1959345

doc/source/admin/domain-specific-config.inc

@@ -146,6 +146,12 @@ then the same public ID will be created. This is useful if you are running
 multiple keystones and want to ensure the same ID would be generated whichever
 server you hit.
 
+.. NOTE::
+
+    In case of the LDAP backend, the names of users and groups are not hashed.
+    As a result, these are length limited to 255 characters. Longer names
+    will result in an error.
+
 While keystone will dynamically maintain the identity mapping, including
 removing entries when entities are deleted via the keystone, for those entities
 in backends that are managed outside of keystone (e.g. a read-only LDAP),

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

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

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))

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,

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)

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):

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

releasenotes/notes/bug-1929066-6e741c9182620a37.yaml

@@ -0,0 +1,7 @@
+---
+upgrade:
+  - |
+    [`bug 1929066 <https://bugs.launchpad.net/keystone/+bug/1929066>`_]
+    Increase the length of the `local_id` column in the `id_mapping` table
+    to accommodate LDAP group names that result in names greater than
+    64 characters.