From fe545dbe5fb434d5fce2d4d0f24c6c4a6bdd7d21 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Thu, 3 Sep 2020 14:25:43 -0500 Subject: [PATCH] Migrate default policy file from JSON to YAML Default value of 'CONF.oslo_policy.policy_file' config option has been changed from 'policy.json' to 'policy.yaml'. If new default file 'policy.yaml' does not exist but old default 'policy.json' exist then fallback to use old default file. An upgrade checks is added to check the policy_file format and fail upgrade checks if it is JSON formatted. Added a warning in policy doc about JSON formatted file is deprecated, also removed all the reference to policy.json file in doc as well as in tests. Related Blueprint: policy-json-to-yaml Closes-Bug: #1875418 Change-Id: Ic4d3b998bb9701cb1e3ef12d9bb6f4d91cc19c18 --- doc/source/cli/nova-status.rst | 4 ++ doc/source/configuration/policy-concepts.rst | 13 +++- doc/source/configuration/policy.rst | 9 +++ doc/source/install/verify.rst | 4 ++ lower-constraints.txt | 10 +-- nova/cmd/status.py | 20 ++++++ nova/policy.py | 41 +++++++++-- nova/tests/unit/cmd/test_status.py | 65 +++++++++++++++++ nova/tests/unit/test_policy.py | 70 +++++++++++++++++++ ...t-policy-file-change-22bd4cc6e27e0091.yaml | 19 +++++ requirements.txt | 10 +-- 11 files changed, 248 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/bug-1875418-default-policy-file-change-22bd4cc6e27e0091.yaml diff --git a/doc/source/cli/nova-status.rst b/doc/source/cli/nova-status.rst index f226ad2f908c..9eae13b43d24 100644 --- a/doc/source/cli/nova-status.rst +++ b/doc/source/cli/nova-status.rst @@ -139,6 +139,10 @@ Upgrade * Checks for the policy files are not automatically overwritten with new defaults. + **22.0.0 (Victoria)** + + * Checks for the policy files is not JSON-formatted. + See Also ======== diff --git a/doc/source/configuration/policy-concepts.rst b/doc/source/configuration/policy-concepts.rst index b9ca84115b36..35c1480da7a0 100644 --- a/doc/source/configuration/policy-concepts.rst +++ b/doc/source/configuration/policy-concepts.rst @@ -1,10 +1,19 @@ Understanding Nova Policies =========================== +.. warning:: + + JSON formatted policy file is deprecated since Nova 22.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 + Nova supports a rich policy system that has evolved significantly over its lifetime. Initially, this took the form of a large, mostly hand-written -``policy.json`` file but, starting in the Newton (14.0.0) release, policy -defaults have been defined in the codebase, requiring the ``policy.json`` +``policy.yaml`` file but, starting in the Newton (14.0.0) release, policy +defaults have been defined in the codebase, requiring the ``policy.yaml`` file only to override these defaults. In the Ussuri (21.0.0) release, further work was undertaken to address some diff --git a/doc/source/configuration/policy.rst b/doc/source/configuration/policy.rst index 60c8fa500501..12b68fd465bf 100644 --- a/doc/source/configuration/policy.rst +++ b/doc/source/configuration/policy.rst @@ -4,6 +4,15 @@ Nova Policies The following is an overview of all available policies in Nova. +.. warning:: + + JSON formatted policy file is deprecated since Nova 22.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 + .. only:: html For a sample configuration file, refer to :doc:`sample-policy`. diff --git a/doc/source/install/verify.rst b/doc/source/install/verify.rst index 974259587943..440abc7e71a5 100644 --- a/doc/source/install/verify.rst +++ b/doc/source/install/verify.rst @@ -127,3 +127,7 @@ Verify operation of the Compute service. | Result: Success | | Details: None | +--------------------------------------------------------------------+ + | Check: Policy File JSON to YAML Migration | + | Result: Success | + | Details: None | + +--------------------------------------------------------------------+ diff --git a/lower-constraints.txt b/lower-constraints.txt index ff6b310fe160..04af416aa660 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -69,21 +69,21 @@ os-xenapi==0.3.4 osc-lib==1.10.0 oslo.cache==1.26.0 oslo.concurrency==3.29.0 -oslo.config==6.1.0 +oslo.config==6.8.0 oslo.context==2.22.0 oslo.db==4.44.0 oslo.i18n==3.15.3 oslo.log==3.36.0 oslo.messaging==10.3.0 oslo.middleware==3.31.0 -oslo.policy==3.1.0 +oslo.policy==3.4.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.utils==4.1.0 +oslo.utils==4.5.0 oslo.versionedobjects==1.35.0 oslo.vmware==2.17.0 oslotest==3.8.0 @@ -127,11 +127,11 @@ python-subunit==1.4.0 pytz==2018.3 PyYAML==3.13 repoze.lru==0.7 -requests==2.14.2 +requests==2.23.0 requests-mock==1.2.0 requestsexceptions==1.4.0 retrying==1.3.3 -rfc3986==1.1.0 +rfc3986==1.2.0 Routes==2.3.1 simplejson==3.13.2 six==1.11.0 diff --git a/nova/cmd/status.py b/nova/cmd/status.py index 3c9c15fb3ff5..fd3c307dca7c 100644 --- a/nova/cmd/status.py +++ b/nova/cmd/status.py @@ -25,6 +25,7 @@ from keystoneauth1 import exceptions as ks_exc from oslo_config import cfg from oslo_serialization import jsonutils from oslo_upgradecheck import upgradecheck +from oslo_utils import fileutils import pkg_resources import six from sqlalchemy import func as sqlfunc @@ -409,6 +410,23 @@ 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 + # The format of the check functions is to return an upgradecheck.Result # object with the appropriate upgradecheck.Code and details set. If the # check hits warnings or failures then those should be stored in the @@ -427,6 +445,8 @@ class UpgradeCommands(upgradecheck.UpgradeCommands): (_('Cinder API'), _check_cinder), # Added in Ussuri (_('Policy Scope-based Defaults'), _check_policy), + # Added in Victoria + (_('Policy File JSON to YAML Migration'), _check_policy_json), ) diff --git a/nova/policy.py b/nova/policy.py index ad1b79c54290..792471d1aa29 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -19,6 +19,7 @@ import re from oslo_config import cfg from oslo_log import log as logging +from oslo_policy import opts from oslo_policy import policy from oslo_utils import excutils @@ -40,6 +41,35 @@ USER_BASED_RESOURCES = ['os-keypairs'] saved_file_rules = [] KEY_EXPR = re.compile(r'%\((\w+)\)s') +# 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) + + +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 @@ -67,11 +97,12 @@ def init(policy_file=None, rules=None, default_rule=None, use_conf=True, global saved_file_rules if not _ENFORCER: - _ENFORCER = policy.Enforcer(CONF, - policy_file=policy_file, - rules=rules, - default_rule=default_rule, - use_conf=use_conf) + _ENFORCER = policy.Enforcer( + CONF, + policy_file=pick_policy_file(policy_file), + rules=rules, + default_rule=default_rule, + use_conf=use_conf) # NOTE(gmann): Explictly disable the warnings for policies # changing their default check_str. During policy-defaults-refresh # work, all the policy defaults have been changed and warning for diff --git a/nova/tests/unit/cmd/test_status.py b/nova/tests/unit/cmd/test_status.py index d593651f95ca..6922a2a6e010 100644 --- a/nova/tests/unit/cmd/test_status.py +++ b/nova/tests/unit/cmd/test_status.py @@ -22,11 +22,16 @@ Unit tests for the nova-status CLI interfaces. import fixtures import mock +import os from six.moves import StringIO +import tempfile +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 @@ -602,3 +607,63 @@ class TestUpgradeCheckPolicyEnableScope(TestUpgradeCheckPolicy): def setUp(self): super(TestUpgradeCheckPolicyEnableScope, self).setUp() 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) diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 8af58e2ee264..31fee0be7461 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -17,10 +17,13 @@ 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 @@ -572,3 +575,70 @@ 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/releasenotes/notes/bug-1875418-default-policy-file-change-22bd4cc6e27e0091.yaml b/releasenotes/notes/bug-1875418-default-policy-file-change-22bd4cc6e27e0091.yaml new file mode 100644 index 000000000000..0cd1e6362dad --- /dev/null +++ b/releasenotes/notes/bug-1875418-default-policy-file-change-22bd4cc6e27e0091.yaml @@ -0,0 +1,19 @@ +--- +upgrade: + - | + The default value of ``[oslo_policy] policy_file`` config option has been + changed from ``policy.json`` + to ``policy.yaml``. Nova policy new defaults since 21.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 `_ tool. + Refer to `bug 1875418 `_ + for more details. + Also check `oslopolicy-convert-json-to-yaml `_ + tool to convert the JSON to YAML formatted policy file in + backward compatible way. +fixes: + - | + Bug `1875418 `_ is fixed + by changing the default value of ``[oslo_policy] policy_file`` config + option to YAML format. diff --git a/requirements.txt b/requirements.txt index 6e27dca79360..e62366561eee 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,27 +28,27 @@ python-cinderclient!=4.0.0,>=3.3.0 # Apache-2.0 keystoneauth1>=3.16.0 # Apache-2.0 python-neutronclient>=6.7.0 # Apache-2.0 python-glanceclient>=2.8.0 # Apache-2.0 -requests>=2.14.2 # Apache-2.0 +requests>=2.23.0 # Apache-2.0 six>=1.11.0 # MIT stevedore>=1.20.0 # Apache-2.0 websockify>=0.9.0 # LGPLv3 oslo.cache>=1.26.0 # Apache-2.0 oslo.concurrency>=3.29.0 # Apache-2.0 -oslo.config>=6.1.0 # Apache-2.0 +oslo.config>=6.8.0 # Apache-2.0 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.utils>=4.1.0 # Apache-2.0 +oslo.utils>=4.5.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.1.0 # Apache-2.0 +oslo.policy>=3.4.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 -rfc3986>=1.1.0 # Apache-2.0 +rfc3986>=1.2.0 # Apache-2.0 oslo.middleware>=3.31.0 # Apache-2.0 psutil>=3.2.2 # BSD oslo.versionedobjects>=1.35.0 # Apache-2.0