From 5d6a0884553f79083b1ae1542e143949d2c4670f Mon Sep 17 00:00:00 2001 From: henriquetruta Date: Mon, 10 Aug 2015 10:48:56 -0300 Subject: [PATCH] Projects acting as domains Moves all domain operations to project table, mapping domains to projects that have the is_domain attribute set to True. Removes all manager references to domain driver calls. The V8 legacy wrapper ensures that manager calls for projects acting as a domain end up calling the underlying driver domain methods, so that older drivers are still be supported. The domain driver methods and the domain table will be removed in follow-up patches. Co-Authored-By: Raildo Mascena Co-Authored-By: Rodrigo Duarte Co-Authored-By: Samuel de Medeiros Queiroz Co-Authored-By: Adam Young Co-Authored-By: Henry Nash Change-Id: Ib22a0f3007cb7ef6b4df6f48da5f4d018e905f55 Implements: bp reseller --- keystone/common/config.py | 7 +- .../093_migrate_domains_to_projects.py | 125 +++++ keystone/models/revoke_model.py | 4 +- keystone/resource/backends/sql.py | 23 +- keystone/resource/controllers.py | 14 +- keystone/resource/core.py | 492 ++++++++++++------ keystone/resource/schema.py | 4 +- keystone/tests/unit/default_fixtures.py | 8 +- keystone/tests/unit/test_backend.py | 212 ++++---- keystone/tests/unit/test_backend_ldap.py | 71 ++- keystone/tests/unit/test_backend_sql.py | 7 +- keystone/tests/unit/test_sql_upgrade.py | 93 ++++ keystone/tests/unit/test_v2_controller.py | 15 +- keystone/tests/unit/test_v3.py | 1 - keystone/tests/unit/test_v3_auth.py | 26 +- keystone/tests/unit/test_v3_resource.py | 45 +- keystone/token/providers/common.py | 8 +- 17 files changed, 836 insertions(+), 319 deletions(-) create mode 100644 keystone/common/sql/migrate_repo/versions/093_migrate_domains_to_projects.py diff --git a/keystone/common/config.py b/keystone/common/config.py index 84a0c72270..468fb64bf1 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -78,9 +78,10 @@ FILE_OPTIONS = { '(e.g. /prefix/v3) or the endpoint should be found ' 'on a different server.'), cfg.IntOpt('max_project_tree_depth', default=5, - help='Maximum depth of the project hierarchy. WARNING: ' - 'setting it to a large value may adversely impact ' - 'performance.'), + help='Maximum depth of the project hierarchy, excluding ' + 'the project acting as a domain at the top of the ' + 'hierarchy. WARNING: setting it to a large value may ' + 'adversely impact performance.'), cfg.IntOpt('max_param_size', default=64, help='Limit the sizes of user & project ID/names.'), # we allow tokens to be a bit larger to accommodate PKI diff --git a/keystone/common/sql/migrate_repo/versions/093_migrate_domains_to_projects.py b/keystone/common/sql/migrate_repo/versions/093_migrate_domains_to_projects.py new file mode 100644 index 0000000000..f6bba7d9ed --- /dev/null +++ b/keystone/common/sql/migrate_repo/versions/093_migrate_domains_to_projects.py @@ -0,0 +1,125 @@ +# 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 sqlalchemy as sql + +from keystone.common.sql import migration_helpers + + +_PROJECT_TABLE_NAME = 'project' +_DOMAIN_TABLE_NAME = 'domain' +_PARENT_ID_COLUMN_NAME = 'parent_id' +_DOMAIN_ID_COLUMN_NAME = 'domain_id' + +# Above the driver level, the domain_id of a project acting as a domain is +# None. However, in order to enable sql integrity constraints to still operate +# on this column, we create a special "root of all domains" row, with an ID of +# NULL_DOMAIN_ID, which all projects acting as a domain reference in their +# domain_id attribute. This special row, as well as NULL_DOMAIN_ID, are never +# exposed outside of sql driver layer. +NULL_DOMAIN_ID = '<>' + + +def list_existing_project_constraints(project_table, domain_table): + constraints = [{'table': project_table, + 'fk_column': _PARENT_ID_COLUMN_NAME, + 'ref_column': project_table.c.id}, + {'table': project_table, + 'fk_column': _DOMAIN_ID_COLUMN_NAME, + 'ref_column': domain_table.c.id}] + + return constraints + + +def list_new_project_constraints(project_table): + constraints = [{'table': project_table, + 'fk_column': _PARENT_ID_COLUMN_NAME, + 'ref_column': project_table.c.id}, + {'table': project_table, + 'fk_column': _DOMAIN_ID_COLUMN_NAME, + 'ref_column': project_table.c.id}] + + return constraints + + +def upgrade(migrate_engine): + + def _project_from_domain(domain): + # Creates a project dict with is_domain=True from the provided + # domain. + + description = None + extra = {} + if domain.extra is not None: + # 'description' property is an extra attribute in domains but a + # first class attribute in projects + extra = json.loads(domain.extra) + description = extra.pop('description', None) + + return { + 'id': domain.id, + 'name': domain.name, + 'enabled': domain.enabled, + 'description': description, + 'domain_id': NULL_DOMAIN_ID, + 'is_domain': True, + 'parent_id': None, + 'extra': json.dumps(extra) + } + + meta = sql.MetaData() + meta.bind = migrate_engine + session = sql.orm.sessionmaker(bind=migrate_engine)() + + project_table = sql.Table(_PROJECT_TABLE_NAME, meta, autoload=True) + domain_table = sql.Table(_DOMAIN_TABLE_NAME, meta, autoload=True) + + # NOTE(htruta): Remove the parent_id constraint during the migration + # because for every root project inside this domain, we will set + # the project domain_id to be its parent_id. We re-enable the constraint + # in the end of this method. We also remove the domain_id constraint, + # while be recreated a FK to the project_id at the end. + migration_helpers.remove_constraints( + list_existing_project_constraints(project_table, domain_table)) + + # For each domain, create a project acting as a domain. We ignore the + # "root of all domains" row, since we already have one of these in the + # project table. + domains = list(domain_table.select().execute()) + for domain in domains: + if domain.id == NULL_DOMAIN_ID: + continue + is_domain_project = _project_from_domain(domain) + new_entry = project_table.insert().values(**is_domain_project) + session.execute(new_entry) + session.commit() + + # For each project, that has no parent (i.e. a top level project), update + # it's parent_id to point at the project acting as its domain. We ignore + # the "root of all domains" row, since its parent_id must always be None. + projects = list(project_table.select().execute()) + for project in projects: + if (project.parent_id is not None or project.is_domain or + project.id == NULL_DOMAIN_ID): + continue + values = {'parent_id': project.domain_id} + update = project_table.update().where( + project_table.c.id == project.id).values(values) + session.execute(update) + session.commit() + + migration_helpers.add_constraints( + list_new_project_constraints(project_table)) + + session.close() diff --git a/keystone/models/revoke_model.py b/keystone/models/revoke_model.py index 5217e9c62d..0fc3e62815 100644 --- a/keystone/models/revoke_model.py +++ b/keystone/models/revoke_model.py @@ -334,7 +334,9 @@ def build_token_values(token_data): project = token_data.get('project', token_data.get('tenant')) if project is not None: token_values['project_id'] = project['id'] - token_values['assignment_domain_id'] = project['domain']['id'] + # The domain_id of projects acting as domains is None + token_values['assignment_domain_id'] = ( + project['domain']['id'] if project['domain'] else None) else: token_values['project_id'] = None diff --git a/keystone/resource/backends/sql.py b/keystone/resource/backends/sql.py index 41d16fd13f..b01161b144 100644 --- a/keystone/resource/backends/sql.py +++ b/keystone/resource/backends/sql.py @@ -28,6 +28,14 @@ class Resource(keystone_resource.ResourceDriverV9): def default_assignment_driver(self): return 'sql' + def _encode_domain_id(self, ref): + if 'domain_id' in ref and ref['domain_id'] is None: + new_ref = ref.copy() + new_ref['domain_id'] = keystone_resource.NULL_DOMAIN_ID + return new_ref + else: + return ref + def _is_hidden_ref(self, ref): return ref.id == keystone_resource.NULL_DOMAIN_ID @@ -99,12 +107,19 @@ class Resource(keystone_resource.ResourceDriverV9): def list_projects_in_domain(self, domain_id): with sql.session_for_read() as session: - self._get_domain(session, domain_id) + try: + self._get_project(session, domain_id) + except exception.ProjectNotFound: + raise exception.DomainNotFound(domain_id=domain_id) query = session.query(Project) - project_refs = query.filter_by(domain_id=domain_id) + project_refs = query.filter(Project.domain_id == domain_id) return [project_ref.to_dict() for project_ref in project_refs] - def _get_children(self, session, project_ids): + def list_projects_acting_as_domain(self, hints): + hints.add_filter('is_domain', True) + return self.list_projects(hints) + + def _get_children(self, session, project_ids, domain_id=None): query = session.query(Project) query = query.filter(Project.parent_id.in_(project_ids)) project_refs = query.all() @@ -308,7 +323,7 @@ class Project(sql.ModelBase, sql.DictBase): 'parent_id', 'is_domain'] id = sql.Column(sql.String(64), primary_key=True) name = sql.Column(sql.String(64), nullable=False) - domain_id = sql.Column(sql.String(64), sql.ForeignKey('domain.id'), + domain_id = sql.Column(sql.String(64), sql.ForeignKey('project.id'), nullable=False) description = sql.Column(sql.Text()) enabled = sql.Column(sql.Boolean) diff --git a/keystone/resource/controllers.py b/keystone/resource/controllers.py index 993c57d1fe..f3c4e543da 100644 --- a/keystone/resource/controllers.py +++ b/keystone/resource/controllers.py @@ -231,19 +231,19 @@ class ProjectV3(controller.V3Controller): def create_project(self, context, project): ref = self._assign_unique_id(self._normalize_dict(project)) - if not ref.get('parent_id') and not ref.get('domain_id'): + if not ref.get('is_domain'): ref = self._normalize_domain_id(context, ref) - - if ref.get('is_domain'): - msg = _('The creation of projects acting as domains is not ' - 'allowed yet.') - raise exception.NotImplemented(msg) + # Our API requires that you specify the location in the hierarchy + # unambiguously. This could be by parent_id or, if it is a top level + # project, just by providing a domain_id. + if not ref.get('parent_id'): + ref['parent_id'] = ref.get('domain_id') initiator = notifications._get_request_audit_info(context) try: ref = self.resource_api.create_project(ref['id'], ref, initiator=initiator) - except exception.DomainNotFound as e: + except (exception.DomainNotFound, exception.ProjectNotFound) as e: raise exception.ValidationError(e) return ProjectV3.wrap_member(context, ref) diff --git a/keystone/resource/core.py b/keystone/resource/core.py index 6c0505d6db..d76243b88f 100644 --- a/keystone/resource/core.py +++ b/keystone/resource/core.py @@ -13,6 +13,7 @@ """Main entry point into the Resource service.""" import abc +import copy from oslo_config import cfg from oslo_log import log @@ -44,6 +45,16 @@ def calc_default_domain(): 'name': u'Default'} +def _get_project_from_domain(domain_ref): + """Creates a project ref from the provided domain ref.""" + project_ref = domain_ref.copy() + project_ref['is_domain'] = True + project_ref['domain_id'] = None + project_ref['parent_id'] = None + + return project_ref + + @dependency.provider('resource_api') @dependency.requires('assignment_api', 'credential_api', 'domain_config_api', 'identity_api', 'revoke_api') @@ -84,12 +95,17 @@ class Manager(manager.Manager): def _assert_max_hierarchy_depth(self, project_id, parents_list=None): if parents_list is None: parents_list = self.list_project_parents(project_id) - max_depth = CONF.max_project_tree_depth + # NOTE(henry-nash): In upgrading to a scenario where domains are + # represented as projects acting as domains, we will effectively + # increase the depth of any existing project hierarchy by one. To avoid + # pushing any existing hierarchies over the limit, we add one to the + # maximum depth allowed, as specified in the configuration file. + max_depth = CONF.max_project_tree_depth + 1 if self._get_hierarchy_depth(parents_list) > max_depth: raise exception.ForbiddenNotSecurity( _('Max hierarchy depth reached for %s branch.') % project_id) - def _assert_is_domain_project_constraints(self, project_ref, parent_ref): + def _assert_is_domain_project_constraints(self, project_ref): """Enforces specific constraints of projects that act as domains Called when is_domain is true, this method ensures that: @@ -108,17 +124,18 @@ class Manager(manager.Manager): self.assert_domain_not_federated(project_ref['id'], project_ref) - if parent_ref: + if project_ref['parent_id']: raise exception.ValidationError( message=_('only root projects are allowed to act as ' 'domains.')) - def _assert_regular_project_constraints(self, project_ref, parent_ref): + def _assert_regular_project_constraints(self, project_ref): """Enforces regular project hierarchy constraints - Called when is_domain is false, and the project must contain a valid - domain_id and may have a parent. If the parent is a regular project, - this project must be in the same domain as its parent. + Called when is_domain is false. The project must contain a valid + domain_id and parent_id. The goal of this method is to check + that the domain_id specified is consistent with the domain of its + parent. :raises keystone.exception.ValidationError: If one of the constraints was not satisfied. @@ -127,34 +144,38 @@ class Manager(manager.Manager): """ # Ensure domain_id is valid, and by inference will not be None. domain = self.get_domain(project_ref['domain_id']) + parent_ref = self.get_project(project_ref['parent_id']) - if parent_ref: - is_domain_parent = parent_ref.get('is_domain') + if parent_ref['is_domain']: + if parent_ref['id'] != domain['id']: + raise exception.ValidationError( + message=_('Cannot create project, since its parent ' + '(%(domain_id)s) is acting as a domain, ' + 'but project\'s specified parent_id ' + '(%(parent_id)s) does not match ' + 'this domain_id.') + % {'domain_id': domain['id'], + 'parent_id': parent_ref['id']}) + else: parent_domain_id = parent_ref.get('domain_id') - - if not is_domain_parent and parent_domain_id != domain['id']: + if parent_domain_id != domain['id']: raise exception.ValidationError( message=_('Cannot create project, since it specifies ' 'its owner as domain %(domain_id)s, but ' 'specifies a parent in a different domain ' '(%(parent_domain_id)s).') % {'domain_id': domain['id'], - 'parent_domain_id': parent_ref.get('domain_id')}) + 'parent_domain_id': parent_domain_id}) - def _find_domain_id(self, project_ref): - parent_ref = self.get_project(project_ref['parent_id']) - return parent_ref['domain_id'] - - def _enforce_project_constraints(self, project_ref, parent_id): - parent_ref = self.get_project(parent_id) if parent_id else None + def _enforce_project_constraints(self, project_ref): if project_ref.get('is_domain'): - self._assert_is_domain_project_constraints(project_ref, parent_ref) + self._assert_is_domain_project_constraints(project_ref) else: - self._assert_regular_project_constraints(project_ref, parent_ref) - - # The whole hierarchy must be enabled - if parent_id: + self._assert_regular_project_constraints(project_ref) + # The whole hierarchy (upwards) must be enabled + parent_id = project_ref['parent_id'] parents_list = self.list_project_parents(parent_id) + parent_ref = self.get_project(parent_id) parents_list.append(parent_ref) for ref in parents_list: if not ref.get('enabled', True): @@ -185,7 +206,7 @@ class Manager(manager.Manager): 'within a domain with the same name : %s' ) % project['name'] - def create_project(self, project_id, project, initiator=None): + def _create_project(self, project_id, project, initiator=None): project = project.copy() if (CONF.resource.project_name_url_safe != 'off' and @@ -196,22 +217,18 @@ class Manager(manager.Manager): project.setdefault('enabled', True) project['enabled'] = clean.project_enabled(project['enabled']) project.setdefault('description', '') - project.setdefault('domain_id', None) - project.setdefault('parent_id', None) - project.setdefault('is_domain', False) - # If this is a top level project acting as a domain, the project_id + # For regular projects, the controller will ensure we have a valid + # domain_id. For projects acting as a domain, the project_id # is, effectively, the domain_id - and for such projects we don't # bother to store a copy of it in the domain_id attribute. - # If this is a non-domain top level project,then the domain_id must - # have been specified by the caller (this is checked as part of the - # project constraints) - domain_id = project.get('domain_id') - parent_id = project.get('parent_id') - if not domain_id and parent_id: - project['domain_id'] = self._find_domain_id(project) + project.setdefault('domain_id', None) + project.setdefault('parent_id', None) + if not project['parent_id']: + project['parent_id'] = project['domain_id'] + project.setdefault('is_domain', False) - self._enforce_project_constraints(project, parent_id) + self._enforce_project_constraints(project) # We leave enforcing name uniqueness to the underlying driver (instead # of doing it in code in the project_constraints above), so as to allow @@ -224,13 +241,21 @@ class Manager(manager.Manager): type='project', details=self._generate_project_name_conflict_msg(project)) - notifications.Audit.created(self._PROJECT, project_id, initiator) + if project.get('is_domain'): + notifications.Audit.created(self._DOMAIN, project_id, initiator) + else: + notifications.Audit.created(self._PROJECT, project_id, initiator) if MEMOIZE.should_cache(ret): self.get_project.set(ret, self, project_id) self.get_project_by_name.set(ret, self, ret['name'], ret['domain_id']) return ret + def create_project(self, project_id, project, initiator=None): + project = self._create_project(project_id, project, initiator) + + return project + def assert_domain_enabled(self, domain_id, domain=None): """Assert the Domain is enabled. @@ -269,7 +294,10 @@ class Manager(manager.Manager): """ if project is None: project = self.get_project(project_id) - self.assert_domain_enabled(domain_id=project['domain_id']) + # If it's a regular project (i.e. it has a domain_id), we need to make + # sure the domain itself is not disabled + if project['domain_id']: + self.assert_domain_enabled(domain_id=project['domain_id']) if not project.get('enabled', True): raise AssertionError(_('Project is disabled: %s') % project_id) @@ -300,17 +328,28 @@ class Manager(manager.Manager): subtree_enabled = [ref.get('enabled', True) for ref in subtree_list] return (not any(subtree_enabled)) - def update_project(self, project_id, project, initiator=None, - cascade=False): + def _update_project(self, project_id, project, initiator=None, + cascade=False): # Use the driver directly to prevent using old cached value. original_project = self.driver.get_project(project_id) project = project.copy() - if (CONF.resource.project_name_url_safe != 'off' and + if original_project['is_domain']: + domain = self._get_domain_from_project(original_project) + self.assert_domain_not_federated(project_id, domain) + if 'enabled' in domain: + domain['enabled'] = clean.domain_enabled(domain['enabled']) + url_safe_option = CONF.resource.domain_name_url_safe + exception_entity = 'Domain' + else: + url_safe_option = CONF.resource.project_name_url_safe + exception_entity = 'Project' + + if (url_safe_option != 'off' and 'name' in project and project['name'] != original_project['name'] and utils.is_not_url_safe(project['name'])): - self._raise_reserved_character_exception('Project', + self._raise_reserved_character_exception(exception_entity, project['name']) parent_id = original_project.get('parent_id') @@ -333,19 +372,17 @@ class Manager(manager.Manager): # the middle of the hierarchy creates an inconsistent project # hierarchy. if update_domain: - # NOTE(henrynash): As soon as projects start to act as domains, - # this check will no longer be valid, since a regular top level - # project will have a parent, the project that acts as a domain. - # Hence, this check must be changed. - is_root_project = original_project['parent_id'] is None - if not is_root_project: - raise exception.ValidationError( - message=_('Update of domain_id is only allowed for ' - 'root projects.')) if original_project['is_domain']: raise exception.ValidationError( message=_('Update of domain_id of projects acting as ' 'domains is not allowed.')) + parent_project = ( + self.driver.get_project(original_project['parent_id'])) + is_root_project = parent_project['is_domain'] + if not is_root_project: + raise exception.ValidationError( + message=_('Update of domain_id is only allowed for ' + 'root projects.')) subtree_list = self.list_projects_in_subtree(project_id) if subtree_list: raise exception.ValidationError( @@ -392,6 +429,13 @@ class Manager(manager.Manager): details=self._generate_project_name_conflict_msg(project)) notifications.Audit.updated(self._PROJECT, project_id, initiator) + if original_project['is_domain']: + notifications.Audit.updated(self._DOMAIN, project_id, initiator) + # If the domain is being disabled, issue the disable notification + # as well + if original_project_enabled and not project_enabled: + self._disable_domain(project_id) + self.get_project.invalidate(self, project_id) self.get_project_by_name.invalidate(self, original_project['name'], original_project['domain_id']) @@ -429,6 +473,15 @@ class Manager(manager.Manager): self.driver.update_project(child['id'], child) + def update_project(self, project_id, project, initiator=None, + cascade=False): + ret = self._update_project(project_id, project, initiator, cascade) + if ret['is_domain']: + self.get_domain.invalidate(self, project_id) + self.get_domain_by_name.invalidate(self, ret['name']) + + return ret + def _pre_delete_cleanup_project(self, project_id, project, initiator=None): project_user_ids = ( self.assignment_api.list_user_ids_for_project(project_id)) @@ -450,6 +503,13 @@ class Manager(manager.Manager): assignment.COMPUTED_ASSIGNMENTS_REGION.invalidate() def delete_project(self, project_id, initiator=None, cascade=False): + project = self.driver.get_project(project_id) + if project.get('is_domain'): + self.delete_domain(project_id, initiator) + else: + self._delete_project(project_id, initiator, cascade) + + def _delete_project(self, project_id, initiator=None, cascade=False): # Use the driver directly to prevent using old cached value. project = self.driver.get_project(project_id) if project['is_domain'] and project['enabled']: @@ -626,37 +686,89 @@ class Manager(manager.Manager): project_id, _projects_indexed_by_parent(subtree_list)) return subtree_as_ids + def list_domains_from_ids(self, domain_ids): + """List domains for the provided list of ids. + + :param domain_ids: list of ids + + :returns: a list of domain_refs. + + This method is used internally by the assignment manager to bulk read + a set of domains given their ids. + + """ + # Retrieve the projects acting as domains get their correspondent + # domains + projects = self.list_projects_from_ids(domain_ids) + domains = [self._get_domain_from_project(project) + for project in projects] + + return domains + @MEMOIZE def get_domain(self, domain_id): - return self.driver.get_domain(domain_id) + try: + # Retrieve the corresponding project that acts as a domain + project = self.driver.get_project(domain_id) + except exception.ProjectNotFound: + raise exception.DomainNotFound(domain_id=domain_id) + + # Return its correspondent domain + return self._get_domain_from_project(project) @MEMOIZE def get_domain_by_name(self, domain_name): - return self.driver.get_domain_by_name(domain_name) + try: + # Retrieve the corresponding project that acts as a domain + project = self.driver.get_project_by_name(domain_name, + domain_id=None) + except exception.ProjectNotFound: + raise exception.DomainNotFound(domain_id=domain_name) + + # Return its correspondent domain + return self._get_domain_from_project(project) + + def _get_domain_from_project(self, project_ref): + """Creates a domain ref from a project ref. + + Based on the provided project ref, create a domain ref, so that the + result can be returned in response to a domain API call. + """ + if not project_ref['is_domain']: + LOG.error(_LE('Asked to convert a non-domain project into a ' + 'domain - Domain: %(domain_id)s, Project ID: ' + '%(id)s, Project Name: %(project_name)s'), + {'domain_id': project_ref['domain_id'], + 'id': project_ref['id'], + 'project_name': project_ref['name']}) + raise exception.DomainNotFound(domain_id=project_ref['id']) + + domain_ref = project_ref.copy() + # As well as the project specific attributes that we need to remove, + # there is an old compatibility issue in that update project (as well + # as extracting an extra attributes), also includes a copy of the + # actual extra dict as well - something that update domain does not do. + for k in ['parent_id', 'domain_id', 'is_domain', 'extra']: + domain_ref.pop(k, None) + + return domain_ref def create_domain(self, domain_id, domain, initiator=None): - if (not self.identity_api.multiple_domains_supported and - domain_id != CONF.identity.default_domain_id): - raise exception.Forbidden(_('Multiple domains are not supported')) if (CONF.resource.domain_name_url_safe != 'off' and utils.is_not_url_safe(domain['name'])): self._raise_reserved_character_exception('Domain', domain['name']) + project_from_domain = _get_project_from_domain(domain) + is_domain_project = self._create_project( + domain_id, project_from_domain, initiator) - self.assert_domain_not_federated(domain_id, domain) - domain.setdefault('enabled', True) - domain['enabled'] = clean.domain_enabled(domain['enabled']) - ret = self.driver.create_domain(domain_id, domain) - - notifications.Audit.created(self._DOMAIN, domain_id, initiator) - - if MEMOIZE.should_cache(ret): - self.get_domain.set(ret, self, domain_id) - self.get_domain_by_name.set(ret, self, ret['name']) - return ret + return self._get_domain_from_project(is_domain_project) @manager.response_truncated def list_domains(self, hints=None): - return self.driver.list_domains(hints or driver_hints.Hints()) + projects = self.list_projects_acting_as_domain(hints) + domains = [self._get_domain_from_project(project) + for project in projects] + return domains @notifications.disabled(_DOMAIN, public=False) def _disable_domain(self, domain_id): @@ -672,31 +784,30 @@ class Manager(manager.Manager): pass def update_domain(self, domain_id, domain, initiator=None): + # TODO(henry-nash): We shouldn't have to check for the federated domain + # here as well as _update_project, but currently our tests assume the + # checks are done in a specific order. The tests should be refactored. self.assert_domain_not_federated(domain_id, domain) - # Use the driver directly to prevent using old cached value. - original_domain = self.driver.get_domain(domain_id) - if (CONF.resource.domain_name_url_safe != 'off' and - 'name' in domain and domain['name'] != original_domain['name'] and - utils.is_not_url_safe(domain['name'])): - self._raise_reserved_character_exception('Domain', domain['name']) - if 'enabled' in domain: - domain['enabled'] = clean.domain_enabled(domain['enabled']) - ret = self.driver.update_domain(domain_id, domain) - notifications.Audit.updated(self._DOMAIN, domain_id, initiator) - # disable owned users & projects when the API user specifically set - # enabled=False - if (original_domain.get('enabled', True) and - not domain.get('enabled', True)): - notifications.Audit.disabled(self._DOMAIN, domain_id, initiator, - public=False) + project = _get_project_from_domain(domain) + try: + original_domain = self.driver.get_project(domain_id) + project = self._update_project(domain_id, project, initiator) + except exception.ProjectNotFound: + raise exception.DomainNotFound(domain_id=domain_id) + domain_from_project = self._get_domain_from_project(project) self.get_domain.invalidate(self, domain_id) self.get_domain_by_name.invalidate(self, original_domain['name']) - return ret + + return domain_from_project def delete_domain(self, domain_id, initiator=None): - # Use the driver directly to prevent using old cached value. - domain = self.driver.get_domain(domain_id) + # Use the driver directly to get the project that acts as a domain and + # prevent using old cached value. + try: + domain = self.driver.get_project(domain_id) + except exception.ProjectNotFound: + raise exception.DomainNotFound(domain_id=domain_id) # To help avoid inadvertent deletes, we insist that the domain # has been previously disabled. This also prevents a user deleting @@ -708,6 +819,7 @@ class Manager(manager.Manager): 'first.')) self._delete_domain_contents(domain_id) + self._delete_project(domain_id, initiator) # Delete any database stored domain config self.domain_config_api.delete_config_options(domain_id) self.domain_config_api.delete_config_options(domain_id, sensitive=True) @@ -719,7 +831,6 @@ class Manager(manager.Manager): # other domains - so we should delete these here by making a call # to the backend to delete all assignments for this domain. # (see Bug #1277847) - self.driver.delete_domain(domain_id) notifications.Audit.deleted(self._DOMAIN, domain_id, initiator) self.get_domain.invalidate(self, domain_id) self.get_domain_by_name.invalidate(self, domain['name']) @@ -752,7 +863,7 @@ class Manager(manager.Manager): _delete_projects(proj, projects, examined) try: - self.delete_project(project['id']) + self.delete_project(project['id'], initiator=None) except exception.ProjectNotFound: LOG.debug(('Project %(projectid)s not found when ' 'deleting domain contents for %(domainid)s, ' @@ -763,7 +874,7 @@ class Manager(manager.Manager): proj_refs = self.list_projects_in_domain(domain_id) # Deleting projects recursively - roots = [x for x in proj_refs if x.get('parent_id') is None] + roots = [x for x in proj_refs if x.get('parent_id') == domain_id] examined = set() for project in roots: _delete_projects(project, proj_refs, examined) @@ -778,6 +889,10 @@ class Manager(manager.Manager): def list_projects_in_domain(self, domain_id): return self.driver.list_projects_in_domain(domain_id) + def list_projects_acting_as_domain(self, hints=None): + return self.driver.list_projects_acting_as_domain( + hints or driver_hints.Hints()) + @MEMOIZE def get_project(self, project_id): return self.driver.get_project(project_id) @@ -803,8 +918,9 @@ class Manager(manager.Manager): # class to the V8 class. Do not modify any of the method signatures in the Base # class - changes should only be made in the V8 and subsequent classes. -# Starting with V9, some drivers support a null domain_id by storing a special -# value in its place. +# Starting with V9, some drivers use a special value to represent a domain_id +# of None. See comment in Project class of resource/backends/sql.py for more +# details. NULL_DOMAIN_ID = '<>' @@ -814,19 +930,6 @@ class ResourceDriverBase(object): def _get_list_limit(self): return CONF.resource.list_limit or CONF.list_limit - def _encode_domain_id(self, ref): - if 'domain_id' in ref and ref['domain_id'] is None: - new_ref = ref.copy() - new_ref['domain_id'] = NULL_DOMAIN_ID - return new_ref - else: - return ref - - def _decode_domain_id(self, ref): - if ref['domain_id'] == NULL_DOMAIN_ID: - ref['domain_id'] = None - return ref - # domain crud @abc.abstractmethod def create_domain(self, domain_id, domain): @@ -1126,6 +1229,18 @@ class ResourceDriverV9(ResourceDriverBase): """ raise exception.NotImplemented() # pragma: no cover + @abc.abstractmethod + def list_projects_acting_as_domain(self, hints): + """List all projects acting as domains. + + :param hints: filter hints which the driver should + implement if at all possible. + + :returns: a list of project_refs or an empty list. + + """ + raise exception.NotImplemented() # pragma: no cover + class V9ResourceWrapperForV8Driver(ResourceDriverV9): """Wrapper class to supported a V8 legacy driver. @@ -1149,15 +1264,12 @@ class V9ResourceWrapperForV8Driver(ResourceDriverV9): This wrapper contains the following support for newer manager code: - - The current manager code expects to be able to store a project domain_id - with a value of None, something that earlier drivers may not support. - Hence we pre and post process the domain_id value to substitute it for - a special value (defined by NULL_DOMAIN_ID). In fact the V9 and later - drivers use this same algorithm internally (to ensure uniqueness - constraints including domain_id can still work), so, although not a - specific goal of our legacy driver support, using this same value may - ease any customers who want to migrate from a legacy customer driver to - the standard community driver. + - The current manager code expects domains to be represented as projects + acting as domains, something that may not be possible in a legacy driver. + Hence the wrapper will map any calls for projects acting as a domain back + onto the driver domain methods. The caveat for this, is that this assumes + that there can not be a clash between a project_id and a domain_id, in + which case it may not be able to locate the correct entry. """ @@ -1169,13 +1281,27 @@ class V9ResourceWrapperForV8Driver(ResourceDriverV9): def __init__(self, wrapped_driver): self.driver = wrapped_driver + def _get_domain_from_project(self, project_ref): + """Creates a domain ref from a project ref. + + Based on the provided project ref (or partial ref), creates a + domain ref, so that the result can be passed to the driver + domain methods. + """ + domain_ref = project_ref.copy() + for k in ['parent_id', 'domain_id', 'is_domain']: + domain_ref.pop(k, None) + return domain_ref + def get_project_by_name(self, project_name, domain_id): if domain_id is None: - actual_domain_id = NULL_DOMAIN_ID + try: + domain_ref = self.driver.get_domain_by_name(project_name) + return _get_project_from_domain(domain_ref) + except exception.DomainNotFound: + raise exception.ProjectNotFound(project_id=project_name) else: - actual_domain_id = domain_id - return self._decode_domain_id( - self.driver.get_project_by_name(project_name, actual_domain_id)) + return self.driver.get_project_by_name(project_name, domain_id) def create_domain(self, domain_id, domain): return self.driver.create_domain(domain_id, domain) @@ -1199,54 +1325,126 @@ class V9ResourceWrapperForV8Driver(ResourceDriverV9): self.driver.delete_domain(domain_id) def create_project(self, project_id, project): - new_project = self._encode_domain_id(project) - return self._decode_domain_id( - self.driver.create_project(project_id, new_project)) + if project['is_domain']: + new_domain = self._get_domain_from_project(project) + domain_ref = self.driver.create_domain(project_id, new_domain) + return _get_project_from_domain(domain_ref) + else: + return self.driver.create_project(project_id, project) def list_projects(self, hints): + """List projects and/or domains. + + We use the hints filter to determine whether we are listing projects, + domains or both. + + If the filter includes domain_id==None, then we should only list + domains (convert to a project acting as a domain) since regular + projcets always have a non-None value for domain_id. + + Likewise, if the filter includes domain_id==, then we + should only list projects. + + If there is no domain_id filter, then we need to do a combained listing + of domains and projects, converting domains to projects acting as a + domain. + + """ + domain_listing_filter = None for f in hints.filters: - if (f['name'] == 'domain_id' and f['value'] is None): - f['value'] = NULL_DOMAIN_ID - refs = self.driver.list_projects(hints) - # We can't assume the driver was able to satisfy any given filter, - # so check that the domain_id filter isn't still in there. If it is, - # see if we need to re-substitute the None value. This will allow - # an unsatisfied filter to be processed by the controller wrapper. - for f in hints.filters: - if (f['name'] == 'domain_id' and f['value'] == NULL_DOMAIN_ID): - f['value'] = None - return [self._decode_domain_id(p) for p in refs] + if (f['name'] == 'domain_id'): + domain_listing_filter = f + + if domain_listing_filter is not None: + if domain_listing_filter['value'] is not None: + proj_list = self.driver.list_projects(hints) + else: + domains = self.driver.list_domains(hints) + proj_list = [_get_project_from_domain(p) for p in domains] + hints.filters.remove(domain_listing_filter) + return proj_list + else: + # No domain_id filter, so combine domains and projects. Although + # we hand any remaining filters into each driver, since each filter + # might need to be carried out more than once, we use copies of the + # filters, allowing the original filters to be passed back up to + # controller level where a final filter will occur. + local_hints = copy.deepcopy(hints) + proj_list = self.driver.list_projects(local_hints) + local_hints = copy.deepcopy(hints) + domains = self.driver.list_domains(local_hints) + for domain in domains: + proj_list.append(_get_project_from_domain(domain)) + return proj_list def list_projects_from_ids(self, project_ids): - return [self._decode_domain_id(p) - for p in self.driver.list_projects_from_ids(project_ids)] + return [self.get_project(id) for id in project_ids] def list_project_ids_from_domain_ids(self, domain_ids): return self.driver.list_project_ids_from_domain_ids(domain_ids) def list_projects_in_domain(self, domain_id): - # Projects within a domain never have a null domain_id, so no need to - # post-process the output - return self.driver.list_projects_in_domain(domain_id) + return self.driver.list_projects_in_domain(domain_id) def get_project(self, project_id): - return self._decode_domain_id( - self.driver.get_project(project_id)) + try: + domain_ref = self.driver.get_domain(project_id) + return _get_project_from_domain(domain_ref) + except exception.DomainNotFound: + return self.driver.get_project(project_id) + + def _is_domain(self, project_id): + ref = self.get_project(project_id) + return ref.get('is_domain', False) def update_project(self, project_id, project): - update_project = self._encode_domain_id(project) - return self._decode_domain_id( - self.driver.update_project(project_id, update_project)) + if self._is_domain(project_id): + update_domain = self._get_domain_from_project(project) + domain_ref = self.driver.update_domain(project_id, update_domain) + return _get_project_from_domain(domain_ref) + else: + return self.driver.update_project(project_id, project) def delete_project(self, project_id): - self.driver.delete_project(project_id) + if self._is_domain(project_id): + try: + self.driver.delete_domain(project_id) + except exception.DomainNotFound: + raise exception.ProjectNotFound(project_id=project_id) + else: + self.driver.delete_project(project_id) def delete_projects_from_ids(self, project_ids): raise exception.NotImplemented() # pragma: no cover def list_project_parents(self, project_id): - return [self._decode_domain_id(p) - for p in self.driver.list_project_parents(project_id)] + """List a project's ancestors. + + The current manager expects the ancestor tree to end with the project + acting as the domain (since that's now the top of the tree), but a + legacy driver will not have that top project in their projects table, + since it's still in the domain table. Hence we lift the algorithm for + traversing up the tree from the driver to here, so that our version of + get_project() is called, which will fetch the "project" from the right + table. + + """ + project = self.get_project(project_id) + parents = [] + examined = set() + while project.get('parent_id') is not None: + if project['id'] in examined: + msg = _LE('Circular reference or a repeated ' + 'entry found in projects hierarchy - ' + '%(project_id)s.') + LOG.error(msg, {'project_id': project['id']}) + return + + examined.add(project['id']) + parent_project = self.get_project(project['parent_id']) + parents.append(parent_project) + project = parent_project + return parents def list_projects_in_subtree(self, project_id): return self.driver.list_projects_in_subtree(project_id) @@ -1254,6 +1452,10 @@ class V9ResourceWrapperForV8Driver(ResourceDriverV9): def is_leaf_project(self, project_id): return self.driver.is_leaf_project(project_id) + def list_projects_acting_as_domain(self, hints): + refs = self.driver.list_domains(hints) + return [_get_project_from_domain(p) for p in refs] + Driver = manager.create_legacy_driver(ResourceDriverV8) diff --git a/keystone/resource/schema.py b/keystone/resource/schema.py index 61daa8f3aa..7e2cd66756 100644 --- a/keystone/resource/schema.py +++ b/keystone/resource/schema.py @@ -16,9 +16,7 @@ from keystone.common.validation import parameter_types _project_properties = { 'description': validation.nullable(parameter_types.description), - # NOTE(lbragstad): domain_id isn't nullable according to some backends. - # The identity-api should be updated to be consistent with the - # implementation. + # NOTE(htruta): domain_id is nullable for projects acting as a domain. 'domain_id': validation.nullable(parameter_types.id_string), 'enabled': parameter_types.boolean, 'is_domain': parameter_types.boolean, diff --git a/keystone/tests/unit/default_fixtures.py b/keystone/tests/unit/default_fixtures.py index e5ca236e2a..19c5ef19d0 100644 --- a/keystone/tests/unit/default_fixtures.py +++ b/keystone/tests/unit/default_fixtures.py @@ -24,7 +24,7 @@ TENANTS = [ 'domain_id': DEFAULT_DOMAIN_ID, 'description': 'description', 'enabled': True, - 'parent_id': None, + 'parent_id': DEFAULT_DOMAIN_ID, 'is_domain': False, }, { 'id': 'baz', @@ -32,7 +32,7 @@ TENANTS = [ 'domain_id': DEFAULT_DOMAIN_ID, 'description': 'description', 'enabled': True, - 'parent_id': None, + 'parent_id': DEFAULT_DOMAIN_ID, 'is_domain': False, }, { 'id': 'mtu', @@ -40,7 +40,7 @@ TENANTS = [ 'description': 'description', 'enabled': True, 'domain_id': DEFAULT_DOMAIN_ID, - 'parent_id': None, + 'parent_id': DEFAULT_DOMAIN_ID, 'is_domain': False, }, { 'id': 'service', @@ -48,7 +48,7 @@ TENANTS = [ 'description': 'description', 'enabled': True, 'domain_id': DEFAULT_DOMAIN_ID, - 'parent_id': None, + 'parent_id': DEFAULT_DOMAIN_ID, 'is_domain': False, } ] diff --git a/keystone/tests/unit/test_backend.py b/keystone/tests/unit/test_backend.py index f5af1495c9..8fb6cd9b13 100644 --- a/keystone/tests/unit/test_backend.py +++ b/keystone/tests/unit/test_backend.py @@ -455,6 +455,9 @@ class AssignmentTestHelperMixin(object): class IdentityTests(AssignmentTestHelperMixin): + + domain_count = len(default_fixtures.DOMAINS) + def _get_domain_fixture(self): domain = unit.new_domain_ref() self.resource_api.create_domain(domain['id'], domain) @@ -581,6 +584,30 @@ class IdentityTests(AssignmentTestHelperMixin): CONF.identity.default_domain_id) self.assertDictEqual(self.tenant_bar, tenant_ref) + @unit.skip_if_no_multiple_domains_support + def test_get_project_by_name_for_project_acting_as_a_domain(self): + """Tests get_project_by_name works when the domain_id is None.""" + project = unit.new_project_ref( + domain_id=CONF.identity.default_domain_id, is_domain=False) + project = self.resource_api.create_project(project['id'], project) + + self.assertRaises(exception.ProjectNotFound, + self.resource_api.get_project_by_name, + project['name'], + None) + + # Test that querying with domain_id as None will find the project + # acting as a domain, even if it's name is the same as the regular + # project above. + project2 = unit.new_project_ref(is_domain=True, + name=project['name']) + project2 = self.resource_api.create_project(project2['id'], project2) + + project_ref = self.resource_api.get_project_by_name( + project2['name'], None) + + self.assertEqual(project2, project_ref) + def test_get_project_by_name_returns_not_found(self): self.assertRaises(exception.ProjectNotFound, self.resource_api.get_project_by_name, @@ -908,9 +935,9 @@ class IdentityTests(AssignmentTestHelperMixin): self.resource_api.create_domain(domain1['id'], domain1) domain2 = unit.new_domain_ref() self.resource_api.create_domain(domain2['id'], domain2) - root_project = unit.new_project_ref(domain_id=domain1['id'], - parent_id=None) - self.resource_api.create_project(root_project['id'], root_project) + root_project = unit.new_project_ref(domain_id=domain1['id']) + root_project = self.resource_api.create_project(root_project['id'], + root_project) root_project['domain_id'] = domain2['id'] self.resource_api.update_project(root_project['id'], root_project) @@ -922,8 +949,7 @@ class IdentityTests(AssignmentTestHelperMixin): def test_update_domain_id_project_is_domain_fails(self): other_domain = unit.new_domain_ref() self.resource_api.create_domain(other_domain['id'], other_domain) - project = unit.new_project_ref(is_domain=True, - domain_id=self.domain_default['id']) + project = unit.new_project_ref(is_domain=True) self.resource_api.create_project(project['id'], project) project['domain_id'] = other_domain['id'] @@ -2563,13 +2589,11 @@ class IdentityTests(AssignmentTestHelperMixin): self.assertIn(domain2['id'], domain_ids) def test_list_projects(self): - projects = self.resource_api.list_projects() - self.assertEqual(4, len(projects)) - project_ids = [] - for project in projects: - project_ids.append(project.get('id')) - self.assertIn(self.tenant_bar['id'], project_ids) - self.assertIn(self.tenant_baz['id'], project_ids) + project_refs = self.resource_api.list_projects() + project_count = len(default_fixtures.TENANTS) + self.domain_count + self.assertEqual(project_count, len(project_refs)) + for project in default_fixtures.TENANTS: + self.assertIn(project, project_refs) def test_list_projects_with_multiple_filters(self): # Create a project @@ -2602,12 +2626,40 @@ class IdentityTests(AssignmentTestHelperMixin): project_ids = ([x['id'] for x in self.resource_api.list_projects_in_domain( CONF.identity.default_domain_id)]) - self.assertEqual(4, len(project_ids)) + # Only the projects from the default fixtures are expected, since + # filtering by domain does not include any project that acts as a + # domain. + self.assertThat( + project_ids, matchers.HasLength(len(default_fixtures.TENANTS))) self.assertIn(self.tenant_bar['id'], project_ids) self.assertIn(self.tenant_baz['id'], project_ids) self.assertIn(self.tenant_mtu['id'], project_ids) self.assertIn(self.tenant_service['id'], project_ids) + @unit.skip_if_no_multiple_domains_support + def test_list_projects_acting_as_domain(self): + initial_domains = self.resource_api.list_domains() + + # Creating 5 projects that act as domains + new_projects_acting_as_domains = [] + for i in range(5): + project = unit.new_project_ref(is_domain=True) + project = self.resource_api.create_project(project['id'], project) + new_projects_acting_as_domains.append(project) + + # Creating a few regular project to ensure it doesn't mess with the + # ones that act as domains + self._create_projects_hierarchy(hierarchy_size=2) + + projects = self.resource_api.list_projects_acting_as_domain() + expected_number_projects = ( + len(initial_domains) + len(new_projects_acting_as_domains)) + self.assertEqual(expected_number_projects, len(projects)) + for project in new_projects_acting_as_domains: + self.assertIn(project, projects) + for domain in initial_domains: + self.assertIn(domain['id'], [p['id'] for p in projects]) + @unit.skip_if_no_multiple_domains_support def test_list_projects_for_alternate_domain(self): domain1 = unit.new_domain_ref() @@ -2656,7 +2708,6 @@ class IdentityTests(AssignmentTestHelperMixin): projects = [project] for i in range(1, hierarchy_size): new_project = unit.new_project_ref(parent_id=project_id, - is_domain=is_domain, domain_id=domain_id) self.resource_api.create_project(new_project['id'], new_project) @@ -2667,11 +2718,10 @@ class IdentityTests(AssignmentTestHelperMixin): @unit.skip_if_no_multiple_domains_support def test_create_domain_with_project_api(self): - project = unit.new_project_ref( - domain_id=CONF.identity.default_domain_id, is_domain=True) + project = unit.new_project_ref(is_domain=True) ref = self.resource_api.create_project(project['id'], project) self.assertTrue(ref['is_domain']) - self.assertEqual(CONF.identity.default_domain_id, ref['domain_id']) + self.resource_api.get_domain(ref['id']) @unit.skip_if_no_multiple_domains_support def test_project_as_a_domain_uniqueness_constraints(self): @@ -2737,7 +2787,6 @@ class IdentityTests(AssignmentTestHelperMixin): self.assertEqual(project['id'], ref['domain_id']) @unit.skip_if_no_multiple_domains_support - @test_utils.wip('waiting for projects acting as domains implementation') def test_delete_domain_with_project_api(self): project = unit.new_project_ref(domain_id=None, is_domain=True) @@ -2746,8 +2795,8 @@ class IdentityTests(AssignmentTestHelperMixin): # Check that a corresponding domain was created self.resource_api.get_domain(project['id']) - # Try to delete is_domain project that is enabled - self.assertRaises(exception.ValidationError, + # Try to delete the enabled project that acts as a domain + self.assertRaises(exception.ForbiddenNotSecurity, self.resource_api.delete_project, project['id']) @@ -2840,7 +2889,6 @@ class IdentityTests(AssignmentTestHelperMixin): # The domain_id should be set to the parent domain_id self.assertEqual(project['domain_id'], ref['domain_id']) - @test_utils.wip('waiting for projects acting as domains implementation') def test_create_project_with_domain_id_and_without_parent_id(self): # First create a domain project = unit.new_project_ref(is_domain=True) @@ -2849,8 +2897,22 @@ class IdentityTests(AssignmentTestHelperMixin): sub_project = unit.new_project_ref(domain_id=project['id']) ref = self.resource_api.create_project(sub_project['id'], sub_project) - # The parent_id should be set to the domain_id + # The parent_id and domain_id should be set to the id of the project + # acting as a domain self.assertEqual(project['id'], ref['parent_id']) + self.assertEqual(project['id'], ref['domain_id']) + + def test_create_project_with_domain_id_mismatch_to_parent_domain(self): + # First create a domain + project = unit.new_project_ref(is_domain=True) + self.resource_api.create_project(project['id'], project) + # Now try to create a child with the above as its parent, but + # specifying a different domain. + sub_project = unit.new_project_ref( + parent_id=project['id'], domain_id=CONF.identity.default_domain_id) + self.assertRaises(exception.ValidationError, + self.resource_api.create_project, + sub_project['id'], sub_project) def test_check_leaf_projects(self): projects_hierarchy = self._create_projects_hierarchy() @@ -2937,7 +2999,7 @@ class IdentityTests(AssignmentTestHelperMixin): self.resource_api.create_project(project4['id'], project4) parents1 = self.resource_api.list_project_parents(project3['id']) - self.assertEqual(2, len(parents1)) + self.assertEqual(3, len(parents1)) self.assertIn(project1, parents1) self.assertIn(project2, parents1) @@ -2945,7 +3007,8 @@ class IdentityTests(AssignmentTestHelperMixin): self.assertEqual(parents1, parents2) parents = self.resource_api.list_project_parents(project1['id']) - self.assertEqual(0, len(parents)) + # It has the default domain as parent + self.assertEqual(1, len(parents)) def test_update_project_enabled_cascade(self): """Test update_project_cascade @@ -3840,13 +3903,15 @@ class IdentityTests(AssignmentTestHelperMixin): return len(self.resource_api.list_project_parents(project_id)) + 1 def test_check_hierarchy_depth(self): - # First create a hierarchy with the max allowed depth + # Should be allowed to have a hierarchy of the max depth specified + # in the config option plus one (to allow for the additional project + # acting as a domain after an upgrade) projects_hierarchy = self._create_projects_hierarchy( CONF.max_project_tree_depth) leaf_project = projects_hierarchy[CONF.max_project_tree_depth - 1] depth = self._get_hierarchy_depth(leaf_project['id']) - self.assertEqual(CONF.max_project_tree_depth, depth) + self.assertEqual(CONF.max_project_tree_depth + 1, depth) # Creating another project in the hierarchy shouldn't be allowed project = unit.new_project_ref( @@ -4074,11 +4139,15 @@ class IdentityTests(AssignmentTestHelperMixin): domain_id = domain['id'] # Create Domain self.resource_api.create_domain(domain_id, domain) + project_domain_ref = self.resource_api.get_project(domain_id) domain_ref = self.resource_api.get_domain(domain_id) + updated_project_domain_ref = copy.deepcopy(project_domain_ref) + updated_project_domain_ref['name'] = uuid.uuid4().hex updated_domain_ref = copy.deepcopy(domain_ref) - updated_domain_ref['name'] = uuid.uuid4().hex + updated_domain_ref['name'] = updated_project_domain_ref['name'] # Update domain, bypassing resource api manager - self.resource_api.driver.update_domain(domain_id, updated_domain_ref) + self.resource_api.driver.update_project(domain_id, + updated_project_domain_ref) # Verify get_domain still returns the domain self.assertDictContainsSubset( domain_ref, self.resource_api.get_domain(domain_id)) @@ -4094,12 +4163,13 @@ class IdentityTests(AssignmentTestHelperMixin): self.assertDictContainsSubset( domain_ref, self.resource_api.get_domain(domain_id)) # Make sure domain is 'disabled', bypass resource api manager - domain_ref_disabled = domain_ref.copy() - domain_ref_disabled['enabled'] = False - self.resource_api.driver.update_domain(domain_id, - domain_ref_disabled) + project_domain_ref_disabled = project_domain_ref.copy() + project_domain_ref_disabled['enabled'] = False + self.resource_api.driver.update_project(domain_id, + project_domain_ref_disabled) + self.resource_api.driver.update_project(domain_id, {'enabled': False}) # Delete domain, bypassing resource api manager - self.resource_api.driver.delete_domain(domain_id) + self.resource_api.driver.delete_project(domain_id) # Verify get_domain still returns the domain self.assertDictContainsSubset( domain_ref, self.resource_api.get_domain(domain_id)) @@ -4114,7 +4184,8 @@ class IdentityTests(AssignmentTestHelperMixin): self.resource_api.get_domain(domain_id) # Make sure domain is 'disabled', bypass resource api manager domain['enabled'] = False - self.resource_api.driver.update_domain(domain_id, domain) + self.resource_api.driver.update_project(domain_id, domain) + self.resource_api.driver.update_project(domain_id, {'enabled': False}) # Delete domain self.resource_api.delete_domain(domain_id) # verify DomainNotFound raised @@ -4740,79 +4811,6 @@ class IdentityTests(AssignmentTestHelperMixin): include_names=False) assert_does_not_contain_names(role_assign_without_names) - def test_delete_project_assignments_same_id_as_domain(self): - """Test deleting project assignments with same project and domain ID. - - Only project assignments must be deleted (i.e USER_PROJECT or - GROUP_PROJECT). - - Test plan: - * Create a project and a domain with the same ID; - * Create a user, a group and four roles; - * Grant one role to user and one to group on project; - * Grant one role to user and one to group on domain; - * Delete all project assignments; - * Domain assignments must stay intact. - """ - # Created a common ID - common_id = uuid.uuid4().hex - # Create a domain - domain = unit.new_domain_ref(id=common_id) - domain = self.resource_api.driver.create_domain(common_id, domain) - self.assertEqual(common_id, domain['id']) - # Create a project - project = unit.new_project_ref( - id=common_id, domain_id=CONF.identity.default_domain_id) - project = self.resource_api.driver.create_project(common_id, project) - self.assertEqual(common_id, project['id']) - # Create a user - user = unit.new_user_ref(domain_id=domain['id']) - user = self.identity_api.driver.create_user(user['id'], user) - # Create a group - group = unit.new_group_ref(domain_id=domain['id']) - group = self.identity_api.driver.create_group(group['id'], group) - # Create four roles - roles = [] - for _ in range(4): - role = unit.new_role_ref() - roles.append(self.role_api.create_role(role['id'], role)) - # Assign roles on domain - self.assignment_api.driver.create_grant(user_id=user['id'], - domain_id=domain['id'], - role_id=roles[0]['id']) - self.assignment_api.driver.create_grant(group_id=group['id'], - domain_id=domain['id'], - role_id=roles[1]['id']) - # Assign roles on project - self.assignment_api.driver.create_grant(user_id=user['id'], - project_id=project['id'], - role_id=roles[2]['id']) - self.assignment_api.driver.create_grant(group_id=group['id'], - project_id=project['id'], - role_id=roles[3]['id']) - # Make sure they were assigned - domain_assignments = self.assignment_api.list_role_assignments( - domain_id=domain['id']) - self.assertThat(domain_assignments, matchers.HasLength(2)) - project_assignments = self.assignment_api.list_role_assignments( - project_id=project['id'] - ) - self.assertThat(project_assignments, matchers.HasLength(2)) - # Delete project assignments - self.assignment_api.delete_project_assignments( - project_id=project['id']) - # Assert only project assignments were deleted - project_assignments = self.assignment_api.list_role_assignments( - project_id=project['id'] - ) - self.assertThat(project_assignments, matchers.HasLength(0)) - domain_assignments = self.assignment_api.list_role_assignments( - domain_id=domain['id']) - self.assertThat(domain_assignments, matchers.HasLength(2)) - # Make sure these remaining assignments are domain-related - for assignment in domain_assignments: - self.assertThat(assignment.keys(), matchers.Contains('domain_id')) - def test_delete_user_assignments_user_same_id_as_group(self): """Test deleting user assignments when user_id == group_id. diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index 7e63fb74a4..85c8e89c95 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -969,13 +969,15 @@ class BaseLDAPIdentity(test_backend.IdentityTests): def test_list_role_assignment_by_domain(self): """Multiple domain assignments are not supported.""" self.assertRaises( - (exception.Forbidden, exception.DomainNotFound), + (exception.Forbidden, exception.DomainNotFound, + exception.ValidationError), super(BaseLDAPIdentity, self).test_list_role_assignment_by_domain) def test_list_role_assignment_by_user_with_domain_group_roles(self): """Multiple domain assignments are not supported.""" self.assertRaises( - (exception.Forbidden, exception.DomainNotFound), + (exception.Forbidden, exception.DomainNotFound, + exception.ValidationError), super(BaseLDAPIdentity, self). test_list_role_assignment_by_user_with_domain_group_roles) @@ -985,10 +987,25 @@ class BaseLDAPIdentity(test_backend.IdentityTests): def test_list_role_assignment_using_sourced_groups_with_domains(self): """Multiple domain assignments are not supported.""" self.assertRaises( - (exception.Forbidden, exception.DomainNotFound), + (exception.Forbidden, exception.ValidationError, + exception.DomainNotFound), super(BaseLDAPIdentity, self). test_list_role_assignment_using_sourced_groups_with_domains) + def test_create_project_with_domain_id_and_without_parent_id(self): + """Multiple domains are not supported.""" + self.assertRaises( + exception.ValidationError, + super(BaseLDAPIdentity, self). + test_create_project_with_domain_id_and_without_parent_id) + + def test_create_project_with_domain_id_mismatch_to_parent_domain(self): + """Multiple domains are not supported.""" + self.assertRaises( + exception.ValidationError, + super(BaseLDAPIdentity, self). + test_create_project_with_domain_id_mismatch_to_parent_domain) + class LDAPIdentity(BaseLDAPIdentity, unit.TestCase): @@ -1015,7 +1032,7 @@ class LDAPIdentity(BaseLDAPIdentity, unit.TestCase): def test_configurable_allowed_project_actions(self): domain = self._get_domain_fixture() project = unit.new_project_ref(domain_id=domain['id']) - self.resource_api.create_project(project['id'], project) + project = self.resource_api.create_project(project['id'], project) project_ref = self.resource_api.get_project(project['id']) self.assertEqual(project['id'], project_ref['id']) @@ -1457,7 +1474,7 @@ class LDAPIdentity(BaseLDAPIdentity, unit.TestCase): def test_create_domain(self): domain = unit.new_domain_ref() - self.assertRaises(exception.Forbidden, + self.assertRaises(exception.ValidationError, self.resource_api.create_domain, domain['id'], domain) @@ -2682,6 +2699,18 @@ class MultiLDAPandSQLIdentity(BaseLDAPIdentity, unit.SQLDriverOverrides, base = super(BaseLDAPIdentity, self) base.test_list_role_assignment_using_sourced_groups_with_domains() + def test_create_project_with_domain_id_and_without_parent_id(self): + # With multi LDAP this method should work, so override the override + # from BaseLDAPIdentity + super(BaseLDAPIdentity, self).\ + test_create_project_with_domain_id_and_without_parent_id + + def test_create_project_with_domain_id_mismatch_to_parent_domain(self): + # With multi LDAP this method should work, so override the override + # from BaseLDAPIdentity + super(BaseLDAPIdentity, self).\ + test_create_project_with_domain_id_mismatch_to_parent_domain + class MultiLDAPandSQLIdentityDomainConfigsInSQL(MultiLDAPandSQLIdentity): """Class to test the use of domain configs stored in the database. @@ -2879,7 +2908,7 @@ class MultiLDAPandSQLIdentityDomainConfigsInSQL(MultiLDAPandSQLIdentity): # should still be able to set another domain to SQL, since we should # self heal this issue. - self.resource_api.driver.delete_domain(domain['id']) + self.resource_api.driver.delete_project(domain['id']) # Invalidate cache (so we will see the domain has gone) self.resource_api.get_domain.invalidate( self.resource_api, domain['id']) @@ -2966,6 +2995,17 @@ class DomainSpecificLDAPandSQLIdentity( self.skipTest( 'N/A: Not relevant for multi ldap testing') + def test_not_delete_domain_with_enabled_subdomains(self): + self.skipTest( + 'N/A: Not relevant for multi ldap testing') + + def test_delete_domain(self): + # With this restricted multi LDAP class, tests that use multiple + # domains and identity, are still not supported + self.assertRaises( + exception.DomainNotFound, + super(BaseLDAPIdentity, self).test_delete_domain_with_project_api) + def test_list_users(self): # Override the standard list users, since we have added an extra user # to the default domain, so the number of expected users is one more @@ -3056,6 +3096,25 @@ class DomainSpecificLDAPandSQLIdentity( base = super(BaseLDAPIdentity, self) base.test_list_role_assignments_filtered_by_role() + def test_delete_domain_with_project_api(self): + # With this restricted multi LDAP class, tests that use multiple + # domains and identity, are still not supported + self.assertRaises( + exception.DomainNotFound, + super(BaseLDAPIdentity, self).test_delete_domain_with_project_api) + + def test_create_project_with_domain_id_and_without_parent_id(self): + # With restricted multi LDAP, tests that don't use identity, but do + # required aditional domains will work + base = super(BaseLDAPIdentity, self) + base.test_create_project_with_domain_id_and_without_parent_id() + + def test_create_project_with_domain_id_mismatch_to_parent_domain(self): + # With restricted multi LDAP, tests that don't use identity, but do + # required aditional domains will work + base = super(BaseLDAPIdentity, self) + base.test_create_project_with_domain_id_mismatch_to_parent_domain() + class DomainSpecificSQLIdentity(DomainSpecificLDAPandSQLIdentity): """Class to test simplest use of domain-specific SQL driver. diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index d5d79ac483..6aed35ac53 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/keystone/tests/unit/test_backend_sql.py @@ -480,7 +480,7 @@ class SqlIdentity(SqlTests, test_backend.IdentityTests): hints = driver_hints.Hints() hints.add_filter('domain_id', None) refs = self.resource_api.list_projects(hints) - self.assertThat(refs, matchers.HasLength(2)) + self.assertThat(refs, matchers.HasLength(2 + self.domain_count)) self.assertIn(project, refs) self.assertIn(project2, refs) @@ -533,6 +533,11 @@ class SqlIdentity(SqlTests, test_backend.IdentityTests): driver.list_projects_in_domain, ref_id) + project_ids = [ + x['id'] for x in + driver.list_projects_acting_as_domain(driver_hints.Hints())] + self.assertNotIn(ref_id, project_ids) + projects = driver.list_projects_in_subtree(ref_id) self.assertThat(projects, matchers.HasLength(0)) diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py index afc8e23184..5a0ea338ba 100644 --- a/keystone/tests/unit/test_sql_upgrade.py +++ b/keystone/tests/unit/test_sql_upgrade.py @@ -832,6 +832,99 @@ class SqlUpgradeTests(SqlMigrateBase): implied_roles = self.role_api.list_implied_roles(role_id_list[0]) self.assertThat(implied_roles, matchers.HasLength(0)) + def test_domain_as_project_upgrade(self): + + def _populate_domain_and_project_tables(session): + # Three domains, with various different attributes + self.domains = [{'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'enabled': True, + 'extra': {'description': uuid.uuid4().hex, + 'another_attribute': True}}, + {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'enabled': True, + 'extra': {'description': uuid.uuid4().hex}}, + {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'enabled': False}] + # Four projects, two top level, two children + self.projects = [] + self.projects.append(unit.new_project_ref( + domain_id=self.domains[0]['id'], + parent_id=None)) + self.projects.append(unit.new_project_ref( + domain_id=self.domains[0]['id'], + parent_id=self.projects[0]['id'])) + self.projects.append(unit.new_project_ref( + domain_id=self.domains[1]['id'], + parent_id=None)) + self.projects.append(unit.new_project_ref( + domain_id=self.domains[1]['id'], + parent_id=self.projects[2]['id'])) + + for domain in self.domains: + this_domain = domain.copy() + if 'extra' in this_domain: + this_domain['extra'] = json.dumps(this_domain['extra']) + self.insert_dict(session, 'domain', this_domain) + for project in self.projects: + self.insert_dict(session, 'project', project) + + def _check_projects(projects): + + def _assert_domain_matches_project(project): + for domain in self.domains: + if project.id == domain['id']: + self.assertEqual(domain['name'], project.name) + self.assertEqual(domain['enabled'], project.enabled) + if domain['id'] == self.domains[0]['id']: + self.assertEqual(domain['extra']['description'], + project.description) + self.assertEqual({'another_attribute': True}, + json.loads(project.extra)) + elif domain['id'] == self.domains[1]['id']: + self.assertEqual(domain['extra']['description'], + project.description) + self.assertEqual({}, json.loads(project.extra)) + + # We had domains 3 we created, which should now be projects acting + # as domains, To this we add the 4 original projects, plus the root + # of all domains row. + self.assertEqual(8, projects.count()) + + project_ids = [] + for project in projects: + if project.is_domain: + self.assertEqual(NULL_DOMAIN_ID, project.domain_id) + self.assertIsNone(project.parent_id) + else: + self.assertIsNotNone(project.domain_id) + self.assertIsNotNone(project.parent_id) + project_ids.append(project.id) + + for domain in self.domains: + self.assertIn(domain['id'], project_ids) + for project in self.projects: + self.assertIn(project['id'], project_ids) + + # Now check the attributes of the domains came across OK + for project in projects: + _assert_domain_matches_project(project) + + NULL_DOMAIN_ID = '<>' + self.upgrade(92) + + session = self.Session() + + _populate_domain_and_project_tables(session) + + self.upgrade(93) + proj_table = sqlalchemy.Table('project', self.metadata, autoload=True) + + projects = session.query(proj_table) + _check_projects(projects) + class VersionTests(SqlMigrateBase): diff --git a/keystone/tests/unit/test_v2_controller.py b/keystone/tests/unit/test_v2_controller.py index 5f92a5702d..6cf8bc53af 100644 --- a/keystone/tests/unit/test_v2_controller.py +++ b/keystone/tests/unit/test_v2_controller.py @@ -16,6 +16,8 @@ import copy import uuid +from testtools import matchers + from keystone.assignment import controllers as assignment_controllers from keystone import exception from keystone.resource import controllers as resource_controllers @@ -79,10 +81,14 @@ class TenantTestCase(unit.TestCase): self.resource_api.create_domain(domain['id'], domain) project1 = unit.new_project_ref(domain_id=domain['id']) self.resource_api.create_project(project1['id'], project1) - # Check the real total number of projects, we should have the above - # plus those in the default features + # Check the real total number of projects, we should have the: + # - tenants in the default fixtures + # - the project representing the default domain + # - the project representing the domain we created above + # - the project we created above refs = self.resource_api.list_projects() - self.assertEqual(len(default_fixtures.TENANTS) + 1, len(refs)) + self.assertThat( + refs, matchers.HasLength(len(default_fixtures.TENANTS) + 3)) # Now list all projects using the v2 API - we should only get # back those in the default features, since only those are in the @@ -97,8 +103,7 @@ class TenantTestCase(unit.TestCase): self.assertIn(tenant_copy, refs['tenants']) def _create_is_domain_project(self): - project = unit.new_project_ref(domain_id='default', - is_domain=True) + project = unit.new_project_ref(is_domain=True) project_ref = self.resource_api.create_project(project['id'], project) return self.tenant_controller.v3_to_v2_project(project_ref) diff --git a/keystone/tests/unit/test_v3.py b/keystone/tests/unit/test_v3.py index 91b118fe08..76f0efa234 100644 --- a/keystone/tests/unit/test_v3.py +++ b/keystone/tests/unit/test_v3.py @@ -977,7 +977,6 @@ class RestfulTestCase(unit.SQLDriverOverrides, rest.RestfulTestCase, **kwargs) def assertValidProject(self, entity, ref=None): - self.assertIsNotNone(entity.get('domain_id')) if ref: self.assertEqual(ref['domain_id'], entity['domain_id']) return entity diff --git a/keystone/tests/unit/test_v3_auth.py b/keystone/tests/unit/test_v3_auth.py index 1f8fc8d476..4a80a06390 100644 --- a/keystone/tests/unit/test_v3_auth.py +++ b/keystone/tests/unit/test_v3_auth.py @@ -1860,15 +1860,19 @@ class TestTokenRevokeApi(TestTokenRevokeById): expected_response = {'events': [{'project_id': project_id}]} self.assertEqual(expected_response, events_response) - def assertDomainInList(self, events_response, domain_id): + def assertDomainAndProjectInList(self, events_response, domain_id): events = events_response['events'] - self.assertEqual(1, len(events)) - self.assertEqual(domain_id, events[0]['domain_id']) + self.assertEqual(2, len(events)) + self.assertEqual(domain_id, events[0]['project_id']) + self.assertEqual(domain_id, events[1]['domain_id']) self.assertIsNotNone(events[0]['issued_before']) + self.assertIsNotNone(events[1]['issued_before']) self.assertIsNotNone(events_response['links']) del (events_response['events'][0]['issued_before']) + del (events_response['events'][1]['issued_before']) del (events_response['links']) - expected_response = {'events': [{'domain_id': domain_id}]} + expected_response = {'events': [{'project_id': domain_id}, + {'domain_id': domain_id}]} self.assertEqual(expected_response, events_response) def assertValidRevokedTokenResponse(self, events_response, **kwargs): @@ -1935,7 +1939,7 @@ class TestTokenRevokeApi(TestTokenRevokeById): events = self.get('/OS-REVOKE/events').json_body - self.assertDomainInList(events, self.domainA['id']) + self.assertDomainAndProjectInList(events, self.domainA['id']) def assertEventDataInList(self, events, **kwargs): found = False @@ -2967,19 +2971,23 @@ class TestAuth(test_v3.RestfulTestCase): def test_disabled_scope_project_domain_result_in_401(self): # create a disabled domain - domain = unit.new_domain_ref(enabled=False) - self.resource_api.create_domain(domain['id'], domain) + domain = unit.new_domain_ref() + domain = self.resource_api.create_domain(domain['id'], domain) - # create a project in the disabled domain + # create a project in the domain project = unit.new_project_ref(domain_id=domain['id']) self.resource_api.create_project(project['id'], project) - # assign some role to self.user for the project in the disabled domain + # assign some role to self.user for the project in the domain self.assignment_api.add_role_to_user_and_project( self.user['id'], project['id'], self.role_id) + # Disable the domain + domain['enabled'] = False + self.resource_api.update_domain(domain['id'], domain) + # user should not be able to auth with project_id auth_data = self.build_authentication_request( user_id=self.user['id'], diff --git a/keystone/tests/unit/test_v3_resource.py b/keystone/tests/unit/test_v3_resource.py index 5c234600fd..f54fcb5749 100644 --- a/keystone/tests/unit/test_v3_resource.py +++ b/keystone/tests/unit/test_v3_resource.py @@ -94,7 +94,6 @@ class ResourceTestCase(test_v3.RestfulTestCase, '/domains', body={'domain': ref}) - @test_utils.wip('waiting for projects acting as domains implementation') def test_create_domain_creates_is_domain_project(self): """Check a project that acts as a domain is created. @@ -117,7 +116,6 @@ class ResourceTestCase(test_v3.RestfulTestCase, self.assertIsNone(r.result['project']['parent_id']) self.assertIsNone(r.result['project']['domain_id']) - @test_utils.wip('waiting for projects acting as domains implementation') def test_create_is_domain_project_creates_domain(self): """Call ``POST /projects`` is_domain and check a domain is created.""" # Create a new project that acts as a domain @@ -187,7 +185,6 @@ class ResourceTestCase(test_v3.RestfulTestCase, 'domain_id': self.domain_id}, body={'domain': ref}) - @test_utils.wip('waiting for projects acting as domains implementation') def test_update_domain_updates_is_domain_project(self): """Check the project that acts as a domain is updated. @@ -375,7 +372,6 @@ class ResourceTestCase(test_v3.RestfulTestCase, r = self.credential_api.get_credential(credential['id']) self.assertDictEqual(credential, r) - @test_utils.wip('waiting for projects acting as domains implementation') def test_delete_domain_deletes_is_domain_project(self): """Check the project that acts as a domain is deleted. @@ -601,18 +597,6 @@ class ResourceTestCase(test_v3.RestfulTestCase, '/projects', body={'project': ref}) - def test_create_project_is_domain_not_allowed(self): - """Call ``POST /projects``. - - Setting is_domain=True is not supported yet and should raise - NotImplemented. - - """ - ref = unit.new_project_ref(domain_id=self.domain_id, is_domain=True) - self.post('/projects', - body={'project': ref}, - expected_status=http_client.NOT_IMPLEMENTED) - def test_create_project_with_parent_id_none_and_domain_id_none(self): """Call ``POST /projects``.""" # Grant a domain role for the user @@ -671,6 +655,7 @@ class ResourceTestCase(test_v3.RestfulTestCase, ref['domain_id'] = self.domain['id'] self.assertValidProjectResponse(r, ref) + @test_utils.wip('waiting for support for parent_id to imply domain_id') def test_create_project_with_parent_id_and_no_domain_id(self): """Call ``POST /projects``.""" # With only the parent_id, the domain_id should be @@ -806,16 +791,20 @@ class ResourceTestCase(test_v3.RestfulTestCase, # Assert parents_as_ids is a structured dictionary correctly # representing the hierarchy. The request was made using projects[2] - # id, hence its parents should be projects[1] and projects[0]. It - # should have the following structure: + # id, hence its parents should be projects[1], projects[0] and the + # is_domain_project, which is the root of the hierarchy. It should + # have the following structure: # { # projects[1]: { - # projects[0]: None + # projects[0]: { + # is_domain_project: None + # } # } # } + is_domain_project_id = projects[0]['project']['domain_id'] expected_dict = { projects[1]['project']['id']: { - projects[0]['project']['id']: None + projects[0]['project']['id']: {is_domain_project_id: None} } } self.assertDictEqual(expected_dict, parents_as_ids) @@ -828,7 +817,21 @@ class ResourceTestCase(test_v3.RestfulTestCase, self.assertValidProjectResponse(r, projects[0]['project']) parents_as_ids = r.result['project']['parents'] - # projects[0] has no parents, parents_as_ids must be None + # projects[0] has only the project that acts as a domain as parent + expected_dict = { + is_domain_project_id: None + } + self.assertDictEqual(expected_dict, parents_as_ids) + + # Query for is_domain_project parents_as_ids + r = self.get( + '/projects/%(project_id)s?parents_as_ids' % { + 'project_id': is_domain_project_id}) + + parents_as_ids = r.result['project']['parents'] + + # the project that acts as a domain has no parents, parents_as_ids + # must be None self.assertIsNone(parents_as_ids) def test_get_project_with_parents_as_list_with_full_access(self): diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py index c9b09cbd47..ea9f498f73 100644 --- a/keystone/token/providers/common.py +++ b/keystone/token/providers/common.py @@ -244,8 +244,12 @@ class V3TokenDataHelper(object): filtered_project = { 'id': project_ref['id'], 'name': project_ref['name']} - filtered_project['domain'] = self._get_filtered_domain( - project_ref['domain_id']) + if project_ref['domain_id'] is not None: + filtered_project['domain'] = ( + self._get_filtered_domain(project_ref['domain_id'])) + else: + # Projects acting as a domain do not have a domain_id attribute + filtered_project['domain'] = None return filtered_project def _populate_scope(self, token_data, domain_id, project_id):