From 5e7a0519cfc246dd2b5ca9a9749428cf0e5217e8 Mon Sep 17 00:00:00 2001
From: Ghanshyam Mann <gmann@ghanshyammann.com>
Date: Thu, 24 Dec 2020 21:51:09 -0600
Subject: [PATCH] [goal] Deprecate the JSON formatted policy file

As per the community goal of migrating the policy file
the format from JSON to YAML[1], we need to do two things:

1. Change the default value of '[oslo_policy] policy_file''
config option from 'policy.json' to 'policy.yaml' with
upgrade checks.

2. Deprecate the JSON formatted policy file on the project side
via warning in doc and releasenotes.

Also replace policy.json to policy.yaml ref from doc and tests.

[1]https://governance.openstack.org/tc/goals/selected/wallaby/migrate-policy-format-from-json-to-yaml.html

Change-Id: Ia0fa4ba1fecf2740a291c5a65b906b089c95404d
---
 aodh/api/hooks.py                             |  8 ++++++++
 aodh/cmd/status.py                            | 13 +++---------
 aodh/conf/defaults.py                         | 16 +++++++++++++++
 aodh/service.py                               |  2 +-
 aodh/tests/functional/api/v2/policy.json-test |  7 -------
 aodh/tests/functional/api/v2/policy.yaml-test |  8 ++++++++
 .../functional/api/v2/test_alarm_scenarios.py |  2 +-
 aodh/tests/unit/cmd/test_status.py            | 14 +++++++++----
 doc/source/configuration/policy.rst           |  8 ++++++++
 .../configuration/sample-policy-yaml.rst      |  8 ++++++++
 ...ormatted-policy-file-fgb26387a9bdb3b9.yaml | 20 +++++++++++++++++++
 requirements.txt                              |  4 ++--
 setup.cfg                                     |  2 +-
 13 files changed, 86 insertions(+), 26 deletions(-)
 delete mode 100644 aodh/tests/functional/api/v2/policy.json-test
 create mode 100644 aodh/tests/functional/api/v2/policy.yaml-test
 create mode 100644 releasenotes/notes/deprecate-json-formatted-policy-file-fgb26387a9bdb3b9.yaml

diff --git a/aodh/api/hooks.py b/aodh/api/hooks.py
index 0965ebca2..dfc813b7a 100644
--- a/aodh/api/hooks.py
+++ b/aodh/api/hooks.py
@@ -13,11 +13,19 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from oslo_config import cfg
+from oslo_policy import opts
 from oslo_policy import policy
 from pecan import hooks
 
 from aodh.api import policies
 
+# TODO(gmann): Remove setting the default value of config policy_file
+# once oslo_policy change the default value to 'policy.yaml'.
+# https://github.com/openstack/oslo.policy/blob/a626ad12fe5a3abd49d70e3e5b95589d279ab578/oslo_policy/opts.py#L49
+DEFAULT_POLICY_FILE = 'policy.yaml'
+opts.set_defaults(cfg.CONF, DEFAULT_POLICY_FILE)
+
 
 class ConfigHook(hooks.PecanHook):
     """Attach the configuration and policy enforcer object to the request.
diff --git a/aodh/cmd/status.py b/aodh/cmd/status.py
index cefe168b3..db7762a32 100644
--- a/aodh/cmd/status.py
+++ b/aodh/cmd/status.py
@@ -15,6 +15,7 @@
 import sys
 
 from oslo_config import cfg
+from oslo_upgradecheck import common_checks
 from oslo_upgradecheck import upgradecheck
 
 from aodh.i18n import _
@@ -30,17 +31,9 @@ class Checks(upgradecheck.UpgradeCommands):
     and added to _upgrade_checks tuple.
     """
 
-    def _sample_check(self):
-        """This is sample check added to test the upgrade check framework
-
-        It needs to be removed after adding any real upgrade check
-        """
-        return upgradecheck.Result(upgradecheck.Code.SUCCESS, 'Sample detail')
-
     _upgrade_checks = (
-        # Sample check added for now.
-        # Whereas in future real checks must be added here in tuple
-        (_('Sample Check'), _sample_check),
+        (_('policy File JSON to YAML Migration'),
+         (common_checks.check_policy_json, {'conf': CONF})),
     )
 
 
diff --git a/aodh/conf/defaults.py b/aodh/conf/defaults.py
index 8750c9c1d..e6ace8550 100644
--- a/aodh/conf/defaults.py
+++ b/aodh/conf/defaults.py
@@ -12,7 +12,23 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from oslo_config import cfg
 from oslo_middleware import cors
