From 07b07d5b83a75219581dabd703020c7a1fd0415e Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Sat, 30 Dec 2017 17:47:46 +0100 Subject: [PATCH] Add expired_at_int column to trusts We've already converted Password objects to use the DateTimeInt format for its datetime attributes[1]. This was necessary to cope with differences in date storage formats between different DBMSs that was causing intermittent test failures. While we're not experiencing those CI problems any more, the DateTimeInt format is the way forward for consistent datetime storage. This patch converts the trust table and model to use the new format. [1] https://review.openstack.org/#/c/493259/ Related-bug: #1702211 Change-Id: If524c743170924e5b8cfdafa862ed31b06db018c --- ...32_contract_add_expired_at_int_to_trust.py | 51 ++++++++++++++++ ...032_migrate_add_expired_at_int_to_trust.py | 22 +++++++ .../032_expand_add_expired_at_int_to_trust.py | 35 +++++++++++ keystone/tests/unit/test_backend_sql.py | 11 +++- keystone/tests/unit/test_sql_upgrade.py | 58 +++++++++++++++++++ keystone/trust/backends/sql.py | 13 ++++- ...res-at-int-to-trusts-60ae3c5d0c00808a.yaml | 8 +++ 7 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 keystone/common/sql/contract_repo/versions/032_contract_add_expired_at_int_to_trust.py create mode 100644 keystone/common/sql/data_migration_repo/versions/032_migrate_add_expired_at_int_to_trust.py create mode 100644 keystone/common/sql/expand_repo/versions/032_expand_add_expired_at_int_to_trust.py create mode 100644 releasenotes/notes/add-expires-at-int-to-trusts-60ae3c5d0c00808a.yaml diff --git a/keystone/common/sql/contract_repo/versions/032_contract_add_expired_at_int_to_trust.py b/keystone/common/sql/contract_repo/versions/032_contract_add_expired_at_int_to_trust.py new file mode 100644 index 0000000000..5839b8caa9 --- /dev/null +++ b/keystone/common/sql/contract_repo/versions/032_contract_add_expired_at_int_to_trust.py @@ -0,0 +1,51 @@ +# 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 + +from migrate import UniqueConstraint +import pytz +import sqlalchemy as sql +from sqlalchemy.orm import sessionmaker + + +_epoch = datetime.datetime.fromtimestamp(0, tz=pytz.UTC) + + +def _convert_value_datetime_to_int(dt): + dt = dt.replace(tzinfo=pytz.utc) + return int((dt - _epoch).total_seconds() * 1000000) + + +def upgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + maker = sessionmaker(bind=migrate_engine) + session = maker() + + trust_table = sql.Table('trust', meta, autoload=True) + trusts = list(trust_table.select().execute()) + + for trust in trusts: + values = {} + if trust.expires_at is not None: + values['expires_at_int'] = _convert_value_datetime_to_int( + trust.expires_at) + + update = trust_table.update().where( + trust_table.c.id == trust.id).values(values) + session.execute(update) + session.commit() + + UniqueConstraint(table=trust_table, + name='duplicate_trust_constraint').drop() + session.close() diff --git a/keystone/common/sql/data_migration_repo/versions/032_migrate_add_expired_at_int_to_trust.py b/keystone/common/sql/data_migration_repo/versions/032_migrate_add_expired_at_int_to_trust.py new file mode 100644 index 0000000000..ce4496ee0a --- /dev/null +++ b/keystone/common/sql/data_migration_repo/versions/032_migrate_add_expired_at_int_to_trust.py @@ -0,0 +1,22 @@ +# 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): + # A migration here is not needed because the actual marshalling of data + # from the old column to the new column is done in the contract phase. This + # is because using triggers to convert datetime objects to integers is + # complex and error-prone. Instead, we'll migrate the data once all + # keystone nodes are on the Queens code-base. From an operator perspective, + # this shouldn't affect operability of a rolling upgrade since all nodes + # must be running Queens before the contract takes place. + pass diff --git a/keystone/common/sql/expand_repo/versions/032_expand_add_expired_at_int_to_trust.py b/keystone/common/sql/expand_repo/versions/032_expand_add_expired_at_int_to_trust.py new file mode 100644 index 0000000000..fd5d6ad65a --- /dev/null +++ b/keystone/common/sql/expand_repo/versions/032_expand_add_expired_at_int_to_trust.py @@ -0,0 +1,35 @@ +# 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. + + +from migrate import UniqueConstraint +import sqlalchemy as sql + +from keystone.common import sql as ks_sql + + +def upgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + + # NOTE(morgan): column is nullable here for migration purposes + # it is set to not-nullable in the contract phase to ensure we can handle + # rolling upgrades in a sane way. This differs from the model in + # keystone.identity.backends.sql_model by design. + expires_at = sql.Column('expires_at_int', ks_sql.DateTimeInt()) + trust_table = sql.Table('trust', meta, autoload=True) + trust_table.create_column(expires_at) + + UniqueConstraint('trustor_user_id', 'trustee_user_id', 'project_id', + 'impersonation', 'expires_at', 'expires_at_int', + table=trust_table, + name='duplicate_trust_constraint_expanded').create() diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index a213f6e69f..fc9806af10 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/keystone/tests/unit/test_backend_sql.py @@ -44,6 +44,7 @@ from keystone.tests.unit.resource import test_backends as resource_tests from keystone.tests.unit.token import test_backends as token_tests from keystone.tests.unit.trust import test_backends as trust_tests from keystone.token.persistence.backends import sql as token_sql +from keystone.trust.backends import sql as trust_sql CONF = keystone.conf.CONF @@ -710,7 +711,15 @@ class SqlIdentity(SqlTests, class SqlTrust(SqlTests, trust_tests.TrustTests): - pass + + def test_trust_expires_at_int_matches_expires_at(self): + with sql.session_for_write() as session: + new_id = uuid.uuid4().hex + self.create_sample_trust(new_id) + trust_ref = session.query(trust_sql.TrustModel).get(new_id) + self.assertIsNotNone(trust_ref._expires_at) + self.assertEqual(trust_ref._expires_at, trust_ref.expires_at_int) + self.assertEqual(trust_ref.expires_at, trust_ref.expires_at_int) class SqlToken(SqlTests, token_tests.TokenTests): diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py index e597911ace..16f2839632 100644 --- a/keystone/tests/unit/test_sql_upgrade.py +++ b/keystone/tests/unit/test_sql_upgrade.py @@ -2531,6 +2531,64 @@ class FullMigration(SqlMigrateBase, unit.TestCase): } system_assignment_table.insert().values(system_group).execute() + def test_migration_032_add_expires_at_int_column_trust(self): + + self.expand(31) + self.migrate(31) + self.contract(31) + + trust_table_name = 'trust' + + self.assertTableColumns( + trust_table_name, + ['id', 'trustor_user_id', 'trustee_user_id', 'project_id', + 'impersonation', 'deleted_at', 'expires_at', 'remaining_uses', + 'extra'], + ) + + self.expand(32) + + self.assertTableColumns( + trust_table_name, + ['id', 'trustor_user_id', 'trustee_user_id', 'project_id', + 'impersonation', 'deleted_at', 'expires_at', 'expires_at_int', + 'remaining_uses', 'extra'], + ) + + # Create Trust + trust_table = sqlalchemy.Table('trust', self.metadata, + autoload=True) + trust_1_data = { + 'id': uuid.uuid4().hex, + 'trustor_user_id': uuid.uuid4().hex, + 'trustee_user_id': uuid.uuid4().hex, + 'project_id': uuid.uuid4().hex, + 'impersonation': False, + 'expires_at': datetime.datetime.utcnow() + } + trust_2_data = { + 'id': uuid.uuid4().hex, + 'trustor_user_id': uuid.uuid4().hex, + 'trustee_user_id': uuid.uuid4().hex, + 'project_id': uuid.uuid4().hex, + 'impersonation': False, + 'expires_at': None + } + trust_table.insert().values(trust_1_data).execute() + trust_table.insert().values(trust_2_data).execute() + + self.migrate(32) + self.contract(32) + trusts = list(trust_table.select().execute()) + + epoch = datetime.datetime.fromtimestamp(0, tz=pytz.UTC) + + for t in trusts: + if t.expires_at: + e = t.expires_at.replace(tzinfo=pytz.UTC) - epoch + e = e.total_seconds() + self.assertEqual(t.expires_at_int, int(e * 1000000)) + class MySQLOpportunisticFullMigration(FullMigration): FIXTURE = test_base.MySQLOpportunisticFixture diff --git a/keystone/trust/backends/sql.py b/keystone/trust/backends/sql.py index 0a10e98438..a4b3470a5e 100644 --- a/keystone/trust/backends/sql.py +++ b/keystone/trust/backends/sql.py @@ -14,6 +14,7 @@ from oslo_utils import timeutils from six.moves import range +from sqlalchemy.ext.hybrid import hybrid_property from keystone.common import sql from keystone import exception @@ -38,7 +39,8 @@ class TrustModel(sql.ModelBase, sql.ModelDictMixinWithExtras): project_id = sql.Column(sql.String(64)) impersonation = sql.Column(sql.Boolean, nullable=False) deleted_at = sql.Column(sql.DateTime) - expires_at = sql.Column(sql.DateTime) + _expires_at = sql.Column('expires_at', sql.DateTime) + expires_at_int = sql.Column(sql.DateTimeInt(), nullable=True) remaining_uses = sql.Column(sql.Integer, nullable=True) extra = sql.Column(sql.JsonBlob()) __table_args__ = (sql.UniqueConstraint( @@ -46,6 +48,15 @@ class TrustModel(sql.ModelBase, sql.ModelDictMixinWithExtras): 'impersonation', 'expires_at', name='duplicate_trust_constraint'),) + @hybrid_property + def expires_at(self): + return self.expires_at_int or self._expires_at + + @expires_at.setter + def expires_at(self, value): + self._expires_at = value + self.expires_at_int = value + class TrustRole(sql.ModelBase): __tablename__ = 'trust_role' diff --git a/releasenotes/notes/add-expires-at-int-to-trusts-60ae3c5d0c00808a.yaml b/releasenotes/notes/add-expires-at-int-to-trusts-60ae3c5d0c00808a.yaml new file mode 100644 index 0000000000..5e76cf447a --- /dev/null +++ b/releasenotes/notes/add-expires-at-int-to-trusts-60ae3c5d0c00808a.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The trusts table now has an expires_at_int column that represents the + expiration time as an integer instead of a datetime object. This will + prevent rounding errors related to the way date objects are stored in some + versions of MySQL. The expires_at column remains, but will be dropped in + Rocky.