From d8ca7c2789b510532be27f6dff32756454ed59f1 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Wed, 26 Aug 2020 08:37:02 -0500 Subject: Deprecate the JSON support for policy_file JSON support for policy_file has been problematic since projects started policy-in-code. For example, generating a sample policy file in JSON results in all the policy-in-code rules being overridden because it is not possible to comment out the default rules in JSON. Asd part of migration of JSON format to YAML, this commit deprecates the: 1. Deprecate JSON support in oslo.policy. 2. Deprecate JSON output in policy CLI tools including '--format' option. Partial implement blueprint policy-json-to-yaml Change-Id: I5432a8cf80903620f48936cbbfb92ea6b6ff30fa --- oslo_policy/generator.py | 11 +++++++ oslo_policy/policy.py | 12 ++++++++ oslo_policy/tests/test_generator.py | 34 ++++++++++++++++++++++ oslo_policy/tests/test_policy.py | 8 +++++ ...e-policy-file-json-format-e1921f15b5d00287.yaml | 12 ++++++++ 5 files changed, 77 insertions(+) create mode 100644 releasenotes/notes/deprecate-policy-file-json-format-e1921f15b5d00287.yaml diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py index 5262784..a35acae 100644 --- a/oslo_policy/generator.py +++ b/oslo_policy/generator.py @@ -34,6 +34,13 @@ RULE_OPTS = [ help='Option namespace(s) under "oslo.policy.policies" in ' 'which to query for options.'), cfg.StrOpt('format', + deprecated_for_removal=True, + deprecated_since='Victoria', + deprecated_reason=""" +``policy_file`` support for JSON formatted file is deprecated. +So these tools also deprecate the support of generating or +upgrading policy file in JSON format. +""", help='Desired format for the output.', default='yaml', choices=['json', 'yaml']), @@ -258,6 +265,7 @@ def _sort_and_format_by_section(policies, output_format='yaml', yield _format_rule_default_yaml(rule_default, include_help=include_help) elif output_format == 'json': + LOG.warning(policy.WARN_JSON) yield _format_rule_default_json(rule_default) @@ -290,6 +298,7 @@ def _generate_sample(namespaces, output_file=None, output_format='yaml', if output_format == 'yaml': output_file.writelines(sections_text) elif output_format == 'json': + LOG.warning(policy.WARN_JSON) output_file.writelines(( '{\n ', ',\n '.join(sections_text), @@ -551,12 +560,14 @@ def upgrade_policy(args=None, conf=None): if conf.format == 'yaml': yaml.safe_dump(policies, fh, default_flow_style=False) elif conf.format == 'json': + LOG.warning(policy.WARN_JSON) jsonutils.dump(policies, fh, indent=4) else: if conf.format == 'yaml': sys.stdout.write(yaml.safe_dump(policies, default_flow_style=False)) elif conf.format == 'json': + LOG.warning(policy.WARN_JSON) sys.stdout.write(jsonutils.dumps(policies, indent=4)) diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index 2b19a66..d808b37 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -296,6 +296,15 @@ RuleCheck = _checks.RuleCheck """Recursively checks credentials based on the defined rules.""" +WARN_JSON = ("JSON formatted policy_file support is deprecated since " + "Victoria release. You need to use YAML format which " + "will be default in future. You can use " + "``oslopolicy-convert-json-to-yaml`` tool to convert existing " + "JSON-formatted policy file to YAML-formatted in backward " + "compatible way: https://docs.openstack.org/oslo.policy/" + "latest/cli/oslopolicy-convert-json-to-yaml.html.") + + class PolicyNotAuthorized(Exception): """Default exception raised for policy enforcement failure.""" @@ -369,6 +378,9 @@ def parse_file_contents(data): # by jsonutils.loads() first. In case of failure yaml.safe_load() # will be used instead. parsed = jsonutils.loads(data) + # NOTE(gmann): If policy file is loaded in JSON format means + # policy_file is JSON formatted so log warning. + LOG.warning(WARN_JSON) except ValueError: try: parsed = yaml.safe_load(data) diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py index ab726dc..91ad901 100644 --- a/oslo_policy/tests/test_generator.py +++ b/oslo_policy/tests/test_generator.py @@ -474,6 +474,23 @@ class GenerateSampleJSONTestCase(base.PolicyBaseTestCase): self.assertEqual(expected, stdout.getvalue()) + @mock.patch.object(generator, 'LOG') + def test_generate_json_file_log_warning(self, mock_log): + extensions = [] + for name, opts in OPTS.items(): + ext = stevedore.extension.Extension(name=name, entry_point=None, + plugin=None, obj=opts) + extensions.append(ext) + test_mgr = stevedore.named.NamedExtensionManager.make_test_instance( + extensions=extensions, namespace=['base_rules', 'rules']) + + output_file = self.get_config_file_fullname('policy.json') + with mock.patch('stevedore.named.NamedExtensionManager', + return_value=test_mgr): + generator._generate_sample(['base_rules', 'rules'], output_file, + output_format='json') + mock_log.warning.assert_any_call(policy.WARN_JSON) + class GeneratorRaiseErrorTestCase(testtools.TestCase): def test_generator_raises_error(self): @@ -679,6 +696,23 @@ class UpgradePolicyTestCase(base.PolicyBaseTestCase): self.assertIsNotNone(new_policy.get('new_policy_name')) self.assertIsNone(new_policy.get('deprecated_name')) + @mock.patch.object(generator, 'LOG') + def test_upgrade_policy_json_file_log_warning(self, mock_log): + test_mgr = stevedore.named.NamedExtensionManager.make_test_instance( + extensions=self.extensions, namespace='test_upgrade') + with mock.patch('stevedore.named.NamedExtensionManager', + return_value=test_mgr): + testargs = ['olsopolicy-policy-upgrade', + '--policy', + self.get_config_file_fullname('policy.json'), + '--namespace', 'test_upgrade', + '--output-file', + self.get_config_file_fullname('new_policy.json'), + '--format', 'json'] + with mock.patch('sys.argv', testargs): + generator.upgrade_policy(conf=self.local_conf) + mock_log.warning.assert_any_call(policy.WARN_JSON) + def test_upgrade_policy_yaml_file(self): test_mgr = stevedore.named.NamedExtensionManager.make_test_instance( extensions=self.extensions, namespace='test_upgrade') diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index 6b5facf..8444379 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -413,6 +413,14 @@ class EnforcerTest(base.PolicyBaseTestCase): 'test_load_directory_caching_with_files_same_but_overwrite_false') # NOQA self._test_scenario_with_opts_registered(test) + @mock.patch.object(policy, 'LOG') + def test_load_json_file_log_warning(self, mock_log): + rules = jsonutils.dumps({'foo': 'rule:bar'}) + self.create_config_file('policy.json', rules) + self.enforcer.load_rules(True) + + mock_log.warning.assert_any_call(policy.WARN_JSON) + def test_load_multiple_directories(self): self.create_config_file( os.path.join('policy.d', 'a.conf'), POLICY_A_CONTENTS) diff --git a/releasenotes/notes/deprecate-policy-file-json-format-e1921f15b5d00287.yaml b/releasenotes/notes/deprecate-policy-file-json-format-e1921f15b5d00287.yaml new file mode 100644 index 0000000..b02a9d8 --- /dev/null +++ b/releasenotes/notes/deprecate-policy-file-json-format-e1921f15b5d00287.yaml @@ -0,0 +1,12 @@ +--- +deprecations: + - | + ``policy_file`` support for JSON formatted file is deprecated. Use + YAML formatted file which will be default in future. + Use `oslopolicy-convert-json-to-yaml `_ + tool to convert the existing JSON to YAML formatted policy file in + backward compatible way. + + JSON format support and ``--format`` option in ``oslopolicy-sample-generator`` + and ``oslopolicy-policy-upgrade`` tools are also deprecated. In future + release, ``--format`` option will be removed. -- cgit v1.2.1