openstack
/
keystone
Code Issues Proposed changes

Merge "sql: Fix incorrect constraints"

This commit is contained in:
Zuul 2023-07-04 10:30:11 +00:00 committed by Gerrit Code Review
parent 8551ac4cb2 56c47d0a39
commit 7d169870fe
7 changed files with 159 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
keystone/common/sql/migrations/env.py
View File

@ -50,31 +50,8 @@ def include_object(object, name, type_, reflected, compare_to):
)
BORKED_UNIQUE_CONSTRAINTS = (
# removed constraints
# duplicate constraints on primary key columns
('project_tag', ['project_id', 'name']),
(
'trust',
[
'trustor_user_id',
'trustee_user_id',
'project_id',
'impersonation',
'expires_at',
],
),
# added constraints
('access_rule', ['external_id']),
(
'trust',
[
'trustor_user_id',
'trustee_user_id',
'project_id',
'impersonation',
'expires_at',
'expires_at_int',
],
),
)
BORKED_FK_CONSTRAINTS = (
@ -122,7 +99,7 @@ def include_object(object, name, type_, reflected, compare_to):
# affected item. However, we only want to skip the actual things we know
# about untl we have enough time to fix them. These issues are listed in
# keystone.tests.unit.common.sql.test_upgrades.KeystoneModelsMigrationsSync
# However, this isn't a bug issues since the test is more specific and will
# However, this isn't an issue since the test is more specific and will
# catch other issues and anyone making changes to the columns and hoping to
# autogenerate them would need to fix the latent issue first anyway.
if type_ == 'column':

2
keystone/common/sql/migrations/versions/CONTRACT_HEAD
View File

@ -1 +1 @@
e25ffa003242
99de3849d860

2
keystone/common/sql/migrations/versions/EXPAND_HEAD
View File

@ -1 +1 @@
29e87d24a316
b4f8b3f584e0

36
keystone/common/sql/migrations/versions/bobcat/contract/99de3849d860_fix_incorrect_constraints.py Normal file
View File

@ -0,0 +1,36 @@
# 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.
"""Fix incorrect constraints.
Revision ID: 99de3849d860
Revises: e25ffa003242
Create Date: 2022-08-02 12:23:25.525035
"""
from alembic import op
# revision identifiers, used by Alembic.
revision = '99de3849d860'
down_revision = 'e25ffa003242'
branch_labels = None
depends_on = None
def upgrade():
with op.batch_alter_table('access_rule', schema=None) as batch_op:
batch_op.drop_constraint('access_rule_external_id_key', type_='unique')
with op.batch_alter_table('trust', schema=None) as batch_op:
batch_op.drop_constraint(
'duplicate_trust_constraint_expanded', type_='unique'
)

40
keystone/common/sql/migrations/versions/bobcat/expand/b4f8b3f584e0_fix_incorrect_constraints.py Normal file
View File

@ -0,0 +1,40 @@
# 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.
"""Fix incorrect constraints.
Revision ID: b4f8b3f584e0
Revises: 29e87d24a316
Create Date: 2022-08-02 12:23:25.520570
"""
from alembic import op
# revision identifiers, used by Alembic.
revision = 'b4f8b3f584e0'
down_revision = '29e87d24a316'
branch_labels = None
depends_on = None
def upgrade():
with op.batch_alter_table('trust', schema=None) as batch_op:
batch_op.create_unique_constraint(
'duplicate_trust_constraint',
[
'trustor_user_id',
'trustee_user_id',
'project_id',
'impersonation',
'expires_at',
],
)

35
keystone/tests/unit/common/sql/test_upgrades.py
View File

@ -147,44 +147,15 @@ class KeystoneModelsMigrationsSync(test_migrations.ModelsMigrationsSync):
):
continue # skip
else:
# FIXME(stephenfin): These unique constraint are unecessary
# since the columns are already included in a primary key
# constraint. The constraints are ignored in PostgreSQL.
if element[0] == 'add_constraint':
if (
element[1].table.name,
[x.name for x in element[1].columns],
) in (
('project_tag', ['project_id', 'name']),
(
'trust',
[
'trustor_user_id',
'trustee_user_id',
'project_id',
'impersonation',
'expires_at',
],
),
):
continue # skip
# FIXME(stephenfin): These have a different name on PostgreSQL.
# Resolve by renaming the constraint on the models.
if element[0] == 'remove_constraint':
if (
element[1].table.name,
[x.name for x in element[1].columns],
) in (
('access_rule', ['external_id']),
(
'trust',
[
'trustor_user_id',
'trustee_user_id',
'project_id',
'impersonation',
'expires_at',
'expires_at_int',
],
),
):
continue # skip

76
keystone/tests/unit/test_sql_banned_operations.py
View File

@ -18,6 +18,7 @@ import fixtures
from oslo_db.sqlalchemy import enginefacade
from oslo_db.sqlalchemy import test_fixtures
from oslo_log import log as logging
import sqlalchemy
from keystone.common import sql
from keystone.common.sql import upgrades
@ -155,6 +156,12 @@ class KeystoneMigrationsWalk(
for branch_label in revision.branch_labels:
banned_ops.extend(self.BANNED_OPS[branch_label])
# SQLite migrations are running in batch mode, which mean we recreate a
# table in all migrations. As such, we can't really blacklist things so
# don't even try.
if self.FIXTURE.DRIVER == 'sqlite':
banned_ops = []
with BannedDBSchemaOperations(banned_ops, version):
alembic_api.upgrade(self.config, version)
@ -178,6 +185,75 @@ class KeystoneMigrationsWalk(
"""This is a no-op migration."""
pass
# 2023.2 bobcat
_99de3849d860_removed_constraints = {
'access_rule': 'access_rule_external_id_key',
'trust': 'duplicate_trust_constraint_expanded',
}
def _pre_upgrade_99de3849d860(self, connection):
inspector = sqlalchemy.inspect(connection)
for table, constraint in (
self._99de3849d860_removed_constraints.items()
):
constraints = [
x['name'] for x in
inspector.get_unique_constraints(table)
]
self.assertIn(constraint, constraints)
def _check_99de3849d860(self, connection):
inspector = sqlalchemy.inspect(connection)
for table, constraint in (
self._99de3849d860_removed_constraints.items()
):
constraints = [
x['name'] for x in
inspector.get_unique_constraints(table)
]
self.assertNotIn(constraint, constraints)
def _pre_upgrade_b4f8b3f584e0(self, connection):
inspector = sqlalchemy.inspect(connection)
constraints = inspector.get_unique_constraints('trust')
self.assertNotIn(
'duplicate_trust_constraint',
{x['name'] for x in constraints},
)
self.assertNotIn(
[
'trustor_user_id',
'trustee_user_id',
'project_id',
'impersonation',
'expires_at',
],
{x['column_names'] for x in constraints},
)
def _check_b4f8b3f584e0(self, connection):
inspector = sqlalchemy.inspect(connection)
constraints = inspector.get_unique_constraints('trust')
self.assertIn(
'duplicate_trust_constraint',
{x['name'] for x in constraints},
)
constraint = [
x for x in constraints if x['name'] ==
'duplicate_trust_constraint'
][0]
self.assertEqual(
[
'trustor_user_id',
'trustee_user_id',
'project_id',
'impersonation',
'expires_at',
],
constraint['column_names'],
)
def test_single_base_revision(self):
"""Ensure we only have a single base revision.