openstack
/
keystone
Code Issues Proposed changes

Correct `role_name` constraint dropping 

The `role_name` 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 change fixes migration 88, updates 96 (for backport to
stable/mitaka), and creates migration 101 for anyone who has moved
beyond the mitaka -> newton placeholders. This is being overly
cautious, but specifically is to handle the case where someone
performed the upgrade and manually fixed the migration resulting
in duplicated constraints in the role_table.

Co-Authored-By: "Matthew Thode" <mthode@mthode.org>
Change-Id: Ie0dc3d2449bace57d3e9323b281a2abd2ad0c983
closes-bug: #1562934
This commit is contained in:
Morgan Fainberg 2016-03-28 10:50:12 -07:00 committed by Steve Martinelli
parent afd29d3fb5
commit 9e81843719
5 changed files with 253 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
keystone/common/sql/migrate_repo/versions/088_domain_specific_roles.py
View File

@ -13,9 +13,10 @@
import migrate
import sqlalchemy as sql
_ROLE_NAME_OLD_CONSTRAINT = 'ixu_role_name'
_ROLE_NAME_NEW_CONSTRAINT = 'ixu_role_name_domain_id'
_ROLE_TABLE_NAME = 'role'
_ROLE_NAME_COLUMN_NAME = 'name'
_DOMAIN_ID_COLUMN_NAME = 'domain_id'
_NULL_DOMAIN_ID = '<<null>>'
@ -27,10 +28,32 @@ def upgrade(migrate_engine):
role_table = sql.Table(_ROLE_TABLE_NAME, meta, autoload=True)
domain_id = sql.Column(_DOMAIN_ID_COLUMN_NAME, sql.String(64),
nullable=False, server_default=_NULL_DOMAIN_ID)
role_table.create_column(domain_id)
migrate.UniqueConstraint(role_table.c.name,
name=_ROLE_NAME_OLD_CONSTRAINT).drop()
# NOTE(morganfainberg): the `role_name` unique constraint is not
# guaranteed to be a fixed name, such as 'ixu_role_name`, so we need to
# search for the correct constraint that only affects role_table.c.name
# and drop that constraint.
to_drop = None
if migrate_engine.name == 'mysql':
for c in role_table.indexes:
if (c.unique and len(c.columns) == 1 and
_ROLE_NAME_COLUMN_NAME in c.columns):
to_drop = c
break
else:
for c in role_table.constraints:
if len(c.columns) == 1 and _ROLE_NAME_COLUMN_NAME in c.columns:
to_drop = c
break
if to_drop is not None:
migrate.UniqueConstraint(role_table.c.name,
name=to_drop.name).drop()
# perform changes after constraint is dropped.
if 'domain_id' not in role_table.columns:
# Only create the column if it doesn't already exist.
role_table.create_column(domain_id)
migrate.UniqueConstraint(role_table.c.name,
role_table.c.domain_id,

50
keystone/common/sql/migrate_repo/versions/096_drop_role_name_constraint.py Normal file
View File

@ -0,0 +1,50 @@
# 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
_ROLE_TABLE_NAME = 'role'
_ROLE_NAME_COLUMN_NAME = 'name'
def upgrade(migrate_engine):
meta = sql.MetaData()
meta.bind = migrate_engine
role_table = sql.Table(_ROLE_TABLE_NAME, meta, autoload=True)
# NOTE(morganfainberg): the `role_name` unique constraint is not
# guaranteed to be named 'ixu_role_name', so we need to search for the
# correct constraint that only affects role_table.c.name and drop
# that constraint.
#
# This is an idempotent change that reflects the fix to migration
# 88 if the role_name 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 c in role_table.indexes:
if (c.unique and len(c.columns) == 1 and
_ROLE_NAME_COLUMN_NAME in c.columns):
to_drop = c
break
else:
for c in role_table.constraints:
if len(c.columns) == 1 and _ROLE_NAME_COLUMN_NAME in c.columns:
to_drop = c
break
if to_drop is not None:
migrate.UniqueConstraint(role_table.c.name,
name=to_drop.name).drop()

18
keystone/common/sql/migrate_repo/versions/096_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 Mitaka backports. Do not use this number for new
# Newton work. New Newton work starts after all the placeholders.
def upgrade(migrate_engine):
pass

53
keystone/common/sql/migrate_repo/versions/101_drop_role_name_constraint.py Normal file
View File

@ -0,0 +1,53 @@
# 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
_ROLE_TABLE_NAME = 'role'
_ROLE_NAME_COLUMN_NAME = 'name'
def upgrade(migrate_engine):
meta = sql.MetaData()
meta.bind = migrate_engine
role_table = sql.Table(_ROLE_TABLE_NAME, meta, autoload=True)
# NOTE(morganfainberg): the `role_name` unique constraint is not
# guaranteed to be named 'ixu_role_name', so we need to search for the
# correct constraint that only affects role_table.c.name and drop
# that constraint.
#
# This is an idempotent change that reflects the fix to migration
# 88 if the role_name unique constraint was not named consistently and
# someone manually fixed the migrations / db without dropping the
# old constraint.
# This is a copy of migration 96 to catch any/all deployments that
# are close to master. migration 96 will be backported to
# stable/mitaka.
to_drop = None
if migrate_engine.name == 'mysql':
for c in role_table.indexes:
if (c.unique and len(c.columns) == 1 and
_ROLE_NAME_COLUMN_NAME in c.columns):
to_drop = c
break
else:
for c in role_table.constraints:
if len(c.columns) == 1 and _ROLE_NAME_COLUMN_NAME in c.columns:
to_drop = c
break
if to_drop is not None:
migrate.UniqueConstraint(role_table.c.name,
name=to_drop.name).drop()

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

@ -32,6 +32,7 @@ WARNING::
import json
import uuid
import migrate
from migrate.versioning import api as versioning_api
from migrate.versioning import repository
import mock
@ -440,6 +441,11 @@ class SqlUpgradeTests(SqlMigrateBase):
table = sqlalchemy.Table(table_name, meta, autoload=True)
return index_name in [idx.name for idx in table.indexes]
def does_constraint_exist(self, table_name, constraint_name):
meta = sqlalchemy.MetaData(bind=self.engine)
table = sqlalchemy.Table(table_name, meta, autoload=True)
return constraint_name in [con.name for con in table.constraints]
def test_endpoint_policy_upgrade(self):
self.assertTableDoesNotExist('policy_association')
self.upgrade(81)
@ -1031,6 +1037,123 @@ class SqlUpgradeTests(SqlMigrateBase):
# assert id column is an integer (after)
self.assertIsInstance(revocation_event_table.c.id.type, sql.Integer)
def _add_unique_constraint_to_role_name(self,
constraint_name='ixu_role_name'):
meta = sqlalchemy.MetaData()
meta.bind = self.engine
role_table = sqlalchemy.Table('role', meta, autoload=True)
migrate.UniqueConstraint(role_table.c.name,
name=constraint_name).create()
def _drop_unique_constraint_to_role_name(self,
constraint_name='ixu_role_name'):
role_table = sqlalchemy.Table('role', self.metadata, autoload=True)
migrate.UniqueConstraint(role_table.c.name,
name=constraint_name).drop()
def test_migration_88_drops_unique_constraint(self):
self.upgrade(87)
if self.engine.name == 'mysql':
self.assertTrue(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertTrue(self.does_constraint_exist('role',
'ixu_role_name'))
self.upgrade(88)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
def test_migration_88_inconsistent_constraint_name(self):
self.upgrade(87)
self._drop_unique_constraint_to_role_name()
constraint_name = uuid.uuid4().hex
self._add_unique_constraint_to_role_name(
constraint_name=constraint_name)
if self.engine.name == 'mysql':
self.assertTrue(self.does_index_exist('role', constraint_name))
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertTrue(self.does_constraint_exist('role',
constraint_name))
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
self.upgrade(88)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('role', constraint_name))
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertFalse(self.does_constraint_exist('role',
constraint_name))
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
def test_migration_96(self):
self.upgrade(95)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
self.upgrade(96)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
def test_migration_96_constraint_exists(self):
self.upgrade(95)
self._add_unique_constraint_to_role_name()
if self.engine.name == 'mysql':
self.assertTrue(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertTrue(self.does_constraint_exist('role',
'ixu_role_name'))
self.upgrade(96)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
def test_migration_101(self):
self.upgrade(100)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
self.upgrade(101)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
def test_migration_101_constraint_exists(self):
self.upgrade(100)
self._add_unique_constraint_to_role_name()
if self.engine.name == 'mysql':
self.assertTrue(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertTrue(self.does_constraint_exist('role',
'ixu_role_name'))
self.upgrade(101)
if self.engine.name == 'mysql':
self.assertFalse(self.does_index_exist('role', 'ixu_role_name'))
else:
self.assertFalse(self.does_constraint_exist('role',
'ixu_role_name'))
class MySQLOpportunisticUpgradeTestCase(SqlUpgradeTests):
FIXTURE = test_base.MySQLOpportunisticFixture