Remove invalid exception RbacConflictingPolicies
This patch set drops RbacConflictingPolicies from Patrole because
its intention is 1) flawed and 2) the way it is being used is
invalid.
First, there is no documented case where conflicting policies
exists in OpenStack, so the intention behind the exception class
is flawed.
Second, the exception is only used in one test in
test_server_rbac. It catches a ServerFault (500) and re-raises
the exception is a pseudo-Forbidden exception. This is very bad
because if Nova raises a 500 on a policy exception then this is
a Nova bug. There is no valid use case where a ServerFault from
Nova should ever be treated as a successful test scenario.
Change-Id: I495b10ccf7b0df523b2551c49ea4db07e5bcaf79
diff --git a/patrole_tempest_plugin/rbac_exceptions.py b/patrole_tempest_plugin/rbac_exceptions.py
index 3958e17..9130fb6 100644
--- a/patrole_tempest_plugin/rbac_exceptions.py
+++ b/patrole_tempest_plugin/rbac_exceptions.py
@@ -20,11 +20,6 @@
message = "An unknown RBAC exception occurred"
-class RbacConflictingPolicies(BasePatroleException):
- message = ("Conflicting policies preventing this action from being "
- "performed.")
-
-
class RbacMalformedResponse(BasePatroleException):
message = ("The response body is missing the expected %(attribute)s due "
"to policy enforcement failure.")
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index d3b057c..828a226 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -198,7 +198,6 @@
test_status = ('Error, %s' % (msg))
LOG.error(msg)
except (expected_exception,
- rbac_exceptions.RbacConflictingPolicies,
rbac_exceptions.RbacMalformedResponse) as actual_exception:
caught_exception = actual_exception
test_status = 'Denied'
diff --git a/patrole_tempest_plugin/tests/api/compute/test_server_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_server_rbac.py
index cae31f6..92ff87f 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_server_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_server_rbac.py
@@ -21,9 +21,7 @@
from tempest.lib.common.utils import data_utils
from tempest.lib.common.utils import test_utils
from tempest.lib import decorators
-from tempest.lib import exceptions
-from patrole_tempest_plugin import rbac_exceptions
from patrole_tempest_plugin import rbac_rule_validation
from patrole_tempest_plugin.tests.api.compute import rbac_base as base
@@ -182,13 +180,7 @@
def test_update_server(self):
new_name = data_utils.rand_name(self.__class__.__name__ + '-server')
with self.rbac_utils.override_role(self):
- try:
- self.servers_client.update_server(self.server['id'],
- name=new_name)
- waiters.wait_for_server_status(self.servers_client,
- self.server['id'], 'ACTIVE')
- except exceptions.ServerFault as e:
- # Some other policy may have blocked it.
- LOG.info("ServerFault exception caught. Some other policy "
- "blocked updating of server")
- raise rbac_exceptions.RbacConflictingPolicies(e)
+ self.servers_client.update_server(self.server['id'],
+ name=new_name)
+ waiters.wait_for_server_status(self.servers_client,
+ self.server['id'], 'ACTIVE')
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
index 1772047..c4cee5a 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -168,48 +168,6 @@
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
- def test_rule_validation_rbac_conflicting_policies_positive(
- self, mock_authority, mock_log):
- """Test RbacConflictingPolicies error is thrown without permission
- passes.
-
- Positive test case: if RbacConflictingPolicies is thrown and the user
- is not allowed to perform the action, then this is a success.
- """
- mock_authority.PolicyAuthority.return_value.allowed.return_value =\
- False
-
- @rbac_rv.action(mock.sentinel.service, rules=[mock.sentinel.action])
- def test_policy(*args):
- raise rbac_exceptions.RbacConflictingPolicies()
-
- mock_log.error.assert_not_called()
-
- @mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
- def test_rule_validation_rbac_conflicting_policies_negative(self,
- mock_authority,
- mock_log):
- """Test RbacConflictingPolicies error is thrown with permission fails.
-
- Negative test case: if RbacConflictingPolicies is thrown and the user
- is allowed to perform the action, then this is an expected failure.
- """
- mock_authority.PolicyAuthority.return_value.allowed.return_value = True
-
- @rbac_rv.action(mock.sentinel.service, rules=[mock.sentinel.action])
- def test_policy(*args):
- raise rbac_exceptions.RbacConflictingPolicies()
-
- test_re = ("Role Member was not allowed to perform the following "
- "actions: \[%s\].*" % (mock.sentinel.action))
- self.assertRaisesRegex(
- rbac_exceptions.RbacUnderPermissionException, test_re, test_policy,
- self.mock_test_args)
- self.assertRegex(mock_log.error.mock_calls[0][1][0], test_re)
-
- @mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_expect_not_found_but_raises_forbidden(self, mock_authority,
mock_log):
"""Test that expecting 404 but getting 403 works for all scenarios.