diff options
| -rw-r--r-- | babel.cfg | 2 | ||||
| -rw-r--r-- | doc/requirements.txt | 6 | ||||
| -rw-r--r-- | doc/source/cli/index.rst | 40 | ||||
| -rw-r--r-- | doc/source/conf.py | 11 | ||||
| -rw-r--r-- | doc/source/index.rst | 13 | ||||
| -rw-r--r-- | lower-constraints.txt | 9 | ||||
| -rw-r--r-- | oslo_policy/generator.py | 69 | ||||
| -rw-r--r-- | oslo_policy/policy.py | 41 | ||||
| -rw-r--r-- | oslo_policy/sphinxext.py | 4 | ||||
| -rw-r--r-- | oslo_policy/sphinxpolicygen.py | 4 | ||||
| -rw-r--r-- | oslo_policy/tests/test_generator.py | 48 | ||||
| -rw-r--r-- | oslo_policy/tests/test_policy.py | 43 | ||||
| -rw-r--r-- | releasenotes/notes/bug-1880959-8f1370a59759d40d.yaml | 8 | ||||
| -rw-r--r-- | releasenotes/notes/policy-file-validator-906d5cff864a2d51.yaml | 6 | ||||
| -rw-r--r-- | releasenotes/source/conf.py | 8 | ||||
| -rw-r--r-- | setup.cfg | 15 |
16 files changed, 279 insertions, 48 deletions
diff --git a/babel.cfg b/babel.cfg deleted file mode 100644 index 15cd6cb..0000000 --- a/babel.cfg +++ /dev/null @@ -1,2 +0,0 @@ -[python: **.py] - diff --git a/doc/requirements.txt b/doc/requirements.txt index 9cf3697..3882a1e 100644 --- a/doc/requirements.txt +++ b/doc/requirements.txt @@ -2,8 +2,8 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -openstackdocstheme>=1.20.0 # Apache-2.0 -sphinx>=1.8.0,!=2.1.0 # BSD +openstackdocstheme>=2.2.0 # Apache-2.0 +sphinx>=2.0.0,!=2.1.0 # BSD sphinxcontrib-apidoc>=0.2.0 # BSD -reno>=2.5.0 # Apache-2.0 +reno>=3.1.0 # Apache-2.0 diff --git a/doc/source/cli/index.rst b/doc/source/cli/index.rst index c7023de..bef7c99 100644 --- a/doc/source/cli/index.rst +++ b/doc/source/cli/index.rst @@ -151,3 +151,43 @@ For more information regarding the options supported by this tool: .. code-block:: bash oslopolicy-list-redundant --help + +oslopolicy_validator +==================== + +The ``oslopolicy-validator`` tool can be used to perform basic sanity checks +against a policy file. It will detect the following problems: + +* A missing policy file +* Rules which have invalid syntax +* Rules which reference non-existent other rules +* Rules which form a cyclical reference with another rule +* Rules which do not exist in the specified namespace + +This tool does very little validation of the content of the rules. Other tools, +such as ``oslopolicy-checker``, should be used to check that rules do what is +intended. + +``oslopolicy-validator`` exits with a ``0`` return code on success and ``1`` on +failure. + +.. note:: At this time the policy validator can only handle single policy + files, not policy dirs. + +Examples +-------- + +Validate the policy file used for Keystone: + +.. code-block:: bash + + oslopolicy-validator --config-file /etc/keystone/keystone.conf --namespace keystone + +Sample output from a failed validation:: + + $ oslopolicy-validator --config-file keystone.conf --namespace keystone + WARNING:oslo_policy.policy:Policies ['foo', 'bar'] are part of a cyclical reference. + Invalid rules found + Failed to parse rule: (role:admin and system_scope:all) or (role:foo and oken.domain.id:%(target.user.domain_id)s)) + Unknown rule found in policy file: foo + Unknown rule found in policy file: bar diff --git a/doc/source/conf.py b/doc/source/conf.py index c2b91e4..0ec69f1 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -25,9 +25,9 @@ extensions = [ ] # openstackdocstheme options -repository_name = 'openstack/oslo.policy' -bug_project = 'oslo.policy' -bug_tag = '' +openstackdocs_repo_name = 'openstack/oslo.policy' +openstackdocs_bug_project = 'oslo.policy' +openstackdocs_bug_tag = '' # autodoc generation is a bit aggressive and a nuisance when doing heavy # text edit cycles. @@ -40,7 +40,6 @@ source_suffix = '.rst' master_doc = 'index' # General information about the project. -project = u'oslo.policy' copyright = u'2014, OpenStack Foundation' source_tree = 'https://opendev.org/openstack/oslo.policy/src/branch/master' @@ -52,19 +51,17 @@ add_function_parentheses = True add_module_names = True # The name of the Pygments (syntax highlighting) style to use. -pygments_style = 'sphinx' +pygments_style = 'native' # A list of ignored prefixes for module index sorting. modindex_common_prefix = ['oslo_policy.'] - # -- Options for HTML output ------------------------------------------------- # The theme to use for HTML and HTML Help pages. Major themes that come with # Sphinx are currently 'default' and 'sphinxdoc'. html_theme = 'openstackdocs' - # -- sphinx.ext.extlinks configuration --------------------------------------- extlinks = { diff --git a/doc/source/index.rst b/doc/source/index.rst index 07a96ca..65b977a 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -5,6 +5,9 @@ An OpenStack library providing support for RBAC policy enforcement across all OpenStack services. +Contents +======== + .. toctree:: :maxdepth: 2 @@ -16,7 +19,15 @@ OpenStack services. reference/index contributor/index -.. rubric:: Indices and tables +Release Notes +============= + +Read also the `oslo.policy Release Notes +<https://docs.openstack.org/releasenotes/oslo.policy/>`_. + + +Indices and tables +================== * :ref:`genindex` * :ref:`modindex` diff --git a/lower-constraints.txt b/lower-constraints.txt index 5b0949e..616bc90 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -8,22 +8,18 @@ docutils==0.11 dulwich==0.15.0 extras==1.0.0 fixtures==3.0.0 -flake8==2.5.5 gitdb==0.6.4 GitPython==1.0.1 -hacking==0.12.0 imagesize==0.7.1 iso8601==0.1.11 Jinja2==2.10 keystoneauth1==3.4.0 linecache2==1.0.0 MarkupSafe==1.0 -mccabe==0.2.1 mox3==0.20.0 msgpack-python==0.4.0 netaddr==0.7.18 netifaces==0.10.4 -openstackdocstheme==1.20.0 os-client-config==1.28.0 oslo.config==5.2.0 oslo.context==2.22.0 @@ -32,15 +28,12 @@ oslo.serialization==2.18.0 oslo.utils==3.40.0 oslotest==3.2.0 pbr==2.0.0 -pep8==1.5.7 -pyflakes==0.8.1 Pygments==2.2.0 pyparsing==2.1.0 python-mimeparse==1.6.0 python-subunit==1.0.0 pytz==2013.6 PyYAML==3.12 -reno==2.5.0 requests==2.14.2 requests-mock==1.2.0 requestsexceptions==1.2.0 @@ -49,8 +42,6 @@ six==1.10.0 stestr==2.0.0 smmap==0.9.0 snowballstemmer==1.2.1 -Sphinx==1.8.0 -sphinxcontrib-websupport==1.0.1 stevedore==1.20.0 testrepository==0.0.18 testtools==2.2.0 diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py index 48a9972..40f374d 100644 --- a/oslo_policy/generator.py +++ b/oslo_policy/generator.py @@ -328,12 +328,70 @@ def _list_redundant(namespace): enforcer.load_rules() for name, file_rule in enforcer.file_rules.items(): - reg_rule = enforcer.registered_rules.get(name, None) + reg_rule = enforcer.registered_rules.get(name) if reg_rule: if file_rule == reg_rule: print(reg_rule) +def _validate_policy(namespace): + """Perform basic sanity checks on a policy file + + Checks for the following errors in the configured policy file: + + * A missing policy file + * Rules which have invalid syntax + * Rules which reference non-existent other rules + * Rules which form a cyclical reference with another rule + * Rules which do not exist in the specified namespace + + :param namespace: The name under which the oslo.policy enforcer is + registered. + :returns: 0 if all policies validated correctly, 1 if not. + """ + return_code = 0 + enforcer = _get_enforcer(namespace) + # NOTE(bnemec): We don't want to see policy deprecation warnings in the + # output of this tool. They tend to overwhelm the output that the user + # actually cares about. If we check for deprecated rules in this tool, + # we need to do it another way. + enforcer.suppress_deprecation_warnings = True + # Disable logging from the parser code. We'll be printing any errors we + # find below. + logging.disable(logging.ERROR) + # Ensure that files have been parsed + enforcer.load_rules() + + if enforcer._informed_no_policy_file: + print('Configured policy file "%s" not found' % enforcer.policy_file) + # If the policy file is completely missing then the rest of our checks + # don't make sense. + return 1 + + # Re-enable logging so we get messages for things like cyclical references + logging.disable(logging.NOTSET) + result = enforcer.check_rules() + if not result: + print('Invalid rules found') + return_code = 1 + + # TODO(bnemec): Allow this to handle policy_dir + with open(cfg.CONF.oslo_policy.policy_file) as f: + unparsed_policies = yaml.safe_load(f.read()) + for name, file_rule in enforcer.file_rules.items(): + reg_rule = enforcer.registered_rules.get(name) + if reg_rule is None: + print('Unknown rule found in policy file:', name) + return_code = 1 + # If a rule has invalid syntax it will be forced to '!'. If the literal + # rule from the policy file isn't '!' then this means there was an + # error parsing it. + if str(enforcer.rules[name]) == '!' and unparsed_policies[name] != '!': + print('Failed to parse rule:', unparsed_policies[name]) + return_code = 1 + return return_code + + def on_load_failure_callback(*args, **kwargs): raise @@ -423,3 +481,12 @@ def list_redundant(args=None): conf(args) _check_for_namespace_opt(conf) _list_redundant(conf.namespace) + + +def validate_policy(args=None): + logging.basicConfig(level=logging.WARN) + conf = cfg.CONF + conf.register_cli_opts(ENFORCER_OPTS) + conf.register_opts(ENFORCER_OPTS) + conf(args) + sys.exit(_validate_policy(conf.namespace)) diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index 406ed35..be0bd00 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -560,6 +560,8 @@ class Enforcer(object): if force_reload: self.use_conf = force_reload + policy_file_rules_changed = False + if self.use_conf: if not self.policy_path: try: @@ -571,18 +573,30 @@ class Enforcer(object): self._informed_no_policy_file = True if self.policy_path: - self._load_policy_file(self.policy_path, force_reload, - overwrite=self.overwrite) + # If the policy file rules have changed any policy.d rules + # also need to be reapplied on top of that change. + policy_file_rules_changed = self._load_policy_file( + self.policy_path, + force_reload, + overwrite=self.overwrite + ) + + force_reload_policy_dir = force_reload + if policy_file_rules_changed: + force_reload_policy_dir = True + for path in self.conf.oslo_policy.policy_dirs: try: path = self._get_policy_path(path) except cfg.ConfigFilesNotFoundError: continue - if (force_reload or self._is_directory_updated( - self._policy_dir_mtimes, path)): - self._walk_through_policy_directory(path, - self._load_policy_file, - force_reload, False) + if (self._is_directory_updated(self._policy_dir_mtimes, path) + or force_reload_policy_dir): + self._walk_through_policy_directory( + path, + self._load_policy_file, + force_reload_policy_dir, False + ) for default in self.registered_rules.values(): if default.deprecated_for_removal: @@ -771,6 +785,8 @@ class Enforcer(object): # is in the cache mtime = 0 if os.path.exists(path): + if not os.path.isdir(path): + raise ValueError('{} is not a directory'.format(path)) # Make a list of all the files files = [path] + [os.path.join(path, file) for file in os.listdir(path)] @@ -809,14 +825,25 @@ class Enforcer(object): self.file_rules[name] = RuleDefault(name, check_str) def _load_policy_file(self, path, force_reload, overwrite=True): + """Load policy rules from the specified policy file. + + :param str path: A path of the policy file to load rules from. + :param bool force_reload: Forcefully reload the policy file content. + :param bool overwrite: Replace policy rules instead of updating them. + :return: A bool indicating whether rules have been changed or not. + :rtype: bool + """ + rules_changed = False reloaded, data = _cache_handler.read_cached_file( self._file_cache, path, force_reload=force_reload) if reloaded or not self.rules: rules = Rules.load(data, self.default_rule) self.set_rules(rules, overwrite=overwrite, use_conf=True) + rules_changed = True self._record_file_rules(data, overwrite) self._loaded_files.append(path) LOG.debug('Reloaded policy file: %(path)s', {'path': path}) + return rules_changed def _get_policy_path(self, path): """Locate the policy YAML/JSON data file/path. diff --git a/oslo_policy/sphinxext.py b/oslo_policy/sphinxext.py index d8b8ce0..a6c02b6 100644 --- a/oslo_policy/sphinxext.py +++ b/oslo_policy/sphinxext.py @@ -165,3 +165,7 @@ class ShowPolicyDirective(rst.Directive): def setup(app): app.add_directive('show-policy', ShowPolicyDirective) + return { + 'parallel_read_safe': True, + 'parallel_write_safe': True, + } diff --git a/oslo_policy/sphinxpolicygen.py b/oslo_policy/sphinxpolicygen.py index 2693e34..1aef057 100644 --- a/oslo_policy/sphinxpolicygen.py +++ b/oslo_policy/sphinxpolicygen.py @@ -92,3 +92,7 @@ def setup(app): app.add_config_value('policy_generator_config_file', None, 'env') app.add_config_value('sample_policy_basename', None, 'env') app.connect('builder-inited', generate_sample) + return { + 'parallel_read_safe': True, + 'parallel_write_safe': True, + } diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py index 86003fe..af6398f 100644 --- a/oslo_policy/tests/test_generator.py +++ b/oslo_policy/tests/test_generator.py @@ -751,3 +751,51 @@ class GetEnforcerTestCase(base.PolicyBaseTestCase): mock_instance.__contains__.return_value = False mock_manager.return_value = mock_instance self.assertRaises(KeyError, generator._get_enforcer, 'nonexistent') + + +class ValidatorTestCase(base.PolicyBaseTestCase): + def _get_test_enforcer(self): + test_rules = [policy.RuleDefault('foo', 'foo:bar=baz'), + policy.RuleDefault('bar', 'bar:foo=baz')] + enforcer = policy.Enforcer(self.conf) + enforcer.register_defaults(test_rules) + return enforcer + + def _test_policy(self, rule, success=False, missing_file=False): + policy_file = self.get_config_file_fullname('test.yaml') + if missing_file: + policy_file = 'bogus.yaml' + self.create_config_file('test.yaml', rule) + self.create_config_file('test.conf', + '[oslo_policy]\npolicy_file=%s' % policy_file) + # Reparse now that we've created our configs + self.conf(args=['--config-dir', self.config_dir]) + + with mock.patch('oslo_policy.generator._get_enforcer') as ge: + ge.return_value = self._get_test_enforcer() + result = generator._validate_policy('test') + if success: + self.assertEqual(0, result) + else: + self.assertEqual(1, result) + + def test_success(self): + self._test_policy('foo: rule:bar', success=True) + + def test_cyclical_reference(self): + self._test_policy('foo: rule:bar\nbar: rule:foo') + + def test_invalid_syntax(self): + self._test_policy('foo: (bar))') + + def test_false_okay(self): + self._test_policy('foo: !', success=True) + + def test_reference_nonexistent(self): + self._test_policy('foo: rule:baz') + + def test_nonexistent(self): + self._test_policy('baz: rule:foo') + + def test_missing_policy_file(self): + self._test_policy('', missing_file=True) diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index b67504b..6b5facf 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -296,6 +296,48 @@ class EnforcerTest(base.PolicyBaseTestCase): os.path.join('policy.d', 'b.conf'), ]) + def test_load_directory_after_file_update(self): + self.create_config_file( + os.path.join('policy.d', 'a.conf'), POLICY_A_CONTENTS) + self.enforcer.load_rules(True) + self.assertIsNotNone(self.enforcer.rules) + loaded_rules = jsonutils.loads(str(self.enforcer.rules)) + self.assertEqual('role:fakeA', loaded_rules['default']) + self.assertEqual('is_admin:True', loaded_rules['admin']) + self.check_loaded_files([ + 'policy.json', + os.path.join('policy.d', 'a.conf'), + ]) + new_policy_json_contents = jsonutils.dumps({ + "default": "rule:admin", + "admin": "is_admin:True", + "foo": "rule:bar", + }) + # Modify the policy.json file and then validate that the rules + # from the policy directory are re-applied on top of the + # new rules from the file. + self.create_config_file('policy.json', new_policy_json_contents) + policy_file_path = self.get_config_file_fullname('policy.json') + # Force the mtime change since the unit test may write to this file + # too fast for mtime to actually change. + stinfo = os.stat(policy_file_path) + os.utime(policy_file_path, (stinfo.st_atime + 42, + stinfo.st_mtime + 42)) + + self.enforcer.load_rules() + + self.assertIsNotNone(self.enforcer.rules) + loaded_rules = jsonutils.loads(str(self.enforcer.rules)) + self.assertEqual('role:fakeA', loaded_rules['default']) + self.assertEqual('is_admin:True', loaded_rules['admin']) + self.assertEqual('rule:bar', loaded_rules['foo']) + self.check_loaded_files([ + 'policy.json', + os.path.join('policy.d', 'a.conf'), + 'policy.json', + os.path.join('policy.d', 'a.conf'), + ]) + def test_load_directory_opts_registered(self): self._test_scenario_with_opts_registered(self.test_load_directory) @@ -421,6 +463,7 @@ class EnforcerTest(base.PolicyBaseTestCase): [os.path.join('policy.d', 'a.conf')], group='oslo_policy') self.assertRaises(ValueError, self.enforcer.load_rules, True) + self.assertRaises(ValueError, self.enforcer.load_rules, False) @mock.patch('oslo_policy.policy.Enforcer.check_rules') def test_load_rules_twice(self, mock_check_rules): diff --git a/releasenotes/notes/bug-1880959-8f1370a59759d40d.yaml b/releasenotes/notes/bug-1880959-8f1370a59759d40d.yaml new file mode 100644 index 0000000..929f509 --- /dev/null +++ b/releasenotes/notes/bug-1880959-8f1370a59759d40d.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + [`bug 1880959 <https://bugs.launchpad.net/keystone/+bug/1880959>`_] + The behavior of policy file reloading from policy directories was fixed. + Previously the rules from policy files located in the directories + specified in the ``policy_dirs`` option were not reapplied after the rules + from the primary policy file have been reapplied due to a change. diff --git a/releasenotes/notes/policy-file-validator-906d5cff864a2d51.yaml b/releasenotes/notes/policy-file-validator-906d5cff864a2d51.yaml new file mode 100644 index 0000000..5e41c62 --- /dev/null +++ b/releasenotes/notes/policy-file-validator-906d5cff864a2d51.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + A new tool, ``oslopolicy-validator``, has been added. It allows deployers + to easily run basic sanity checks against their policy files. See the + documentation for full details. diff --git a/releasenotes/source/conf.py b/releasenotes/source/conf.py index 5084a9d..23e98c1 100644 --- a/releasenotes/source/conf.py +++ b/releasenotes/source/conf.py @@ -40,9 +40,9 @@ extensions = [ ] # openstackdocstheme options -repository_name = 'openstack/oslo.policy' -bug_project = 'oslo.policy' -bug_tag = '' +openstackdocs_repo_name = 'openstack/oslo.policy' +openstackdocs_bug_project = 'oslo.policy' +openstackdocs_bug_tag = '' # Add any paths that contain templates here, relative to this directory. templates_path = ['_templates'] @@ -96,7 +96,7 @@ exclude_patterns = [] # show_authors = False # The name of the Pygments (syntax highlighting) style to use. -pygments_style = 'sphinx' +pygments_style = 'native' # A list of ignored prefixes for module index sorting. # modindex_common_prefix = [] @@ -35,21 +35,8 @@ console_scripts = oslopolicy-policy-generator = oslo_policy.generator:generate_policy oslopolicy-list-redundant = oslo_policy.generator:list_redundant oslopolicy-policy-upgrade = oslo_policy.generator:upgrade_policy + oslopolicy-validator = oslo_policy.generator:validate_policy oslo.policy.rule_checks = http = oslo_policy._external:HttpCheck https = oslo_policy._external:HttpsCheck - -[compile_catalog] -directory = oslo_policy/locale -domain = oslo_policy - -[update_catalog] -domain = oslo_policy -output_dir = oslo_policy/locale -input_file = oslo_policy/locale/oslo_policy.pot - -[extract_messages] -keywords = _ gettext ngettext l_ lazy_gettext -mapping_file = babel.cfg -output_file = oslo_policy/locale/oslo_policy.pot |