Improve exception that is raised following invalid service
This patchset changes the exception that is raised following
an invalid service being passed in for testing. Instead of
a NotFound getting raised (which doesn't make sense), the
original exception, RbacInvalidServiceException, is raised
instead. In addition "Exception" is appended to the original
exception name for greater clarity and consistency with other
exception names.
This patchset is similar to [0] which aims to simplify and
consolidate how service input is validated.
[0] https://review.openstack.org/#/c/577963/
Change-Id: Ib91a6581f89a528630d756840176b0e16663fa6f
diff --git a/patrole_tempest_plugin/policy_authority.py b/patrole_tempest_plugin/policy_authority.py
index b813f88..3339a5d 100644
--- a/patrole_tempest_plugin/policy_authority.py
+++ b/patrole_tempest_plugin/policy_authority.py
@@ -136,7 +136,7 @@
if not service or service not in cls.available_services:
LOG.debug("%s is NOT a valid service.", service)
- raise rbac_exceptions.RbacInvalidService(
+ raise rbac_exceptions.RbacInvalidServiceException(
"%s is NOT a valid service." % service)
@classmethod
diff --git a/patrole_tempest_plugin/rbac_exceptions.py b/patrole_tempest_plugin/rbac_exceptions.py
index 980672a..809a7ed 100644
--- a/patrole_tempest_plugin/rbac_exceptions.py
+++ b/patrole_tempest_plugin/rbac_exceptions.py
@@ -64,7 +64,10 @@
"instead. Actual exception: %(exception)s")
-class RbacInvalidService(exceptions.TempestException):
+class RbacInvalidServiceException(exceptions.TempestException):
+ """Raised when an invalid service is passed to ``rbac_rule_validation``
+ decorator.
+ """
message = "Attempted to test an invalid service"
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 2f6759d..e6d1e80 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -22,7 +22,7 @@
import six
from tempest import config
-from tempest.lib import exceptions
+from tempest.lib import exceptions as lib_exc
from tempest import test
from patrole_tempest_plugin import policy_authority
@@ -123,7 +123,7 @@
"os_alt.auth_provider.credentials.user_id"
})
- :raises NotFound: If ``service`` is invalid.
+ :raises RbacInvalidServiceException: If ``service`` is invalid.
:raises RbacUnderPermissionException: For item (2) above.
:raises RbacOverPermissionException: For item (3) above.
:raises RbacExpectedWrongException: When a 403 is expected but a 404
@@ -184,12 +184,13 @@
try:
test_func(*args, **kwargs)
- except rbac_exceptions.RbacInvalidService as e:
- msg = ("%s is not a valid service." % service)
- test_status = ('Error, %s' % (msg))
- LOG.error(msg)
- raise exceptions.NotFound(
- "%s RbacInvalidService was: %s" % (msg, e))
+ except rbac_exceptions.RbacInvalidServiceException:
+ with excutils.save_and_reraise_exception():
+ msg = ("%s is not a valid service." % service)
+ # FIXME(felipemonteiro): This test_status is logged too
+ # late. Need a function to log it before re-raising.
+ test_status = ('Error, %s' % (msg))
+ LOG.error(msg)
except (expected_exception,
rbac_exceptions.RbacConflictingPolicies,
rbac_exceptions.RbacMalformedResponse) as e:
@@ -382,14 +383,13 @@
raise rbac_exceptions.RbacInvalidErrorCode(msg)
if expected_error_code == 403:
- expected_exception = exceptions.Forbidden
+ expected_exception = lib_exc.Forbidden
elif expected_error_code == 404:
- expected_exception = exceptions.NotFound
+ expected_exception = lib_exc.NotFound
irregular_msg = ("NotFound exception was caught for test %s. Expected "
"policies which may have caused the error: %s. The "
"service %s throws a 404 instead of a 403, which is "
"irregular.")
-
return expected_exception, irregular_msg
@@ -431,7 +431,7 @@
def _check_for_expected_mismatch_exception(expected_exception,
actual_exception):
- permission_exceptions = (exceptions.Forbidden, exceptions.NotFound)
+ permission_exceptions = (lib_exc.Forbidden, lib_exc.NotFound)
if isinstance(actual_exception, permission_exceptions):
if not isinstance(actual_exception, expected_exception.__class__):
return True
diff --git a/patrole_tempest_plugin/tests/unit/test_policy_authority.py b/patrole_tempest_plugin/tests/unit/test_policy_authority.py
index 3e2cc4c..d396a29 100644
--- a/patrole_tempest_plugin/tests/unit/test_policy_authority.py
+++ b/patrole_tempest_plugin/tests/unit/test_policy_authority.py
@@ -238,7 +238,7 @@
test_user_id = mock.sentinel.user_id
service = 'invalid_service'
- self.assertRaises(rbac_exceptions.RbacInvalidService,
+ self.assertRaises(rbac_exceptions.RbacInvalidServiceException,
policy_authority.PolicyAuthority,
test_tenant_id,
test_user_id,
@@ -253,7 +253,7 @@
test_user_id = mock.sentinel.user_id
service = None
- self.assertRaises(rbac_exceptions.RbacInvalidService,
+ self.assertRaises(rbac_exceptions.RbacInvalidServiceException,
policy_authority.PolicyAuthority,
test_tenant_id,
test_user_id,
@@ -528,8 +528,9 @@
policy_parser = None
expected_exception = 'invalid_service is NOT a valid service'
- with self.assertRaisesRegex(rbac_exceptions.RbacInvalidService,
- expected_exception):
+ with self.assertRaisesRegex(
+ rbac_exceptions.RbacInvalidServiceException,
+ expected_exception):
policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "INVALID_SERVICE")
else:
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 2e275dc..1bf5510 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -430,6 +430,22 @@
"Allowed")
+class RBACRuleValidationNegativeTest(BaseRBACRuleValidationTest):
+
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_invalid_service_raises_exc(self, mock_authority):
+ """Test that invalid service raises the appropriate exception."""
+ mock_authority.PolicyAuthority.return_value.allowed.side_effect = (
+ rbac_exceptions.RbacInvalidServiceException)
+
+ @rbac_rv.action(mock.sentinel.service, mock.sentinel.action)
+ def test_policy(*args):
+ pass
+
+ self.assertRaises(rbac_exceptions.RbacInvalidServiceException,
+ test_policy, self.mock_test_args)
+
+
class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
"""Test suite for validating multi-policy support for the
``rbac_rule_validation`` decorator.