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

lower-constraints.txt

@@ -21,7 +21,7 @@ msgpack-python==0.4.0
 netaddr==0.7.18
 netifaces==0.10.4
 os-client-config==1.28.0
-oslo.config==5.2.0
+oslo.config==6.0.0
 oslo.context==2.22.0
 oslo.i18n==3.15.3
 oslo.serialization==2.18.0

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

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')

requirements.txt

@@ -3,7 +3,7 @@
 # process, which may cause wedges in the gate later.
 
 requests>=2.14.2 # Apache-2.0
-oslo.config>=5.2.0 # Apache-2.0
+oslo.config>=6.0.0 # Apache-2.0
 oslo.context>=2.22.0 # Apache-2.0
 oslo.i18n>=3.15.3 # Apache-2.0
 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0