Correct domain_id and name constraint dropping9.1.0

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

3 files changed, 230 insertions, 2 deletions

diff --git a/keystone/common/sql/migrate_repo/versions/091_migrate_data_to_local_user_and_password_tables.py b/keystone/common/sql/migrate_repo/versions/091_migrate_data_to_local_user_and_password_tables.py
index 1f41fd896..7e98b99b7 100644
--- a/keystone/common/sql/migrate_repo/versions/091_migrate_data_to_local_user_and_password_tables.py
+++ b/keystone/common/sql/migrate_repo/versions/091_migrate_data_to_local_user_and_password_tables.py
@@ -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()
diff --git a/keystone/common/sql/migrate_repo/versions/097_drop_user_name_domainid_constraint.py b/keystone/common/sql/migrate_repo/versions/097_drop_user_name_domainid_constraint.py
new file mode 100644
index 000000000..bb1aea882
--- /dev/null
+++ b/keystone/common/sql/migrate_repo/versions/097_drop_user_name_domainid_constraint.py
@@ -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()
diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py
index 5ca12f66f..7ad34f630 100644
--- a/keystone/tests/unit/test_sql_upgrade.py
+++ b/keystone/tests/unit/test_sql_upgrade.py
@@ -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):