From a72ace1c6c67ab1f5318a39638ba718c81307726 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Thu, 19 Nov 2020 18:44:47 -0600 Subject: [PATCH] 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 | 6 +-- nova/cmd/status.py | 24 +++------- nova/policy.py | 25 +---------- nova/tests/unit/cmd/test_status.py | 65 --------------------------- nova/tests/unit/test_policy.py | 70 ------------------------------ requirements.txt | 6 +-- 6 files changed, 12 insertions(+), 184 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index 35abd1eaff01..6cbd35586014 100644 --- a/lower-constraints.txt +++ b/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 diff --git a/nova/cmd/status.py b/nova/cmd/status.py index 78b49b39efdb..69a69969baa4 100644 --- a/nova/cmd/status.py +++ b/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) ) diff --git a/nova/policy.py b/nova/policy.py index 792471d1aa29..55455a9271dc 100644 --- a/nova/policy.py +++ b/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) diff --git a/nova/tests/unit/cmd/test_status.py b/nova/tests/unit/cmd/test_status.py index 8e2a8602a451..1d2074d7daf7 100644 --- a/nova/tests/unit/cmd/test_status.py +++ b/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): diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 5e7751e6ca1d..8239b62e0082 100644 --- a/nova/tests/unit/test_policy.py +++ b/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') diff --git a/requirements.txt b/requirements.txt index 0db846c9488a..9ccccc1468cf 100644 --- a/requirements.txt +++ b/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