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
This commit is contained in:
Felipe Monteiro 2018-10-05 21:29:25 +01:00
parent 91e33c6ef1
commit 8dd5f19901
4 changed files with 4 additions and 60 deletions

View File

@ -20,11 +20,6 @@ class BasePatroleException(exceptions.TempestException):
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.")

View File

@ -198,7 +198,6 @@ def action(service,
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'

View File

@ -21,9 +21,7 @@ from tempest import config
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 @@ class ComputeServersRbacTest(base.BaseV2ComputeRbacTest):
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')

View File

@ -166,48 +166,6 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
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_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,