openstack/keystone
Code Issues Proposed changes

sql: Squash liberty migrations

Browse Source 
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 <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane
2021-12-23 12:01:11 +00:00
parent e92bf89f71
commit c80b183aa5
10 changed files with 29 additions and 379 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
keystone/common/sql/migrate_repo/versions/068_placeholder.py
View File

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

18
keystone/common/sql/migrate_repo/versions/069_placeholder.py
View File

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

18
keystone/common/sql/migrate_repo/versions/070_placeholder.py
View File

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

18
keystone/common/sql/migrate_repo/versions/071_placeholder.py
View File

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

18
keystone/common/sql/migrate_repo/versions/072_placeholder.py
View File

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

113
keystone/common/sql/migrate_repo/versions/073_insert_assignment_inherited_pk.py
View File

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

27
keystone/common/sql/migrate_repo/versions/074_add_is_domain_project.py
View File

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

29
keystone/common/sql/migrate_repo/versions/075_confirm_config_registration.py
View File

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

25
keystone/common/sql/migrate_repo/versions/067_kilo.py → keystone/common/sql/migrate_repo/versions/075_liberty.py
View File

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

124
keystone/tests/unit/test_sql_upgrade.py
View File

@@ -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
# { <DB_TABLE_NAME>: [<COLUMN>, <COLUMN>, ...], ... }
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,