From 5180e9674f30a44a530deeabbc02804bec66351d Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Wed, 18 Nov 2020 14:37:47 -0600 Subject: Add policy file selection logic when default changing to yaml As part of community goal[1], each services are changing the default value of 'CONF.oslo_policy.policy_file' config option from 'policy.json' to 'policy.yaml'. oslo policy select the default value from CONF.oslo_policy.policy_file which will be policy.yaml as service will start changing the default. To avoid breaking the existing deployment which are relying on old default (policy.json) file, a new fallback logic is implemented. If new default file 'policy.yaml' does not exist but old default 'policy.json' exist then fallback to use old default file. Each services are going to add upgrade checks and warnings for using JSON formatted policy file so in future we cna remove this fallback logic. This logic was done in nova in Victoria cycle when nova changed the default value - https://review.opendev.org/#/c/748059/ . Moving this to oslo policy side will avoid the duplication on services side. Also it provides a flag to disable this fallback. [1] https://governance.openstack.org/tc/goals/selected/wallaby/migrate-policy-format-from-json-to-yaml.html Change-Id: If72b2fcc3cfd8116b575ed7b9e3870df634fd9af --- oslo_policy/policy.py | 29 +++++++++++++- oslo_policy/tests/test_policy.py | 81 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) (limited to 'oslo_policy') diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index 9495b0e..f21ebe9 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -360,6 +360,29 @@ class InvalidContextObject(Exception): super(InvalidContextObject, self).__init__(msg) +def pick_default_policy_file(conf, fallback_to_json_file=True): + # TODO(gmann): If service changed the default value of + # CONF.oslo_policy.policy_file option to 'policy.yaml' then 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 once + # oslo_policy stop supporting the JSON formatted policy file. + + new_default_policy_file = 'policy.yaml' + old_default_policy_file = 'policy.json' + if ((conf.oslo_policy.policy_file == new_default_policy_file) and + fallback_to_json_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]: + if conf.find_file(old_default_policy_file): + return old_default_policy_file + # Return overridden policy file + return conf.oslo_policy.policy_file + + def parse_file_contents(data): """Parse the raw contents of a policy file. @@ -494,7 +517,8 @@ class Enforcer(object): """ def __init__(self, conf, policy_file=None, rules=None, - default_rule=None, use_conf=True, overwrite=True): + default_rule=None, use_conf=True, overwrite=True, + fallback_to_json_file=True): self.conf = conf opts._register(conf) @@ -506,7 +530,8 @@ class Enforcer(object): self.policy_path = None - self.policy_file = policy_file or self.conf.oslo_policy.policy_file + self.policy_file = policy_file or pick_default_policy_file( + self.conf, fallback_to_json_file=fallback_to_json_file) self.use_conf = use_conf self._need_check_rule = True self.overwrite = overwrite diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index d9f6fa0..f27f6b1 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -19,6 +19,7 @@ import os from unittest import mock import yaml +import fixtures from oslo_config import cfg from oslo_context import context from oslo_serialization import jsonutils @@ -1898,3 +1899,83 @@ class EnforcerCheckRulesTest(base.PolicyBaseTestCase): self.enforcer.load_rules(True) self.assertTrue(self.enforcer.check_rules()) + + +class PickPolicyFileTestCase(base.PolicyBaseTestCase): + + def setUp(self): + super(PickPolicyFileTestCase, self).setUp() + self.data = { + 'rule_admin': 'True', + 'rule_admin2': 'is_admin:True' + } + 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) + + mock_search_dir = self.useFixture( + fixtures.MockPatch('oslo_config.cfg._search_dirs')).mock + mock_search_dir.side_effect = fake_search_dirs + + mock_cfg_location = self.useFixture( + fixtures.MockPatchObject(self.conf, 'get_location')).mock + mock_cfg_location.return_value = cfg.LocationInfo( + cfg.Locations.set_default, 'None') + + def test_no_fallback_to_json_file(self): + tmpfilename = 'policy.yaml' + self.conf.set_override('policy_file', tmpfilename, group='oslo_policy') + jsonfile = os.path.join(self.tmpdir.path, 'policy.json') + with open(jsonfile, 'w') as fh: + jsonutils.dump(self.data, fh) + + selected_policy_file = policy.pick_default_policy_file( + self.conf, fallback_to_json_file=False) + self.assertEqual(self.conf.oslo_policy.policy_file, tmpfilename) + self.assertEqual(selected_policy_file, tmpfilename) + + def test_overridden_policy_file(self): + tmpfilename = 'nova-policy.yaml' + self.conf.set_override('policy_file', tmpfilename, group='oslo_policy') + selected_policy_file = policy.pick_default_policy_file(self.conf) + self.assertEqual(self.conf.oslo_policy.policy_file, tmpfilename) + self.assertEqual(selected_policy_file, tmpfilename) + + def test_only_new_default_policy_file_exist(self): + self.conf.set_override('policy_file', 'policy.yaml', + group='oslo_policy') + 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_default_policy_file(self.conf) + self.assertEqual(self.conf.oslo_policy.policy_file, 'policy.yaml') + self.assertEqual(selected_policy_file, 'policy.yaml') + + def test_only_old_default_policy_file_exist(self): + self.conf.set_override('policy_file', 'policy.yaml', + group='oslo_policy') + 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_default_policy_file(self.conf) + self.assertEqual(self.conf.oslo_policy.policy_file, 'policy.yaml') + self.assertEqual(selected_policy_file, 'policy.json') + + def test_both_default_policy_file_exist(self): + self.conf.set_override('policy_file', 'policy.yaml', + group='oslo_policy') + 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_default_policy_file(self.conf) + self.assertEqual(self.conf.oslo_policy.policy_file, 'policy.yaml') + self.assertEqual(selected_policy_file, 'policy.yaml') -- cgit v1.2.1