From c80b183aa5eb78b68e41edae4fccf8a8d831ded1 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 23 Dec 2021 12:01:11 +0000 Subject: [PATCH] sql: Squash liberty migrations Make the following changes to the new "initial" migration. - Add 'inherited' column to primary key constraint for 'assignment' table (073) - Add 'is_domain' column to 'project' table (074) - Add 'config_register' table (075) Schemas can be compared using the techniques discussed at [1]. A sample command for SQLite: # with change before this python manage.py version_control \ --database 'sqlite:///before.db' --repository . --version 066 python manage.py upgrade \ --database 'sqlite:///before.db' --repository . --version 075 # with this change python manage.py version_control \ --database 'sqlite:///after.db' --repository . --version 074 python manage.py upgrade \ --database 'sqlite:///after.db' --repository . --version 075 [1] https://that.guru/blog/comparing-nova-db-migrations/ Change-Id: I161454bb5d112abf32e8ae363955b36a7529889d Signed-off-by: Stephen Finucane --- .../migrate_repo/versions/068_placeholder.py | 18 --- .../migrate_repo/versions/069_placeholder.py | 18 --- .../migrate_repo/versions/070_placeholder.py | 18 --- .../migrate_repo/versions/071_placeholder.py | 18 --- .../migrate_repo/versions/072_placeholder.py | 18 --- .../073_insert_assignment_inherited_pk.py | 113 ---------------- .../versions/074_add_is_domain_project.py | 27 ---- .../075_confirm_config_registration.py | 29 ---- .../versions/{067_kilo.py => 075_liberty.py} | 25 +++- keystone/tests/unit/test_sql_upgrade.py | 124 +----------------- 10 files changed, 29 insertions(+), 379 deletions(-) delete mode 100644 keystone/common/sql/migrate_repo/versions/068_placeholder.py delete mode 100644 keystone/common/sql/migrate_repo/versions/069_placeholder.py delete mode 100644 keystone/common/sql/migrate_repo/versions/070_placeholder.py delete mode 100644 keystone/common/sql/migrate_repo/versions/071_placeholder.py delete mode 100644 keystone/common/sql/migrate_repo/versions/072_placeholder.py delete mode 100644 keystone/common/sql/migrate_repo/versions/073_insert_assignment_inherited_pk.py delete mode 100644 keystone/common/sql/migrate_repo/versions/074_add_is_domain_project.py delete mode 100644 keystone/common/sql/migrate_repo/versions/075_confirm_config_registration.py rename keystone/common/sql/migrate_repo/versions/{067_kilo.py => 075_liberty.py} (95%) diff --git a/keystone/common/sql/migrate_repo/versions/068_placeholder.py b/keystone/common/sql/migrate_repo/versions/068_placeholder.py deleted file mode 100644 index 111df9d4a7..0000000000 --- a/keystone/common/sql/migrate_repo/versions/068_placeholder.py +++ /dev/null @@ -1,18 +0,0 @@ -# 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 Kilo backports. Do not use this number for new -# Liberty work. New Liberty work starts after all the placeholders. - - -def upgrade(migrate_engine): - pass diff --git a/keystone/common/sql/migrate_repo/versions/069_placeholder.py b/keystone/common/sql/migrate_repo/versions/069_placeholder.py deleted file mode 100644 index 111df9d4a7..0000000000 --- a/keystone/common/sql/migrate_repo/versions/069_placeholder.py +++ /dev/null @@ -1,18 +0,0 @@ -# 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 Kilo backports. Do not use this number for new -# Liberty work. New Liberty work starts after all the placeholders. - - -def upgrade(migrate_engine): - pass diff --git a/keystone/common/sql/migrate_repo/versions/070_placeholder.py b/keystone/common/sql/migrate_repo/versions/070_placeholder.py deleted file mode 100644 index 111df9d4a7..0000000000 --- a/keystone/common/sql/migrate_repo/versions/070_placeholder.py +++ /dev/null @@ -1,18 +0,0 @@ -# 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 Kilo backports. Do not use this number for new -# Liberty work. New Liberty work starts after all the placeholders. - - -def upgrade(migrate_engine): - pass diff --git a/keystone/common/sql/migrate_repo/versions/071_placeholder.py b/keystone/common/sql/migrate_repo/versions/071_placeholder.py deleted file mode 100644 index 111df9d4a7..0000000000 --- a/keystone/common/sql/migrate_repo/versions/071_placeholder.py +++ /dev/null @@ -1,18 +0,0 @@ -# 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 Kilo backports. Do not use this number for new -# Liberty work. New Liberty work starts after all the placeholders. - - -def upgrade(migrate_engine): - pass diff --git a/keystone/common/sql/migrate_repo/versions/072_placeholder.py b/keystone/common/sql/migrate_repo/versions/072_placeholder.py deleted file mode 100644 index 111df9d4a7..0000000000 --- a/keystone/common/sql/migrate_repo/versions/072_placeholder.py +++ /dev/null @@ -1,18 +0,0 @@ -# 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 Kilo backports. Do not use this number for new -# Liberty work. New Liberty work starts after all the placeholders. - - -def upgrade(migrate_engine): - pass diff --git a/keystone/common/sql/migrate_repo/versions/073_insert_assignment_inherited_pk.py b/keystone/common/sql/migrate_repo/versions/073_insert_assignment_inherited_pk.py deleted file mode 100644 index dc0905b5f9..0000000000 --- a/keystone/common/sql/migrate_repo/versions/073_insert_assignment_inherited_pk.py +++ /dev/null @@ -1,113 +0,0 @@ -# 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 migrate -import sqlalchemy as sql -from sqlalchemy.orm import sessionmaker - -from keystone.assignment.backends import sql as assignment_sql - - -def upgrade(migrate_engine): - """Insert inherited column to assignment table PK constraints. - - For non-SQLite databases, it changes the constraint in the existing table. - - For SQLite, since changing constraints is not supported, it recreates the - assignment table with the new PK constraint and migrates the existing data. - - """ - ASSIGNMENT_TABLE_NAME = 'assignment' - - metadata = sql.MetaData() - metadata.bind = migrate_engine - - # Retrieve the existing assignment table - assignment_table = sql.Table(ASSIGNMENT_TABLE_NAME, metadata, - autoload=True) - - if migrate_engine.name == 'sqlite': - ACTOR_ID_INDEX_NAME = 'ix_actor_id' - TMP_ASSIGNMENT_TABLE_NAME = 'tmp_assignment' - - # Define the new assignment table with a temporary name - new_assignment_table = sql.Table( - TMP_ASSIGNMENT_TABLE_NAME, metadata, - sql.Column('type', sql.Enum( - assignment_sql.AssignmentType.USER_PROJECT, - assignment_sql.AssignmentType.GROUP_PROJECT, - assignment_sql.AssignmentType.USER_DOMAIN, - assignment_sql.AssignmentType.GROUP_DOMAIN, - name='type'), - nullable=False), - sql.Column('actor_id', sql.String(64), nullable=False), - sql.Column('target_id', sql.String(64), nullable=False), - sql.Column('role_id', sql.String(64), sql.ForeignKey('role.id'), - nullable=False), - sql.Column('inherited', sql.Boolean, default=False, - nullable=False), - sql.PrimaryKeyConstraint('type', 'actor_id', 'target_id', - 'role_id', 'inherited'), - mysql_engine='InnoDB', - mysql_charset='utf8') - - # Create the new assignment table - new_assignment_table.create(migrate_engine, checkfirst=True) - - # Change the index from the existing assignment table to the new one - sql.Index(ACTOR_ID_INDEX_NAME, assignment_table.c.actor_id).drop() - sql.Index(ACTOR_ID_INDEX_NAME, - new_assignment_table.c.actor_id).create() - - # Instantiate session - maker = sessionmaker(bind=migrate_engine) - session = maker() - - # Migrate existing data - insert = new_assignment_table.insert().from_select( - assignment_table.c, select=session.query(assignment_table)) - session.execute(insert) - session.commit() - - # Drop the existing assignment table, in favor of the new one - assignment_table.deregister() - assignment_table.drop() - - # Finally, rename the new table to the original assignment table name - new_assignment_table.rename(ASSIGNMENT_TABLE_NAME) - elif migrate_engine.name == 'ibm_db_sa': - # Recreate the existing constraint, marking the inherited column as PK - # for DB2. - - # This is a workaround to the general case in the else statement below. - # Due to a bug in the DB2 sqlalchemy dialect, Column.alter() actually - # creates a primary key over only the "inherited" column. This is wrong - # because the primary key for the table actually covers other columns - # too, not just the "inherited" column. Since the primary key already - # exists for the table after the Column.alter() call, it causes the - # next line to fail with an error that the primary key already exists. - - # The workaround here skips doing the Column.alter(). This causes a - # warning message since the metadata is out of sync. We can remove this - # workaround once the DB2 sqlalchemy dialect is fixed. - # DB2 Issue: https://code.google.com/p/ibm-db/issues/detail?id=173 - - migrate.PrimaryKeyConstraint(table=assignment_table).drop() - migrate.PrimaryKeyConstraint( - assignment_table.c.type, assignment_table.c.actor_id, - assignment_table.c.target_id, assignment_table.c.role_id, - assignment_table.c.inherited).create() - else: - # Recreate the existing constraint, marking the inherited column as PK - migrate.PrimaryKeyConstraint(table=assignment_table).drop() - assignment_table.c.inherited.alter(primary_key=True) - migrate.PrimaryKeyConstraint(table=assignment_table).create() diff --git a/keystone/common/sql/migrate_repo/versions/074_add_is_domain_project.py b/keystone/common/sql/migrate_repo/versions/074_add_is_domain_project.py deleted file mode 100644 index dcb89b07c7..0000000000 --- a/keystone/common/sql/migrate_repo/versions/074_add_is_domain_project.py +++ /dev/null @@ -1,27 +0,0 @@ -# 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 - - -_PROJECT_TABLE_NAME = 'project' -_IS_DOMAIN_COLUMN_NAME = 'is_domain' - - -def upgrade(migrate_engine): - meta = sql.MetaData() - meta.bind = migrate_engine - - project_table = sql.Table(_PROJECT_TABLE_NAME, meta, autoload=True) - is_domain = sql.Column(_IS_DOMAIN_COLUMN_NAME, sql.Boolean, nullable=False, - server_default='0', default=False) - project_table.create_column(is_domain) diff --git a/keystone/common/sql/migrate_repo/versions/075_confirm_config_registration.py b/keystone/common/sql/migrate_repo/versions/075_confirm_config_registration.py deleted file mode 100644 index 576842c69f..0000000000 --- a/keystone/common/sql/migrate_repo/versions/075_confirm_config_registration.py +++ /dev/null @@ -1,29 +0,0 @@ -# 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 - -REGISTRATION_TABLE = 'config_register' - - -def upgrade(migrate_engine): - meta = sql.MetaData() - meta.bind = migrate_engine - - registration_table = sql.Table( - REGISTRATION_TABLE, - meta, - sql.Column('type', sql.String(64), primary_key=True), - sql.Column('domain_id', sql.String(64), nullable=False), - mysql_engine='InnoDB', - mysql_charset='utf8') - registration_table.create(migrate_engine, checkfirst=True) diff --git a/keystone/common/sql/migrate_repo/versions/067_kilo.py b/keystone/common/sql/migrate_repo/versions/075_liberty.py similarity index 95% rename from keystone/common/sql/migrate_repo/versions/067_kilo.py rename to keystone/common/sql/migrate_repo/versions/075_liberty.py index 09d2342143..8673b7f739 100644 --- a/keystone/common/sql/migrate_repo/versions/067_kilo.py +++ b/keystone/common/sql/migrate_repo/versions/075_liberty.py @@ -116,6 +116,22 @@ def upgrade(migrate_engine): sql.Column('enabled', sql.Boolean), sql.Column('domain_id', sql.String(length=64), nullable=False), sql.Column('parent_id', sql.String(64), nullable=True), + sql.Column( + 'is_domain', + sql.Boolean, + nullable=False, + server_default='0', + default=False, + ), + mysql_engine='InnoDB', + mysql_charset='utf8', + ) + + config_register = sql.Table( + 'config_register', + meta, + sql.Column('type', sql.String(64), primary_key=True), + sql.Column('domain_id', sql.String(64), nullable=False), mysql_engine='InnoDB', mysql_charset='utf8', ) @@ -241,7 +257,13 @@ def upgrade(migrate_engine): sql.Column('target_id', sql.String(64), nullable=False), sql.Column('role_id', sql.String(64), nullable=False), sql.Column('inherited', sql.Boolean, default=False, nullable=False), - sql.PrimaryKeyConstraint('type', 'actor_id', 'target_id', 'role_id'), + sql.PrimaryKeyConstraint( + 'type', + 'actor_id', + 'target_id', + 'role_id', + 'inherited', + ), mysql_engine='InnoDB', mysql_charset='utf8', ) @@ -307,6 +329,7 @@ def upgrade(migrate_engine): id_mapping, whitelisted_config, sensitive_config, + config_register, ] for table in tables: diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py index 8d22ace541..296639b2ac 100644 --- a/keystone/tests/unit/test_sql_upgrade.py +++ b/keystone/tests/unit/test_sql_upgrade.py @@ -75,6 +75,9 @@ from keystone.tests.unit.ksfixtures import database # is done to mirror the expected structure of the DB in the format of # { : [, , ...], ... } INITIAL_TABLE_STRUCTURE = { + 'config_register': [ + 'type', 'domain_id', + ], 'credential': [ 'id', 'user_id', 'project_id', 'blob', 'type', 'extra', ], @@ -93,7 +96,7 @@ INITIAL_TABLE_STRUCTURE = { ], 'project': [ 'id', 'name', 'extra', 'description', 'enabled', 'domain_id', - 'parent_id', + 'parent_id', 'is_domain', ], 'role': [ 'id', 'name', 'extra', @@ -364,109 +367,6 @@ class SqlLegacyRepoUpgradeTests(SqlMigrateBase): for table in INITIAL_TABLE_STRUCTURE: self.assertTableColumns(table, INITIAL_TABLE_STRUCTURE[table]) - def test_kilo_squash(self): - self.upgrade(67) - - # In 053 the size of ID and parent region ID columns were changed - table = sqlalchemy.Table('region', self.metadata, autoload=True) - self.assertEqual(255, table.c.id.type.length) - self.assertEqual(255, table.c.parent_region_id.type.length) - table = sqlalchemy.Table('endpoint', self.metadata, autoload=True) - self.assertEqual(255, table.c.region_id.type.length) - - # In 054 an index was created for the actor_id of the assignment table - table = sqlalchemy.Table('assignment', self.metadata, autoload=True) - index_data = [(idx.name, list(idx.columns.keys())) - for idx in table.indexes] - self.assertIn(('ix_actor_id', ['actor_id']), index_data) - - # In 055 indexes were created for user and trust IDs in the token table - table = sqlalchemy.Table('token', self.metadata, autoload=True) - index_data = [(idx.name, list(idx.columns.keys())) - for idx in table.indexes] - self.assertIn(('ix_token_user_id', ['user_id']), index_data) - self.assertIn(('ix_token_trust_id', ['trust_id']), index_data) - - # In 062 the role ID foreign key was removed from the assignment table - if self.engine.name == "mysql": - self.assertFalse(self.does_fk_exist('assignment', 'role_id')) - - # In 064 the domain ID FK was removed from the group and user tables - if self.engine.name != 'sqlite': - # sqlite does not support FK deletions (or enforcement) - self.assertFalse(self.does_fk_exist('group', 'domain_id')) - self.assertFalse(self.does_fk_exist('user', 'domain_id')) - - # In 067 the role ID index was removed from the assignment table - if self.engine.name == "mysql": - self.assertFalse(self.does_index_exist('assignment', - 'assignment_role_id_fkey')) - - def test_insert_assignment_inherited_pk(self): - ASSIGNMENT_TABLE_NAME = 'assignment' - INHERITED_COLUMN_NAME = 'inherited' - ROLE_TABLE_NAME = 'role' - - self.upgrade(72) - - # Check that the 'inherited' column is not part of the PK - self.assertFalse(self.does_pk_exist(ASSIGNMENT_TABLE_NAME, - INHERITED_COLUMN_NAME)) - - session = self.sessionmaker() - - role = {'id': uuid.uuid4().hex, - 'name': uuid.uuid4().hex} - self.insert_dict(session, ROLE_TABLE_NAME, role) - - # Create both inherited and noninherited role assignments - inherited = {'type': 'UserProject', - 'actor_id': uuid.uuid4().hex, - 'target_id': uuid.uuid4().hex, - 'role_id': role['id'], - 'inherited': True} - - noninherited = inherited.copy() - noninherited['inherited'] = False - - # Create another inherited role assignment as a spoiler - spoiler = inherited.copy() - spoiler['actor_id'] = uuid.uuid4().hex - - self.insert_dict(session, ASSIGNMENT_TABLE_NAME, inherited) - self.insert_dict(session, ASSIGNMENT_TABLE_NAME, spoiler) - - # Since 'inherited' is not part of the PK, we can't insert noninherited - self.assertRaises(db_exception.DBDuplicateEntry, - self.insert_dict, - session, - ASSIGNMENT_TABLE_NAME, - noninherited) - - session.close() - - self.upgrade(73) - - session = self.sessionmaker() - - # Check that the 'inherited' column is now part of the PK - self.assertTrue(self.does_pk_exist(ASSIGNMENT_TABLE_NAME, - INHERITED_COLUMN_NAME)) - - # The noninherited role assignment can now be inserted - self.insert_dict(session, ASSIGNMENT_TABLE_NAME, noninherited) - - assignment_table = sqlalchemy.Table(ASSIGNMENT_TABLE_NAME, - self.metadata, - autoload=True) - - assignments = session.query(assignment_table).all() - for assignment in (inherited, spoiler, noninherited): - self.assertIn((assignment['type'], assignment['actor_id'], - assignment['target_id'], assignment['role_id'], - assignment['inherited']), - assignments) - def test_endpoint_policy_upgrade(self): self.assertTableDoesNotExist('policy_association') self.upgrade(81) @@ -616,13 +516,6 @@ class SqlLegacyRepoUpgradeTests(SqlMigrateBase): # that 084 did not create the table. self.assertTableDoesNotExist('revocation_event') - def test_project_is_domain_upgrade(self): - self.upgrade(74) - self.assertTableColumns('project', - ['id', 'name', 'extra', 'description', - 'enabled', 'domain_id', 'parent_id', - 'is_domain']) - def test_implied_roles_upgrade(self): self.upgrade(87) self.assertTableColumns('implied_role', @@ -630,13 +523,6 @@ class SqlLegacyRepoUpgradeTests(SqlMigrateBase): self.assertTrue(self.does_fk_exist('implied_role', 'prior_role_id')) self.assertTrue(self.does_fk_exist('implied_role', 'implied_role_id')) - def test_add_config_registration(self): - config_registration = 'config_register' - self.upgrade(74) - self.assertTableDoesNotExist(config_registration) - self.upgrade(75) - self.assertTableColumns(config_registration, ['type', 'domain_id']) - def test_endpoint_filter_upgrade(self): def assert_tables_columns_exist(): self.assertTableColumns('project_endpoint', @@ -1713,7 +1599,7 @@ class MigrationValidation(SqlMigrateBase, unit.TestCase): def test_running_db_sync_expand_without_up_to_date_legacy_fails(self): # Set Legacy version and then test that running expand fails if Legacy # isn't at the latest version. - self.upgrade(67) + self.upgrade(75) latest_version = self.repos[EXPAND_REPO].max_version self.assertRaises( db_exception.DBMigrationError,