Reuse code from oslo lib for JSON policy migration

In Victoria cycle, we migrated the JSON formatted policy file to YAML
- https://review.opendev.org/#/c/748059/

Which added the upgrade check and policy fallback logic to select
the default JSON file if exist.

In Wallaby cycle, this work is defined as community wide goal
- https://governance.openstack.org/tc/goals/selected/wallaby/migrate-policy-format-from-json-to-yaml.html

and common part of 1. upgrade check 2. policy fallback logic is moved
from nova to oslo.upgradechecks and oslo.policy respectively.

- oslo.upgradechecks(1.3.0): https://review.opendev.org/#/c/763484/
- oslo.policy(3.6.0): https://review.opendev.org/#/c/763261/

This commit make use these code form oslo lib.

Change-Id: I1a8bc19b77abdcb6867eb61fe6ea1945142b32d2

lower-constraints.txt

@@ -76,13 +76,13 @@ oslo.i18n==3.15.3
 oslo.log==3.36.0
 oslo.messaging==10.3.0
 oslo.middleware==3.31.0
-oslo.policy==3.4.0
+oslo.policy==3.6.0
 oslo.privsep==1.33.2
 oslo.reports==1.18.0
 oslo.rootwrap==5.8.0
 oslo.serialization==2.21.1
 oslo.service==1.40.1
-oslo.upgradecheck==0.1.1
+oslo.upgradecheck==1.3.0
 oslo.utils==4.7.0
 oslo.versionedobjects==1.35.0
 oslo.vmware==2.17.0
@@ -125,7 +125,7 @@ python-mimeparse==1.6.0
 python-neutronclient==6.7.0
 python-subunit==1.4.0
 pytz==2018.3
-PyYAML==3.13
+PyYAML==5.1
 repoze.lru==0.7
 requests==2.23.0
 requests-mock==1.2.0

nova/cmd/status.py

@@ -24,8 +24,8 @@ import traceback
 from keystoneauth1 import exceptions as ks_exc
 from oslo_config import cfg
 from oslo_serialization import jsonutils
+from oslo_upgradecheck import common_checks
 from oslo_upgradecheck import upgradecheck
-from oslo_utils import fileutils
 import pkg_resources
 from sqlalchemy import func as sqlfunc
 from sqlalchemy import MetaData, Table, and_, select
@@ -409,23 +409,6 @@ class UpgradeCommands(upgradecheck.UpgradeCommands):
         policy.reset()
         return status
 
-    def _check_policy_json(self):
-        "Checks to see if policy file is JSON-formatted policy file."
-        msg = _("Your policy file  is JSON-formatted which is "
-                "deprecated since Victoria release (Nova 22.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)
-        # Check if policy file exist and is JSON-formatted.
-        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
-
     def _check_old_computes(self):
         # warn if there are computes in the system older than the previous
         # major release
@@ -455,7 +438,10 @@ class UpgradeCommands(upgradecheck.UpgradeCommands):
         # Added in Ussuri
         (_('Policy Scope-based Defaults'), _check_policy),
         # Added in Victoria
-        (_('Policy File JSON to YAML Migration'), _check_policy_json),
+        (
+            _('Policy File JSON to YAML Migration'),
+            (common_checks.check_policy_json, {'conf': CONF})
+        ),
         # Added in Wallaby
         (_('Older than N-1 computes'), _check_old_computes)
     )

nova/policy.py

@@ -48,29 +48,6 @@ DEFAULT_POLICY_FILE = 'policy.yaml'
 opts.set_defaults(cfg.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.json) 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
-
-
 def reset():
     global _ENFORCER
     if _ENFORCER:
@@ -99,7 +76,7 @@ def init(policy_file=None, rules=None, default_rule=None, use_conf=True,
     if not _ENFORCER:
         _ENFORCER = policy.Enforcer(
             CONF,
-            policy_file=pick_policy_file(policy_file),
+            policy_file=policy_file,
             rules=rules,
             default_rule=default_rule,
             use_conf=use_conf)

nova/tests/unit/cmd/test_status.py

@@ -21,18 +21,13 @@ Unit tests for the nova-status CLI interfaces.
 # PlacementFixture, which is only available in functioanl tests.
 
 from io import StringIO
-import os
-import tempfile
 
 import fixtures
 import mock
-import yaml
 
 from keystoneauth1 import exceptions as ks_exc
 from keystoneauth1 import loading as keystone
 from keystoneauth1 import session
-from oslo_config import cfg
-from oslo_serialization import jsonutils
 from oslo_upgradecheck import upgradecheck
 from oslo_utils.fixture import uuidsentinel as uuids
 from oslo_utils import uuidutils
@@ -611,66 +606,6 @@ class TestUpgradeCheckPolicyEnableScope(TestUpgradeCheckPolicy):
         self.flags(enforce_scope=True, group="oslo_policy")
 
 
-class TestUpgradeCheckPolicyJSON(test.NoDBTestCase):
-
-    def setUp(self):
-        super(TestUpgradeCheckPolicyJSON, self).setUp()
-        self.cmd = status.UpgradeCommands()
-        policy.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)
-
-        with open(self.json_file, 'w') as fh:
-            jsonutils.dump(self.data, fh)
-        with open(self.yaml_file, 'w') as fh:
-            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.stub_out('oslo_config.cfg._search_dirs', 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)
-
-
 class TestUpgradeCheckOldCompute(test.NoDBTestCase):
 
     def setUp(self):

