Merge "Add periodic-stable entry to .zuul.yaml"
diff --git a/.zuul.yaml b/.zuul.yaml
index 77755ed..fe4bd4f 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -2,8 +2,8 @@
name: patrole-base
parent: devstack-tempest
description: |
- Patrole base job for admin and member roles. This job executes RBAC tests
- for all the "core" services that Tempest covers, excluding Swift.
+ Patrole base job for admin and member roles. This job executes RBAC tests
+ for all the "core" services that Tempest covers, excluding Swift.
required-projects:
- name: openstack/tempest
- name: openstack/patrole
@@ -139,16 +139,15 @@
name: patrole-plugin-base
parent: patrole-base
description: |
- Patrole plugin job for admin and member roles which
- runs RBAC tests for neutron-tempest-plugin APIs (if the plugin is installed).
+ Patrole plugin job for admin and member roles which
+ runs RBAC tests for neutron-tempest-plugin APIs (if the plugin is installed).
required-projects:
- name: openstack/tempest
- name: openstack/patrole
- name: openstack/neutron-tempest-plugin
vars:
devstack_localrc:
- TEMPEST_PLUGINS: "'/opt/stack/patrole
- /opt/stack/neutron-tempest-plugin'"
+ TEMPEST_PLUGINS: "'/opt/stack/patrole /opt/stack/neutron-tempest-plugin'"
devstack_plugins:
neutron: git://git.openstack.org/openstack/neutron.git
patrole: git://git.openstack.org/openstack/patrole.git
@@ -178,7 +177,14 @@
- project:
templates:
+ - openstack-cover-jobs
+ - openstack-lower-constraints-jobs
- openstack-python36-jobs
+ - openstack-python-jobs
+ - openstack-python35-jobs
+ - check-requirements
+ - publish-openstack-docs-pti
+ - release-notes-jobs-python3
check:
jobs:
- patrole-admin
@@ -189,7 +195,6 @@
- patrole-py35-member
- patrole-multinode-admin
- patrole-multinode-member
- - openstack-tox-lower-constraints
- patrole-plugin-admin
- patrole-plugin-member
gate:
diff --git a/patrole_tempest_plugin/rbac_exceptions.py b/patrole_tempest_plugin/rbac_exceptions.py
index 809a7ed..3958e17 100644
--- a/patrole_tempest_plugin/rbac_exceptions.py
+++ b/patrole_tempest_plugin/rbac_exceptions.py
@@ -16,12 +16,16 @@
from tempest.lib import exceptions
-class RbacConflictingPolicies(exceptions.TempestException):
+class BasePatroleException(exceptions.TempestException):
+ message = "An unknown RBAC exception occurred"
+
+
+class RbacConflictingPolicies(BasePatroleException):
message = ("Conflicting policies preventing this action from being "
"performed.")
-class RbacMalformedResponse(exceptions.TempestException):
+class RbacMalformedResponse(BasePatroleException):
message = ("The response body is missing the expected %(attribute)s due "
"to policy enforcement failure.")
@@ -37,25 +41,25 @@
super(RbacMalformedResponse, self).__init__(**kwargs)
-class RbacResourceSetupFailed(exceptions.TempestException):
+class RbacResourceSetupFailed(BasePatroleException):
message = "RBAC resource setup failed"
-class RbacOverPermissionException(exceptions.TempestException):
+class RbacOverPermissionException(BasePatroleException):
"""Raised when the expected result is failure but the actual result is
pass.
"""
message = "Unauthorized action was allowed to be performed"
-class RbacUnderPermissionException(exceptions.TempestException):
+class RbacUnderPermissionException(BasePatroleException):
"""Raised when the expected result is pass but the actual result is
failure.
"""
message = "Authorized action was not allowed to be performed"
-class RbacExpectedWrongException(exceptions.TempestException):
+class RbacExpectedWrongException(BasePatroleException):
"""Raised when the expected exception does not match the actual exception
raised, when both are instances of Forbidden or NotFound, indicating
the test provides a wrong argument to `expected_error_codes`.
@@ -64,16 +68,30 @@
"instead. Actual exception: %(exception)s")
-class RbacInvalidServiceException(exceptions.TempestException):
+class RbacInvalidServiceException(BasePatroleException):
"""Raised when an invalid service is passed to ``rbac_rule_validation``
decorator.
"""
message = "Attempted to test an invalid service"
-class RbacParsingException(exceptions.TempestException):
+class RbacParsingException(BasePatroleException):
message = "Attempted to test an invalid policy file or action"
-class RbacInvalidErrorCode(exceptions.TempestException):
+class RbacInvalidErrorCode(BasePatroleException):
message = "Unsupported error code passed in test"
+
+
+class RbacOverrideRoleException(BasePatroleException):
+ """Raised when override_role is used incorrectly or fails somehow.
+
+ Used for safeguarding against false positives that might occur when the
+ expected exception isn't raised inside the ``override_role`` context.
+ Specifically, when:
+
+ * ``override_role`` isn't called
+ * an exception is raised before ``override_role`` context
+ * an exception is raised after ``override_role`` context
+ """
+ message = "Override role failure or incorrect usage"
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 32deb9f..a7927fc 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -180,6 +180,7 @@
expected_exception, irregular_msg = _get_exception_type(
exp_error_code)
+ caught_exception = None
test_status = 'Allowed'
try:
@@ -193,13 +194,16 @@
LOG.error(msg)
except (expected_exception,
rbac_exceptions.RbacConflictingPolicies,
- rbac_exceptions.RbacMalformedResponse) as e:
+ rbac_exceptions.RbacMalformedResponse) as actual_exception:
+ caught_exception = actual_exception
test_status = 'Denied'
+
if irregular_msg:
LOG.warning(irregular_msg,
test_func.__name__,
', '.join(rules),
service)
+
if allowed:
msg = ("Role %s was not allowed to perform the following "
"actions: %s. Expected allowed actions: %s. "
@@ -209,8 +213,10 @@
sorted(disallowed_rules)))
LOG.error(msg)
raise rbac_exceptions.RbacUnderPermissionException(
- "%s Exception was: %s" % (msg, e))
+ "%s Exception was: %s" % (msg, actual_exception))
except Exception as actual_exception:
+ caught_exception = actual_exception
+
if _check_for_expected_mismatch_exception(expected_exception,
actual_exception):
LOG.error('Expected and actual exceptions do not match. '
@@ -249,6 +255,14 @@
"Allowed" if allowed else "Denied",
test_status)
+ # Sanity-check that ``override_role`` was called to eliminate
+ # false-positives and bad test flows resulting from exceptions
+ # getting raised too early, too late or not at all, within
+ # the scope of an RBAC test.
+ _validate_override_role_called(
+ test_obj,
+ actual_exception=caught_exception)
+
return wrapper
return decorator
@@ -389,7 +403,7 @@
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.")
+ "irregular")
return expected_exception, irregular_msg
@@ -431,8 +445,63 @@
def _check_for_expected_mismatch_exception(expected_exception,
actual_exception):
+ """Checks that ``expected_exception`` matches ``actual_exception``.
+
+ Since Patrole must handle 403/404 it is important that the expected and
+ actual error codes match.
+
+ :param excepted_exception: Expected exception for test.
+ :param actual_exception: Actual exception raised by test.
+ :returns: True if match, else False.
+ :rtype: boolean
+ """
permission_exceptions = (lib_exc.Forbidden, lib_exc.NotFound)
if isinstance(actual_exception, permission_exceptions):
if not isinstance(actual_exception, expected_exception.__class__):
return True
return False
+
+
+def _validate_override_role_called(test_obj, actual_exception):
+ """Validates that :func:`rbac_utils.RbacUtils.override_role` is called
+ during each Patrole test.
+
+ Useful for validating that the expected exception isn't raised too early
+ (before ``override_role`` call) or too late (after ``override_call``) or
+ at all (which is a bad test).
+
+ :param test_obj: An instance or subclass of ``tempest.test.BaseTestCase``.
+ :param actual_exception: Actual exception raised by test.
+ :raises RbacOverrideRoleException: If ``override_role`` isn't called, is
+ called too early, or is called too late.
+ """
+ called = test_obj._validate_override_role_called()
+ base_msg = ('This error is unrelated to RBAC and is due to either '
+ 'an API or override role failure. Exception: %s' %
+ actual_exception)
+
+ if not called:
+ if actual_exception is not None:
+ msg = ('Caught exception (%s) but it was raised before the '
+ '`override_role` context. ' % actual_exception.__class__)
+ else:
+ msg = 'Test missing required `override_role` call. '
+ msg += base_msg
+ LOG.error(msg)
+ raise rbac_exceptions.RbacOverrideRoleException(msg)
+ else:
+ exc_caught_in_ctx = test_obj._validate_override_role_caught_exc()
+ # This block is only executed if ``override_role`` is called. If
+ # an exception is raised and the exception wasn't raised in the
+ # ``override_role`` context and if the exception isn't a valid
+ # exception type (instance of ``BasePatroleException``), then this is
+ # a legitimate error.
+ if (not exc_caught_in_ctx and
+ actual_exception is not None and
+ not isinstance(actual_exception,
+ rbac_exceptions.BasePatroleException)):
+ msg = ('Caught exception (%s) but it was raised after the '
+ '`override_role` context. ' % actual_exception.__class__)
+ msg += base_msg
+ LOG.error(msg)
+ raise rbac_exceptions.RbacOverrideRoleException(msg)
diff --git a/patrole_tempest_plugin/rbac_utils.py b/patrole_tempest_plugin/rbac_utils.py
index 9a9f864..c928f40 100644
--- a/patrole_tempest_plugin/rbac_utils.py
+++ b/patrole_tempest_plugin/rbac_utils.py
@@ -14,6 +14,7 @@
# under the License.
from contextlib import contextmanager
+import sys
import time
from oslo_log import log as logging
@@ -95,11 +96,17 @@
# if the API call above threw an exception, any code below this
# point in the test is not executed.
"""
+ test_obj._set_override_role_called()
self._override_role(test_obj, True)
try:
# Execute the test.
yield
finally:
+ # Check whether an exception was raised. If so, remember that
+ # for future validation.
+ exc = sys.exc_info()[0]
+ if exc is not None:
+ test_obj._set_override_role_caught_exc()
# This code block is always executed, no matter the result of the
# test. Automatically switch back to the admin role for test clean
# up.
@@ -222,6 +229,11 @@
cls.setup_rbac_utils()
"""
+ # Shows if override_role was called.
+ __override_role_called = False
+ # Shows if exception raised during override_role.
+ __override_role_caught_exc = False
+
@classmethod
def get_auth_providers(cls):
"""Returns list of auth_providers used within test.
@@ -238,7 +250,7 @@
"deprecated and will be removed in the S "
"release. Patrole tests will always be enabled "
"following installation of the Patrole Tempest "
- "plugin. Use a regex to skip tests.")
+ "plugin. Use a regex to skip tests")
versionutils.report_deprecated_feature(LOG, deprecation_msg)
raise cls.skipException(
'Patrole testing not enabled so skipping %s.' % cls.__name__)
@@ -247,6 +259,33 @@
def setup_rbac_utils(cls):
cls.rbac_utils = RbacUtils(cls)
+ def _set_override_role_called(self):
+ """Helper for tracking whether ``override_role`` was called."""
+ self.__override_role_called = True
+
+ def _set_override_role_caught_exc(self):
+ """Helper for tracking whether exception was thrown inside
+ ``override_role``.
+ """
+ self.__override_role_caught_exc = True
+
+ def _validate_override_role_called(self):
+ """Idempotently validate that ``override_role`` is called and reset
+ its value to False for sequential tests.
+ """
+ was_called = self.__override_role_called
+ self.__override_role_called = False
+ return was_called
+
+ def _validate_override_role_caught_exc(self):
+ """Idempotently validate that exception was caught inside
+ ``override_role``, so that, by process of elimination, it can be
+ determined whether one was thrown outside (which is invalid).
+ """
+ caught_exception = self.__override_role_caught_exc
+ self.__override_role_caught_exc = False
+ return caught_exception
+
def is_admin():
"""Verifies whether the current test role equals the admin role.
diff --git a/patrole_tempest_plugin/tests/api/compute/rbac_base.py b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
index 18d2f48..99137cc 100644
--- a/patrole_tempest_plugin/tests/api/compute/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
@@ -12,14 +12,11 @@
# under the License.
from tempest.api.compute import base as compute_base
-from tempest import config
from tempest.lib.common.utils import data_utils
from tempest.lib.common.utils import test_utils
from patrole_tempest_plugin import rbac_utils
-CONF = config.CONF
-
class BaseV2ComputeRbacTest(rbac_utils.RbacUtilsMixin,
compute_base.BaseV2ComputeTest):
diff --git a/patrole_tempest_plugin/tests/api/identity/rbac_base.py b/patrole_tempest_plugin/tests/api/identity/rbac_base.py
index 91b3d1e..d34caaa 100644
--- a/patrole_tempest_plugin/tests/api/identity/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/identity/rbac_base.py
@@ -16,13 +16,11 @@
from oslo_log import log as logging
from tempest.api.identity import base
-from tempest import config
from tempest.lib.common.utils import data_utils
from tempest.lib.common.utils import test_utils
from patrole_tempest_plugin import rbac_utils
-CONF = config.CONF
LOG = logging.getLogger(__name__)
diff --git a/patrole_tempest_plugin/tests/api/identity/v3/test_tokens_negative_rbac.py b/patrole_tempest_plugin/tests/api/identity/v3/test_tokens_negative_rbac.py
index 00d522c..da5d4cd 100644
--- a/patrole_tempest_plugin/tests/api/identity/v3/test_tokens_negative_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v3/test_tokens_negative_rbac.py
@@ -13,7 +13,6 @@
# License for the specific language governing permissions and limitations
# under the License.
-from tempest import config
from tempest.lib import decorators
from tempest.lib import exceptions as lib_exc
@@ -21,8 +20,6 @@
from patrole_tempest_plugin import rbac_utils
from patrole_tempest_plugin.tests.api.identity import rbac_base
-CONF = config.CONF
-
class IdentityTokenV3RbacTest(rbac_base.BaseIdentityV3RbacTest):
diff --git a/patrole_tempest_plugin/tests/api/image/rbac_base.py b/patrole_tempest_plugin/tests/api/image/rbac_base.py
index 954790d..2220335 100644
--- a/patrole_tempest_plugin/tests/api/image/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/image/rbac_base.py
@@ -12,12 +12,9 @@
# under the License.
from tempest.api.image import base as image_base
-from tempest import config
from patrole_tempest_plugin import rbac_utils
-CONF = config.CONF
-
class BaseV2ImageRbacTest(rbac_utils.RbacUtilsMixin,
image_base.BaseV2ImageTest):
diff --git a/patrole_tempest_plugin/tests/api/network/rbac_base.py b/patrole_tempest_plugin/tests/api/network/rbac_base.py
index 9d3e28b..39ad311 100644
--- a/patrole_tempest_plugin/tests/api/network/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/network/rbac_base.py
@@ -14,12 +14,9 @@
# under the License.
from tempest.api.network import base as network_base
-from tempest import config
from patrole_tempest_plugin import rbac_utils
-CONF = config.CONF
-
class BaseNetworkRbacTest(rbac_utils.RbacUtilsMixin,
network_base.BaseNetworkTest):
diff --git a/patrole_tempest_plugin/tests/api/volume/rbac_base.py b/patrole_tempest_plugin/tests/api/volume/rbac_base.py
index bac173e..8fa3b67 100644
--- a/patrole_tempest_plugin/tests/api/volume/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/volume/rbac_base.py
@@ -13,14 +13,11 @@
from tempest.api.volume import base as vol_base
from tempest.common import waiters
-from tempest import config
from tempest.lib.common.utils import data_utils
from tempest.lib.common.utils import test_utils
from patrole_tempest_plugin import rbac_utils
-CONF = config.CONF
-
class BaseVolumeRbacTest(rbac_utils.RbacUtilsMixin,
vol_base.BaseVolumeTest):
diff --git a/patrole_tempest_plugin/tests/unit/fixtures.py b/patrole_tempest_plugin/tests/unit/fixtures.py
index 1c47985..4552224 100644
--- a/patrole_tempest_plugin/tests/unit/fixtures.py
+++ b/patrole_tempest_plugin/tests/unit/fixtures.py
@@ -16,6 +16,7 @@
"""Fixtures for Patrole tests."""
from __future__ import absolute_import
+from contextlib import contextmanager
import fixtures
import mock
import time
@@ -117,6 +118,17 @@
new_role = 'member' if role_toggle else 'admin'
self.set_roles(['admin', 'member'], [new_role])
+ @contextmanager
+ def real_override_role(self, test_obj):
+ """Actual call to ``override_role``.
+
+ Useful for ensuring all the necessary mocks are performed before
+ the method in question is called.
+ """
+ _rbac_utils = rbac_utils.RbacUtils(test_obj)
+ with _rbac_utils.override_role(test_obj):
+ yield
+
def set_roles(self, roles, roles_on_project=None):
"""Set the list of available roles in the system.
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 1bf5510..fe36f2c 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -12,9 +12,12 @@
# License for the specific language governing permissions and limitations
# under the License.
+from __future__ import absolute_import
+
import mock
from oslo_config import cfg
+import fixtures
from tempest.lib import exceptions
from tempest import manager
from tempest import test
@@ -23,7 +26,7 @@
from patrole_tempest_plugin import rbac_exceptions
from patrole_tempest_plugin import rbac_rule_validation as rbac_rv
from patrole_tempest_plugin import rbac_utils
-from patrole_tempest_plugin.tests.unit import fixtures
+from patrole_tempest_plugin.tests.unit import fixtures as patrole_fixtures
CONF = cfg.CONF
@@ -43,10 +46,12 @@
setattr(self.mock_test_args.os_primary, 'credentials', mock_creds)
self.useFixture(
- fixtures.ConfPatcher(rbac_test_role='Member', group='patrole'))
+ patrole_fixtures.ConfPatcher(rbac_test_role='Member',
+ group='patrole'))
# Disable patrole log for unit tests.
self.useFixture(
- fixtures.ConfPatcher(enable_reporting=False, group='patrole_log'))
+ patrole_fixtures.ConfPatcher(enable_reporting=False,
+ group='patrole_log'))
class RBACRuleValidationTest(BaseRBACRuleValidationTest):
@@ -54,6 +59,12 @@
``rbac_rule_validation`` decorator.
"""
+ def setUp(self):
+ super(RBACRuleValidationTest, self).setUp()
+ # This behavior is tested in separate test class below.
+ self.useFixture(fixtures.MockPatchObject(
+ rbac_rv, '_validate_override_role_called'))
+
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_rule_validation_have_permission_no_exc(self, mock_authority,
@@ -269,7 +280,7 @@
mock_log.warning.assert_called_with(
"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.",
+ "404 instead of a 403, which is irregular",
test_policy.__name__,
', '.join(policy_names),
mock.sentinel.service)
@@ -334,7 +345,7 @@
expected_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.")
+ "404 instead of a 403, which is irregular")
actual_exception, actual_irregular_msg = \
rbac_rv._get_exception_type(404)
@@ -385,6 +396,12 @@
Patrole RBAC validation work flows.
"""
+ def setUp(self):
+ super(RBACRuleValidationLoggingTest, self).setUp()
+ # This behavior is tested in separate test class below.
+ self.useFixture(fixtures.MockPatchObject(
+ rbac_rv, '_validate_override_role_called'))
+
@mock.patch.object(rbac_rv, 'RBACLOG', autospec=True)
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_rbac_report_logging_disabled(self, mock_authority, mock_rbaclog):
@@ -392,7 +409,8 @@
is False
"""
self.useFixture(
- fixtures.ConfPatcher(enable_reporting=False, group='patrole_log'))
+ patrole_fixtures.ConfPatcher(enable_reporting=False,
+ group='patrole_log'))
mock_authority.PolicyAuthority.return_value.allowed.return_value = True
@@ -410,7 +428,8 @@
True
"""
self.useFixture(
- fixtures.ConfPatcher(enable_reporting=True, group='patrole_log'))
+ patrole_fixtures.ConfPatcher(enable_reporting=True,
+ group='patrole_log'))
mock_authority.PolicyAuthority.return_value.allowed.return_value = True
policy_names = ['foo:bar', 'baz:qux']
@@ -432,6 +451,12 @@
class RBACRuleValidationNegativeTest(BaseRBACRuleValidationTest):
+ def setUp(self):
+ super(RBACRuleValidationNegativeTest, self).setUp()
+ # This behavior is tested in separate test class below.
+ self.useFixture(fixtures.MockPatchObject(
+ rbac_rv, '_validate_override_role_called'))
+
@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."""
@@ -451,6 +476,12 @@
``rbac_rule_validation`` decorator.
"""
+ def setUp(self):
+ super(RBACRuleValidationTestMultiPolicy, self).setUp()
+ # This behavior is tested in separate test class below.
+ self.useFixture(fixtures.MockPatchObject(
+ rbac_rv, '_validate_override_role_called'))
+
def _assert_policy_authority_called_with(self, rules, mock_authority):
m_authority = mock_authority.PolicyAuthority.return_value
m_authority.allowed.assert_has_calls([
@@ -708,3 +739,189 @@
# When expected_error_codes is provided rules must be as well.
self.assertRaisesRegex(ValueError, error_re, _do_test,
None, None, None, [404])
+
+
+class RBACOverrideRoleValidationTest(BaseRBACRuleValidationTest):
+ """Class for validating that untimely exceptions (outside
+ ``override_role`` is called) result in test failures.
+
+ This regression tests false positives caused by test exceptions matching
+ the expected exception before or after the ``override_role`` context is
+ called. Also tests case where ``override_role`` is never called which is
+ an invalid Patrole test.
+
+ """
+
+ def setUp(self):
+ super(RBACOverrideRoleValidationTest, self).setUp()
+
+ # Mixin automatically initializes __override_role_called to False.
+ class FakeRbacTest(rbac_utils.RbacUtilsMixin, test.BaseTestCase):
+ def runTest(self):
+ pass
+
+ # Stub out problematic function calls.
+ FakeRbacTest.os_primary = mock.Mock(spec=manager.Manager)
+ FakeRbacTest.rbac_utils = self.useFixture(
+ patrole_fixtures.RbacUtilsFixture())
+ mock_creds = mock.Mock(user_id=mock.sentinel.user_id,
+ project_id=mock.sentinel.project_id)
+ setattr(FakeRbacTest.os_primary, 'credentials', mock_creds)
+ setattr(FakeRbacTest.os_primary, 'auth_provider', mock.Mock())
+
+ self.parent_class = FakeRbacTest
+
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_override_role_called_inside_ctx(self,
+ mock_authority):
+ """Test success case when the expected exception is raised within the
+ override_role context.
+ """
+ mock_authority.PolicyAuthority.return_value.allowed.return_value =\
+ False
+
+ class ChildRbacTest(self.parent_class):
+
+ @rbac_rv.action(mock.sentinel.service, rules=["fake:rule"],
+ expected_error_codes=[404])
+ def test_called(self_):
+ with self_.rbac_utils.real_override_role(self_):
+ raise exceptions.NotFound()
+
+ child_test = ChildRbacTest()
+ child_test.test_called()
+
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_override_role_patrole_exception_ignored(
+ self, mock_authority):
+ """Test success case where Patrole exception is raised (which is
+ valid in case of e.g. RbacMalformedException) after override_role
+ passes.
+ """
+ mock_authority.PolicyAuthority.return_value.allowed.return_value =\
+ True
+
+ class ChildRbacTest(self.parent_class):
+
+ @rbac_rv.action(mock.sentinel.service, rules=["fake:rule"],
+ expected_error_codes=[404])
+ def test_called(self_):
+ with self_.rbac_utils.real_override_role(self_):
+ pass
+ # Instances of BasePatroleException don't count as they are
+ # part of the validation work flow.
+ raise rbac_exceptions.BasePatroleException()
+
+ child_test = ChildRbacTest()
+ self.assertRaises(rbac_exceptions.BasePatroleException,
+ child_test.test_called)
+
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_override_role_called_before_ctx(self,
+ mock_authority):
+ """Test failure case when an exception that happens before
+ ``override_role`` context, even if it is the expected exception,
+ raises ``RbacOverrideRoleException``.
+ """
+ mock_authority.PolicyAuthority.return_value.allowed.return_value =\
+ False
+
+ # This behavior should work for supported (NotFound/Forbidden) and
+ # miscellaneous exceptions alike.
+ for exception_type in (exceptions.NotFound,
+ Exception):
+ class ChildRbacTest(self.parent_class):
+
+ @rbac_rv.action(mock.sentinel.service, rules=["fake:rule"],
+ expected_error_codes=[404])
+ def test_called_before(self_):
+ raise exception_type()
+
+ child_test = ChildRbacTest()
+ test_re = ".*before.*"
+ self.assertRaisesRegex(rbac_exceptions.RbacOverrideRoleException,
+ test_re, child_test.test_called_before)
+
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_override_role_called_after_ctx(self,
+ mock_authority):
+ """Test failure case when an exception that happens before
+ ``override_role`` context, even if it is the expected exception,
+ raises ``RbacOverrideRoleException``.
+ """
+ mock_authority.PolicyAuthority.return_value.allowed.return_value =\
+ False
+
+ # This behavior should work for supported (NotFound/Forbidden) and
+ # miscellaneous exceptions alike.
+ for exception_type in (exceptions.NotFound,
+ Exception):
+ class ChildRbacTest(self.parent_class):
+
+ @rbac_rv.action(mock.sentinel.service, rules=["fake:rule"],
+ expected_error_codes=[404])
+ def test_called_after(self_):
+ with self_.rbac_utils.real_override_role(self_):
+ pass
+ # Simulates a test tearDown failure or some such.
+ raise exception_type()
+
+ child_test = ChildRbacTest()
+ test_re = ".*after.*"
+ self.assertRaisesRegex(rbac_exceptions.RbacOverrideRoleException,
+ test_re, child_test.test_called_after)
+
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_override_role_never_called(self, mock_authority):
+ """Test failure case where override_role is **never** called."""
+ mock_authority.PolicyAuthority.return_value.allowed.return_value =\
+ False
+
+ class ChildRbacTest(self.parent_class):
+
+ @rbac_rv.action(mock.sentinel.service, rules=["fake:rule"],
+ expected_error_codes=[404])
+ def test_never_called(self_):
+ pass
+
+ child_test = ChildRbacTest()
+ test_re = ".*missing required `override_role` call.*"
+ self.assertRaisesRegex(rbac_exceptions.RbacOverrideRoleException,
+ test_re, child_test.test_never_called)
+
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_override_role_sequential_test_calls(
+ self, mock_authority):
+ """Test success/failure scenarios above across sequential test calls.
+ """
+ mock_authority.PolicyAuthority.return_value.allowed.return_value =\
+ False
+
+ class ChildRbacTest(self.parent_class):
+
+ @rbac_rv.action(mock.sentinel.service, rules=["fake:rule1"],
+ expected_error_codes=[404])
+ def test_called(self_):
+ with self_.rbac_utils.real_override_role(self_):
+ raise exceptions.NotFound()
+
+ @rbac_rv.action(mock.sentinel.service, rules=["fake:rule2"],
+ expected_error_codes=[404])
+ def test_called_before(self_):
+ raise exceptions.NotFound()
+
+ test_re = ".*before.*"
+
+ # Test case where override role is called in first test but *not* in
+ # second test.
+ child_test1 = ChildRbacTest()
+ child_test1.test_called()
+ self.assertRaisesRegex(rbac_exceptions.RbacOverrideRoleException,
+ test_re, child_test1.test_called_before)
+
+ # Test case where override role is *not* called in first test but is
+ # in second test.
+ child_test2 = ChildRbacTest()
+ self.assertRaisesRegex(rbac_exceptions.RbacOverrideRoleException,
+ test_re, child_test2.test_called_before)
+ child_test2.test_called()
diff --git a/releasenotes/notes/check-expected-errors-only-in-override-role-f7109a73f5ff70e2.yaml b/releasenotes/notes/check-expected-errors-only-in-override-role-f7109a73f5ff70e2.yaml
new file mode 100644
index 0000000..e0ac744
--- /dev/null
+++ b/releasenotes/notes/check-expected-errors-only-in-override-role-f7109a73f5ff70e2.yaml
@@ -0,0 +1,19 @@
+---
+features:
+ - |
+ Add new exception called ``RbacOverrideRoleException``. Used for
+ safeguarding against false positives that might occur when the expected
+ exception isn't raised inside the ``override_role`` context. Specifically,
+ when:
+
+ * ``override_role`` isn't called
+ * an exception is raised before ``override_role`` context
+ * an exception is raised after ``override_role`` context
+fixes:
+ - |
+ Previously, the ``rbac_rule_validation.action`` decorator could catch
+ expected exceptions with no regard to where the error happened. Such
+ behavior could cause false-positive results. To prevent this from
+ happening from now on, if an exception happens outside of the
+ ``override_role`` context, it will cause
+ ``rbac_exceptions.RbacOverrideRoleException`` to be raised.