diff --git a/keystone/identity/core.py b/keystone/identity/core.py index 8170950720..cdcb702eee 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -573,8 +573,8 @@ class Manager(manager.Manager): return self._set_domain_id_and_mapping_for_single_ref( ref, domain_id, driver, entity_type, conf) elif isinstance(ref, list): - return [self._set_domain_id_and_mapping( - x, domain_id, driver, entity_type) for x in ref] + return self._set_domain_id_and_mapping_for_list( + ref, domain_id, driver, entity_type, conf) else: raise ValueError(_('Expected dict or list: %s') % type(ref)) @@ -583,6 +583,16 @@ class Manager(manager.Manager): return (driver is not self.driver or not driver.generates_uuids() or not driver.is_domain_aware()) + def _insert_new_public_id(self, local_entity, ref, driver): + # Need to create a mapping. If the driver generates UUIDs + # then pass the local UUID in as the public ID to use. + public_id = None + if driver.generates_uuids(): + public_id = ref['id'] + ref['id'] = self.id_mapping_api.create_id_mapping( + local_entity, public_id) + LOG.debug('Created new mapping to public ID: %s', ref['id']) + def _set_domain_id_and_mapping_for_single_ref(self, ref, domain_id, driver, entity_type, conf): LOG.debug('Local ID: %s', ref['id']) @@ -600,16 +610,60 @@ class Manager(manager.Manager): LOG.debug('Found existing mapping to public ID: %s', ref['id']) else: - # Need to create a mapping. If the driver generates UUIDs - # then pass the local UUID in as the public ID to use. - if driver.generates_uuids(): - public_id = ref['id'] - ref['id'] = self.id_mapping_api.create_id_mapping( - local_entity, public_id) - LOG.debug('Created new mapping to public ID: %s', - ref['id']) + self._insert_new_public_id(local_entity, ref, driver) return ref + def _set_domain_id_and_mapping_for_list(self, ref_list, domain_id, driver, + entity_type, conf): + """Set domain id and mapping for a list of refs. + + The method modifies refs in-place. + """ + if not ref_list: + return [] + + for r in ref_list: + self._insert_domain_id_if_needed(r, driver, domain_id, conf) + + if not self._is_mapping_needed(driver): + return ref_list + + # build a map of refs for fast look-up + refs_map = {} + for r in ref_list: + refs_map[(r['id'], entity_type, r['domain_id'])] = r + + # NOTE(breton): there are cases when the driver is not domain aware and + # no domain_id was explicitely provided for list operation. domain_id + # gets inserted into refs, but not passed into this method. Lets use + # domain_id from one of the refs. + if not domain_id: + domain_id = ref_list[0]['domain_id'] + + # fetch all mappings for the domain, lookup the user at the map built + # at previous step and replace his id. + domain_mappings = self.id_mapping_api.get_domain_mapping_list( + domain_id) + for _mapping in domain_mappings: + idx = (_mapping.local_id, _mapping.entity_type, _mapping.domain_id) + try: + ref = refs_map.pop(idx) + # due to python specifics, `ref` still points to an item in + # `ref_list`. That's why when we change it here, it gets + # changed in `ref_list`. + ref['id'] = _mapping.public_id + except KeyError: + pass # some old entry, skip it + + # at this point, all known refs were granted a public_id. For the refs + # left, there are no mappings. They need to be created. + for ref in refs_map.values(): + local_entity = {'domain_id': ref['domain_id'], + 'local_id': ref['id'], + 'entity_type': entity_type} + self._insert_new_public_id(local_entity, ref, driver) + return ref_list + def _insert_domain_id_if_needed(self, ref, driver, domain_id, conf): """Insert the domain ID into the ref, if required. diff --git a/keystone/identity/mapping_backends/base.py b/keystone/identity/mapping_backends/base.py index abf09d5676..1d15502c30 100644 --- a/keystone/identity/mapping_backends/base.py +++ b/keystone/identity/mapping_backends/base.py @@ -34,6 +34,15 @@ class MappingDriverV8(object): """ raise exception.NotImplemented() # pragma: no cover + @abc.abstractmethod + def get_domain_mapping_list(self, domain_id): + """Return mappings for the domain. + + :param domain_id: Domain ID to get mappings for. + :returns: list of mappings. + """ + raise exception.NotImplemented() # pragma: no cover + @abc.abstractmethod def get_id_mapping(self, public_id): """Return the local mapping. diff --git a/keystone/identity/mapping_backends/sql.py b/keystone/identity/mapping_backends/sql.py index 7da264dd9e..0ec9be0f51 100644 --- a/keystone/identity/mapping_backends/sql.py +++ b/keystone/identity/mapping_backends/sql.py @@ -57,6 +57,10 @@ class Mapping(base.MappingDriverV8): except sql.NotFound: return None + def get_domain_mapping_list(self, domain_id): + with sql.session_for_read() as session: + return session.query(IDMapping).filter_by(domain_id=domain_id) + def get_id_mapping(self, public_id): with sql.session_for_read() as session: mapping_ref = session.query(IDMapping).get(public_id) diff --git a/keystone/tests/unit/test_backend_id_mapping_sql.py b/keystone/tests/unit/test_backend_id_mapping_sql.py index da8bd3176d..74b9b55c00 100644 --- a/keystone/tests/unit/test_backend_id_mapping_sql.py +++ b/keystone/tests/unit/test_backend_id_mapping_sql.py @@ -290,3 +290,45 @@ class SqlIDMapping(test_backend_sql.SqlTests): # Purge mappings the remaining mappings self.id_mapping_api.purge_mappings({}) self.assertIsNone(self.id_mapping_api.get_public_id(local_entity5)) + + def test_get_domain_mapping_list(self): + local_id1 = uuid.uuid4().hex + local_id2 = uuid.uuid4().hex + local_id3 = uuid.uuid4().hex + local_id4 = uuid.uuid4().hex + local_id5 = uuid.uuid4().hex + + # Create five mappings,two in domainA, three in domainB + local_entity1 = {'domain_id': self.domainA['id'], + 'local_id': local_id1, + 'entity_type': mapping.EntityType.USER} + local_entity2 = {'domain_id': self.domainA['id'], + 'local_id': local_id2, + 'entity_type': mapping.EntityType.USER} + local_entity3 = {'domain_id': self.domainB['id'], + 'local_id': local_id3, + 'entity_type': mapping.EntityType.GROUP} + local_entity4 = {'domain_id': self.domainB['id'], + 'local_id': local_id4, + 'entity_type': mapping.EntityType.USER} + local_entity5 = {'domain_id': self.domainB['id'], + 'local_id': local_id5, + 'entity_type': mapping.EntityType.USER} + + local_entity1['public_id'] = self.id_mapping_api.create_id_mapping( + local_entity1) + local_entity2['public_id'] = self.id_mapping_api.create_id_mapping( + local_entity2) + local_entity3['public_id'] = self.id_mapping_api.create_id_mapping( + local_entity3) + local_entity4['public_id'] = self.id_mapping_api.create_id_mapping( + local_entity4) + local_entity5['public_id'] = self.id_mapping_api.create_id_mapping( + local_entity5) + + # list mappings for domainA + domain_a_mappings = self.id_mapping_api.get_domain_mapping_list( + self.domainA['id']) + domain_a_mappings = [m.to_dict() for m in domain_a_mappings] + self.assertItemsEqual([local_entity1, local_entity2], + domain_a_mappings) diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index 9e8ab38c50..40e4182050 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -3121,6 +3121,23 @@ class DomainSpecificLDAPandSQLIdentity( domain_scope=self.domains['domain1']['id']), matchers.HasLength(1)) + def test_get_domain_mapping_list_is_used(self): + # before get_domain_mapping_list was introduced, it was required to + # make N calls to the database for N users, and it was slow. + # get_domain_mapping_list solves this problem and should be used + # when multiple users are fetched from domain-specific backend. + for i in range(5): + unit.create_user(self.identity_api, + domain_id=self.domains['domain1']['id']) + + with mock.patch.multiple(self.id_mapping_api, + get_domain_mapping_list=mock.DEFAULT, + get_id_mapping=mock.DEFAULT) as mocked: + self.identity_api.list_users( + domain_scope=self.domains['domain1']['id']) + mocked['get_domain_mapping_list'].assert_called() + mocked['get_id_mapping'].assert_not_called() + def test_user_id_comma(self): self.skip_test_overrides('Only valid if it is guaranteed to be ' 'talking to the fakeldap backend') diff --git a/releasenotes/notes/domain_mapping_list-a368ac5a252ec84f.yaml b/releasenotes/notes/domain_mapping_list-a368ac5a252ec84f.yaml new file mode 100644 index 0000000000..0390ad2a02 --- /dev/null +++ b/releasenotes/notes/domain_mapping_list-a368ac5a252ec84f.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - ID Mapping driver interface has changed. A new method + ``get_domain_mapping_list`` was added for fetching mapping list + for a domain. If you have a custom implementation for the identity + driver, you will need to implement this new method.