From 9a69cc52b6d993b639f541134f56acddfd9f9012 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Fri, 25 Dec 2020 16:03:57 -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: Ib2101f13171940857fe81f64dd9798dfe489743a --- .../admin/advanced-configuration-guide.rst | 2 +- doc/source/admin/configuration-guide.rst | 10 +++++++++- lower-constraints.txt | 18 ++++++++--------- ...ormatted-policy-file-b267f288cba7e325.yaml | 20 +++++++++++++++++++ requirements.txt | 12 +++++------ sahara/api/acl.py | 7 +++++++ sahara/cli/sahara_status.py | 13 +++--------- sahara/common/config.py | 8 ++++++++ sahara/tests/unit/api/test_acl.py | 10 ++++++++-- sahara/tests/unit/cli/test_sahara_cli.py | 10 ++++++---- sahara/tests/unit/cli/test_sahara_status.py | 18 +++++++++++++---- setup.cfg | 2 +- 12 files changed, 92 insertions(+), 38 deletions(-) create mode 100644 releasenotes/notes/deprecate-json-formatted-policy-file-b267f288cba7e325.yaml diff --git a/doc/source/admin/advanced-configuration-guide.rst b/doc/source/admin/advanced-configuration-guide.rst index 1ddc8aab72..18b4168dcd 100644 --- a/doc/source/admin/advanced-configuration-guide.rst +++ b/doc/source/admin/advanced-configuration-guide.rst @@ -519,7 +519,7 @@ set to ``true`` and some extra configurations are needed: be returned by the Compute service. This can be done by: - * by changing nova's ``policy.json`` to allow the user access to the + * by changing nova's ``policy.yaml`` to allow the user access to the ``extended_server_attributes`` option. * by designating an account with privileged rights in the cinder configuration: diff --git a/doc/source/admin/configuration-guide.rst b/doc/source/admin/configuration-guide.rst index 24b5a4ba36..a0400f58c3 100644 --- a/doc/source/admin/configuration-guide.rst +++ b/doc/source/admin/configuration-guide.rst @@ -161,11 +161,19 @@ for instance provisioning. Policy configuration -------------------- +.. warning:: + + JSON formatted policy file is deprecated since Sahara 15.0.0 (Xena). + 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/victoria/cli/oslopolicy-convert-json-to-yaml.html + Sahara's public API calls may be restricted to certain sets of users by using a policy configuration file. The location of the policy file(s) is controlled by the ``policy_file`` and ``policy_dirs`` parameters in the ``[oslo_policy]`` section. By default sahara will search for -a ``policy.json`` file in the same directory as the ``sahara.conf`` +a ``policy.yaml`` file in the same directory as the ``sahara.conf`` configuration file. Examples diff --git a/lower-constraints.txt b/lower-constraints.txt index 23d6ecf2dd..a811c86e74 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -67,21 +67,21 @@ os-service-types==1.2.0 osc-lib==1.10.0 oslo.cache==1.29.0 oslo.concurrency==3.26.0 -oslo.config==5.2.0 -oslo.context==2.19.2 +oslo.config==6.8.0 +oslo.context==2.22.0 oslo.db==6.0.0 oslo.i18n==3.15.3 oslo.log==3.36.0 oslo.messaging==10.2.0 oslo.middleware==3.31.0 -oslo.policy==1.30.0 +oslo.policy==3.6.0 oslo.rootwrap==5.8.0 oslo.serialization==2.18.0 oslo.service==1.31.0 -oslo.upgradecheck==0.1.0 -oslo.utils==3.33.0 +oslo.upgradecheck==1.3.0 +oslo.utils==4.5.0 oslotest==3.2.0 -packaging==17.1 +packaging==20.4 paramiko==2.7.1 Paste==2.0.3 PasteDeploy==1.5.2 @@ -117,13 +117,13 @@ python-saharaclient==1.4.0 python-subunit==1.4.0 python-swiftclient==3.2.0 pytz==2018.3 -PyYAML==3.13 +PyYAML==5.1 reno==2.5.0 repoze.lru==0.7 -requests==2.14.2 +requests==2.23.0 requestsexceptions==1.4.0 restructuredtext-lint==1.1.3 -rfc3986==1.1.0 +rfc3986==1.2.0 Routes==2.4.1 simplejson==3.13.2 six==1.14.0 diff --git a/releasenotes/notes/deprecate-json-formatted-policy-file-b267f288cba7e325.yaml b/releasenotes/notes/deprecate-json-formatted-policy-file-b267f288cba7e325.yaml new file mode 100644 index 0000000000..00519ae7be --- /dev/null +++ b/releasenotes/notes/deprecate-json-formatted-policy-file-b267f288cba7e325.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 + `_ + 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 Xena 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 8f10d3a040..769b84d0b6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,22 +15,22 @@ jsonschema>=3.2.0 # MIT keystoneauth1>=3.4.0 # Apache-2.0 keystonemiddleware>=4.17.0 # Apache-2.0 microversion-parse>=0.2.1 # Apache-2.0 -oslo.config>=5.2.0 # Apache-2.0 +oslo.config>=6.8.0 # Apache-2.0 oslo.concurrency>=3.26.0 # Apache-2.0 -oslo.context>=2.19.2 # Apache-2.0 +oslo.context>=2.22.0 # Apache-2.0 oslo.db>=6.0.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.log>=3.36.0 # Apache-2.0 oslo.messaging>=10.2.0 # Apache-2.0 oslo.middleware>=3.31.0 # Apache-2.0 -oslo.policy>=1.30.0 # Apache-2.0 +oslo.policy>=3.6.0 # Apache-2.0 oslo.rootwrap>=5.8.0 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.service>=1.31.0 # Apache-2.0 -oslo.upgradecheck>=0.1.0 # Apache-2.0 -oslo.utils>=3.33.0 # Apache-2.0 +oslo.upgradecheck>=1.3.0 # Apache-2.0 +oslo.utils>=4.5.0 # Apache-2.0 paramiko>=2.7.1 # LGPLv2.1+ -requests>=2.14.2 # Apache-2.0 +requests>=2.23.0 # Apache-2.0 python-cinderclient!=4.0.0,>=3.3.0 # Apache-2.0 python-keystoneclient>=3.8.0 # Apache-2.0 python-manilaclient>=1.16.0 # Apache-2.0 diff --git a/sahara/api/acl.py b/sahara/api/acl.py index f9a8cff97a..0dcdce041b 100644 --- a/sahara/api/acl.py +++ b/sahara/api/acl.py @@ -18,6 +18,7 @@ import functools from oslo_config import cfg +from oslo_policy import opts from oslo_policy import policy from sahara.common import policies @@ -26,6 +27,12 @@ from sahara import exceptions ENFORCER = None +# TODO(gmann): Remove setting the default value of config policy_file +# once oslo_policy change the default value to 'policy.yaml'. +# https://opendev.org/openstack/oslo.policy/src/commit/d8534850d9238e85ae0ea55bf2ac8583681fdb2b/oslo_policy/opts.py#L49 +DEFAULT_POLICY_FILE = 'policy.yaml' +opts.set_defaults(cfg.CONF, DEFAULT_POLICY_FILE) + def setup_policy(): global ENFORCER diff --git a/sahara/cli/sahara_status.py b/sahara/cli/sahara_status.py index ea9cdc3710..d6ebcf7124 100644 --- a/sahara/cli/sahara_status.py +++ b/sahara/cli/sahara_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 sahara.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/sahara/common/config.py b/sahara/common/config.py index 4ab0e3fbc6..0cbd1ad449 100644 --- a/sahara/common/config.py +++ b/sahara/common/config.py @@ -14,12 +14,20 @@ # limitations under the License. from oslo_middleware import cors +from oslo_policy import opts + +from sahara import config def set_config_defaults(): """This method updates all configuration default values.""" set_cors_middleware_defaults() + # TODO(gmann): Remove setting the default value of config policy_file + # once oslo_policy change the default value to 'policy.yaml'. + # https://opendev.org/openstack/oslo.policy/src/commit/d8534850d9238e85ae0ea55bf2ac8583681fdb2b/oslo_policy/opts.py#L49 + opts.set_defaults(config.CONF, 'policy.yaml') + def set_cors_middleware_defaults(): """Update default configuration options for oslo.middleware.""" diff --git a/sahara/tests/unit/api/test_acl.py b/sahara/tests/unit/api/test_acl.py index a7d799c76b..875fe9cd90 100644 --- a/sahara/tests/unit/api/test_acl.py +++ b/sahara/tests/unit/api/test_acl.py @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +from unittest import mock + from oslo_policy import policy as cpolicy from sahara.api import acl @@ -27,7 +29,9 @@ class TestAcl(base.SaharaTestCase): rules = cpolicy.Rules.load_json(json) acl.ENFORCER.set_rules(rules, use_conf=False) - def test_policy_allow(self): + @mock.patch('oslo_config.cfg.ConfigOpts.find_file') + def test_policy_allow(self, mock_config): + mock_config.return_value = '/etc/sahara/' @acl.enforce("data-processing:clusters:get_all") def test(): pass @@ -37,7 +41,9 @@ class TestAcl(base.SaharaTestCase): test() - def test_policy_deny(self): + @mock.patch('oslo_config.cfg.ConfigOpts.find_file') + def test_policy_deny(self, mock_config): + mock_config.return_value = '/etc/sahara/' @acl.enforce("data-processing:clusters:get_all") def test(): pass diff --git a/sahara/tests/unit/cli/test_sahara_cli.py b/sahara/tests/unit/cli/test_sahara_cli.py index 4add2ed318..b7f7a48862 100644 --- a/sahara/tests/unit/cli/test_sahara_cli.py +++ b/sahara/tests/unit/cli/test_sahara_cli.py @@ -52,9 +52,10 @@ class TestSaharaCLI(base.SaharaTestCase): for patcher in reversed(self.patchers): patcher.stop() + @mock.patch('oslo_config.cfg.ConfigOpts.find_file') @mock.patch('sahara.main.setup_sahara_api') - def test_main_start_api(self, mock_setup_sahara_api): - + def test_main_start_api(self, mock_setup_sahara_api, mock_config): + mock_config.return_value = '/etc/sahara/' sahara_api.main() self.mock_start_server.assert_called_once_with() @@ -76,8 +77,9 @@ class TestSaharaCLI(base.SaharaTestCase): mock_pl.launch_service.assert_called_once_with(mock_get_service) mock_pl.wait.assert_called_once_with() - def test_main_start_all(self): - + @mock.patch('oslo_config.cfg.ConfigOpts.find_file') + def test_main_start_all(self, mock_config): + mock_config.return_value = '/etc/sahara/' sahara_all.main() self.mock_start_server.assert_called_once_with() diff --git a/sahara/tests/unit/cli/test_sahara_status.py b/sahara/tests/unit/cli/test_sahara_status.py index 3ac3c179e1..d7f87e4b56 100644 --- a/sahara/tests/unit/cli/test_sahara_status.py +++ b/sahara/tests/unit/cli/test_sahara_status.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +from unittest import mock + from oslo_upgradecheck.upgradecheck import Code from sahara.cli import sahara_status @@ -24,7 +26,15 @@ class TestUpgradeChecks(base.SaharaTestCase): super(TestUpgradeChecks, self).setUp() self.cmd = sahara_status.Checks() - def test__sample_check(self): - check_result = self.cmd._sample_check() - self.assertEqual( - Code.SUCCESS, check_result.code) + @mock.patch('oslo_config.cfg.ConfigOpts.find_file') + @mock.patch('oslo_utils.fileutils.is_json') + def test_checks(self, mock_util, mock_config): + mock_config.return_value = '/etc/sahara/' + mock_util.return_value = False + 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/setup.cfg b/setup.cfg index 36d7ed8738..952c129d25 100644 --- a/setup.cfg +++ b/setup.cfg @@ -74,7 +74,7 @@ oslo.config.opts = sahara.config = sahara.config:list_opts oslo.config.opts.defaults = - sahara.config = sahara.common.config:set_cors_middleware_defaults + sahara.config = sahara.common.config:set_config_defaults oslo.policy.policies = sahara = sahara.common.policies:list_rules