openstack
/
patrole
Code Issues Proposed changes

Deprecate [rbac] configuration group. 

The [rbac] configuration group has been deprecated
and will be removed in the next release. It has been
renamed to the [patrole] group which contains
the exact same options.

This commit makes necessary deprecation changes,
along with renaming changes to documentation, unit
tests and framework.

Change-Id: I71198506b97b98ac18a969b7e6b13b664579c081
This commit is contained in:
Felipe Monteiro 2017-08-06 06:08:02 +01:00
parent 11b023243f
commit f6eb862105
20 changed files with 114 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
doc/source/configuration.rst
View File

@ -5,7 +5,7 @@ Patrole Configuration Guide
Patrole can be customized by updating Tempest's ``tempest.conf`` configuration
file. All Patrole-specific configuration options should be included under
the ``rbac`` group.
the ``patrole`` group.
RBAC Test Role
--------------

2
doc/source/sampleconf.rst
View File

@ -7,7 +7,7 @@ The following is a sample Patrole configuration for adaptation and use.
.. code-block:: ini
[rbac]
[patrole]
# The role that you want the RBAC tests to use for RBAC testing
# This needs to be edited to run the test as a different role.

2
doc/source/usage.rst
View File

@ -30,7 +30,7 @@ To execute patrole tests for a specific module, run::
To change the role that the patrole tests are being run as, edit
``rbac_test_role`` in the ``rbac`` section of tempest.conf: ::
[rbac]
[patrole]
rbac_test_role = Member
...

25
patrole_tempest_plugin/config.py
View File

