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

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

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)

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')

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)

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 <https://docs.openstack.org/oslo.policy/latest/cli/oslopolicy-convert-json-to-yaml.html>`_
+    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.