diff --git a/keystone/contrib/federation/migrate_repo/versions/006_fixup_service_provider_attributes.py b/keystone/contrib/federation/migrate_repo/versions/006_fixup_service_provider_attributes.py new file mode 100644 index 0000000000..8a42ce3a4f --- /dev/null +++ b/keystone/contrib/federation/migrate_repo/versions/006_fixup_service_provider_attributes.py @@ -0,0 +1,48 @@ +# 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 sqlalchemy as sql + +_SP_TABLE_NAME = 'service_provider' + + +def _update_null_columns(migrate_engine, sp_table): + stmt = (sp_table.update(). + where(sp_table.c.auth_url.is_(None)). + values(auth_url='')) + migrate_engine.execute(stmt) + + stmt = (sp_table.update(). + where(sp_table.c.sp_url.is_(None)). + values(sp_url='')) + migrate_engine.execute(stmt) + + +def upgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + sp_table = sql.Table(_SP_TABLE_NAME, meta, autoload=True) + # The columns are being changed to non-nullable. To prevent + # database errors when both are altered, all the existing + # null-records should be filled with not null values. + _update_null_columns(migrate_engine, sp_table) + + sp_table.c.auth_url.alter(nullable=False) + sp_table.c.sp_url.alter(nullable=False) + + +def downgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + sp_table = sql.Table(_SP_TABLE_NAME, meta, autoload=True) + sp_table.c.auth_url.alter(nullable=True) + sp_table.c.sp_url.alter(nullable=True) diff --git a/keystone/tests/unit/test_sql_migrate_extensions.py b/keystone/tests/unit/test_sql_migrate_extensions.py index 4b23ba5174..edfb91d71a 100644 --- a/keystone/tests/unit/test_sql_migrate_extensions.py +++ b/keystone/tests/unit/test_sql_migrate_extensions.py @@ -29,6 +29,10 @@ WARNING:: all data will be lost. """ +import sqlalchemy +import uuid + +from oslo_db import exception as db_exception from oslo_db.sqlalchemy import utils from keystone.contrib import endpoint_filter @@ -216,11 +220,19 @@ class FederationExtension(test_sql_upgrade.SqlMigrateBase): super(FederationExtension, self).setUp() self.identity_provider = 'identity_provider' self.federation_protocol = 'federation_protocol' + self.service_provider = 'service_provider' self.mapping = 'mapping' def repo_package(self): return federation + def insert_dict(self, session, table_name, d): + """Naively inserts key-value pairs into a table, given a dictionary.""" + table = sqlalchemy.Table(table_name, self.metadata, autoload=True) + insert = table.insert().values(**d) + session.execute(insert) + session.commit() + def test_upgrade(self): self.assertTableDoesNotExist(self.identity_provider) self.assertTableDoesNotExist(self.federation_protocol) @@ -272,6 +284,77 @@ class FederationExtension(test_sql_upgrade.SqlMigrateBase): self.assertTableDoesNotExist(self.federation_protocol) self.assertTableDoesNotExist(self.mapping) + def test_fixup_service_provider_attributes(self): + self.upgrade(6, repository=self.repo_path) + self.assertTableColumns(self.service_provider, + ['id', 'description', 'enabled', 'auth_url', + 'sp_url']) + + session = self.Session() + sp1 = {'id': uuid.uuid4().hex, + 'auth_url': None, + 'sp_url': uuid.uuid4().hex, + 'description': uuid.uuid4().hex, + 'enabled': True} + sp2 = {'id': uuid.uuid4().hex, + 'auth_url': uuid.uuid4().hex, + 'sp_url': None, + 'description': uuid.uuid4().hex, + 'enabled': True} + sp3 = {'id': uuid.uuid4().hex, + 'auth_url': None, + 'sp_url': None, + 'description': uuid.uuid4().hex, + 'enabled': True} + + # Insert with 'auth_url' or 'sp_url' set to null must fail + self.assertRaises(db_exception.DBError, + self.insert_dict, + session, + self.service_provider, + sp1) + self.assertRaises(db_exception.DBError, + self.insert_dict, + session, + self.service_provider, + sp2) + self.assertRaises(db_exception.DBError, + self.insert_dict, + session, + self.service_provider, + sp3) + + session.close() + self.downgrade(5, repository=self.repo_path) + self.assertTableColumns(self.service_provider, + ['id', 'description', 'enabled', 'auth_url', + 'sp_url']) + session = self.Session() + self.metadata.clear() + + # Before the migration, the table should accept null values + self.insert_dict(session, self.service_provider, sp1) + self.insert_dict(session, self.service_provider, sp2) + self.insert_dict(session, self.service_provider, sp3) + + # Check if null values are updated to empty string when migrating + session.close() + self.upgrade(6, repository=self.repo_path) + sp_table = sqlalchemy.Table(self.service_provider, + self.metadata, + autoload=True) + session = self.Session() + self.metadata.clear() + + sp = session.query(sp_table).filter(sp_table.c.id == sp1['id'])[0] + self.assertEqual('', sp.auth_url) + + sp = session.query(sp_table).filter(sp_table.c.id == sp2['id'])[0] + self.assertEqual('', sp.sp_url) + + sp = session.query(sp_table).filter(sp_table.c.id == sp3['id'])[0] + self.assertEqual('', sp.auth_url) + self.assertEqual('', sp.sp_url) _REVOKE_COLUMN_NAMES = ['id', 'domain_id', 'project_id', 'user_id', 'role_id', 'trust_id', 'consumer_id', 'access_token_id',