@ -15,19 +15,23 @@
from oslo_config import cfg
rbac_group = cfg.OptGroup(name='rbac',
title='RBAC testing options')
RbacGroup = [
patrole_group = cfg.OptGroup(name='patrole', title='Patrole Testing Options')
PatroleGroup = [
cfg.StrOpt('rbac_test_role',
default='admin',
deprecated_group='rbac',
help="""The current RBAC role against which to run Patrole
tests."""),
cfg.BoolOpt('enable_rbac',
default=True,
deprecated_group='rbac',
help="Enables RBAC tests."),
cfg.BoolOpt('strict_policy_check',
default=False,
deprecated_group='rbac',
help="""If true, throws RbacParsingException for policies which
don't exist or are not included in the service's policy file. If false, throws
skipException."""),
@ -35,6 +39,7 @@ skipException."""),
# other hosts. It may be possible to leverage the v3 identity policy API.
cfg.ListOpt('custom_policy_files',
default=['/etc/%s/policy.json'],
deprecated_group='rbac',
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
@ -45,6 +50,7 @@ first path that is found to contain the service's policy file will be used.
default='/etc/cinder/policy.json',
help="""Location of the Cinder policy file. Assumed to be on
the same host as Patrole.""",
deprecated_group='rbac',
deprecated_for_removal=True,
deprecated_reason="It is better to use `custom_policy_files` "
"which supports any OpenStack service."),
@ -52,6 +58,7 @@ the same host as Patrole.""",
default='/etc/glance/policy.json',
help="""Location of the Glance policy file. Assumed to be on
the same host as Patrole.""",
deprecated_group='rbac',
deprecated_for_removal=True,
deprecated_reason="It is better to use `custom_policy_files` "
"which supports any OpenStack service."),
@ -59,6 +66,7 @@ the same host as Patrole.""",
default='/etc/keystone/policy.json',
help="""Location of the custom Keystone policy file. Assumed to
be on the same host as Patrole.""",
deprecated_group='rbac',
deprecated_for_removal=True,
deprecated_reason="It is better to use `custom_policy_files` "
"which supports any OpenStack service."),
@ -66,6 +74,7 @@ be on the same host as Patrole.""",
default='/etc/neutron/policy.json',
help="""Location of the Neutron policy file. Assumed to be on
the same host as Patrole.""",
deprecated_group='rbac',
deprecated_for_removal=True,
deprecated_reason="It is better to use `custom_policy_files` "
"which supports any OpenStack service."),
@ -73,11 +82,13 @@ the same host as Patrole.""",
default='/etc/nova/policy.json',
help="""Location of the custom Nova policy file. Assumed to be
on the same host as Patrole.""",
deprecated_group='rbac',
deprecated_for_removal=True,
deprecated_reason="It is better to use `custom_policy_files` "
"which supports any OpenStack service."),
cfg.BoolOpt('test_custom_requirements',
default=False,
deprecated_group='rbac',
help="""
This option determines whether Patrole should run against a
`custom_requirements_file` which defines RBAC requirements. The
@ -101,6 +112,7 @@ test run: allowed
test result: fail (over-permission)
"""),
cfg.StrOpt('custom_requirements_file',
deprecated_group='rbac',
help="""
File path of the yaml file that defines your RBAC requirements. This
file must be located on the same host that Patrole runs on. The yaml
@ -128,3 +140,10 @@ api_action = the policy action that is being tested. Examples:
allowed_role = the Keystone role that is allowed to perform the API
""")
]
rbac_group = cfg.OptGroup(name='rbac',
title='RBAC testing options',
help="This group is deprecated and will be removed "
"in the next release. Use the [patrole] group "
"instead.")

11
patrole_tempest_plugin/plugin.py
View File

@ -30,10 +30,17 @@ class PatroleTempestPlugin(plugins.TempestPlugin):
return full_test_dir, base_path
def register_opts(self, conf):
# TODO(fmontei): Remove ``rbac_group`` in a future release as it is
# currently deprecated.
config.register_opt_group(
conf,
project_config.rbac_group,
project_config.RbacGroup)
project_config.PatroleGroup)
config.register_opt_group(
conf,
project_config.patrole_group,
project_config.PatroleGroup)
def get_opt_lists(self):
return [(project_config.rbac_group.name, project_config.RbacGroup)]
return [(project_config.patrole_group.name,
project_config.PatroleGroup)]

10
patrole_tempest_plugin/rbac_policy_parser.py
View File

@ -72,11 +72,11 @@ class RbacPolicyParser(RbacAuthority):
# Prioritize dynamically searching for policy files over relying on
# deprecated service-specific policy file locations.
if CONF.rbac.custom_policy_files:
if CONF.patrole.custom_policy_files:
self.discover_policy_files()
self.path = self.policy_files.get(service)
else:
self.path = getattr(CONF.rbac, '%s_policy_file' % str(service),
self.path = getattr(CONF.patrole, '%s_policy_file' % str(service),
None)
self.rules = policy.Rules.load(self._get_policy_data(service),
@ -110,11 +110,11 @@ class RbacPolicyParser(RbacAuthority):
def discover_policy_files(cls):
# Dynamically discover the policy file for each service in
# ``cls.available_services``. Pick the first ``candidate_path`` found
# out of the potential paths in ``CONF.rbac.custom_policy_files``.
# out of the potential paths in ``CONF.patrole.custom_policy_files``.
if not hasattr(cls, 'policy_files'):
cls.policy_files = {}
for service in cls.available_services:
for candidate_path in CONF.rbac.custom_policy_files:
for candidate_path in CONF.patrole.custom_policy_files:
if os.path.isfile(candidate_path % service):
cls.policy_files.setdefault(service,
candidate_path % service)
@ -178,7 +178,7 @@ class RbacPolicyParser(RbacAuthority):
error_message = (
'Policy file for {0} service neither found in code nor at {1}.'
.format(service, [loc % service for loc in
CONF.rbac.custom_policy_files])
CONF.patrole.custom_policy_files])
)
raise rbac_exceptions.RbacParsingException(error_message)

14
patrole_tempest_plugin/rbac_rule_validation.py
View File

@ -72,7 +72,7 @@ def action(service, rule='', admin_only=False, expected_error_code=403,
extra_target_data = {}
def decorator(func):
role = CONF.rbac.rbac_test_role
role = CONF.patrole.rbac_test_role
def wrapper(*args, **kwargs):
if args and isinstance(args[0], test.BaseTestCase):
@ -149,9 +149,9 @@ def _is_authorized(test_obj, service, rule_name, extra_target_data):
:raises RbacResourceSetupFailed: if project_id or user_id are missing from
the Tempest test object's `auth_provider`
:raises RbacParsingException: if ``CONF.rbac.strict_policy_check`` is
:raises RbacParsingException: if ``CONF.patrole.strict_policy_check`` is
enabled and the ``rule_name`` does not exist in the system
:raises skipException: if ``CONF.rbac.strict_policy_check`` is
:raises skipException: if ``CONF.patrole.strict_policy_check`` is
disabled and the ``rule_name`` does not exist in the system
"""
try:
@ -164,11 +164,11 @@ def _is_authorized(test_obj, service, rule_name, extra_target_data):
raise rbac_exceptions.RbacResourceSetupFailed(msg)
try:
role = CONF.rbac.rbac_test_role
role = CONF.patrole.rbac_test_role
# Test RBAC against custom requirements. Otherwise use oslo.policy
if CONF.rbac.test_custom_requirements:
if CONF.patrole.test_custom_requirements:
authority = requirements_authority.RequirementsAuthority(
CONF.rbac.custom_requirements_file, service)
CONF.patrole.custom_requirements_file, service)
else:
formatted_target_data = _format_extra_target_data(
test_obj, extra_target_data)
@ -185,7 +185,7 @@ def _is_authorized(test_obj, service, rule_name, extra_target_data):
rule_name, role)
return is_allowed
except rbac_exceptions.RbacParsingException as e:
if CONF.rbac.strict_policy_check:
if CONF.patrole.strict_policy_check:
raise e
else:
raise testtools.TestCase.skipException(str(e))

