diff options
author | Pavlo Shchelokovskyy <shchelokovskyy@gmail.com> | 2018-05-30 21:27:58 +0300 |
---|---|---|
committer | Raildo <rmascena@redhat.com> | 2020-11-10 17:29:02 -0300 |
commit | 0153ab781d387cc0fdb3c93fbd7be5c897bcf933 (patch) | |
tree | d5ef1b950210e9d47fd347caeff1fe9a7e0bfde9 | |
parent | 654dd5ee47d0b2a38506cd76f91faf1497a19f14 (diff) | |
download | keystone-0153ab781d387cc0fdb3c93fbd7be5c897bcf933.tar.gz |
Filter by entity_type in get_domain_mapping_list
with many users and groups in a domain fetching all mappings (for both
users and groups) may become inefficient.
In an environment with approx 125k users and 150 groups in the mapping
table and SAML2+LDAP auth/backend, this patch reduced the time
for first (uncached) 'openstack token issue' command from 12 to 3 seconds.
Similar improvements were seen with time to login to Horizon as well.
Change-Id: Iccbef534ff7e723f8b1461bb1169e2da66cc1dea
Closes-Bug: #1775207
(cherry picked from commit 4abd9926ab7fb79bbe86a22657a2474790a8fcb8)
-rw-r--r-- | keystone/identity/core.py | 2 | ||||
-rw-r--r-- | keystone/identity/mapping_backends/base.py | 5 | ||||
-rw-r--r-- | keystone/identity/mapping_backends/sql.py | 7 | ||||
-rw-r--r-- | keystone/tests/unit/test_backend_id_mapping_sql.py | 79 | ||||
-rw-r--r-- | releasenotes/notes/filter-mappings-by-entity-77162a146d375385.yaml | 11 |
5 files changed, 59 insertions, 45 deletions
diff --git a/keystone/identity/core.py b/keystone/identity/core.py index dd2944c21..1eb7d018d 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -671,7 +671,7 @@ class Manager(manager.Manager): # fetch all mappings for the domain, lookup the user at the map built # at previous step and replace his id. domain_mappings = PROVIDERS.id_mapping_api.get_domain_mapping_list( - domain_id) + domain_id, entity_type=entity_type) for _mapping in domain_mappings: idx = (_mapping.local_id, _mapping.entity_type, _mapping.domain_id) try: diff --git a/keystone/identity/mapping_backends/base.py b/keystone/identity/mapping_backends/base.py index 320d60ac1..fb7f0ead6 100644 --- a/keystone/identity/mapping_backends/base.py +++ b/keystone/identity/mapping_backends/base.py @@ -36,10 +36,13 @@ class MappingDriverBase(provider_api.ProviderAPIMixin, object): raise exception.NotImplemented() # pragma: no cover @abc.abstractmethod - def get_domain_mapping_list(self, domain_id): + def get_domain_mapping_list(self, domain_id, entity_type=None): """Return mappings for the domain. :param domain_id: Domain ID to get mappings for. + :param entity_type: Optional entity_type to get mappings for. + :type entity_type: String, one of mappings defined in + keystone.identity.mapping_backends.mapping.EntityType :returns: list of mappings. """ raise exception.NotImplemented() # pragma: no cover diff --git a/keystone/identity/mapping_backends/sql.py b/keystone/identity/mapping_backends/sql.py index 991a890ae..676d14492 100644 --- a/keystone/identity/mapping_backends/sql.py +++ b/keystone/identity/mapping_backends/sql.py @@ -55,9 +55,12 @@ class Mapping(base.MappingDriverBase): except sql.NotFound: return None - def get_domain_mapping_list(self, domain_id): + def get_domain_mapping_list(self, domain_id, entity_type=None): + filters = {'domain_id': domain_id} + if entity_type is not None: + filters['entity_type'] = entity_type with sql.session_for_read() as session: - return session.query(IDMapping).filter_by(domain_id=domain_id) + return session.query(IDMapping).filter_by(**filters) def get_id_mapping(self, public_id): with sql.session_for_read() as session: diff --git a/keystone/tests/unit/test_backend_id_mapping_sql.py b/keystone/tests/unit/test_backend_id_mapping_sql.py index a89a8be93..172de3660 100644 --- a/keystone/tests/unit/test_backend_id_mapping_sql.py +++ b/keystone/tests/unit/test_backend_id_mapping_sql.py @@ -318,46 +318,28 @@ class SqlIDMapping(test_backend_sql.SqlTests): PROVIDERS.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'] = ( - PROVIDERS.id_mapping_api.create_id_mapping(local_entity1) - ) - local_entity2['public_id'] = ( - PROVIDERS.id_mapping_api.create_id_mapping(local_entity2) - ) - local_entity3['public_id'] = ( - PROVIDERS.id_mapping_api.create_id_mapping(local_entity3) - ) - local_entity4['public_id'] = ( - PROVIDERS.id_mapping_api.create_id_mapping(local_entity4) - ) - local_entity5['public_id'] = ( - PROVIDERS.id_mapping_api.create_id_mapping(local_entity5) - ) + def _prepare_domain_mappings_for_list(self): + # Create five mappings: + # two users in domainA, one group and two users in domainB + local_entities = [ + {'domain_id': self.domainA['id'], + 'entity_type': mapping.EntityType.USER}, + {'domain_id': self.domainA['id'], + 'entity_type': mapping.EntityType.USER}, + {'domain_id': self.domainB['id'], + 'entity_type': mapping.EntityType.GROUP}, + {'domain_id': self.domainB['id'], + 'entity_type': mapping.EntityType.USER}, + {'domain_id': self.domainB['id'], + 'entity_type': mapping.EntityType.USER} + ] + for e in local_entities: + e['local_id'] = uuid.uuid4().hex + e['public_id'] = PROVIDERS.id_mapping_api.create_id_mapping(e) + return local_entities + def test_get_domain_mapping_list(self): + local_entities = self._prepare_domain_mappings_for_list() # NOTE(notmorgan): Always call to_dict in an active session context to # ensure that lazy-loaded relationships succeed. Edge cases could cause # issues especially in attribute mappers. @@ -369,5 +351,20 @@ class SqlIDMapping(test_backend_sql.SqlTests): ) ) domain_a_mappings = [m.to_dict() for m in domain_a_mappings] - self.assertItemsEqual([local_entity1, local_entity2], - domain_a_mappings) + self.assertItemsEqual(local_entities[:2], domain_a_mappings) + + def test_get_domain_mapping_list_by_entity(self): + local_entities = self._prepare_domain_mappings_for_list() + # NOTE(notmorgan): Always call to_dict in an active session context to + # ensure that lazy-loaded relationships succeed. Edge cases could cause + # issues especially in attribute mappers. + with sql.session_for_read(): + # list user mappings for domainB + domain_b_mappings_user = ( + PROVIDERS.id_mapping_api.get_domain_mapping_list( + self.domainB['id'], entity_type=mapping.EntityType.USER + ) + ) + domain_b_mappings_user = [m.to_dict() + for m in domain_b_mappings_user] + self.assertItemsEqual(local_entities[-2:], domain_b_mappings_user) diff --git a/releasenotes/notes/filter-mappings-by-entity-77162a146d375385.yaml b/releasenotes/notes/filter-mappings-by-entity-77162a146d375385.yaml new file mode 100644 index 000000000..9f30e34a0 --- /dev/null +++ b/releasenotes/notes/filter-mappings-by-entity-77162a146d375385.yaml @@ -0,0 +1,11 @@ +--- +upgrade: + - | + As a performance improvement, the base mapping driver's method + ``get_domain_mapping_list`` now accepts an optional named argument + ``entity_type`` that can be used to get the mappings for a given + entity type only. + As this new call signature is already used in the ``identity.core`` + module, authors/maintainers of out-of-tree custom mapping drivers + are expected to update their implementations of ``get_domain_mapping_list`` + method accordingly. |