Correct domain_id and name constraint dropping

The 'domain_id' and 'name' unique constraint was not properly dropped
in some cases because the unique constraint was not consistently
named. In all cases we must search for the constraint expected,
not assume the name of the constraint will be consistent
(especially from older installs that have been moved forward in
releases). This fix is modeled on the fix for a similair issue
authored by Morgan Fainberg & Matthew Thode for Bug #1562934

Migration 091:
    Fix to broken migration to prevent failed migrations when database is
    upgraded from Kilo (or below) to Mitaka
Migration 097:
    Ensure that when Mitaka point release is applied the constraint and tables
    have been dropped if migration 91 was previously worked around.

Migration 91 drops 3 columns from the user table after the code to disable
the constraint. I have included code in migrations 97 to also drop
those columns if they are still present in case they were missed when working
around Bug #1572341. This may be over kill.

The following file conflicted since Opportunistic DB testing was included
in the Newton release.

  keystone/tests/unit/test_sql_upgrade.py

Note that migration 104 was removed since it does not exist in the Mitaka
release. The unit tests were also modified accordingly.

Change-Id: I076d7139b388e30be8826d0a4550256b5617d992
Closes-bug: #1572341
This commit is contained in:
Liam Young 2016-06-15 10:01:43 +00:00 committed by Steve Martinelli
parent 582596c440
commit a4be3399ee
3 changed files with 230 additions and 2 deletions

View File

@ -54,11 +54,29 @@ def upgrade(migrate_engine):
if password_values:
password_table.insert().values(password_values).execute()
# NOTE(gnuoy): the `domain_id` unique constraint is not guaranteed to
# be a fixed name, such as 'ixu_user_name_domain_id`, so we need to
# search for the correct constraint that only affects
# user_table.c.domain_id and drop that constraint. (Fix based on
# morganfainbergs fix in 088_domain_specific_roles.py)
to_drop = None
if migrate_engine.name == 'mysql':
for index in user_table.indexes:
if (index.unique and len(index.columns) == 2 and
'domain_id' in index.columns and 'name' in index.columns):
to_drop = index
break
else:
for index in user_table.constraints:
if (len(index.columns) == 2 and 'domain_id' in index.columns and
'name' in index.columns):
to_drop = index
break
# remove domain_id and name unique constraint
if migrate_engine.name != 'sqlite':
if migrate_engine.name != 'sqlite' and to_drop is not None:
migrate.UniqueConstraint(user_table.c.domain_id,
user_table.c.name,
name='ixu_user_name_domain_id').drop()
name=to_drop.name).drop()
# drop user columns
user_table.c.domain_id.drop()

View File

@ -0,0 +1,67 @@
# 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
_USER_TABLE_NAME = 'user'
_USER_NAME_COLUMN_NAME = 'name'
_USER_DOMAINID_COLUMN_NAME = 'domain_id'
_USER_PASSWORD_COLUMN_NAME = 'password'
def upgrade(migrate_engine):
meta = sql.MetaData()
meta.bind = migrate_engine
user_table = sql.Table(_USER_TABLE_NAME, meta, autoload=True)
# NOTE(gnuoy): the `domain_id` unique constraint is not guaranteed to
# be a fixed name, such as 'ixu_user_name_domain_id`, so we need to
# search for the correct constraint that only affects
# user_table.c.domain_id and drop that constraint. (Fix based on
# morganfainbergs fix in 088_domain_specific_roles.py)
#
# This is an idempotent change that reflects the fix to migration
# 91 if the user name & domain_id unique constraint was not named
# consistently and someone manually fixed the migrations / db
# without dropping the old constraint.
to_drop = None
if migrate_engine.name == 'mysql':
for index in user_table.indexes:
if (index.unique and len(index.columns) == 2 and
_USER_DOMAINID_COLUMN_NAME in index.columns and
_USER_NAME_COLUMN_NAME in index.columns):
to_drop = index
break
else:
for index in user_table.constraints:
if (len(index.columns) == 2 and
_USER_DOMAINID_COLUMN_NAME in index.columns and
_USER_NAME_COLUMN_NAME in index.columns):
to_drop = index
break
# remove domain_id and name unique constraint
if to_drop is not None:
migrate.UniqueConstraint(user_table.c.domain_id,
user_table.c.name,
name=to_drop.name).drop()
# If migration 91 was aborted due to Bug #1572341 then columns may not
# have been dropped.
if _USER_DOMAINID_COLUMN_NAME in user_table.c:
user_table.c.domain_id.drop()
if _USER_NAME_COLUMN_NAME in user_table.c:
user_table.c.name.drop()
if _USER_PASSWORD_COLUMN_NAME in user_table.c:
user_table.c.password.drop()

View File