39
patrole_tempest_plugin/rbac_utils.py
View File

@ -39,7 +39,7 @@ class RbacUtils(object):
seamlessly swap between admin credentials, needed for setup and clean up,
and primary credentials, needed to perform the API call which does
policy enforcement. The primary credentials always cycle between roles
defined by ``[identity] admin_role`` and ``[rbac] rbac_test_role``.
defined by ``CONF.identity.admin_role`` and `CONF.patrole.rbac_test_role``.
"""
def __init__(self, test_obj):
@ -78,14 +78,10 @@ class RbacUtils(object):
Switch the role used by `os_primary` credentials to:
* admin if `toggle_rbac_role` is False
* `[rbac] rbac_test_role` if `toggle_rbac_role` is True
* `CONF.patrole.rbac_test_role` if `toggle_rbac_role` is True
:param test_obj: An instance of `tempest.test.BaseTestCase`.
:param toggle_rbac_role: Role to switch `os_primary` Tempest creds to.
:returns: None.
:raises RbacResourceSetupFailed: If admin or test roles are missing. Or
if `toggle_rbac_role` is not a boolean value or role validation
fails.
:param test_obj: test object of type tempest.lib.base.BaseTestCase
:param toggle_rbac_role: role to switch `os_primary` Tempest creds to
"""
self.user_id = test_obj.os_primary.credentials.user_id
self.project_id = test_obj.os_primary.credentials.tenant_id
@ -127,14 +123,14 @@ class RbacUtils(object):
admin_role_id = rbac_role_id = None
for role in available_roles['roles']:
if role['name'] == CONF.rbac.rbac_test_role:
if role['name'] == CONF.patrole.rbac_test_role:
rbac_role_id = role['id']
if role['name'] == CONF.identity.admin_role:
admin_role_id = role['id']
if not all([admin_role_id, rbac_role_id]):
msg = ("Roles defined by `[rbac] rbac_test_role` and `[identity] "
"admin_role` must be defined in the system.")
msg = ("Roles defined by `[patrole] rbac_test_role` and "
"`[identity] admin_role` must be defined in the system.")
raise rbac_exceptions.RbacResourceSetupFailed(msg)
self.admin_role_id = admin_role_id
@ -201,13 +197,32 @@ class RbacUtils(object):
else:
self.switch_role_history[key] = toggle_rbac_role
def _get_roles(self):
available_roles = self.admin_roles_client.list_roles()
admin_role_id = rbac_role_id = None
for role in available_roles['roles']:
if role['name'] == CONF.patrole.rbac_test_role:
rbac_role_id = role['id']
if role['name'] == CONF.identity.admin_role:
admin_role_id = role['id']
if not admin_role_id or not rbac_role_id:
msg = "Role with name 'admin' does not exist in the system."\
if not admin_role_id else "Role defined by rbac_test_role "\
"does not exist in the system."
raise rbac_exceptions.RbacResourceSetupFailed(msg)
self.admin_role_id = admin_role_id
self.rbac_role_id = rbac_role_id
def is_admin():
"""Verifies whether the current test role equals the admin role.
:returns: True if ``rbac_test_role`` is the admin role.
"""
return CONF.rbac.rbac_test_role == CONF.identity.admin_role
return CONF.patrole.rbac_test_role == CONF.identity.admin_role
@six.add_metaclass(abc.ABCMeta)

