openstack/rally-openstack
Code Issues Proposed changes

Ensure cleanup is performed correctly on roles

Browse Source 
When using existing users (for e.g. LDAP setup) rally when specified
roles context it tends to assign and remove roles, which can be problematic
if the roles are already assigned to the users. This fix checks whether the
user already a member of the role, if so it doesn't attempt to cleanup

Closes-Bug: #1720270

Change-Id: I8638eb0e59fdea2fdbbdf87728da70530fd22e14
This commit is contained in:
Srinivas Sakhamuri 2018-01-04 15:18:02 -05:00
parent fd33530491
commit 8039a9543b
2 changed files with 33 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
rally/plugins/openstack/context/keystone/roles.py
View File

@ -64,6 +64,12 @@ class RoleGenerator(context.Context):
raise exceptions.NotFoundException(
"There is no role with name `%s`" % context_role)
def _get_user_role_ids(self, user_id, project_id):
keystone = identity.Identity(osclients.Clients(self.credential))
user_roles = keystone.list_roles(user_id=user_id,
project_id=project_id)
return [role.id for role in user_roles]
def _get_consumer(self, func_name):
def consume(cache, args):
role_id, user_id, project_id = args
@ -90,21 +96,29 @@ class RoleGenerator(context.Context):
"role_id": role.id,
"threads": threads})
for user in self.context["users"]:
args = (role.id, user["id"], user["tenant_id"])
queue.append(args)
if "roles" not in user:
user["roles"] = self._get_user_role_ids(
user["id"],
user["tenant_id"])
user["assigned_roles"] = []
if role.id not in user["roles"]:
args = (role.id, user["id"], user["tenant_id"])
queue.append(args)
user["assigned_roles"].append(role.id)
broker.run(publish, self._get_consumer("add_role"), threads)
self.context["roles"] = roles_dict
def cleanup(self):
"""Remove all roles from users."""
"""Remove assigned roles from users."""
threads = self.workers
def publish(queue):
for role_id in self.context["roles"]:
LOG.debug("Removing role %s from all users" % role_id)
LOG.debug("Removing assigned role %s from all users" % role_id)
for user in self.context["users"]:
args = (role_id, user["id"], user["tenant_id"])
queue.append(args)
if role_id in user["assigned_roles"]:
args = (role_id, user["id"], user["tenant_id"])
queue.append(args)
broker.run(publish, self._get_consumer("revoke_role"), threads)

18
tests/unit/plugins/openstack/context/keystone/test_roles.py
View File

@ -87,8 +87,10 @@ class RoleGeneratorTestCase(test.TestCase):
ctx = roles.RoleGenerator(self.context)
ctx.context["roles"] = {"r1": "test_role1",
"r2": "test_role2"}
ctx.context["users"] = [{"id": "u1", "tenant_id": "t1"},
{"id": "u2", "tenant_id": "t2"}]
ctx.context["users"] = [{"id": "u1", "tenant_id": "t1",
"assigned_roles": ["r1", "r2"]},
{"id": "u2", "tenant_id": "t2",
"assigned_roles": ["r1", "r2"]}]
ctx.credential = mock.MagicMock()
ctx.cleanup()
calls = [
@ -107,17 +109,23 @@ class RoleGeneratorTestCase(test.TestCase):
mock_osclients.Clients.return_value = fc
self.create_default_roles_and_patch_add_remove_functions(fc)
def _get_user_role_ids_side_effect(user_id, project_id):
return ["r1", "r2"] if user_id == "u3" else []
with roles.RoleGenerator(self.context) as ctx:
ctx.context["users"] = [{"id": "u1", "tenant_id": "t1"},
{"id": "u2", "tenant_id": "t2"}]
{"id": "u2", "tenant_id": "t2"},
{"id": "u3", "tenant_id": "t3"}]
ctx._get_user_role_ids = mock.MagicMock()
ctx._get_user_role_ids.side_effect = _get_user_role_ids_side_effect
ctx.setup()
ctx.credential = mock.MagicMock()
calls = [
mock.call(user="u1", role="r1", tenant="t1"),
mock.call(user="u2", role="r1", tenant="t2"),
mock.call(user="u1", role="r2", tenant="t1"),
mock.call(user="u2", role="r2", tenant="t2")
mock.call(user="u2", role="r2", tenant="t2"),
]
fc.keystone().roles.add_user_role.assert_has_calls(calls,
any_order=True)
@ -128,7 +136,7 @@ class RoleGeneratorTestCase(test.TestCase):
self.assertEqual(2, len(ctx.context["roles"]))
self.assertEqual(2, len(fc.keystone().roles.list()))
# Cleanup (called by content manager)
# Cleanup (called by context manager)
self.assertEqual(2, len(fc.keystone().roles.list()))
self.assertEqual(4, fc.keystone().roles.add_user_role.call_count)
self.assertEqual(4, fc.keystone().roles.remove_user_role.call_count)