+from oslo_policy import opts as policy_opts
+
+
+def set_lib_defaults():
+    """Update default value for configuration options from other namespace.
+
+    Example, oslo lib config options. This is needed for
+    config generator tool to pick these default value changes.
+    https://docs.openstack.org/oslo.config/latest/cli/
+    generator.html#modifying-defaults-from-other-namespaces
+    """
+    set_cors_middleware_defaults()
+
+    # Update default value of oslo.policy policy_file config option.
+    policy_opts.set_defaults(cfg.CONF, 'policy.yaml')
 
 
 def set_cors_middleware_defaults():
diff --git a/aodh/service.py b/aodh/service.py
index e83198791..6e747709e 100644
--- a/aodh/service.py
+++ b/aodh/service.py
@@ -87,7 +87,7 @@ def prepare_service(argv=None, config_files=None):
     if profiler_opts:
         profiler_opts.set_defaults(conf)
     policy_opts.set_defaults(conf, policy_file=os.path.abspath(
-        os.path.join(os.path.dirname(__file__), "api", "policy.json")))
+        os.path.join(os.path.dirname(__file__), "api", "policy.yaml")))
     from aodh import opts
     # Register our own Aodh options
     for group, options in opts.list_opts():
diff --git a/aodh/tests/functional/api/v2/policy.json-test b/aodh/tests/functional/api/v2/policy.json-test
deleted file mode 100644
index 58a01a7ab..000000000
--- a/aodh/tests/functional/api/v2/policy.json-test
+++ /dev/null
@@ -1,7 +0,0 @@
-{
-    "context_is_admin": "role:admin",
-    "segregation": "rule:context_is_admin",
-    "admin_or_owner": "rule:context_is_admin or project_id:%(project_id)s",
-    "default": "rule:admin_or_owner",
-    "telemetry:get_alarms": "role:admin"
-}
diff --git a/aodh/tests/functional/api/v2/policy.yaml-test b/aodh/tests/functional/api/v2/policy.yaml-test
new file mode 100644
index 000000000..7a09cb594
--- /dev/null
+++ b/aodh/tests/functional/api/v2/policy.yaml-test
@@ -0,0 +1,8 @@
+# WARNING: Below rules are either deprecated rules
+# or extra rules in policy file, it is strongly
+# recommended to switch to new rules.
+"context_is_admin": "role:admin"
+"segregation": "rule:context_is_admin"
+"admin_or_owner": "rule:context_is_admin or project_id:%(project_id)s"
+"default": "rule:admin_or_owner"
+"telemetry:get_alarms": "role:admin"
diff --git a/aodh/tests/functional/api/v2/test_alarm_scenarios.py b/aodh/tests/functional/api/v2/test_alarm_scenarios.py
index a6e380f62..3d2e55de3 100644
--- a/aodh/tests/functional/api/v2/test_alarm_scenarios.py
+++ b/aodh/tests/functional/api/v2/test_alarm_scenarios.py
@@ -414,7 +414,7 @@ class TestAlarms(TestAlarmsBase):
         _test('project_id')
 
     def test_get_alarm_forbiden(self):
-        pf = os.path.abspath('aodh/tests/functional/api/v2/policy.json-test')
+        pf = os.path.abspath('aodh/tests/functional/api/v2/policy.yaml-test')
         self.CONF.set_override('policy_file', pf, group='oslo_policy')
         self.CONF.set_override('auth_mode', None, group='api')
         self.app = webtest.TestApp(app.load_app(self.CONF))
diff --git a/aodh/tests/unit/cmd/test_status.py b/aodh/tests/unit/cmd/test_status.py
index b5dc275ff..f479b2cde 100644
--- a/aodh/tests/unit/cmd/test_status.py
+++ b/aodh/tests/unit/cmd/test_status.py
@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_config import cfg
 from oslo_upgradecheck.upgradecheck import Code
 
 from aodh.cmd import status
@@ -23,8 +24,13 @@ class TestUpgradeChecks(base.BaseTestCase):
     def setUp(self):
         super(TestUpgradeChecks, self).setUp()
         self.cmd = status.Checks()
+        cfg.CONF(args=[], project='aodh')
 
-    def test__sample_check(self):
-        check_result = self.cmd._sample_check()
-        self.assertEqual(
-            Code.SUCCESS, check_result.code)
+    def test_checks(self):
+        for name, func in self.cmd._upgrade_checks:
+            if isinstance(func, tuple):
+                func_name, kwargs = func
+                result = func_name(self, **kwargs)
+            else:
+                result = func(self)
+            self.assertEqual(Code.SUCCESS, result.code)
diff --git a/doc/source/configuration/policy.rst b/doc/source/configuration/policy.rst
index 1593f24cc..f47f8bdcb 100644
--- a/doc/source/configuration/policy.rst
+++ b/doc/source/configuration/policy.rst
@@ -2,6 +2,14 @@
 Aodh Sample Policy Configuration File
 =====================================
 
