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

4 files changed, 110 insertions, 4 deletions

diff --git a/lower-constraints.txt b/lower-constraints.txt
index 347aa71..c83f09b 100644
--- a/lower-constraints.txt
+++ b/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
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')
diff --git a/requirements.txt b/requirements.txt
index b0c5d09..f2f2f85 100644
--- a/requirements.txt
+++ b/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