openstack
/
patrole
Code Issues Proposed changes

Add support for multiple policy files 

Most of the neutron plugins provide an updated version of policy.json
file with full list of rules, but at the same time there are a lot of
other plugins which provide their own policy files and store them in
the policy.d/ folder: neutron-fwaas, networking-bgpvpn, vmware-nsx,
ect...
To implement the tests for such plugins the Patrole should be able to
load and merge multiple policy files for any of the services.

Modify the discover_policy_files function to discover all policy files
for each of the services. Using glob.glob() function makes it possible
to use patterns like '*.json' to discover the policy files.
Modify the _get_policy_data function to load a data from all discovered
policy files for a service.

Update the unit test according to the changes.

Change-Id: Ib24f3d6d7a5ffdeaecce579af9795fd897dce872
This commit is contained in:
Sergey Vilgelm 2018-09-06 20:51:57 -05:00
parent a3c15da1cd
commit 062fb157b8
No known key found for this signature in database
GPG Key ID: 08D0E2FF778887E6
4 changed files with 100 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
patrole_tempest_plugin/config.py
View File

@ -39,8 +39,9 @@ This option is paradoxical with the Tempest plugin architecture.
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,

70
patrole_tempest_plugin/policy_authority.py
View File

@ -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 @@ class PolicyAuthority(RbacAuthority):
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 @@ class PolicyAuthority(RbacAuthority):
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 @@ class PolicyAuthority(RbacAuthority):
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 @@ class PolicyAuthority(RbacAuthority):
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 @@ class PolicyAuthority(RbacAuthority):
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 @@ class PolicyAuthority(RbacAuthority):
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 @@ class PolicyAuthority(RbacAuthority):
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:

70
patrole_tempest_plugin/tests/unit/test_policy_authority.py
View File

@ -270,9 +270,9 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
]
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 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
]
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 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
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 @@ class PolicyAuthorityTest(base.TestCase):
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):

17
releasenotes/notes/multiple-policy-files-9aa7f7583283739e.yaml Normal file
View File

@ -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.