2
patrole_tempest_plugin/tests/api/compute/rbac_base.py
View File

@ -26,7 +26,7 @@ class BaseV2ComputeRbacTest(compute_base.BaseV2ComputeTest):
@classmethod
def skip_checks(cls):
super(BaseV2ComputeRbacTest, cls).skip_checks()
if not CONF.rbac.enable_rbac:
if not CONF.patrole.enable_rbac:
raise cls.skipException(
'%s skipped as RBAC testing not enabled' % cls.__name__)

2
patrole_tempest_plugin/tests/api/identity/rbac_base.py
View File

@ -31,7 +31,7 @@ class BaseIdentityV2RbacTest(base.BaseIdentityV2Test):
@classmethod
def skip_checks(cls):
super(BaseIdentityV2RbacTest, cls).skip_checks()
if not CONF.rbac.enable_rbac:
if not CONF.patrole.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC testing not enabled" % cls.__name__)

4
patrole_tempest_plugin/tests/api/image/rbac_base.py
View File

@ -24,7 +24,7 @@ class BaseV1ImageRbacTest(image_base.BaseV1ImageTest):
@classmethod
def skip_checks(cls):
super(BaseV1ImageRbacTest, cls).skip_checks()
if not CONF.rbac.enable_rbac:
if not CONF.patrole.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC testing not enabled" % cls.__name__)
@ -39,7 +39,7 @@ class BaseV2ImageRbacTest(image_base.BaseV2ImageTest):
@classmethod
def skip_checks(cls):
super(BaseV2ImageRbacTest, cls).skip_checks()
if not CONF.rbac.enable_rbac:
if not CONF.patrole.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC testing not enabled" % cls.__name__)

2
patrole_tempest_plugin/tests/api/network/rbac_base.py
View File

@ -26,7 +26,7 @@ class BaseNetworkRbacTest(network_base.BaseNetworkTest):
@classmethod
def skip_checks(cls):
super(BaseNetworkRbacTest, cls).skip_checks()
if not CONF.rbac.enable_rbac:
if not CONF.patrole.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC testing not enabled" % cls.__name__)

2
patrole_tempest_plugin/tests/api/volume/rbac_base.py
View File

@ -26,7 +26,7 @@ class BaseVolumeRbacTest(vol_base.BaseVolumeTest):
@classmethod
def skip_checks(cls):
super(BaseVolumeRbacTest, cls).skip_checks()
if not CONF.rbac.enable_rbac:
if not CONF.patrole.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC testing not enabled" % cls.__name__)

2
patrole_tempest_plugin/tests/unit/fixtures.py
View File

@ -61,7 +61,7 @@ class RbacUtilsFixture(fixtures.Fixture):
def setUp(self):
super(RbacUtilsFixture, self).setUp()
self.useFixture(ConfPatcher(rbac_test_role='member', group='rbac'))
self.useFixture(ConfPatcher(rbac_test_role='member', group='patrole'))
self.useFixture(ConfPatcher(
admin_role='admin', auth_version='v3', group='identity'))

17
patrole_tempest_plugin/tests/unit/test_patrole.py
View File

@ -14,16 +14,23 @@
# under the License.
"""
test_patrole
----------------------------------
Tests for `patrole` module.
"""
from tempest import config
from patrole_tempest_plugin.tests.unit import base
CONF = config.CONF
class TestPatrole(base.TestCase):
def test_something(self):
pass
def test_rbac_group_backwards_compatability(self):
"""Validate that the deprecated group [rbac] is available and has the
same options and option values as [patrole] group, which is current.
"""
self.assertTrue(hasattr(CONF, 'patrole'))
self.assertTrue(hasattr(CONF, 'rbac'))
# Validate that both groups are identical.
self.assertEqual(CONF.patrole.items(), CONF.rbac.items())

10
patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
View File

