Only sleep following a role switch
This commit drops support for not sleeping if the token being
used for keystone auth is UUID, as it is being deprecated
during the "R" release [0]. Thus, the condition for sleeping
has completely changed: Now, time.sleep(1) is only carried
out if the role actually switched (causing a token revocation
event in keystone and hence the need to sleep).
For example, if rbac_test_role = "admin" then there is no need
to sleep (except once, at the beginning, which strips the user
of any additional roles) as setup, API execution and clean up
all require admin. If rbac_test_role = "Member" then sleep
must be carried out each test at least twice, as setup requires
admin, API execution requires Member, and clean up requires
admin.
This commit also adds unit tests to confirm that sleep is
only performed if the role switched. This required
changes to some testing logic.
[0] http://lists.openstack.org/pipermail/openstack-dev/2017-August/121067.html
Change-Id: Iedc97397d9ae76c7f5efed5913850ced470c6c5f
Closes-Bug: #1710237
Depends-On: I34d324575c8117022724a944c034f089eb0c7541
diff --git a/patrole_tempest_plugin/tests/unit/fixtures.py b/patrole_tempest_plugin/tests/unit/fixtures.py
index f25e05d..ed50e15 100644
--- a/patrole_tempest_plugin/tests/unit/fixtures.py
+++ b/patrole_tempest_plugin/tests/unit/fixtures.py
@@ -53,15 +53,11 @@
class RbacUtilsFixture(fixtures.Fixture):
- """Fixture for RbacUtils class."""
+ """Fixture for `RbacUtils` class."""
USER_ID = mock.sentinel.user_id
PROJECT_ID = mock.sentinel.project_id
- def __init__(self, **kwargs):
- super(RbacUtilsFixture, self).__init__()
- self.available_roles = None
-
def setUp(self):
super(RbacUtilsFixture, self).setUp()
@@ -81,29 +77,54 @@
self.roles_v3_client = (
self.mock_test_obj.get_client_manager.return_value.roles_v3_client)
+ self.set_roles(['admin', 'member'], [])
+
def switch_role(self, *role_toggles):
"""Instantiate `rbac_utils.RbacUtils` and call `switch_role`.
Create an instance of `rbac_utils.RbacUtils` and call `switch_role`
for each boolean value in `role_toggles`. The number of calls to
- `switch_role` is always 1 + len(role_toggles) because the
+ `switch_role` is always 1 + len(`role_toggles`) because the
`rbac_utils.RbacUtils` constructor automatically calls `switch_role`.
:param role_toggles: the list of boolean values iterated over and
passed to `switch_role`.
"""
- if not self.available_roles:
- self.set_roles('admin', 'member')
-
self.fake_rbac_utils = rbac_utils.RbacUtils(self.mock_test_obj)
+
for role_toggle in role_toggles:
self.fake_rbac_utils.switch_role(self.mock_test_obj, role_toggle)
+ # NOTE(felipemonteiro): Simulate that a role switch has occurred
+ # by updating the user's current role to the new role. This means
+ # that all API actions involved during a role switch -- listing,
+ # deleting and adding roles -- are executed, making it easier to
+ # assert that mock calls were called as expected.
+ new_role = 'member' if role_toggle else 'admin'
+ self.set_roles(['admin', 'member'], [new_role])
- def set_roles(self, *roles):
- """Set the list of available roles in the system to `roles`."""
- self.available_roles = {
+ def set_roles(self, roles, roles_on_project=None):
+ """Set the list of available roles in the system.
+
+ :param roles: List of roles returned by ``list_roles``.
+ :param roles_on_project: List of roles returned by
+ ``list_user_roles_on_project``.
+ :returns: None.
+ """
+ if not roles_on_project:
+ roles_on_project = []
+ if not isinstance(roles, list):
+ roles = [roles]
+ if not isinstance(roles_on_project, list):
+ roles_on_project = [roles_on_project]
+
+ available_roles = {
'roles': [{'name': role, 'id': '%s_id' % role} for role in roles]
}
- self.roles_v3_client.list_user_roles_on_project.return_value =\
- self.available_roles
- self.roles_v3_client.list_roles.return_value = self.available_roles
+ available_project_roles = {
+ 'roles': [{'name': role, 'id': '%s_id' % role}
+ for role in roles_on_project]
+ }
+
+ self.roles_v3_client.list_roles.return_value = available_roles
+ self.roles_v3_client.list_user_roles_on_project.return_value = (
+ available_project_roles)
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
index 79503b1..540283a 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
@@ -34,17 +34,19 @@
def test_switch_role_with_missing_admin_role(self):
self.rbac_utils.set_roles('member')
- e = self.assertRaises(rbac_exceptions.RbacResourceSetupFailed,
- self.rbac_utils.switch_role, True)
- self.assertIn("Role with name 'admin' does not exist in the system.",
- e.__str__())
+ error_re = (
+ 'Roles defined by `\[rbac\] rbac_test_role` and `\[identity\] '
+ 'admin_role` must be defined in the system.')
+ self.assertRaisesRegex(rbac_exceptions.RbacResourceSetupFailed,
+ error_re, self.rbac_utils.switch_role)
def test_switch_role_with_missing_rbac_role(self):
self.rbac_utils.set_roles('admin')
- e = self.assertRaises(rbac_exceptions.RbacResourceSetupFailed,
- self.rbac_utils.switch_role, True)
- self.assertIn("Role defined by rbac_test_role does not exist in the "
- "system.", e.__str__())
+ error_re = (
+ 'Roles defined by `\[rbac\] rbac_test_role` and `\[identity\] '
+ 'admin_role` must be defined in the system.')
+ self.assertRaisesRegex(rbac_exceptions.RbacResourceSetupFailed,
+ error_re, self.rbac_utils.switch_role)
def test_switch_role_to_admin_role(self):
self.rbac_utils.switch_role()
@@ -61,6 +63,16 @@
.assert_called_once_with()
mock_time.sleep.assert_called_once_with(1)
+ def test_switch_role_to_admin_role_avoids_role_switch(self):
+ self.rbac_utils.set_roles(['admin', 'member'], 'admin')
+ self.rbac_utils.switch_role()
+
+ roles_client = self.rbac_utils.roles_v3_client
+ mock_time = self.rbac_utils.mock_time
+
+ roles_client.create_user_role_on_project.assert_not_called()
+ mock_time.sleep.assert_not_called()
+
def test_switch_role_to_member_role(self):
self.rbac_utils.switch_role(True)
@@ -80,6 +92,19 @@
[mock.call()] * 2)
mock_time.sleep.assert_has_calls([mock.call(1)] * 2)
+ def test_switch_role_to_member_role_avoids_role_switch(self):
+ self.rbac_utils.set_roles(['admin', 'member'], 'member')
+ self.rbac_utils.switch_role(True)
+
+ roles_client = self.rbac_utils.roles_v3_client
+ mock_time = self.rbac_utils.mock_time
+
+ roles_client.create_user_role_on_project.assert_has_calls([
+ mock.call(self.rbac_utils.PROJECT_ID, self.rbac_utils.USER_ID,
+ 'admin_id')
+ ])
+ mock_time.sleep.assert_called_once_with(1)
+
def test_switch_role_to_member_role_then_admin_role(self):
self.rbac_utils.switch_role(True, False)
@@ -137,19 +162,22 @@
self.assertIn(expected_error_message, str(e))
def test_clear_user_roles(self):
- self.rbac_utils.switch_role(True)
+ # NOTE(felipemonteiro): Set the user's roles on the project to
+ # include 'random' to coerce a role switch, or else it will be
+ # skipped.
+ self.rbac_utils.set_roles(['admin', 'member'], ['member', 'random'])
+ self.rbac_utils.switch_role()
roles_client = self.rbac_utils.roles_v3_client
- roles_client.list_user_roles_on_project.assert_has_calls([
- mock.call(self.rbac_utils.PROJECT_ID, self.rbac_utils.USER_ID)
- ] * 2)
+ roles_client.list_user_roles_on_project.assert_called_once_with(
+ self.rbac_utils.PROJECT_ID, self.rbac_utils.USER_ID)
roles_client.delete_role_from_user_on_project.\
assert_has_calls([
mock.call(mock.sentinel.project_id, mock.sentinel.user_id,
- 'admin_id'),
+ 'member_id'),
mock.call(mock.sentinel.project_id, mock.sentinel.user_id,
- 'member_id')] * 2)
+ 'random_id')])
@mock.patch.object(rbac_utils, 'LOG', autospec=True)
@mock.patch.object(rbac_utils, 'sys', autospec=True)