Merge "Change default policy file from JSON to YAML"

.gitignore

@@ -16,6 +16,7 @@ doc/source/_static/cyborg.conf.sample
 doc/source/_static/cyborg.policy.yaml.sample
 # Sample profile
 etc/cyborg/policy.json.sample
+etc/cyborg/policy.yaml.sample
 etc/cyborg/cyborg.conf.sample
 .idea/*
 .vscode/*

cyborg/cmd/status.py

@@ -16,20 +16,38 @@ import sys
 
 from oslo_config import cfg
 from oslo_upgradecheck import upgradecheck
+from oslo_utils import fileutils
 
 from cyborg.common.i18n import _
 
 
+CONF = cfg.CONF
+
+
 class Checks(upgradecheck.UpgradeCommands):
 
     """Various upgrade checks should be added as separate methods in this class
     and added to _upgrade_checks tuple.
     """
 
-    def _check_placeholder(self):
+    def _check_policy_json(self):
-        # TODO(whoami-rajat):This is just a placeholder for upgrade checks,
+        "Checks to see if policy file is JSON-formatted policy file."
-        # it should be removed when the actual checks are added
+        msg = _("Your policy file is JSON-formatted which is "
-        return upgradecheck.Result(upgradecheck.Code.SUCCESS)
+                "deprecated since Victoria release (Cyborg 5.0.0). "
+                "You need to switch to YAML-formatted file. You can use the "
+                "``oslopolicy-convert-json-to-yaml`` tool to convert existing "
+                "JSON-formatted files to YAML-formatted files in a "
+                "backwards-compatible manner: "
+                "https://docs.openstack.org/oslo.policy/"
+                "latest/cli/oslopolicy-convert-json-to-yaml.html.")
+        status = upgradecheck.Result(upgradecheck.Code.SUCCESS)
+        # NOTE(gmann): Check if policy file exist and is in
+        # JSON format by actually loading the file not just
+        # by checking the extension.
+        policy_path = CONF.find_file(CONF.oslo_policy.policy_file)
+        if policy_path and fileutils.is_json(policy_path):
+            status = upgradecheck.Result(upgradecheck.Code.FAILURE, msg)
+        return status
 
     # The format of the check functions is to return an
     # oslo_upgradecheck.upgradecheck.Result
@@ -39,8 +57,8 @@ class Checks(upgradecheck.UpgradeCommands):
     # in the returned Result's "details" attribute. The
     # summary will be rolled up at the end of the check() method.
     _upgrade_checks = (
-        # In the future there should be some real checks added here
+        # Added in Victoria
-        (_('Placeholder'), _check_placeholder),
+        (_('Policy File JSON to YAML Migration'), _check_policy_json),
     )
 
 

cyborg/common/authorize_wsgi.py

@@ -16,6 +16,7 @@ import sys
 from oslo_concurrency import lockutils
 from oslo_config import cfg
 from oslo_log import log
+from oslo_policy import opts
 from oslo_policy import policy
 from oslo_versionedobjects import base as object_base
 import pecan
@@ -30,6 +31,36 @@ CONF = cfg.CONF
 LOG = log.getLogger(__name__)
 
 
+# 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(CONF, DEFAULT_POLICY_FILE)
+
+
+def pick_policy_file(policy_file):
+    # TODO(gmann): We have changed the default value of
+    # CONF.oslo_policy.policy_file option to 'policy.yaml' in Victoria
+    # release. To avoid breaking any deployment relying on default
+    # value, we need to add this is fallback logic to pick the old default
+    # policy file (policy.yaml) if exist. We can to remove this fallback
+    # logic sometime in future.
+    if policy_file:
+        return policy_file
+
+    if CONF.oslo_policy.policy_file == DEFAULT_POLICY_FILE:
+        location = CONF.get_location('policy_file', 'oslo_policy').location
+        if CONF.find_file(CONF.oslo_policy.policy_file):
+            return CONF.oslo_policy.policy_file
+        elif location in [cfg.Locations.opt_default,
+                          cfg.Locations.set_default]:
+            old_default = 'policy.json'
+            if CONF.find_file(old_default):
+                return old_default
+    # Return overridden value
+    return CONF.oslo_policy.policy_file
+
+
 @lockutils.synchronized('policy_enforcer', 'cyborg-')
 def init_enforcer(policy_file=None, rules=None,
                   default_rule=None, use_conf=True,
@@ -55,7 +86,9 @@ def init_enforcer(policy_file=None, rules=None,
     # loaded exactly once - when this module-global is initialized.
     # Defining these in the relevant API modules won't work
     # because API classes lack singletons and don't use globals.
-    _ENFORCER = policy.Enforcer(CONF, policy_file=policy_file,
+    _ENFORCER = policy.Enforcer(
+        CONF,
+        policy_file=pick_policy_file(policy_file),
         rules=rules,
         default_rule=default_rule,
         use_conf=use_conf)

cyborg/common/policy.py

@@ -19,7 +19,7 @@
 """
 from oslo_policy import policy
 # NOTE: to follow policy-in-code spec, we define defaults for
-#       the granular policies in code, rather than in policy.json.
+#       the granular policies in code, rather than in policy.yaml.
 #       All of these may be overridden by configuration, but we can
 #       depend on their existence throughout the code.
 

cyborg/tests/unit/cmd/test_status.py

@@ -12,19 +12,77 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from oslo_upgradecheck.upgradecheck import Code
+import fixtures
+import os
+import tempfile
+import yaml
+
+from oslo_config import cfg
+from oslo_serialization import jsonutils
+from oslo_upgradecheck import upgradecheck
 
 from cyborg.cmd import status
+from cyborg.common import authorize_wsgi
 from cyborg.tests import base
 
 
-class TestUpgradeChecks(base.TestCase):
+class TestUpgradeCheckPolicyJSON(base.TestCase):
 
     def setUp(self):
-        super(TestUpgradeChecks, self).setUp()
+        super(TestUpgradeCheckPolicyJSON, self).setUp()
-        self.cmd = status.Checks()
+        self.cmd = status.UpgradeCommands()
+        authorize_wsgi.CONF.clear_override('policy_file', group='oslo_policy')
+        self.data = {
+            'rule_admin': 'True',
+            'rule_admin2': 'is_admin:True'
+        }
+        self.temp_dir = self.useFixture(fixtures.TempDir())
+        fd, self.json_file = tempfile.mkstemp(dir=self.temp_dir.path)
+        fd, self.yaml_file = tempfile.mkstemp(dir=self.temp_dir.path)
 
-    def test__check_placeholder(self):
+        with open(self.json_file, 'w') as fh:
-        check_result = self.cmd._check_placeholder()
+            jsonutils.dump(self.data, fh)
-        self.assertEqual(
+        with open(self.yaml_file, 'w') as fh:
-            Code.SUCCESS, check_result.code)
+            yaml.dump(self.data, fh)
+
+        original_search_dirs = cfg._search_dirs
+
+        def fake_search_dirs(dirs, name):
+            dirs.append(self.temp_dir.path)
+            return original_search_dirs(dirs, name)
+
+        self.mock_search = self.useFixture(fixtures.MockPatch(
+            'oslo_config.cfg._search_dirs')).mock
+        self.mock_search.side_effect = fake_search_dirs
+
+    def test_policy_json_file_fail_upgrade(self):
+        # Test with policy json file full path set in config.
+        self.flags(policy_file=self.json_file, group="oslo_policy")
+        self.assertEqual(upgradecheck.Code.FAILURE,
+                         self.cmd._check_policy_json().code)
+
+    def test_policy_yaml_file_pass_upgrade(self):
+        # Test with full policy yaml file path set in config.
+        self.flags(policy_file=self.yaml_file, group="oslo_policy")
+        self.assertEqual(upgradecheck.Code.SUCCESS,
+                         self.cmd._check_policy_json().code)
+
+    def test_no_policy_file_pass_upgrade(self):
+        # Test with no policy file exist.
+        self.assertEqual(upgradecheck.Code.SUCCESS,
+                         self.cmd._check_policy_json().code)
+
+    def test_default_policy_yaml_file_pass_upgrade(self):
+        tmpfilename = os.path.join(self.temp_dir.path, 'policy.yaml')
+        with open(tmpfilename, 'w') as fh:
+            yaml.dump(self.data, fh)
+        self.assertEqual(upgradecheck.Code.SUCCESS,
+                         self.cmd._check_policy_json().code)
+
+    def test_old_default_policy_json_file_fail_upgrade(self):
+        self.flags(policy_file='policy.json', group="oslo_policy")
+        tmpfilename = os.path.join(self.temp_dir.path, 'policy.json')
+        with open(tmpfilename, 'w') as fh:
+            jsonutils.dump(self.data, fh)
+        self.assertEqual(upgradecheck.Code.FAILURE,
+                         self.cmd._check_policy_json().code)

cyborg/tests/unit/policy_fixture.py

@@ -36,7 +36,7 @@ class PolicyFixture(fixtures.Fixture):
         super(PolicyFixture, self).setUp()
         self.policy_dir = self.useFixture(fixtures.TempDir())
         self.policy_file_name = os.path.join(self.policy_dir.path,
-                                             'policy.json')
+                                             'policy.yaml')
         with open(self.policy_file_name, 'w') as policy_file:
             policy_file.write(policy_data)
         policy_opts.set_defaults(CONF)

devstack/settings

@@ -15,7 +15,7 @@ CYBORG_AUTH_CACHE_DIR=${CYBORG_AUTH_CACHE_DIR:-/var/cache/cyborg}
 CYBORG_CONF_DIR=${CYBORG_CONF_DIR:-/etc/cyborg}
 CYBORG_CONF_FILE=$CYBORG_CONF_DIR/cyborg.conf
 CYBORG_API_PASTE_INI=$CYBORG_CONF_DIR/api-paste.ini
-CYBORG_POLICY_JSON=$CYBORG_CONF_DIR/policy.json
+CYBORG_POLICY_JSON=$CYBORG_CONF_DIR/policy.yaml
 CYBORG_SERVICE_HOST=${CYBORG_SERVICE_HOST:-$SERVICE_HOST}
 CYBORG_SERVICE_PORT=${CYBORG_SERVICE_PORT:-6666}
 CYBORG_SERVICE_PROTOCOL=${CYBORG_SERVICE_PROTOCOL:-$SERVICE_PROTOCOL}

doc/source/configuration/sample_policy.rst

@@ -2,6 +2,15 @@
 Cyborg Sample Policy
 ====================
 
+.. warning::
+
+   JSON formatted policy file is deprecated since Cyborg 5.0.0(Victoria).
+   Use YAML formatted file. Use `oslopolicy-convert-json-to-yaml`__ tool
+   to convert the existing JSON to YAML formatted policy file in backward
+   compatible way.
+
+.. __: https://docs.openstack.org/oslo.policy/latest/cli/oslopolicy-convert-json-to-yaml.html
+
 The following is a sample cyborg policy file that has been auto-generated
 from default policy values in code. If you're using the default policies, then
 the maintenance of this file is not necessary, and it should not be copied into

etc/cyborg/README.policy.yaml.txt

@@ -1,4 +1,4 @@
-To generate the sample policy.json file, run the following command from the top
+To generate the sample policy.yaml file, run the following command from the top
 level of the cyborg directory:
 
     tox -egenpolicy

releasenotes/notes/policy-file-default-value-change-de14a3688357b081.yaml

@@ -0,0 +1,14 @@
+---
+upgrade:
+  - |
+    The default value of ``[oslo_policy] policy_file`` config option has been
+    changed from ``policy.json``
+    to ``policy.yaml``. Cyborg policy new defaults since 5.0.0 and current
+    default value of ``[oslo_policy] policy_file`` config option (``policy.json``)
+    does not work when ``policy.json`` is generated by
+    `oslopolicy-sample-generator <https://docs.openstack.org/oslo.policy/latest/cli/oslopolicy-sample-generator.html>`_  tool.
+    Refer to `bug 1875418 <https://bugs.launchpad.net/nova/+bug/1875418>`_
+    for more details.
+    Also check `oslopolicy-convert-json-to-yaml <https://docs.openstack.org/oslo.policy/latest/cli/oslopolicy-convert-json-to-yaml.html>`_
+    tool to convert the JSON to YAML formatted policy file in
+    backward compatible way.

requirements.txt

@@ -17,9 +17,9 @@ oslo.service>=1.0.0,!=1.28.1 # Apache-2.0
 oslo.db>=4.44.0 # Apache-2.0
 os-resource-classes>=0.5.0 # Apache-2.0
 oslo.upgradecheck>=0.1.0 # Apache-2.0
-oslo.utils>=3.33.0 # Apache-2.0
+oslo.utils>=4.5.0 # Apache-2.0
 oslo.versionedobjects>=1.31.2 # Apache-2.0
-oslo.policy>=2.3.0 # Apache-2.0
+oslo.policy>=3.4.0 # Apache-2.0
 SQLAlchemy>=0.9.0,!=1.1.5,!=1.1.6,!=1.1.7,!=1.1.8 # MIT
 alembic>=0.8.10 # MIT
 stevedore>=1.5.0 # Apache-2.0

setup.cfg

@@ -26,7 +26,7 @@ packages =
     cyborg
 data_files =
     etc/cyborg =
-        etc/cyborg/policy.json
+        etc/cyborg/policy.yaml
         etc/cyborg/api-paste.ini
 
 [entry_points]

tools/config/cyborg-policy-generator.conf

@@ -1,3 +1,3 @@
 [DEFAULT]
-output_file = etc/cyborg/policy.json.sample
+output_file = etc/cyborg/policy.yaml.sample
 namespace = cyborg.api