@ -64,9 +64,9 @@ class RbacPolicyTest(base.TestCase):
current_directory, 'resources', '%s.json')
CONF.set_override(
'custom_policy_files', [self.conf_policy_path], group='rbac')
'custom_policy_files', [self.conf_policy_path], group='patrole')
self.addCleanup(CONF.clear_override, 'custom_policy_files',
group='rbac')
group='patrole')
# Guarantee a blank slate for each test.
for attr in ('available_services', 'policy_files'):
@ -393,7 +393,7 @@ class RbacPolicyTest(base.TestCase):
'Policy file for {0} service neither found in code '\
'nor at {1}.'.format(
'test_service',
[CONF.rbac.custom_policy_files[0] % 'test_service'])
[CONF.patrole.custom_policy_files[0] % 'test_service'])
self.assertIn(expected_error, str(e))
@ -439,7 +439,7 @@ class RbacPolicyTest(base.TestCase):
expected_error = (
'Policy file for {0} service neither found in code nor at {1}.'
.format('tenant_rbac_policy', [CONF.rbac.custom_policy_files[0]
.format('tenant_rbac_policy', [CONF.patrole.custom_policy_files[0]
% 'tenant_rbac_policy']))
self.assertIn(expected_error, str(e))
@ -473,7 +473,7 @@ class RbacPolicyTest(base.TestCase):
# The expected policy will be 'baz/test_service'.
CONF.set_override(
'custom_policy_files', ['foo/%s', 'bar/%s', 'baz/%s'],
group='rbac')
group='patrole')
policy_parser = rbac_policy_parser.RbacPolicyParser(
None, None, 'test_service')

8
patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
View File

@ -37,8 +37,8 @@ class RBACRuleValidationTest(base.TestCase):
self.mock_args.os_primary.credentials.user_id = \
mock.sentinel.user_id
CONF.set_override('rbac_test_role', 'Member', group='rbac')
self.addCleanup(CONF.clear_override, 'rbac_test_role', group='rbac')
CONF.set_override('rbac_test_role', 'Member', group='patrole')
self.addCleanup(CONF.clear_override, 'rbac_test_role', group='patrole')
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
@ -310,9 +310,9 @@ class RBACRuleValidationTest(base.TestCase):
def test_invalid_policy_rule_throws_parsing_exception(
self, mock_rbac_policy_parser):
"""Test that invalid policy action causes test to be skipped."""
CONF.set_override('strict_policy_check', True, group='rbac')
CONF.set_override('strict_policy_check', True, group='patrole')
self.addCleanup(CONF.clear_override, 'strict_policy_check',
group='rbac')
group='patrole')
mock_rbac_policy_parser.RbacPolicyParser.return_value.allowed.\
side_effect = rbac_exceptions.RbacParsingException

4
patrole_tempest_plugin/tests/unit/test_rbac_utils.py
View File

@ -35,7 +35,7 @@ class RBACUtilsTest(base.TestCase):
def test_switch_role_with_missing_admin_role(self):
self.rbac_utils.set_roles('member')
error_re = (
'Roles defined by `\[rbac\] rbac_test_role` and `\[identity\] '
'Roles defined by `\[patrole\] rbac_test_role` and `\[identity\] '
'admin_role` must be defined in the system.')
self.assertRaisesRegex(rbac_exceptions.RbacResourceSetupFailed,
error_re, self.rbac_utils.switch_role)
@ -43,7 +43,7 @@ class RBACUtilsTest(base.TestCase):
def test_switch_role_with_missing_rbac_role(self):
self.rbac_utils.set_roles('admin')
error_re = (
'Roles defined by `\[rbac\] rbac_test_role` and `\[identity\] '
'Roles defined by `\[patrole\] rbac_test_role` and `\[identity\] '
'admin_role` must be defined in the system.')
self.assertRaisesRegex(rbac_exceptions.RbacResourceSetupFailed,
error_re, self.rbac_utils.switch_role)

6
releasenotes/notes/deprecate-rbac-group-148e222913dc74cc.yaml Normal file
View File

@ -0,0 +1,6 @@
---
deprecations:
- |
The ``[rbac]`` configuration group has been deprecated and will be removed
in the next release. Use ``[patrole]`` group instead, which has the exact
same options.

6
tox.ini
View File

@ -59,7 +59,11 @@ commands = oslo_debug_helper -t patrole_tempest_plugin/tests {posargs}
enable-extensions = H106,H203,H904
show-source = True
# E123, E125 skipped as they are invalid PEP-8.
ignore = E123,E125
#
# H405 is another one that is good as a guideline, but sometimes
# multiline doc strings just don't have a natural summary
# line. Rejecting code for this reason is wrong.
ignore = E123,E125,H405
builtins = _
exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build