nova/tests/unit/test_policy.py

@@ -17,13 +17,10 @@
 
 import os.path
 
-import fixtures
 import mock
-from oslo_config import cfg
 from oslo_policy import policy as oslo_policy
 from oslo_serialization import jsonutils
 import requests_mock
-import yaml
 
 from nova import context
 from nova import exception
@@ -570,70 +567,3 @@ class RealRolePolicyTestCase(test.NoDBTestCase):
             self.system_reader_or_owner_rules +
             self.allow_nobody_rules + special_rules)
         self.assertEqual(set([]), result)
-
-
-class PickPolicyFileTestCase(test.NoDBTestCase):
-
-    def setUp(self):
-        super(PickPolicyFileTestCase, self).setUp()
-        self.data = {
-            'rule_admin': 'True',
-            'rule_admin2': 'is_admin:True'
-        }
-        policy.CONF.clear_override('policy_file', group='oslo_policy')
-        self.tmpdir = self.useFixture(fixtures.TempDir())
-        original_search_dirs = cfg._search_dirs
-
-        def fake_search_dirs(dirs, name):
-            dirs.append(self.tmpdir.path)
-            return original_search_dirs(dirs, name)
-
-        self.stub_out('oslo_config.cfg._search_dirs', fake_search_dirs)
-
-    def test_non_config_policy_file(self):
-        tmpfilename = 'nova-policy.yaml'
-        self.flags(policy_file=tmpfilename, group='oslo_policy')
-        selected_policy_file = policy.pick_policy_file(
-            policy_file='non-config-file')
-        self.assertEqual(policy.CONF.oslo_policy.policy_file, tmpfilename)
-        self.assertEqual(selected_policy_file, 'non-config-file')
-
-    def test_overridden_policy_file(self):
-        tmpfilename = 'nova-policy.yaml'
-        self.flags(policy_file=tmpfilename, group='oslo_policy')
-        selected_policy_file = policy.pick_policy_file(policy_file=None)
-        self.assertEqual(policy.CONF.oslo_policy.policy_file, tmpfilename)
-        self.assertEqual(selected_policy_file, tmpfilename)
-
-    def test_only_new_default_policy_file_exist(self):
-        tmpfilename = os.path.join(self.tmpdir.path, 'policy.yaml')
-        with open(tmpfilename, 'w') as fh:
-            yaml.dump(self.data, fh)
-
-        selected_policy_file = policy.pick_policy_file(policy_file=None)
-        self.assertEqual(policy.CONF.oslo_policy.policy_file, 'policy.yaml')
-        self.assertEqual(selected_policy_file, 'policy.yaml')
-
-    @mock.patch.object(policy.CONF, 'get_location')
-    def test_only_old_default_policy_file_exist(self, mock_get):
-        mock_get.return_value = cfg.LocationInfo(cfg.Locations.set_default,
-                                                 'None')
-        tmpfilename = os.path.join(self.tmpdir.path, 'policy.json')
-        with open(tmpfilename, 'w') as fh:
-            jsonutils.dump(self.data, fh)
-
-        selected_policy_file = policy.pick_policy_file(policy_file=None)
-        self.assertEqual(policy.CONF.oslo_policy.policy_file, 'policy.yaml')
-        self.assertEqual(selected_policy_file, 'policy.json')
-
-    def test_both_default_policy_file_exist(self):
-        tmpfilename1 = os.path.join(self.tmpdir.path, 'policy.json')
-        with open(tmpfilename1, 'w') as fh:
-            jsonutils.dump(self.data, fh)
-        tmpfilename2 = os.path.join(self.tmpdir.path, 'policy.yaml')
-        with open(tmpfilename2, 'w') as fh:
-            yaml.dump(self.data, fh)
-
-        selected_policy_file = policy.pick_policy_file(policy_file=None)
-        self.assertEqual(policy.CONF.oslo_policy.policy_file, 'policy.yaml')
-        self.assertEqual(selected_policy_file, 'policy.yaml')

requirements.txt

@@ -38,12 +38,12 @@ oslo.context>=2.22.0 # Apache-2.0
 oslo.log>=3.36.0 # Apache-2.0
 oslo.reports>=1.18.0 # Apache-2.0
 oslo.serialization!=2.19.1,>=2.21.1 # Apache-2.0
-oslo.upgradecheck>=0.1.1
+oslo.upgradecheck>=1.3.0
 oslo.utils>=4.7.0 # Apache-2.0
 oslo.db>=4.44.0 # Apache-2.0
 oslo.rootwrap>=5.8.0 # Apache-2.0
 oslo.messaging>=10.3.0 # Apache-2.0
-oslo.policy>=3.4.0 # Apache-2.0
+oslo.policy>=3.6.0 # Apache-2.0
 oslo.privsep>=1.33.2 # Apache-2.0
 oslo.i18n>=3.15.3 # Apache-2.0
 oslo.service>=1.40.1 # Apache-2.0
@@ -70,4 +70,4 @@ zVMCloudConnector>=1.3.0;sys_platform!='win32'  # Apache 2.0 License
 futurist>=1.8.0 # Apache-2.0
 openstacksdk>=0.35.0 # Apache-2.0
 dataclasses>=0.7;python_version=='3.6'  # Apache 2.0 License
-PyYAML>=3.13 # MIT
+PyYAML>=5.1 # MIT