From d465a58f02f134086d6322c5b858c056a3aea025 Mon Sep 17 00:00:00 2001 From: Jose Castro Leon Date: Tue, 9 Oct 2018 15:11:48 +0200 Subject: [PATCH] Add caching on trust role validation to improve performance In the token model, the trust roles are not cached. This behavior impacts services that are using trusts heavily like heat or magnum. It introduces new cache data to improve the performance on token validation requests on trusts. Change-Id: I974907b427c34fd5db3228b6139d93bbcdc38df5 Closes-Bug: #1796887 --- keystone/assignment/core.py | 20 ++++++++++++++ keystone/models/token_model.py | 20 +++++--------- .../tests/unit/assignment/test_backends.py | 26 +++++++++++++++++++ keystone/tests/unit/test_backend_ldap.py | 3 +++ .../notes/bug-1796887-eaea84e3f9a8ff9f.yaml | 7 +++++ 5 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/bug-1796887-eaea84e3f9a8ff9f.yaml diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py index c9f90de5ee..722402950b 100644 --- a/keystone/assignment/core.py +++ b/keystone/assignment/core.py @@ -129,6 +129,26 @@ class Manager(manager.Manager): # Use set() to process the list to remove any duplicates return list(set([x['role_id'] for x in assignment_list])) + @MEMOIZE_COMPUTED_ASSIGNMENTS + def get_roles_for_trustor_and_project(self, trustor_id, project_id): + """Get the roles associated with a trustor within given project. + + This includes roles directly assigned to the trustor on the + project, as well as those by virtue of group membership or + inheritance, but it doesn't include the domain roles. + + :returns: a list of role ids. + :raises keystone.exception.ProjectNotFound: If the project doesn't + exist. + + """ + PROVIDERS.resource_api.get_project(project_id) + assignment_list = self.list_role_assignments( + user_id=trustor_id, project_id=project_id, effective=True, + strip_domain_roles=False) + # Use set() to process the list to remove any duplicates + return list(set([x['role_id'] for x in assignment_list])) + @MEMOIZE_COMPUTED_ASSIGNMENTS def get_roles_for_user_and_domain(self, user_id, domain_id): """Get the roles associated with a user within given domain. diff --git a/keystone/models/token_model.py b/keystone/models/token_model.py index 37cc56342e..a7fd89edcc 100644 --- a/keystone/models/token_model.py +++ b/keystone/models/token_model.py @@ -294,15 +294,10 @@ class TokenModel(object): set([r['role_id'] for r in effective_trust_roles]) ) - trustor_assignments = ( - PROVIDERS.assignment_api.list_role_assignments( - user_id=original_trustor_id, - project_id=self.trust.get('project_id'), - effective=True, strip_domain_roles=False - ) - ) current_effective_trustor_roles = ( - set([x['role_id'] for x in trustor_assignments]) + PROVIDERS.assignment_api.get_roles_for_trustor_and_project( + original_trustor_id, self.trust.get('project_id') + ) ) for trust_role_id in effective_trust_role_ids: @@ -518,13 +513,10 @@ class TokenModel(object): effective_trust_role_ids = ( set([r['role_id'] for r in effective_trust_roles]) ) - assignments = PROVIDERS.assignment_api.list_role_assignments( - user_id=self.trustor['id'], system=self.system, - project_id=self.project_id, effective=True, - strip_domain_roles=False - ) current_effective_trustor_roles = ( - set([x['role_id'] for x in assignments]) + PROVIDERS.assignment_api.get_roles_for_trustor_and_project( + self.trustor['id'], self.trust.get('project_id') + ) ) # Go through each of the effective trust roles, making sure the # trustor still has them, if any have been removed, then we diff --git a/keystone/tests/unit/assignment/test_backends.py b/keystone/tests/unit/assignment/test_backends.py index 97f098092f..d53211c350 100644 --- a/keystone/tests/unit/assignment/test_backends.py +++ b/keystone/tests/unit/assignment/test_backends.py @@ -757,6 +757,32 @@ class AssignmentTests(AssignmentTestHelperMixin): self.assertIn(self.role_admin['id'], roles_ref) self.assertIn(default_fixtures.MEMBER_ROLE_ID, roles_ref) + def test_get_role_by_trustor_and_project(self): + new_domain = unit.new_domain_ref() + PROVIDERS.resource_api.create_domain(new_domain['id'], new_domain) + new_user = unit.new_user_ref(domain_id=new_domain['id']) + new_user = PROVIDERS.identity_api.create_user(new_user) + new_project = unit.new_project_ref(domain_id=new_domain['id']) + PROVIDERS.resource_api.create_project(new_project['id'], new_project) + role = self._create_role(domain_id=new_domain['id']) + + # Now create the grants (roles are defined in default_fixtures) + PROVIDERS.assignment_api.create_grant( + user_id=new_user['id'], + project_id=new_project['id'], + role_id=default_fixtures.MEMBER_ROLE_ID) + PROVIDERS.assignment_api.create_grant( + user_id=new_user['id'], + domain_id=new_domain['id'], + role_id=role['id'], + inherited_to_projects=True) + + roles_ids = PROVIDERS.assignment_api.get_roles_for_trustor_and_project( + new_user['id'], new_project['id']) + self.assertEqual(2, len(roles_ids)) + self.assertIn(self.role_member['id'], roles_ids) + self.assertIn(role['id'], roles_ids) + def test_get_roles_for_user_and_domain(self): """Test for getting roles for user on a domain. diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index d8e2a82b6e..13667a9198 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -199,6 +199,9 @@ class AssignmentTests(assignment_tests.AssignmentTests): 'N/A: LDAP does not implement get_roles_for_groups; ' 'see bug 1333712 for details') + def test_get_role_by_trustor_and_project(self): + self.skip_test_overrides('Domains are read-only against LDAP') + def test_get_roles_for_groups_on_project(self): self.skip_test_overrides( 'N/A: LDAP does not implement get_roles_for_groups; ' diff --git a/releasenotes/notes/bug-1796887-eaea84e3f9a8ff9f.yaml b/releasenotes/notes/bug-1796887-eaea84e3f9a8ff9f.yaml new file mode 100644 index 0000000000..fd27b9175a --- /dev/null +++ b/releasenotes/notes/bug-1796887-eaea84e3f9a8ff9f.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`bug 1796887 `_] + Add caching on trust role validation to improve performance. Services + relying heavily on trusts are impacted as the trusts are validated against + the database. This adds caching on those operations to improve performance