Merge "Adds tests to cover address scopes"
diff --git a/.zuul.yaml b/.zuul.yaml
index 5e6deeb..fb110f0 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
@@ -87,6 +87,11 @@
RBAC_TEST_ROLE: member
- job:
+ name: patrole-member-rocky
+ parent: patrole-member
+ override-checkout: stable/rocky
+
+- job:
name: patrole-member-queens
parent: patrole-member
override-checkout: stable/queens
@@ -134,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
@@ -152,6 +156,7 @@
tempest: true
neutron: true
neutron-segments: true
+ neutron-qos: true
- job:
name: patrole-plugin-member
@@ -173,20 +178,32 @@
- 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
- patrole-member
+ - patrole-member-rocky
- patrole-member-queens
- patrole-member-pike
- patrole-py35-member
- patrole-multinode-admin
- patrole-multinode-member
- - openstack-tox-lower-constraints
- patrole-plugin-admin
- patrole-plugin-member
gate:
jobs:
- patrole-admin
- patrole-member
+ periodic-stable:
+ jobs:
+ - patrole-member-rocky
+ - patrole-member-queens
+ - patrole-member-pike
diff --git a/devstack/plugin.sh b/devstack/plugin.sh
index 4826d21..01be7d6 100644
--- a/devstack/plugin.sh
+++ b/devstack/plugin.sh
@@ -40,7 +40,6 @@
iniset $TEMPEST_CONFIG policy-feature-enabled removed_nova_policies_stein False
fi
- iniset $TEMPEST_CONFIG patrole enable_rbac True
iniset $TEMPEST_CONFIG patrole rbac_test_role $RBAC_TEST_ROLE
}
diff --git a/doc/source/framework/policy_authority.rst b/doc/source/framework/policy_authority.rst
index 822c7b6..37b698c 100644
--- a/doc/source/framework/policy_authority.rst
+++ b/doc/source/framework/policy_authority.rst
@@ -60,3 +60,4 @@
.. automodule:: patrole_tempest_plugin.policy_authority
:members:
:undoc-members:
+ :special-members:
diff --git a/doc/source/framework/rbac_authority.rst b/doc/source/framework/rbac_authority.rst
index 84c372b..40f2a8d 100644
--- a/doc/source/framework/rbac_authority.rst
+++ b/doc/source/framework/rbac_authority.rst
@@ -35,3 +35,4 @@
.. automodule:: patrole_tempest_plugin.rbac_authority
:members:
:undoc-members:
+ :special-members:
diff --git a/doc/source/framework/rbac_utils.rst b/doc/source/framework/rbac_utils.rst
index 7143928..d0fe27e 100644
--- a/doc/source/framework/rbac_utils.rst
+++ b/doc/source/framework/rbac_utils.rst
@@ -176,3 +176,4 @@
.. automodule:: patrole_tempest_plugin.rbac_utils
:members:
:private-members:
+ :special-members:
diff --git a/doc/source/framework/rbac_validation.rst b/doc/source/framework/rbac_validation.rst
index 186dfe2..6cd1534 100644
--- a/doc/source/framework/rbac_validation.rst
+++ b/doc/source/framework/rbac_validation.rst
@@ -17,3 +17,4 @@
.. automodule:: patrole_tempest_plugin.rbac_rule_validation
:members:
:private-members:
+ :special-members:
diff --git a/doc/source/framework/requirements_authority.rst b/doc/source/framework/requirements_authority.rst
index 6c4fcc0..628f0c0 100644
--- a/doc/source/framework/requirements_authority.rst
+++ b/doc/source/framework/requirements_authority.rst
@@ -103,3 +103,4 @@
.. automodule:: patrole_tempest_plugin.requirements_authority
:members:
:undoc-members:
+ :special-members:
diff --git a/patrole_tempest_plugin/config.py b/patrole_tempest_plugin/config.py
index 47b76d4..4ad7f08 100644
--- a/patrole_tempest_plugin/config.py
+++ b/patrole_tempest_plugin/config.py
@@ -24,23 +24,14 @@
default='admin',
help="""The current RBAC role against which to run
Patrole tests."""),
- cfg.BoolOpt('enable_rbac',
- default=True,
- deprecated_for_removal=True,
- deprecated_reason="""This is a legacy option that was
-meaningful when Patrole existed downstream as a suite of tests inside Tempest.
-Installing the Patrole plugin necessarily means that RBAC tests should be run.
-This option is paradoxical with the Tempest plugin architecture.
-""",
- deprecated_since='R',
- help="Enables Patrole RBAC tests."),
cfg.ListOpt('custom_policy_files',
default=['/etc/%s/policy.json'],
help="""List of the paths to search for policy files. Each
policy path assumes that the service name is included in the path once. Also
assumes Patrole is on the same host as the policy files. The paths should be
-ordered by precedence, with high-priority paths before low-priority paths. The
-first path that is found to contain the service's policy file will be used.
+ordered by precedence, with high-priority paths before low-priority paths. All
+the paths that are found to contain the service's policy file will be used and
+all policy files will be merged.
"""),
cfg.BoolOpt('test_custom_requirements',
default=False,
diff --git a/patrole_tempest_plugin/policy_authority.py b/patrole_tempest_plugin/policy_authority.py
index 3339a5d..27786ae 100644
--- a/patrole_tempest_plugin/policy_authority.py
+++ b/patrole_tempest_plugin/policy_authority.py
@@ -13,7 +13,9 @@
# License for the specific language governing permissions and limitations
# under the License.
+import collections
import copy
+import glob
import json
import os
@@ -103,17 +105,14 @@
if extra_target_data is None:
extra_target_data = {}
- self.validate_service(service)
+ self.service = self.validate_service(service)
# Prioritize dynamically searching for policy files over relying on
# deprecated service-specific policy file locations.
- self.path = None
if CONF.patrole.custom_policy_files:
self.discover_policy_files()
- self.path = self.policy_files.get(service)
- self.rules = policy.Rules.load(self._get_policy_data(service),
- 'default')
+ self.rules = policy.Rules.load(self._get_policy_data(), 'default')
self.project_id = project_id
self.user_id = user_id
self.extra_target_data = extra_target_data
@@ -139,19 +138,22 @@
raise rbac_exceptions.RbacInvalidServiceException(
"%s is NOT a valid service." % service)
+ return service
+
@classmethod
def discover_policy_files(cls):
"""Dynamically discover the policy file for each service in
- ``cls.available_services``. Pick the first candidate path found
+ ``cls.available_services``. Pick all candidate paths found
out of the potential paths in ``[patrole] custom_policy_files``.
"""
if not hasattr(cls, 'policy_files'):
- cls.policy_files = {}
+ cls.policy_files = collections.defaultdict(list)
for service in cls.available_services:
for candidate_path in CONF.patrole.custom_policy_files:
- if os.path.isfile(candidate_path % service):
- cls.policy_files.setdefault(service,
- candidate_path % service)
+ path = candidate_path % service
+ for filename in glob.iglob(path):
+ if os.path.isfile(filename):
+ cls.policy_files[service].append(filename)
def allowed(self, rule_name, role):
"""Checks if a given rule in a policy is allowed with given role.
@@ -168,17 +170,28 @@
is_admin=is_admin_context)
return is_allowed
- def _get_policy_data(self, service):
+ def _get_policy_data(self):
file_policy_data = {}
mgr_policy_data = {}
policy_data = {}
# Check whether policy file exists and attempt to read it.
- if self.path and os.path.isfile(self.path):
+ for path in self.policy_files[self.service]:
try:
- with open(self.path, 'r') as policy_file:
- file_policy_data = policy_file.read()
- file_policy_data = json.loads(file_policy_data)
+ with open(path, 'r') as fp:
+ for k, v in json.load(fp).items():
+ if k not in file_policy_data:
+ file_policy_data[k] = v
+ else:
+ # If the policy name and rule are the same, no
+ # ambiguity, so no reason to warn.
+ if v != file_policy_data[k]:
+ LOG.warning(
+ "The same policy name: %s was found in "
+ "multiple policies files for service %s. "
+ "This can lead to policy rule ambiguity. "
+ "Using rule: %s", k, self.service,
+ file_policy_data[k])
except (IOError, ValueError) as e:
msg = "Failed to read policy file for service. "
if isinstance(e, IOError):
@@ -186,21 +199,20 @@
else:
msg += "JSON may be improperly formatted."
LOG.debug(msg)
- file_policy_data = {}
# Check whether policy actions are defined in code. Nova and Keystone,
# for example, define their default policy actions in code.
mgr = stevedore.named.NamedExtensionManager(
'oslo.policy.policies',
- names=[service],
+ names=[self.service],
on_load_failure_callback=None,
invoke_on_load=True,
warn_on_missing_entrypoint=False)
if mgr:
- policy_generator = {policy.name: policy.obj for policy in mgr}
- if policy_generator and service in policy_generator:
- for rule in policy_generator[service]:
+ policy_generator = {plc.name: plc.obj for plc in mgr}
+ if policy_generator and self.service in policy_generator:
+ for rule in policy_generator[self.service]:
mgr_policy_data[rule.name] = str(rule.check)
# If data from both file and code exist, combine both together.
@@ -217,10 +229,10 @@
policy_data = mgr_policy_data
else:
error_message = (
- 'Policy file for {0} service was not found among the '
+ 'Policy files for {0} service were not found among the '
'registered in-code policies or in any of the possible policy '
- 'files: {1}.'.format(service,
- [loc % service for loc in
+ 'files: {1}.'.format(self.service,
+ [loc % self.service for loc in
CONF.patrole.custom_policy_files])
)
raise rbac_exceptions.RbacParsingException(error_message)
@@ -228,8 +240,8 @@
try:
policy_data = json.dumps(policy_data)
except (TypeError, ValueError):
- error_message = 'Policy file for {0} service is invalid.'.format(
- service)
+ error_message = 'Policy files for {0} service are invalid.'.format(
+ self.service)
raise rbac_exceptions.RbacParsingException(error_message)
return policy_data
@@ -296,9 +308,11 @@
def _try_rule(self, apply_rule, target, access_data, o):
if apply_rule not in self.rules:
- message = ("Policy action \"{0}\" not found in policy file: {1} or"
- " among registered policy in code defaults for service."
- ).format(apply_rule, self.path)
+ message = ('Policy action "{0}" not found in policy files: '
+ '{1} or among registered policy in code defaults for '
+ '{2} service.').format(apply_rule,
+ self.policy_files[self.service],
+ self.service)
LOG.debug(message)
raise rbac_exceptions.RbacParsingException(message)
else:
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..366e033 100644
--- a/patrole_tempest_plugin/rbac_utils.py
+++ b/patrole_tempest_plugin/rbac_utils.py
@@ -14,10 +14,10 @@
# under the License.
from contextlib import contextmanager
+import sys
import time
from oslo_log import log as logging
-from oslo_log import versionutils
from oslo_utils import excutils
from tempest import clients
@@ -95,11 +95,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 +228,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.
@@ -232,21 +243,36 @@
return [cls.os_primary.auth_provider]
@classmethod
- def skip_rbac_checks(cls):
- if not CONF.patrole.enable_rbac:
- deprecation_msg = ("The `[patrole].enable_rbac` option is "
- "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.")
- versionutils.report_deprecated_feature(LOG, deprecation_msg)
- raise cls.skipException(
- 'Patrole testing not enabled so skipping %s.' % cls.__name__)
-
- @classmethod
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/README.rst b/patrole_tempest_plugin/tests/api/README.rst
new file mode 120000
index 0000000..e2853ec
--- /dev/null
+++ b/patrole_tempest_plugin/tests/api/README.rst
@@ -0,0 +1 @@
+../../../doc/source/field_guide/rbac.rst
\ No newline at end of file
diff --git a/patrole_tempest_plugin/tests/api/compute/rbac_base.py b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
index 18d2f48..ab4551e 100644
--- a/patrole_tempest_plugin/tests/api/compute/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
@@ -12,24 +12,16 @@
# 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):
@classmethod
- def skip_checks(cls):
- super(BaseV2ComputeRbacTest, cls).skip_checks()
- cls.skip_rbac_checks()
-
- @classmethod
def setup_clients(cls):
super(BaseV2ComputeRbacTest, cls).setup_clients()
cls.setup_rbac_utils()
diff --git a/patrole_tempest_plugin/tests/api/identity/rbac_base.py b/patrole_tempest_plugin/tests/api/identity/rbac_base.py
index 91b3d1e..44f5962 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__)
@@ -30,11 +28,6 @@
base.BaseIdentityTest):
@classmethod
- def skip_checks(cls):
- super(BaseIdentityRbacTest, cls).skip_checks()
- cls.skip_rbac_checks()
-
- @classmethod
def setup_clients(cls):
super(BaseIdentityRbacTest, cls).setup_clients()
cls.setup_rbac_utils()
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..becd564 100644
--- a/patrole_tempest_plugin/tests/api/image/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/image/rbac_base.py
@@ -12,22 +12,14 @@
# 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):
@classmethod
- def skip_checks(cls):
- super(BaseV2ImageRbacTest, cls).skip_checks()
- cls.skip_rbac_checks()
-
- @classmethod
def setup_clients(cls):
super(BaseV2ImageRbacTest, cls).setup_clients()
cls.setup_rbac_utils()
diff --git a/patrole_tempest_plugin/tests/api/network/rbac_base.py b/patrole_tempest_plugin/tests/api/network/rbac_base.py
index 9d3e28b..6102347 100644
--- a/patrole_tempest_plugin/tests/api/network/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/network/rbac_base.py
@@ -14,22 +14,14 @@
# 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):
@classmethod
- def skip_checks(cls):
- super(BaseNetworkRbacTest, cls).skip_checks()
- cls.skip_rbac_checks()
-
- @classmethod
def setup_clients(cls):
super(BaseNetworkRbacTest, cls).setup_clients()
cls.setup_rbac_utils()
diff --git a/patrole_tempest_plugin/tests/api/network/test_dscp_marking_rule_rbac.py b/patrole_tempest_plugin/tests/api/network/test_dscp_marking_rule_rbac.py
new file mode 100644
index 0000000..b9f8365
--- /dev/null
+++ b/patrole_tempest_plugin/tests/api/network/test_dscp_marking_rule_rbac.py
@@ -0,0 +1,106 @@
+# Copyright 2018 AT&T Corporation.
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from tempest.common import utils
+from tempest.lib.common.utils import data_utils
+from tempest.lib.common.utils import test_utils
+from tempest.lib import decorators
+
+from patrole_tempest_plugin import rbac_rule_validation
+from patrole_tempest_plugin.tests.api.network import rbac_base as base
+
+
+class DscpMarkingRulePluginRbacTest(base.BaseNetworkPluginRbacTest):
+
+ @classmethod
+ def skip_checks(cls):
+ super(DscpMarkingRulePluginRbacTest, cls).skip_checks()
+ if not utils.is_extension_enabled('qos', 'network'):
+ msg = "qos extension not enabled."
+ raise cls.skipException(msg)
+
+ @classmethod
+ def resource_setup(cls):
+ super(DscpMarkingRulePluginRbacTest, cls).resource_setup()
+ name = data_utils.rand_name(cls.__class__.__name__ + '-qos')
+ cls.policy_id = cls.ntp_client.create_qos_policy(
+ name=name)["policy"]["id"]
+ cls.addClassResourceCleanup(
+ cls.ntp_client.delete_qos_policy, cls.policy_id)
+
+ def create_policy_dscp_marking_rule(cls):
+ rule = cls.ntp_client.create_dscp_marking_rule(cls.policy_id, 10)
+ rule_id = rule['dscp_marking_rule']['id']
+ cls.addCleanup(
+ test_utils.call_and_ignore_notfound_exc,
+ cls.ntp_client.delete_dscp_marking_rule, cls.policy_id, rule_id)
+ return rule_id
+
+ @decorators.idempotent_id('2717AB75-E4CF-4CA4-AF04-5BEC0C808AA5')
+ @rbac_rule_validation.action(service="neutron",
+ rules=["create_policy_dscp_marking_rule"])
+ def test_create_policy_dscp_marking_rule(self):
+ """Create policy_dscp_marking_rule.
+
+ RBAC test for the neutron "create_policy_dscp_marking_rule" policy
+ """
+
+ with self.rbac_utils.override_role(self):
+ self.create_policy_dscp_marking_rule()
+
+ @decorators.idempotent_id('3D68F50E-B948-4B25-8A72-F6F4890BBC6F')
+ @rbac_rule_validation.action(service="neutron",
+ rules=["get_policy_dscp_marking_rule"],
+ expected_error_codes=[404])
+ def test_show_policy_dscp_marking_rule(self):
+ """Show policy_dscp_marking_rule.
+
+ RBAC test for the neutron "get_policy_dscp_marking_rule" policy
+ """
+ rule_id = self.create_policy_dscp_marking_rule()
+
+ with self.rbac_utils.override_role(self):
+ self.ntp_client.show_dscp_marking_rule(self.policy_id, rule_id)
+
+ @decorators.idempotent_id('33830794-8731-45C3-BC97-17718555DD7C')
+ @rbac_rule_validation.action(service="neutron",
+ rules=["get_policy_dscp_marking_rule",
+ "update_policy_dscp_marking_rule"],
+ expected_error_codes=[404, 403])
+ def test_update_policy_dscp_marking_rule(self):
+ """Update policy_dscp_marking_rule.
+
+ RBAC test for the neutron "update_policy_dscp_marking_rule" policy
+ """
+ rule_id = self.create_policy_dscp_marking_rule()
+
+ with self.rbac_utils.override_role(self):
+ self.ntp_client.update_dscp_marking_rule(
+ self.policy_id, rule_id, dscp_mark=16)
+
+ @decorators.idempotent_id('7BF564DD-3648-4D12-8A8B-6D5E576D1843')
+ @rbac_rule_validation.action(service="neutron",
+ rules=["get_policy_dscp_marking_rule",
+ "delete_policy_dscp_marking_rule"],
+ expected_error_codes=[404, 403])
+ def test_delete_policy_dscp_marking_rule(self):
+ """Delete policy_dscp_marking_rule.
+
+ RBAC test for the neutron "delete_policy_dscp_marking_rule" policy
+ """
+ rule_id = self.create_policy_dscp_marking_rule()
+
+ with self.rbac_utils.override_role(self):
+ self.ntp_client.delete_dscp_marking_rule(self.policy_id, rule_id)
diff --git a/patrole_tempest_plugin/tests/api/network/test_flavors_rbac.py b/patrole_tempest_plugin/tests/api/network/test_flavors_rbac.py
index cdc9852..f8ef0bb 100644
--- a/patrole_tempest_plugin/tests/api/network/test_flavors_rbac.py
+++ b/patrole_tempest_plugin/tests/api/network/test_flavors_rbac.py
@@ -13,6 +13,8 @@
# License for the specific language governing permissions and limitations
# under the License.
+from oslo_serialization import jsonutils as json
+
from tempest.lib.common.utils import data_utils
from tempest.lib.common.utils import test_utils
from tempest.lib import decorators
@@ -116,3 +118,70 @@
with self.rbac_utils.override_role(self):
self.ntp_client.list_flavors()
+
+
+class FlavorsServiceProfilePluginRbacTest(base.BaseNetworkPluginRbacTest):
+ @classmethod
+ def resource_setup(cls):
+ super(FlavorsServiceProfilePluginRbacTest, cls).resource_setup()
+ providers = cls.ntp_client.list_service_providers()
+ if not providers["service_providers"]:
+ raise cls.skipException("No service_providers available.")
+ cls.service_type = providers["service_providers"][0]["service_type"]
+
+ cls.flavor_id = cls.create_flavor()
+ cls.service_profile_id = cls.create_service_profile()
+
+ @classmethod
+ def create_flavor(cls):
+ flavor = cls.ntp_client.create_flavor(service_type=cls.service_type)
+ flavor_id = flavor["flavor"]["id"]
+ cls.addClassResourceCleanup(
+ test_utils.call_and_ignore_notfound_exc,
+ cls.ntp_client.delete_flavor, flavor_id)
+ return flavor_id
+
+ @classmethod
+ def create_service_profile(cls):
+ service_profile = cls.ntp_client.create_service_profile(
+ metainfo=json.dumps({'foo': 'bar'}))
+ service_profile_id = service_profile["service_profile"]["id"]
+ cls.addClassResourceCleanup(
+ test_utils.call_and_ignore_notfound_exc,
+ cls.ntp_client.delete_service_profile, service_profile_id)
+ return service_profile_id
+
+ def create_flavor_service_profile(self, flavor_id, service_profile_id):
+ self.ntp_client.create_flavor_service_profile(
+ flavor_id, service_profile_id)
+ self.addCleanup(
+ test_utils.call_and_ignore_notfound_exc,
+ self.ntp_client.delete_flavor_service_profile,
+ flavor_id, service_profile_id)
+
+ @decorators.idempotent_id('aa84b4c5-0dd6-4c34-aa81-3a76507f9b81')
+ @rbac_rule_validation.action(service="neutron",
+ rules=["create_flavor_service_profile"])
+ def test_create_flavor_service_profile(self):
+ """Create flavor_service_profile.
+
+ RBAC test for the neutron "create_flavor_service_profile" policy
+ """
+ with self.rbac_utils.override_role(self):
+ self.create_flavor_service_profile(self.flavor_id,
+ self.service_profile_id)
+
+ @decorators.idempotent_id('3b680d9e-946a-4670-ab7f-0e4576675833')
+ @rbac_rule_validation.action(service="neutron",
+ rules=["delete_flavor_service_profile"])
+ def test_delete_flavor_service_profile(self):
+ """Delete flavor_service_profile.
+
+ RBAC test for the neutron "delete_flavor_service_profile" policy
+ """
+ self.create_flavor_service_profile(self.flavor_id,
+ self.service_profile_id)
+
+ with self.rbac_utils.override_role(self):
+ self.ntp_client.delete_flavor_service_profile(
+ self.flavor_id, self.service_profile_id)
diff --git a/patrole_tempest_plugin/tests/api/network/test_qos_rbac.py b/patrole_tempest_plugin/tests/api/network/test_qos_rbac.py
new file mode 100644
index 0000000..20f9e61
--- /dev/null
+++ b/patrole_tempest_plugin/tests/api/network/test_qos_rbac.py
@@ -0,0 +1,100 @@
+# Copyright 2018 AT&T Corporation.
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from tempest.common import utils
+from tempest.lib.common.utils import data_utils
+from tempest.lib.common.utils import test_utils
+from tempest.lib import decorators
+
+from patrole_tempest_plugin import rbac_rule_validation
+from patrole_tempest_plugin.tests.api.network import rbac_base as base
+
+
+class QosRbacTest(base.BaseNetworkPluginRbacTest):
+
+ @classmethod
+ def skip_checks(cls):
+ super(QosRbacTest, cls).skip_checks()
+ if not utils.is_extension_enabled('qos', 'network'):
+ msg = "qos extension not enabled."
+ raise cls.skipException(msg)
+
+ @classmethod
+ def resource_setup(cls):
+ super(QosRbacTest, cls).resource_setup()
+ cls.network = cls.create_network()
+
+ def create_policy(self, name=None):
+ name = name or data_utils.rand_name(self.__class__.__name__)
+ policy = self.ntp_client.create_qos_policy(name)['policy']
+ self.addCleanup(test_utils.call_and_ignore_notfound_exc,
+ self.ntp_client.delete_qos_policy, policy['id'])
+ return policy
+
+ @rbac_rule_validation.action(service="neutron",
+ rules=["create_policy"],
+ expected_error_codes=[403])
+ @decorators.idempotent_id('2ade2e48-7f82-4650-a69c-933d8d594636')
+ def test_create_policy(self):
+
+ """Create Policy Test
+
+ RBAC test for the neutron create_policy policy
+ """
+ with self.rbac_utils.override_role(self):
+ self.create_policy()
+
+ @rbac_rule_validation.action(service="neutron",
+ rules=["get_policy"],
+ expected_error_codes=[404])
+ @decorators.idempotent_id('d004a8de-b226-4eb4-9fdc-8202a7f64c56')
+ def test_get_policy(self):
+
+ """Show Policy Test
+
+ RBAC test for the neutron get_policy policy
+ """
+ policy = self.create_policy()
+ with self.rbac_utils.override_role(self):
+ self.ntp_client.show_qos_policy(policy['id'])
+
+ @rbac_rule_validation.action(service="neutron",
+ rules=["get_policy", "update_policy"],
+ expected_error_codes=[404, 403])
+ @decorators.idempotent_id('fb74d56f-1dfc-490b-a9e1-454af583eefb')
+ def test_update_policy(self):
+
+ """Update Policy Test
+
+ RBAC test for the neutron update_policy policy
+ """
+ policy = self.create_policy()
+ with self.rbac_utils.override_role(self):
+ self.ntp_client.update_qos_policy(policy['id'],
+ description='updated')
+
+ @rbac_rule_validation.action(service="neutron",
+ rules=["get_policy", "delete_policy"],
+ expected_error_codes=[404, 403])
+ @decorators.idempotent_id('ef4c23a6-4095-47a6-958e-1df585f7d8db')
+ def test_delete_policy(self):
+
+ """Delete Policy Test
+
+ RBAC test for the neutron delete_policy policy
+ """
+ policy = self.create_policy()
+ with self.rbac_utils.override_role(self):
+ self.ntp_client.delete_qos_policy(policy['id'])
diff --git a/patrole_tempest_plugin/tests/api/network/test_subnetpools_rbac.py b/patrole_tempest_plugin/tests/api/network/test_subnetpools_rbac.py
index 7d02271..62735d7 100644
--- a/patrole_tempest_plugin/tests/api/network/test_subnetpools_rbac.py
+++ b/patrole_tempest_plugin/tests/api/network/test_subnetpools_rbac.py
@@ -65,7 +65,33 @@
@rbac_rule_validation.action(service="neutron",
rules=["create_subnetpool",
- "create_subnetpool:shared"])
+ "create_subnetpool:is_default"],
+ expected_error_codes=[403, 403])
+ @decorators.idempotent_id('1b5509fd-2c32-44a8-a786-1b6ca162dbd2')
+ def test_create_subnetpool_default(self):
+ """Create default subnetpool.
+
+ RBAC test for the neutron create_subnetpool:is_default policy
+ """
+ # Most likely we already have default subnetpools for ipv4 and ipv6,
+ # so we temporary mark them as is_default=False, to let this test pass.
+ def_pools = self.subnetpools_client.list_subnetpools(is_default=True)
+ for default_pool in def_pools["subnetpools"]:
+ self.subnetpools_client.update_subnetpool(default_pool["id"],
+ is_default=False)
+
+ self.addCleanup(self.subnetpools_client.update_subnetpool,
+ default_pool["id"], is_default=True)
+
+ with self.rbac_utils.override_role(self):
+ # It apparently only enforces the policy for is_default=True.
+ # It does nothing for is_default=False
+ self._create_subnetpool(is_default=True)
+
+ @rbac_rule_validation.action(service="neutron",
+ rules=["create_subnetpool",
+ "create_subnetpool:shared"],
+ expected_error_codes=[403, 403])
@decorators.idempotent_id('cf730989-0d47-40bc-b39a-99e7de484723')
def test_create_subnetpool_shared(self):
"""Create subnetpool shared.
diff --git a/patrole_tempest_plugin/tests/api/volume/rbac_base.py b/patrole_tempest_plugin/tests/api/volume/rbac_base.py
index bac173e..14b3151 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):
@@ -32,11 +29,6 @@
_api_version = 3
@classmethod
- def skip_checks(cls):
- super(BaseVolumeRbacTest, cls).skip_checks()
- cls.skip_rbac_checks()
-
- @classmethod
def setup_clients(cls):
super(BaseVolumeRbacTest, cls).setup_clients()
cls.setup_rbac_utils()
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_policy_authority.py b/patrole_tempest_plugin/tests/unit/test_policy_authority.py
index d396a29..b2af1c6 100644
--- a/patrole_tempest_plugin/tests/unit/test_policy_authority.py
+++ b/patrole_tempest_plugin/tests/unit/test_policy_authority.py
@@ -270,9 +270,9 @@
fake_rule = 'fake_rule'
expected_message = (
- "Policy action \"{0}\" not found in policy file: {1} or among "
- "registered policy in code defaults for service.").format(
- fake_rule, self.custom_policy_file)
+ 'Policy action "{0}" not found in policy files: {1} or among '
+ 'registered policy in code defaults for {2} service.').format(
+ fake_rule, [self.custom_policy_file], "custom_rbac_policy")
e = self.assertRaises(rbac_exceptions.RbacParsingException,
authority.allowed, fake_rule, None)
@@ -292,9 +292,10 @@
mock.sentinel.error)})
expected_message = (
- "Policy action \"{0}\" not found in policy file: {1} or among "
- "registered policy in code defaults for service.").format(
- mock.sentinel.rule, self.custom_policy_file)
+ 'Policy action "{0}" not found in policy files: {1} or among '
+ 'registered policy in code defaults for {2} service.').format(
+ mock.sentinel.rule, [self.custom_policy_file],
+ "custom_rbac_policy")
e = self.assertRaises(rbac_exceptions.RbacParsingException,
authority.allowed, mock.sentinel.rule, None)
@@ -313,7 +314,7 @@
]
mock_manager = mock.Mock(obj=fake_policy_rules, __name__='foo')
- mock_manager.configure_mock(name='fake_service')
+ mock_manager.configure_mock(name='tenant_rbac_policy')
mock_stevedore.named.NamedExtensionManager.return_value = [
mock_manager
]
@@ -323,7 +324,7 @@
authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "tenant_rbac_policy")
- policy_data = authority._get_policy_data('fake_service')
+ policy_data = authority._get_policy_data()
self.assertIsInstance(policy_data, str)
actual_policy_data = json.loads(policy_data)
@@ -354,7 +355,7 @@
]
mock_manager = mock.Mock(obj=fake_policy_rules, __name__='foo')
- mock_manager.configure_mock(name='fake_service')
+ mock_manager.configure_mock(name='tenant_rbac_policy')
mock_stevedore.named.NamedExtensionManager.return_value = [
mock_manager
]
@@ -364,7 +365,7 @@
authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, 'tenant_rbac_policy')
- policy_data = authority._get_policy_data('fake_service')
+ policy_data = authority._get_policy_data()
self.assertIsInstance(policy_data, str)
actual_policy_data = json.loads(policy_data)
@@ -388,7 +389,7 @@
None, None, 'test_service')
expected_error = (
- 'Policy file for {0} service was not found among the registered '
+ 'Policy files for {0} service were not found among the registered '
'in-code policies or in any of the possible policy files: {1}.'
.format('test_service',
[CONF.patrole.custom_policy_files[0] % 'test_service']))
@@ -413,7 +414,7 @@
policy_authority.PolicyAuthority,
None, None, 'test_service')
- expected_error = "Policy file for {0} service is invalid."\
+ expected_error = "Policy files for {0} service are invalid."\
.format("test_service")
self.assertIn(expected_error, str(e))
@@ -435,7 +436,7 @@
None, None, 'tenant_rbac_policy')
expected_error = (
- 'Policy file for {0} service was not found among the registered '
+ 'Policy files for {0} service were not found among the registered '
'in-code policies or in any of the possible policy files: {1}.'
.format('tenant_rbac_policy', [CONF.patrole.custom_policy_files[0]
% 'tenant_rbac_policy']))
@@ -450,7 +451,7 @@
dir(policy_authority.PolicyAuthority))
self.assertIn('policy_files', dir(policy_parser))
self.assertIn('tenant_rbac_policy', policy_parser.policy_files)
- self.assertEqual(self.conf_policy_path % 'tenant_rbac_policy',
+ self.assertEqual([self.conf_policy_path % 'tenant_rbac_policy'],
policy_parser.policy_files['tenant_rbac_policy'])
@mock.patch.object(policy_authority, 'policy', autospec=True)
@@ -458,35 +459,40 @@
autospec=True)
@mock.patch.object(policy_authority, 'clients', autospec=True)
@mock.patch.object(policy_authority, 'os', autospec=True)
- def test_discover_policy_files_with_many_invalid_one_valid(self, m_os,
- m_creds, *args):
+ @mock.patch.object(policy_authority, 'glob', autospec=True)
+ def test_discover_policy_files_with_many_invalid_one_valid(self, m_glob,
+ m_os, m_creds,
+ *args):
+ service = 'test_service'
+ custom_policy_files = ['foo/%s', 'bar/%s', 'baz/%s']
+ m_glob.iglob.side_effect = [iter([path % service])
+ for path in custom_policy_files]
# Only the 3rd path is valid.
- m_os.path.isfile.side_effect = [False, False, True, False]
+ m_os.path.isfile.side_effect = [False, False, True]
# Ensure the outer for loop runs only once in `discover_policy_files`.
m_creds.Manager().identity_services_v3_client.\
list_services.return_value = {
- 'services': [{'name': 'test_service'}]}
+ 'services': [{'name': service}]}
# The expected policy will be 'baz/test_service'.
self.useFixture(fixtures.ConfPatcher(
- custom_policy_files=['foo/%s', 'bar/%s', 'baz/%s'],
+ custom_policy_files=custom_policy_files,
group='patrole'))
policy_parser = policy_authority.PolicyAuthority(
- None, None, 'test_service')
+ None, None, service)
# Ensure that "policy_files" is set at class and instance levels.
- self.assertIn('policy_files',
- dir(policy_authority.PolicyAuthority))
- self.assertIn('policy_files', dir(policy_parser))
- self.assertIn('test_service', policy_parser.policy_files)
- self.assertEqual('baz/test_service',
- policy_parser.policy_files['test_service'])
+ self.assertTrue(hasattr(policy_authority.PolicyAuthority,
+ 'policy_files'))
+ self.assertTrue(hasattr(policy_parser, 'policy_files'))
+ self.assertEqual(['baz/%s' % service],
+ policy_parser.policy_files[service])
def test_discover_policy_files_with_no_valid_files(self):
expected_error = (
- 'Policy file for {0} service was not found among the registered '
+ 'Policy files for {0} service were not found among the registered '
'in-code policies or in any of the possible policy files: {1}.'
.format('test_service', [self.conf_policy_path % 'test_service']))
@@ -495,11 +501,11 @@
None, None, 'test_service')
self.assertIn(expected_error, str(e))
- self.assertIn('policy_files',
- dir(policy_authority.PolicyAuthority))
- self.assertNotIn(
- 'test_service',
- policy_authority.PolicyAuthority.policy_files.keys())
+ self.assertTrue(hasattr(policy_authority.PolicyAuthority,
+ 'policy_files'))
+ self.assertEqual(
+ [],
+ policy_authority.PolicyAuthority.policy_files['test_service'])
def _test_validate_service(self, v2_services, v3_services,
expected_failure=False, expected_services=None):
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/patrole_tempest_plugin/tests/unit/test_rbac_utils.py b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
index c5264aa..5132079 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
@@ -208,11 +208,6 @@
class FakeRbacTest(rbac_utils.RbacUtilsMixin, test.BaseTestCase):
@classmethod
- def skip_checks(cls):
- super(FakeRbacTest, cls).skip_checks()
- cls.skip_rbac_checks()
-
- @classmethod
def setup_clients(cls):
super(FakeRbacTest, cls).setup_clients()
cls.setup_rbac_utils()
@@ -237,21 +232,3 @@
self.assertTrue(hasattr(child_test, 'rbac_utils'))
self.assertIsInstance(child_test.rbac_utils, rbac_utils.RbacUtils)
-
- def test_skip_rbac_checks(self):
- """Validate that the child class is skipped if `[patrole] enable_rbac`
- is False and that the child class's name is in the skip message.
- """
- self.useFixture(patrole_fixtures.ConfPatcher(enable_rbac=False,
- group='patrole'))
-
- class ChildRbacTest(self.parent_class):
- pass
-
- child_test = ChildRbacTest()
-
- with testtools.ExpectedException(
- testtools.TestCase.skipException,
- value_re=('Patrole testing not enabled so skipping %s.'
- % ChildRbacTest.__name__)):
- child_test.setUpClass()
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.
diff --git a/releasenotes/notes/multiple-policy-files-9aa7f7583283739e.yaml b/releasenotes/notes/multiple-policy-files-9aa7f7583283739e.yaml
new file mode 100644
index 0000000..a3555e6
--- /dev/null
+++ b/releasenotes/notes/multiple-policy-files-9aa7f7583283739e.yaml
@@ -0,0 +1,17 @@
+---
+features:
+ - |
+ In order to implement the tests for plugins which do not maintain the
+ ``policy.json`` with full list of the policy rules and provide policy file
+ with only their own policy rules, the Patrole should be able to load and
+ merge multiple policy files for any of the services.
+
+ - Discovery all policy files for each of the services.
+ The updated ``discover_policy_files`` function picks all candidate paths
+ found out of the potential paths in the ``[patrole].custom_policy_files``
+ config option. Using ``glob.glob()`` function makes it possible to use
+ the patterns like '\*.json' to discover the policy files.
+
+ - Loading and merging a data from multiple policy files.
+ Patrole loads a data from each of the discovered policy files for a
+ service and merge the data from all files.
diff --git a/releasenotes/notes/remove-deprecated-enable-rbac-config-option-a5e46ce1053b7dea.yaml b/releasenotes/notes/remove-deprecated-enable-rbac-config-option-a5e46ce1053b7dea.yaml
new file mode 100644
index 0000000..53b1710
--- /dev/null
+++ b/releasenotes/notes/remove-deprecated-enable-rbac-config-option-a5e46ce1053b7dea.yaml
@@ -0,0 +1,5 @@
+---
+upgrade:
+ - |
+ Remove deprecated ``[patrole].enable_rbac`` configuration option. To skip
+ Patrole tests going forward, use an appropriate regex.