Merge "Fixes password created_at errors due to the server_default"

keystone/common/sql/contract_repo/versions/004_reset_password_created_at.py

@@ -0,0 +1,37 @@
+# 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 datetime
+
+import sqlalchemy as sql
+import sqlalchemy.sql.expression as expression
+
+
+def upgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+
+    password = sql.Table('password', meta, autoload=True)
+    # reset created_at column
+    password.c.created_at.drop()
+    created_at = sql.Column('created_at', sql.DateTime(),
+                            nullable=True,
+                            default=datetime.datetime.utcnow)
+    password.create_column(created_at)
+    # update created_at value
+    now = datetime.datetime.utcnow()
+    values = {'created_at': now}
+    stmt = password.update().where(
+        password.c.created_at == expression.null()).values(values)
+    stmt.execute()
+    # set not nullable
+    password.c.created_at.alter(nullable=False)

keystone/common/sql/data_migration_repo/versions/004_reset_password_created_at.py

@@ -0,0 +1,15 @@
+# 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.
+
+
+def upgrade(migrate_engine):
+    pass

keystone/common/sql/expand_repo/versions/004_reset_password_created_at.py

@@ -0,0 +1,15 @@
+# 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.
+
+
+def upgrade(migrate_engine):
+    pass

keystone/identity/backends/sql_model.py

@@ -234,7 +234,7 @@ class Password(sql.ModelBase, sql.DictBase):
                                ondelete='CASCADE'))
     password = sql.Column(sql.String(128), nullable=True)
     # created_at default set here to safe guard in case it gets missed
-    created_at = sql.Column(sql.TIMESTAMP, nullable=False,
+    created_at = sql.Column(sql.DateTime, nullable=False,
                             default=datetime.datetime.utcnow)
     expires_at = sql.Column(sql.DateTime, nullable=True)
     self_service = sql.Column(sql.Boolean, default=False, nullable=False,

keystone/tests/unit/test_backend_sql.py

@@ -152,7 +152,7 @@ class SqlModels(SqlTests):
         cols = (('id', sql.Integer, None),
                 ('local_user_id', sql.Integer, None),
                 ('password', sql.String, 128),
-                ('created_at', sql.TIMESTAMP, None),
+                ('created_at', sql.DateTime, None),
                 ('expires_at', sql.DateTime, None),
                 ('self_service', sql.Boolean, False))
         self.assertExpectedSchema('password', cols)

keystone/tests/unit/test_sql_banned_operations.py

@@ -236,7 +236,12 @@ class TestKeystoneExpandSchemaMigrations(
         # to make `blob` nullable. This allows the triggers added in 003 to
         # catch writes when the `blob` attribute isn't populated. We do this so
         # that the triggers aren't aware of the encryption implementation.
-        3
+        3,
+        # Migration 004 changes the password created_at column type, from
+        # timestamp to datetime and updates the initial value in the contract
+        # phase. Adding an exception here to pass expand banned tests,
+        # otherwise fails.
+        4
     ]
 
     def setUp(self):
@@ -271,7 +276,12 @@ class TestKeystoneDataMigrations(
         # Migration 002 changes the column type, from datetime to timestamp in
         # the contract phase. Adding exception here to pass banned data
         # migration tests. Fails otherwise.
-        2
+        2,
+        # Migration 004 changes the password created_at column type, from
+        # timestamp to datetime and updates the initial value in the contract
+        # phase. Adding an exception here to pass data migrations banned tests,
+        # otherwise fails.
+        4
     ]
 
     def setUp(self):
@@ -315,7 +325,16 @@ class TestKeystoneContractSchemaMigrations(
         # Migration 002 changes the column type, from datetime to timestamp.
         # To do this, the column is first dropped and recreated. This should
         # not have any negative impact on a rolling upgrade deployment.
-        2
+        2,
+        # Migration 004 changes the password created_at column type, from
+        # timestamp to datetime and updates the created_at value. This is
+        # likely not going to impact a rolling upgrade as the contract repo is
+        # executed once the code has been updated; thus the created_at column
+        # would be populated for any password changes. That being said, there
+        # could be a performance issue for existing large password tables, as
+        # the migration is not batched. However, it's a compromise and not
+        # likely going to be a problem for operators.
+        4
     ]
 
     def setUp(self):

keystone/tests/unit/test_sql_upgrade.py

@@ -1794,6 +1794,28 @@ class FullMigration(SqlMigrateBase, unit.TestCase):
                       'type': 'cert'}
         self.insert_dict(session, credential_table_name, credential)
 
+    def test_migration_004_reset_password_created_at(self):
+        # upgrade each repository to 003 and test
+        self.expand(3)
+        self.migrate(3)
+        self.contract(3)
+        password = sqlalchemy.Table('password', self.metadata, autoload=True)
+        # postgresql returns 'TIMESTAMP WITHOUT TIME ZONE'
+        self.assertTrue(
+            str(password.c.created_at.type).startswith('TIMESTAMP'))
+        # upgrade each repository to 004 and test
+        self.expand(4)
+        self.migrate(4)
+        self.contract(4)
+        password = sqlalchemy.Table('password', self.metadata, autoload=True)
+        # type would still be TIMESTAMP with postgresql
+        if self.engine.name == 'postgresql':
+            self.assertTrue(
+                str(password.c.created_at.type).startswith('TIMESTAMP'))
+        else:
+            self.assertEqual('DATETIME', str(password.c.created_at.type))
+        self.assertFalse(password.c.created_at.nullable)
+
 
 class MySQLOpportunisticFullMigration(FullMigration):
     FIXTURE = test_base.MySQLOpportunisticFixture