diff -Nru keystone-10.0.3/debian/changelog keystone-10.0.3/debian/changelog --- keystone-10.0.3/debian/changelog 2017-09-18 11:36:51.000000000 -0700 +++ keystone-10.0.3/debian/changelog 2018-01-10 14:09:56.000000000 -0700 @@ -1,3 +1,15 @@ +keystone (2:10.0.3-0ubuntu1~cloud1) xenial-newton; urgency=medium + + * LDAP backend performance improvements (LP: #1582585) + - d/p/faster-id-mapping-lookup.patch: Allow querying for all + public ids in a domain at once instead of N queries (one per + entity). + - d/p/fallback-for-custom-id-map-drivers.patch: Add fallback + path for faster-id-mapping lookup for any customer id mapping + drivers that may be in use or existing deployments. + + -- Billy Olsen Wed, 10 Jan 2018 14:09:56 -0700 + keystone (2:10.0.3-0ubuntu1~cloud0) xenial-newton; urgency=medium * d/p/*: Rebased. diff -Nru keystone-10.0.3/debian/patches/fallback-for-custom-id-map-drivers.patch keystone-10.0.3/debian/patches/fallback-for-custom-id-map-drivers.patch --- keystone-10.0.3/debian/patches/fallback-for-custom-id-map-drivers.patch 1969-12-31 17:00:00.000000000 -0700 +++ keystone-10.0.3/debian/patches/fallback-for-custom-id-map-drivers.patch 2018-01-10 14:09:56.000000000 -0700 @@ -0,0 +1,75 @@ +Author: Billy Olsen +Forwarded: not-needed +Origin: vendor, https://bugs.launchpad.net/ubuntu/+source/keystone/+bug/1582585 +Bug: https://bugs.launchpad.net/ubuntu/+source/keystone/+bug/1582585 +Description: Don't break custom identity mapping drivers + +Backporting the faster id mapping lookup table into Newton +introduces a change to the internal API for ID Mapping drivers. +This patch adds a shim to fallback to existing behavior for +custom drivers which do not implement the get_domain_mapping_list +method. +--- + keystone/identity/core.py | 12 ++++++++++-- + keystone/tests/unit/test_backend_ldap.py | 25 +++++++++++++++++++++++++ + 2 files changed, 35 insertions(+), 2 deletions(-) + +Index: keystone-10.0.3/keystone/identity/core.py +=================================================================== +--- keystone-10.0.3.orig/keystone/identity/core.py ++++ keystone-10.0.3/keystone/identity/core.py +@@ -572,8 +572,16 @@ 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_for_list( +- ref, domain_id, driver, entity_type, conf) ++ try: ++ return self._set_domain_id_and_mapping_for_list( ++ ref, domain_id, driver, entity_type, conf) ++ except exception.NotImplemented: ++ # The id_mapping_api.get_domain_mapping_list is not ++ # implemented by the backend. Fallback to the previous way of ++ # setting the domain_id_and_mapping for backwards ++ # compatibility. ++ return [self._set_domain_id_and_mapping( ++ x, domain_id, driver, entity_type) for x in ref] + else: + raise ValueError(_('Expected dict or list: %s') % type(ref)) + +Index: keystone-10.0.3/keystone/tests/unit/test_backend_ldap.py +=================================================================== +--- keystone-10.0.3.orig/keystone/tests/unit/test_backend_ldap.py ++++ keystone-10.0.3/keystone/tests/unit/test_backend_ldap.py +@@ -3132,6 +3132,31 @@ class DomainSpecificLDAPandSQLIdentity( + mocked['get_domain_mapping_list'].assert_called() + mocked['get_id_mapping'].assert_not_called() + ++ def test_get_domain_mapping_list_is_not_implemented(self): ++ # By introducing get_domain_mapping_list, the code needs to support ++ # any custom IdentityMapping drivers with the legacy behavior so ++ # that they are not broken. This test ensures that the fallback path is ++ # used in driver implementations which have not implemented the ++ # get_domain_mapping_list method. ++ 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, ++ get_public_id=mock.DEFAULT) as mocked: ++ mocked['get_domain_mapping_list'].side_effect = \ ++ exception.NotImplemented() ++ self.identity_api.list_users( ++ domain_scope = self.domains['domain1']['id']) ++ self.assertTrue(mocked['get_domain_mapping_list'].called) ++ mocked['get_id_mapping'].assert_not_called() ++ # Note call_count is expected to be 6 as there are 6 total users ++ # which will be returned by this call. 1 is created as part of ++ # the overall test setup. ++ self.assertEqual(mocked['get_public_id'].call_count, 6) ++ + def test_user_id_comma(self): + self.skip_test_overrides('Only valid if it is guaranteed to be ' + 'talking to the fakeldap backend') diff -Nru keystone-10.0.3/debian/patches/faster-id-mapping-lookup.patch keystone-10.0.3/debian/patches/faster-id-mapping-lookup.patch --- keystone-10.0.3/debian/patches/faster-id-mapping-lookup.patch 1969-12-31 17:00:00.000000000 -0700 +++ keystone-10.0.3/debian/patches/faster-id-mapping-lookup.patch 2018-01-10 14:09:24.000000000 -0700 @@ -0,0 +1,258 @@ +Author: Boris Bobrov +Origin: upstream, https://git.openstack.org/cgit/openstack/keystone/commit/?id=f534f36246fd0b41bcdc2a664369507f9e299266 +Bug: https://bugs.launchpad.net/ubuntu/+source/keystone/+bug/1582585 +Description: Faster id mapping lookup + +id_mapping_api was designed to make a query per entity to fetch +public ids. This lead to a very poor performance when there were many +entries in LDAP. For example, for 15k entries 15k MySQL queries were +required. For the first run 15k INSERTs were required, which makes +things even worse. + +Change this behavior to fetch related mappings from MySQL as a list and +perform the necessary join in-memory. + +bp ldap-preprocessing +Partial-Bug: 1582585 +Change-Id: I2c266e91f2f05be760f8a3eea3738868243cc9c6 + +(cherry picked from commit f534f36246fd0b41bcdc2a664369507f9e299266) +--- + keystone/identity/core.py | 74 +++++++++++++++++++--- + keystone/identity/mapping_backends/base.py | 9 +++ + keystone/identity/mapping_backends/sql.py | 4 ++ + keystone/tests/unit/test_backend_id_mapping_sql.py | 42 ++++++++++++ + keystone/tests/unit/test_backend_ldap.py | 17 +++++ + .../domain_mapping_list-a368ac5a252ec84f.yaml | 6 ++ + 6 files changed, 142 insertions(+), 10 deletions(-) + create mode 100644 releasenotes/notes/domain_mapping_list-a368ac5a252ec84f.yaml + +diff --git a/keystone/identity/core.py b/keystone/identity/core.py +index ebbec69f5..7eaaeeae8 100644 +--- a/keystone/identity/core.py ++++ b/keystone/identity/core.py +@@ -572,8 +572,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)) + +@@ -582,6 +582,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']) +@@ -599,16 +609,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 abf09d567..1d15502c3 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 7da264dd9..0ec9be0f5 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 da8bd3176..74b9b55c0 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 845aacea1..fe113e44d 100644 +--- a/keystone/tests/unit/test_backend_ldap.py ++++ b/keystone/tests/unit/test_backend_ldap.py +@@ -3115,6 +3115,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 000000000..0390ad2a0 +--- /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. +-- +2.14.1 + diff -Nru keystone-10.0.3/debian/patches/series keystone-10.0.3/debian/patches/series --- keystone-10.0.3/debian/patches/series 2017-09-18 11:36:51.000000000 -0700 +++ keystone-10.0.3/debian/patches/series 2018-01-10 14:09:35.000000000 -0700 @@ -2,3 +2,5 @@ ubuntu-test-overrides.patch default-log-dir.patch add-version-info.patch +faster-id-mapping-lookup.patch +fallback-for-custom-id-map-drivers.patch