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) 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. Note that migration 100 was not backported since
it was a placeholder migration.

Conflicts:
    keystone/tests/unit/test_sql_upgrade.py

Co-Authored-By: "Matthew Thode" <mthode@mthode.org>
Change-Id: Ie0dc3d2449bace57d3e9323b281a2abd2ad0c983
closes-bug: #1562934
(cherry picked from commit 276e57e2083dcad8cbabf9aa9b3bd06c6079c415)
This commit is contained in:
Morgan Fainberg 2016-03-28 10:50:12 -07:00 committed by Steve Martinelli
parent cf1fd9dad7
commit dba04cdd23
3 changed files with 170 additions and 4 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()

93
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
@ -494,6 +495,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)
@ -1059,6 +1065,93 @@ class SqlUpgradeTests(SqlMigrateBase):
# assert id column is an integer (after)
self.assertEqual('INTEGER', str(revocation_event_table.c.id.type))
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'))
class VersionTests(SqlMigrateBase):