From 1012bd42df5906bca67a82663f23b5c8a4eafe68 Mon Sep 17 00:00:00 2001 From: Adam Young Date: Tue, 6 Nov 2012 17:16:56 -0500 Subject: [PATCH] normalize identity modify tables by adding columns, and modify entities by adding attributes for password, description and enabled update tests to deal with change from 'False' and 'True' to the python values False and True Added a Text type from SQL Alchemy Bug 1070351 Bug 1023544 Change-Id: I066c788b5d08a8f42a9b5412ea9e29e4fe9ba205 --- keystone/common/sql/core.py | 1 + .../versions/008_normalize_identity.py | 61 ++++++++ .../versions/008_sqlite_downgrade.sql | 5 + .../009_normalize_identity_migration.py | 146 ++++++++++++++++++ keystone/identity/backends/sql.py | 8 +- tests/default_fixtures.py | 8 + tests/test_backend.py | 18 +-- tests/test_backend_sql.py | 2 + tests/test_sql_upgrade.py | 53 ++++++- 9 files changed, 290 insertions(+), 12 deletions(-) create mode 100644 keystone/common/sql/migrate_repo/versions/008_normalize_identity.py create mode 100644 keystone/common/sql/migrate_repo/versions/008_sqlite_downgrade.sql create mode 100644 keystone/common/sql/migrate_repo/versions/009_normalize_identity_migration.py diff --git a/keystone/common/sql/core.py b/keystone/common/sql/core.py index a376ed8778..0e18f8d5d3 100644 --- a/keystone/common/sql/core.py +++ b/keystone/common/sql/core.py @@ -46,6 +46,7 @@ IntegrityError = sql.exc.IntegrityError NotFound = sql.orm.exc.NoResultFound Boolean = sql.Boolean +Text = sql.Text def set_global_engine(engine): diff --git a/keystone/common/sql/migrate_repo/versions/008_normalize_identity.py b/keystone/common/sql/migrate_repo/versions/008_normalize_identity.py new file mode 100644 index 0000000000..c473e78744 --- /dev/null +++ b/keystone/common/sql/migrate_repo/versions/008_normalize_identity.py @@ -0,0 +1,61 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2012 OpenStack LLC +# +# 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 json +import string + +from sqlalchemy import Column, MetaData, String, Table, Text, types +from sqlalchemy.orm import sessionmaker + + +#this won't work on sqlite. It doesn't support dropping columns +def downgrade_user_table(meta, migrate_engine): + user_table = Table('user', meta, autoload=True) + user_table.columns["password"].drop() + user_table.columns["enabled"].drop() + + +def downgrade_tenant_table(meta, migrate_engine): + tenant_table = Table('tenant', meta, autoload=True) + tenant_table.columns["description"].drop() + tenant_table.columns["enabled"].drop() + + +def upgrade_user_table(meta, migrate_engine): + user_table = Table('user', meta, autoload=True) + user_table.create_column(Column("password", String(128))) + user_table.create_column(Column("enabled", types.Boolean, + default=True)) + + +def upgrade_tenant_table(meta, migrate_engine): + tenant_table = Table('tenant', meta, autoload=True) + tenant_table.create_column(Column("description", Text())) + tenant_table.create_column(Column("enabled", types.Boolean)) + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + upgrade_user_table(meta, migrate_engine) + upgrade_tenant_table(meta, migrate_engine) + + +def downgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + downgrade_user_table(meta, migrate_engine) + downgrade_tenant_table(meta, migrate_engine) diff --git a/keystone/common/sql/migrate_repo/versions/008_sqlite_downgrade.sql b/keystone/common/sql/migrate_repo/versions/008_sqlite_downgrade.sql new file mode 100644 index 0000000000..4dec683afa --- /dev/null +++ b/keystone/common/sql/migrate_repo/versions/008_sqlite_downgrade.sql @@ -0,0 +1,5 @@ +-- not supported by sqlite, but should be: +-- alter TABLE tenant drop column description; +-- alter TABLE tenant drop column enabled; +-- The downgrade process will fail without valid SQL in this file +select count(*) from tenant; diff --git a/keystone/common/sql/migrate_repo/versions/009_normalize_identity_migration.py b/keystone/common/sql/migrate_repo/versions/009_normalize_identity_migration.py new file mode 100644 index 0000000000..888334a623 --- /dev/null +++ b/keystone/common/sql/migrate_repo/versions/009_normalize_identity_migration.py @@ -0,0 +1,146 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2012 OpenStack LLC +# +# 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 json +import string + +from sqlalchemy import Column, MetaData, String, Table, types +from sqlalchemy.orm import sessionmaker + +disabled_values = ['false', 'disabled', 'no', '0'] + + +def is_enabled(enabled): + #no explicit value means enabled + if enabled is None: + return 1 + if enabled is str: + if str(enabled).lower() in disabled_values: + return 0 + if enabled: + return 1 + else: + return 0 + + +def downgrade_user_table(meta, migrate_engine): + user_table = Table('user', meta, autoload=True) + maker = sessionmaker(bind=migrate_engine) + session = maker() + user_data = [] + for a_user in session.query(user_table): + id, name, extra, password, enabled = a_user + extra_parsed = json.loads(extra) + extra_parsed['password'] = password + extra_parsed['enabled'] = "%r" % enabled + user_data.append((password, + json.dumps(extra_parsed), + is_enabled(enabled), id)) + for user in user_data: + session.execute("update user " + "set extra = '%s' " + "where id = '%s'" % + user) + + session.commit() + + +def downgrade_tenant_table(meta, migrate_engine): + tenant_table = Table('tenant', meta, autoload=True) + maker = sessionmaker(bind=migrate_engine) + session = maker() + tenant_data = [] + for a_tenant in session.query(tenant_table): + id, name, extra, password, enabled = a_tenant + extra_parsed = json.loads(extra) + extra_parsed['description'] = description + extra_parsed['enabled'] = "%r" % enabled + tenant_data.append((password, + json.dumps(extra_parsed), + is_enabled(enabled), id)) + for tenant in tenant_data: + session.execute("update tenant " + "set extra = '%s' " + "where id = '%s'" % + tenant) + + session.commit() + + +def upgrade_user_table(meta, migrate_engine): + user_table = Table('user', meta, autoload=True) + maker = sessionmaker(bind=migrate_engine) + session = maker() + + new_user_data = [] + for a_user in session.query(user_table): + id, name, extra, password, enabled = a_user + extra_parsed = json.loads(extra) + if 'password' in extra_parsed: + password = extra_parsed['password'] + extra_parsed.pop('password') + if 'enabled' in extra_parsed: + enabled = extra_parsed['enabled'] + extra_parsed.pop('enabled') + new_user_data.append((password, + json.dumps(extra_parsed), + is_enabled(enabled), id)) + for new_user in new_user_data: + session.execute("update user " + "set password = '%s', extra = '%s', enabled = '%s' " + "where id = '%s'" % + new_user) + session.commit() + + +def upgrade_tenant_table(meta, migrate_engine): + tenant_table = Table('tenant', meta, autoload=True) + + maker = sessionmaker(bind=migrate_engine) + session = maker() + new_tenant_data = [] + for a_tenant in session.query(tenant_table): + id, name, extra, description, enabled = a_tenant + extra_parsed = json.loads(extra) + if 'description' in extra_parsed: + description = extra_parsed['description'] + extra_parsed.pop('description') + if 'enabled' in extra_parsed: + enabled = extra_parsed['enabled'] + extra_parsed.pop('enabled') + new_tenant_data.append((description, + json.dumps(extra_parsed), + is_enabled(enabled), id)) + for new_tenant in new_tenant_data: + session.execute("update tenant " + "set description = '%s', extra = '%s', enabled = '%s' " + "where id = '%s'" % + new_tenant) + session.commit() + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + upgrade_user_table(meta, migrate_engine) + upgrade_tenant_table(meta, migrate_engine) + + +def downgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + downgrade_user_table(meta, migrate_engine) + downgrade_tenant_table(meta, migrate_engine) diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index 209ec2f094..73d58b9423 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -39,9 +39,11 @@ def wrapper(*args, **kwargs): class User(sql.ModelBase, sql.DictBase): __tablename__ = 'user' - attributes = ['id', 'name'] + attributes = ['id', 'name', 'password', 'enabled'] id = sql.Column(sql.String(64), primary_key=True) name = sql.Column(sql.String(64), unique=True, nullable=False) + password = sql.Column(sql.String(128)) + enabled = sql.Column(sql.Boolean) extra = sql.Column(sql.JsonBlob()) @@ -73,6 +75,8 @@ class Tenant(sql.ModelBase, sql.DictBase): attributes = ['id', 'name'] id = sql.Column(sql.String(64), primary_key=True) name = sql.Column(sql.String(64), unique=True, nullable=False) + description = sql.Column(sql.Text()) + enabled = sql.Column(sql.Boolean) extra = sql.Column(sql.JsonBlob()) @@ -562,6 +566,8 @@ def list_user_projects(self, user_id): @handle_conflicts(type='user') def create_user(self, user_id, user): user['name'] = clean.user_name(user['name']) + if not 'enabled' in user: + user['enabled'] = True user = utils.hash_user_password(user) session = self.get_session() with session.begin(): diff --git a/tests/default_fixtures.py b/tests/default_fixtures.py index b1ea51981c..4a844a50ab 100644 --- a/tests/default_fixtures.py +++ b/tests/default_fixtures.py @@ -44,6 +44,14 @@ 'enabled': True, 'tenant_id': 'baz', 'tenants': ['baz'], + }, { + 'id': 'badguy', + 'name': 'BadGuy', + 'password': 'bad', + 'email': 'bad@guy.com', + 'enabled': False, + 'tenant_id': 'baz', + 'tenants': ['baz'], } ] diff --git a/tests/test_backend.py b/tests/test_backend.py index 5fd8d3cc38..e20011b9dc 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -659,39 +659,39 @@ def test_create_tenant_doesnt_modify_passed_in_dict(self): def test_create_user_doesnt_modify_passed_in_dict(self): new_user = {'id': 'user_id', 'name': 'new_user', - 'password': 'secret'} + 'password': 'secret', 'enabled': True} original_user = new_user.copy() self.identity_api.create_user('user_id', new_user) self.assertDictEqual(original_user, new_user) def test_update_user_enable(self): - user = {'id': 'fake1', 'name': 'fake1', 'enabled': 'True'} + user = {'id': 'fake1', 'name': 'fake1', 'enabled': True} self.identity_api.create_user('fake1', user) user_ref = self.identity_api.get_user('fake1') - self.assertEqual(user_ref['enabled'], 'True') + self.assertEqual(user_ref['enabled'], True) - user['enabled'] = 'False' + user['enabled'] = False self.identity_api.update_user('fake1', user) user_ref = self.identity_api.get_user('fake1') self.assertEqual(user_ref['enabled'], user['enabled']) - user['enabled'] = 'True' + user['enabled'] = True self.identity_api.update_user('fake1', user) user_ref = self.identity_api.get_user('fake1') self.assertEqual(user_ref['enabled'], user['enabled']) def test_update_tenant_enable(self): - tenant = {'id': 'fake1', 'name': 'fake1', 'enabled': 'True'} + tenant = {'id': 'fake1', 'name': 'fake1', 'enabled': True} self.identity_api.create_tenant('fake1', tenant) tenant_ref = self.identity_api.get_tenant('fake1') - self.assertEqual(tenant_ref['enabled'], 'True') + self.assertEqual(tenant_ref['enabled'], True) - tenant['enabled'] = 'False' + tenant['enabled'] = False self.identity_api.update_tenant('fake1', tenant) tenant_ref = self.identity_api.get_tenant('fake1') self.assertEqual(tenant_ref['enabled'], tenant['enabled']) - tenant['enabled'] = 'True' + tenant['enabled'] = True self.identity_api.update_tenant('fake1', tenant) tenant_ref = self.identity_api.get_tenant('fake1') self.assertEqual(tenant_ref['enabled'], tenant['enabled']) diff --git a/tests/test_backend_sql.py b/tests/test_backend_sql.py index c089bac700..c163acbada 100644 --- a/tests/test_backend_sql.py +++ b/tests/test_backend_sql.py @@ -53,6 +53,8 @@ def setUp(self): # populate the engine with tables & fixtures self.load_fixtures(default_fixtures) + #defaulted by the data load + self.user_foo['enabled'] = True def tearDown(self): sql.set_global_engine(None) diff --git a/tests/test_sql_upgrade.py b/tests/test_sql_upgrade.py index 9b67fd5f59..6ade77f714 100644 --- a/tests/test_sql_upgrade.py +++ b/tests/test_sql_upgrade.py @@ -19,6 +19,7 @@ from migrate.versioning import api as versioning_api import sqlalchemy +from sqlalchemy.orm import sessionmaker from keystone.common import sql from keystone import config @@ -84,6 +85,45 @@ def test_upgrade_5_to_6(self): self.assertTableExists('policy') self.assertTableColumns('policy', ['id', 'type', 'blob', 'extra']) + def test_upgrade_7_to_9(self): + + self.assertEqual(self.schema.version, 0) + self._migrate(self.repo_path, 7) + self.populate_user_table() + self.populate_tenant_table() + self._migrate(self.repo_path, 9) + self.assertEqual(self.schema.version, 9) + self.assertTableColumns("user", + ["id", "name", "extra", "password", + "enabled"]) + self.assertTableColumns("tenant", + ["id", "name", "extra", "description", + "enabled"]) + self.assertTableColumns("role", ["id", "name", "extra"]) + self.assertTableColumns("user_tenant_membership", + ["user_id", "tenant_id"]) + self.assertTableColumns("metadata", ["user_id", "tenant_id", "data"]) + maker = sessionmaker(bind=self.engine) + session = maker() + user_table = sqlalchemy.Table("user", + self.metadata, + autoload=True) + a_user = session.query(user_table).filter("id='foo'").one() + self.assertTrue(a_user.enabled) + a_user = session.query(user_table).filter("id='badguy'").one() + self.assertFalse(a_user.enabled) + tenant_table = sqlalchemy.Table("tenant", + self.metadata, + autoload=True) + a_tenant = session.query(tenant_table).filter("id='baz'").one() + self.assertEqual(a_tenant.description, 'description') + session.commit() + + def test_downgrade_9_to_7(self): + self.assertEqual(self.schema.version, 0) + self._migrate(self.repo_path, 9) + self._migrate(self.repo_path, 7, False) + def populate_user_table(self): for user in default_fixtures.USERS: extra = copy.deepcopy(user) @@ -94,6 +134,16 @@ def populate_user_table(self): user['name'], json.dumps(extra))) + def populate_tenant_table(self): + for tenant in default_fixtures.TENANTS: + extra = copy.deepcopy(tenant) + extra.pop('id') + extra.pop('name') + self.engine.execute("insert into tenant values ('%s', '%s', '%s')" + % (tenant['id'], + tenant['name'], + json.dumps(extra))) + def select_table(self, name): table = sqlalchemy.Table(name, self.metadata, @@ -117,8 +167,7 @@ def assertTableDoesNotExist(self, table_name): else: raise AssertionError('Table "%s" already exists' % table_name) - def _migrate(self, repository, version): - upgrade = True + def _migrate(self, repository, version, upgrade=True): err = "" version = versioning_api._migrate_version(self.schema, version,