+.. warning::
+
+   JSON formatted policy file is deprecated since Aodh 12.0.0 (Wallaby).
+   This `oslopolicy-convert-json-to-yaml`__ tool will migrate your existing
+   JSON-formatted policy file to YAML in a backward-compatible way.
+
+.. __: https://docs.openstack.org/oslo.policy/latest/cli/oslopolicy-convert-json-to-yaml.html
+
 The following is an overview of all available policies in Aodh.
 For a sample configuration file, refer to :doc:`sample-policy-yaml`.
 
diff --git a/doc/source/configuration/sample-policy-yaml.rst b/doc/source/configuration/sample-policy-yaml.rst
index 90e1e1183..01b29fd44 100644
--- a/doc/source/configuration/sample-policy-yaml.rst
+++ b/doc/source/configuration/sample-policy-yaml.rst
@@ -2,6 +2,14 @@
 policy.yaml
 ===========
 
+.. warning::
+
+   JSON formatted policy file is deprecated since Aodh 12.0.0 (Wallaby).
+   This `oslopolicy-convert-json-to-yaml`__ tool will migrate your existing
+   JSON-formatted policy file to YAML in a backward-compatible way.
+
+.. __: https://docs.openstack.org/oslo.policy/latest/cli/oslopolicy-convert-json-to-yaml.html
+
 Use the ``policy.yaml`` file to define additional access controls that will be
 applied to Aodh:
 
diff --git a/releasenotes/notes/deprecate-json-formatted-policy-file-fgb26387a9bdb3b9.yaml b/releasenotes/notes/deprecate-json-formatted-policy-file-fgb26387a9bdb3b9.yaml
new file mode 100644
index 000000000..c9c530004
--- /dev/null
+++ b/releasenotes/notes/deprecate-json-formatted-policy-file-fgb26387a9bdb3b9.yaml
@@ -0,0 +1,20 @@
+---
+upgrade:
+  - |
+    The default value of ``[oslo_policy] policy_file`` config option has
+    been changed from ``policy.json`` to ``policy.yaml``.
+    Operators who are utilizing customized or previously generated
+    static policy JSON files (which are not needed by default), should
+    generate new policy files or convert them in YAML format. Use the
+    `oslopolicy-convert-json-to-yaml
+    <https://docs.openstack.org/oslo.policy/latest/cli/oslopolicy-convert-json-to-yaml.html>`_
+    tool to convert a JSON to YAML formatted policy file in
+    backward compatible way.
+deprecations:
+  - |
+    Use of JSON policy files was deprecated by the ``oslo.policy`` library
+    during the Victoria development cycle. As a result, this deprecation is
+    being noted in the Wallaby cycle with an anticipated future removal of support
+    by ``oslo.policy``. As such operators will need to convert to YAML policy
+    files. Please see the upgrade notes for details on migration of any
+    custom policy files.
diff --git a/requirements.txt b/requirements.txt
index 435b8f900..54dabb670 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -10,12 +10,12 @@ keystonemiddleware>=5.1.0 # Apache-2.0
 gnocchiclient>=3.1.0  # Apache-2.0
 lxml>=2.3
 oslo.db>=4.8.0,!=4.13.1,!=4.13.2,!=4.15.0 # Apache-2.0
-oslo.config>=2.6.0 # Apache-2.0
+oslo.config>=6.8.0 # Apache-2.0
 oslo.context>=2.22.0 # Apache-2.0
 oslo.i18n>=1.5.0 # Apache-2.0
 oslo.log>=4.3.0 # Apache-2.0
 oslo.policy>=3.6.0 # Apache-2.0
-oslo.upgradecheck>=0.1.1 # Apache-2.0
+oslo.upgradecheck>=1.3.0 # Apache-2.0
 PasteDeploy>=1.5.0
 pbr>=2.0.0 # Apache-2.0
 pecan>=0.8.0
diff --git a/setup.cfg b/setup.cfg
index d88c8faaf..539661748 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -110,7 +110,7 @@ oslo.config.opts =
     aodh-auth = aodh.opts:list_keystoneauth_opts
 
 oslo.config.opts.defaults =
-    aodh = aodh.conf.defaults:set_cors_middleware_defaults
+    aodh = aodh.conf.defaults:set_lib_defaults
 
 oslo.policy.policies =
     aodh = aodh.api.policies:list_rules