@ -1079,6 +1079,31 @@ class SqlUpgradeTests(SqlMigrateBase):
migrate.UniqueConstraint(role_table.c.name,
name=constraint_name).drop()
def _add_unique_constraint_to_user_name_domainid(
self,
constraint_name='ixu_role_name'):
meta = sqlalchemy.MetaData()
meta.bind = self.engine
user_table = sqlalchemy.Table('user', meta, autoload=True)
migrate.UniqueConstraint(user_table.c.name, user_table.c.domain_id,
name=constraint_name).create()
def _add_name_domain_id_columns_to_user(self):
meta = sqlalchemy.MetaData()
meta.bind = self.engine
user_table = sqlalchemy.Table('user', meta, autoload=True)
column_name = sqlalchemy.Column('name', sql.String(255))
column_domain_id = sqlalchemy.Column('domain_id', sql.String(64))
user_table.create_column(column_name)
user_table.create_column(column_domain_id)
def _drop_unique_constraint_to_user_name_domainid(
self,
constraint_name='ixu_user_name_domain_id'):
user_table = sqlalchemy.Table('user', self.metadata, autoload=True)
migrate.UniqueConstraint(user_table.c.name, user_table.c.domain_id,
name=constraint_name).drop()
def test_migration_88_drops_unique_constraint(self):
self.upgrade(87)
if self.engine.name == 'mysql':
@ -1120,6 +1145,58 @@ class SqlUpgradeTests(SqlMigrateBase):
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
def test_migration_91_drops_unique_constraint(self):
self.upgrade(90)
if self.engine.name == 'mysql':
self.assertTrue(self.does_index_exist('user',
'ixu_user_name_domain_id'))
else:
self.assertTrue(self.does_constraint_exist(
'user',
'ixu_user_name_domain_id'))
self.upgrade(91)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist(
'user',
'ixu_user_name_domain_id'))
else:
self.assertFalse(self.does_constraint_exist(
'user',
'ixu_user_name_domain_id'))
def test_migration_91_inconsistent_constraint_name(self):
self.upgrade(90)
self._drop_unique_constraint_to_user_name_domainid()
constraint_name = uuid.uuid4().hex
self._add_unique_constraint_to_user_name_domainid(
constraint_name=constraint_name)
if self.engine.name == 'mysql':
self.assertTrue(self.does_index_exist('user', constraint_name))
self.assertFalse(self.does_index_exist(
'user',
'ixu_user_name_domain_id'))
else:
self.assertTrue(self.does_constraint_exist('user',
constraint_name))
self.assertFalse(self.does_constraint_exist(
'user',
'ixu_user_name_domain_id'))
self.upgrade(91)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('user', constraint_name))
self.assertFalse(self.does_index_exist(
'user',
'ixu_user_name_domain_id'))
else:
self.assertFalse(self.does_constraint_exist('user',
constraint_name))
self.assertFalse(self.does_constraint_exist(
'user',
'ixu_user_name_domain_id'))
def test_migration_96(self):
self.upgrade(95)
if self.engine.name == 'mysql':
@ -1152,6 +1229,72 @@ class SqlUpgradeTests(SqlMigrateBase):
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
def test_migration_97(self):
self.upgrade(96)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist(
'user',
'ixu_user_name_domain_id'))
else:
self.assertFalse(self.does_constraint_exist(
'user',
'ixu_user_name_domain_id'))
self.upgrade(97)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist(
'user',
'ixu_user_name_domain_id'))
else:
self.assertFalse(self.does_constraint_exist(
'user',
'ixu_user_name_domain_id'))
def test_migration_97_constraint_exists(self):
self.upgrade(96)
self._add_name_domain_id_columns_to_user()
self._add_unique_constraint_to_user_name_domainid(
constraint_name='ixu_user_name_domain_id')
if self.engine.name == 'mysql':
self.assertTrue(self.does_index_exist(
'user',
'ixu_user_name_domain_id'))
else:
self.assertTrue(self.does_constraint_exist(
'user',
'ixu_user_name_domain_id'))
self.upgrade(97)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist(
'user',
'ixu_user_name_domain_id'))
else:
self.assertFalse(self.does_constraint_exist(
'user',
'ixu_user_name_domain_id'))
def test_migration_97_inconsistent_constraint_exists(self):
self.upgrade(96)
constraint_name = uuid.uuid4().hex
self._add_name_domain_id_columns_to_user()
self._add_unique_constraint_to_user_name_domainid(
constraint_name=constraint_name)
if self.engine.name == 'mysql':
self.assertTrue(self.does_index_exist('user', constraint_name))
else:
self.assertTrue(self.does_constraint_exist('user',
constraint_name))
self.upgrade(97)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('user', constraint_name))
else:
self.assertFalse(self.does_constraint_exist('user',
constraint_name))
class VersionTests(SqlMigrateBase):