Base implementation of override_role for automatic role re-switch
This PS deprecated switch_role in rbac_utils and replaces it with
override_role. override_role realizes the same functionality as
switch_role, but uses @contextmanager so that role-switching can be
streamlined. This approach offers the following advantages:
1) Role switching is performed in 1 class only. There is no
need to call ``test_obj.switch_role(test_obj, toggle_rbac_role=False)``
from ``rbac_rule_validation``. This de-coupling between both modules
leads to cleaner, more readable code.
2) Improves test code readability.
3) Improve role switch granularity, meaning the role remains switched
within the narrowest scope possible.
4) Simplifies interface, making it easier for test-writers to use
the Patrole framework.
Rather than doing:
# setup code here
rand_name = data_utils.rand_name(...)
# ...
# more setup code here
self.rbac_utils.switch_role(self, toggle_rbac_role=True)
# execute the test here
(Without newlines, this code is very hard to read.)
It is instead possible to now do:
# setup code here
rand_name = data_utils.rand_name(...)
# ...
# more setup code here
with self.rbac_utils.override_role(self):
# execute the test here
# notice the indentation... visually it is easy to see
# that this block here is where the role is switched
# now we are back to admin credentials in case we still
# need it in the test... this was not possible before w/o
# calling ``switch_role`` yet again...
waiters.wait_for_volume_status(self.volumes_client, ...)
This commit:
* Adds the necessary logic to rbac_utils to allow for automatic
role re-switch following test execution (i.e. override_role)
* Deprecates switch_role method in rbac_utils.
* Refactors RBAC tests in test_volumes_extend_rbac to prove
the concept introduced here.
* Removes _validate_switch_role functionality since its purpose
was to overcompensate for the old switch_role interface which
allowed users to pass in a boolean flag; now this is no longer
needed. Also removes associated unit tests.
* Updates a docstring in rbac_utils module.
Partially Implements: blueprint rbac-utils-contextmanager
Change-Id: I670fba358bf321eae0d22d18cea6d2f530f00716
diff --git a/patrole_tempest_plugin/tests/unit/fixtures.py b/patrole_tempest_plugin/tests/unit/fixtures.py
index 52c2598..4e3387e 100644
--- a/patrole_tempest_plugin/tests/unit/fixtures.py
+++ b/patrole_tempest_plugin/tests/unit/fixtures.py
@@ -92,21 +92,21 @@
self.set_roles(['admin', 'member'], [])
- def switch_role(self, *role_toggles):
- """Instantiate `rbac_utils.RbacUtils` and call `switch_role`.
+ def override_role(self, *role_toggles):
+ """Instantiate `rbac_utils.RbacUtils` and call `override_role`.
- Create an instance of `rbac_utils.RbacUtils` and call `switch_role`
+ Create an instance of `rbac_utils.RbacUtils` and call `override_role`
for each boolean value in `role_toggles`. The number of calls to
- `switch_role` is always 1 + len(`role_toggles`) because the
- `rbac_utils.RbacUtils` constructor automatically calls `switch_role`.
+ `override_role` is always 1 + len(`role_toggles`) because the
+ `rbac_utils.RbacUtils` constructor automatically calls `override_role`.
:param role_toggles: the list of boolean values iterated over and
- passed to `switch_role`.
+ passed to `override_role`.
"""
- self.fake_rbac_utils = rbac_utils.RbacUtils(self.mock_test_obj)
+ _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)
+ _rbac_utils._override_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,
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
index 87adff0..0d75c3e 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
@@ -16,6 +16,7 @@
import mock
import testtools
+from tempest.lib import exceptions as lib_exc
from tempest.tests import base
from patrole_tempest_plugin import rbac_exceptions
@@ -29,27 +30,27 @@
super(RBACUtilsTest, self).setUp()
# Reset the role history after each test run to avoid validation
# errors between tests.
- rbac_utils.RbacUtils.switch_role_history = {}
+ rbac_utils.RbacUtils.override_role_history = {}
self.rbac_utils = self.useFixture(patrole_fixtures.RbacUtilsFixture())
- def test_switch_role_with_missing_admin_role(self):
+ def test_override_role_with_missing_admin_role(self):
self.rbac_utils.set_roles('member')
error_re = (
'Roles defined by `\[patrole\] 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)
+ error_re, self.rbac_utils.override_role)
- def test_switch_role_with_missing_rbac_role(self):
+ def test_override_role_with_missing_rbac_role(self):
self.rbac_utils.set_roles('admin')
error_re = (
'Roles defined by `\[patrole\] 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)
+ error_re, self.rbac_utils.override_role)
- def test_switch_role_to_admin_role(self):
- self.rbac_utils.switch_role()
+ def test_override_role_to_admin_role(self):
+ self.rbac_utils.override_role()
mock_test_obj = self.rbac_utils.mock_test_obj
roles_client = self.rbac_utils.roles_v3_client
@@ -63,9 +64,9 @@
.assert_called_once_with()
mock_time.sleep.assert_called_once_with(1)
- def test_switch_role_to_admin_role_avoids_role_switch(self):
+ def test_override_role_to_admin_role_avoids_role_switch(self):
self.rbac_utils.set_roles(['admin', 'member'], 'admin')
- self.rbac_utils.switch_role()
+ self.rbac_utils.override_role()
roles_client = self.rbac_utils.roles_v3_client
mock_time = self.rbac_utils.mock_time
@@ -73,8 +74,8 @@
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)
+ def test_override_role_to_member_role(self):
+ self.rbac_utils.override_role(True)
mock_test_obj = self.rbac_utils.mock_test_obj
roles_client = self.rbac_utils.roles_v3_client
@@ -92,9 +93,9 @@
[mock.call()] * 2)
mock_time.sleep.assert_has_calls([mock.call(1)] * 2)
- def test_switch_role_to_member_role_avoids_role_switch(self):
+ def test_override_role_to_member_role_avoids_role_switch(self):
self.rbac_utils.set_roles(['admin', 'member'], 'member')
- self.rbac_utils.switch_role(True)
+ self.rbac_utils.override_role(True)
roles_client = self.rbac_utils.roles_v3_client
mock_time = self.rbac_utils.mock_time
@@ -105,8 +106,8 @@
])
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)
+ def test_override_role_to_member_role_then_admin_role(self):
+ self.rbac_utils.override_role(True, False)
mock_test_obj = self.rbac_utils.mock_test_obj
roles_client = self.rbac_utils.roles_v3_client
@@ -126,47 +127,12 @@
[mock.call()] * 3)
mock_time.sleep.assert_has_calls([mock.call(1)] * 3)
- def test_switch_role_without_boolean_value(self):
- self.assertRaises(rbac_exceptions.RbacResourceSetupFailed,
- self.rbac_utils.switch_role, "admin")
- self.assertRaises(rbac_exceptions.RbacResourceSetupFailed,
- self.rbac_utils.switch_role, None)
-
- def test_switch_role_with_false_value_twice(self):
- expected_error_message = (
- '`toggle_rbac_role` must not be called with the same bool value '
- 'twice. Make sure that you included a rbac_utils.switch_role '
- 'method call inside the test.')
-
- e = self.assertRaises(rbac_exceptions.RbacResourceSetupFailed,
- self.rbac_utils.switch_role, False)
- self.assertIn(expected_error_message, str(e))
-
- e = self.assertRaises(rbac_exceptions.RbacResourceSetupFailed,
- self.rbac_utils.switch_role, True, False, False)
- self.assertIn(expected_error_message, str(e))
-
- def test_switch_role_with_true_value_twice(self):
- expected_error_message = (
- '`toggle_rbac_role` must not be called with the same bool value '
- 'twice. Make sure that you included a rbac_utils.switch_role '
- 'method call inside the test.')
-
- e = self.assertRaises(rbac_exceptions.RbacResourceSetupFailed,
- self.rbac_utils.switch_role, True, True)
- self.assertIn(expected_error_message, str(e))
-
- e = self.assertRaises(rbac_exceptions.RbacResourceSetupFailed,
- self.rbac_utils.switch_role, True, False, True,
- True)
- self.assertIn(expected_error_message, str(e))
-
def test_clear_user_roles(self):
# 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()
+ self.rbac_utils.override_role()
roles_client = self.rbac_utils.roles_v3_client
@@ -179,20 +145,57 @@
mock.call(mock.sentinel.project_id, mock.sentinel.user_id,
'random_id')])
- @mock.patch.object(rbac_utils, 'LOG', autospec=True)
- @mock.patch.object(rbac_utils, 'sys', autospec=True)
- def test_switch_roles_with_unexpected_exception(self, mock_sys, mock_log):
- """Test whether unexpected exceptions don't throw error.
-
- If an unexpected exception or skip exception is raised, then that
- should not result in an error being raised.
+ @mock.patch.object(rbac_utils.RbacUtils, '_override_role', autospec=True)
+ def test_override_role_context_manager_simulate_pass(self,
+ mock_override_role):
+ """Validate that expected override_role calls are made when switching
+ to admin role for success path.
"""
- unexpected_exceptions = [testtools.TestCase.skipException,
- AttributeError]
+ test_obj = mock.MagicMock()
+ _rbac_utils = rbac_utils.RbacUtils(test_obj)
- for unexpected_exception in unexpected_exceptions:
- mock_sys.exc_info.return_value = [unexpected_exception()]
- # Ordinarily calling switch_role twice with the same value should
- # result in an error being thrown -- but not in this case.
- self.rbac_utils.switch_role(False)
- mock_log.error.assert_not_called()
+ # Validate constructor called _override_role with False.
+ mock_override_role.assert_called_once_with(_rbac_utils, test_obj,
+ False)
+ mock_override_role.reset_mock()
+
+ with _rbac_utils.override_role(test_obj):
+ # Validate `override_role` public method called private method
+ # `_override_role` with True.
+ mock_override_role.assert_called_once_with(_rbac_utils, test_obj,
+ True)
+ mock_override_role.reset_mock()
+ # Validate that `override_role` switched back to admin role after
+ # contextmanager.
+ mock_override_role.assert_called_once_with(_rbac_utils, test_obj,
+ False)
+
+ @mock.patch.object(rbac_utils.RbacUtils, '_override_role', autospec=True)
+ def test_override_role_context_manager_simulate_fail(self,
+ mock_override_role):
+ """Validate that expected override_role calls are made when switching
+ to admin role for failure path (i.e. when test raises exception).
+ """
+ test_obj = mock.MagicMock()
+ _rbac_utils = rbac_utils.RbacUtils(test_obj)
+
+ # Validate constructor called _override_role with False.
+ mock_override_role.assert_called_once_with(_rbac_utils, test_obj,
+ False)
+ mock_override_role.reset_mock()
+
+ def _do_test():
+ with _rbac_utils.override_role(test_obj):
+ # Validate `override_role` public method called private method
+ # `_override_role` with True.
+ mock_override_role.assert_called_once_with(
+ _rbac_utils, test_obj, True)
+ mock_override_role.reset_mock()
+ # Raise exc to verify role switch works for negative case.
+ raise lib_exc.Forbidden()
+
+ # Validate that role is switched back to admin, despite test failure.
+ with testtools.ExpectedException(lib_exc.Forbidden):
+ _do_test()
+ mock_override_role.assert_called_once_with(_rbac_utils, test_obj,
+ False)