From 4c8f38cd1d088c46be314b47f6774e721813c6d9 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Tue, 31 Mar 2015 18:17:22 -0400 Subject: Avoid reloading policy files in policy.d for every call We added policy files in policy.d, currently, the code will reload the policy in policy.d every enforce been called. This patch caches the file mtime of the most newest file in the directory (or the directory itself if it is empty) and uses that to detect if we need to reload files in the policy.d directory. Change-Id: I3ab1ce1f2132ea8672bf0765ee4b30ad126d4559 Closes-Bug: 1437992 Co-Authored-By: Eli Qiao Cherry-pick: Ia8c273c8566a4b472cb8807b1576ef1dee8ef054 --- oslo_policy/policy.py | 28 ++++++++++++++++++--- oslo_policy/tests/test_policy.py | 54 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index 6b7a8ca..943a396 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -331,6 +331,7 @@ class Enforcer(object): self.use_conf = use_conf self.overwrite = overwrite self._loaded_files = [] + self._policy_dir_mtimes = {} def set_rules(self, rules, overwrite=True, use_conf=False): """Create a new :class:`Rules` based on the provided dict of rules. @@ -357,6 +358,7 @@ class Enforcer(object): self.default_rule = None self.policy_path = None self._loaded_files = [] + self._policy_dir_mtimes = {} def load_rules(self, force_reload=False): """Loads policy_path's rules. @@ -380,9 +382,29 @@ class Enforcer(object): path = self._get_policy_path(path) except cfg.ConfigFilesNotFoundError: continue - self._walk_through_policy_directory(path, - self._load_policy_file, - force_reload, False) + if (force_reload or self._is_directory_updated( + self._policy_dir_mtimes, path)): + self._walk_through_policy_directory(path, + self._load_policy_file, + force_reload, False) + + @staticmethod + def _is_directory_updated(cache, path): + # Get the current modified time and compare it to what is in + # the cache and check if the new mtime is greater than what + # is in the cache + mtime = 0 + if os.path.exists(path): + # Make a list of all the files + files = [path] + [os.path.join(path, file) for file in + os.listdir(path)] + # Pick the newest one, let's use its time. + mtime = os.path.getmtime(max(files, key=os.path.getmtime)) + cache_info = cache.setdefault(path, {}) + if mtime > cache_info.get('mtime', 0): + cache_info['mtime'] = mtime + return True + return False @staticmethod def _walk_through_policy_directory(path, func, *args): diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index f1f7ddb..85cbfc9 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -15,10 +15,13 @@ """Test of Policy Engine""" +import os + import mock from oslo_config import cfg from oslo_serialization import jsonutils from oslotest import base as test_base +import six from oslo_policy import _checks from oslo_policy import _parser @@ -172,6 +175,56 @@ class EnforcerTest(base.PolicyBaseTestCase): 'policy.d/b.conf', ]) + def test_load_directory_caching_with_files_updated(self): + self.create_config_file('policy.d/a.conf', POLICY_A_CONTENTS) + + self.enforcer.load_rules(False) + self.assertIsNotNone(self.enforcer.rules) + + old = six.next(six.itervalues( + self.enforcer._policy_dir_mtimes)) + self.assertEqual(1, len(self.enforcer._policy_dir_mtimes)) + + # Touch the file + conf_path = os.path.join(self.config_dir, 'policy.d/a.conf') + stinfo = os.stat(conf_path) + os.utime(conf_path, (stinfo.st_atime + 10, stinfo.st_mtime + 10)) + + self.enforcer.load_rules(False) + self.assertEqual(1, len(self.enforcer._policy_dir_mtimes)) + self.assertEqual(old, six.next(six.itervalues( + self.enforcer._policy_dir_mtimes))) + + loaded_rules = jsonutils.loads(str(self.enforcer.rules)) + self.assertEqual('is_admin:True', loaded_rules['admin']) + self.check_loaded_files([ + 'policy.json', + 'policy.d/a.conf', + 'policy.d/a.conf', + ]) + + def test_load_directory_caching_with_files_same(self): + self.create_config_file('policy.d/a.conf', POLICY_A_CONTENTS) + + self.enforcer.load_rules(False) + self.assertIsNotNone(self.enforcer.rules) + + old = six.next(six.itervalues( + self.enforcer._policy_dir_mtimes)) + self.assertEqual(1, len(self.enforcer._policy_dir_mtimes)) + + self.enforcer.load_rules(False) + self.assertEqual(1, len(self.enforcer._policy_dir_mtimes)) + self.assertEqual(old, six.next(six.itervalues( + self.enforcer._policy_dir_mtimes))) + + loaded_rules = jsonutils.loads(str(self.enforcer.rules)) + self.assertEqual('is_admin:True', loaded_rules['admin']) + self.check_loaded_files([ + 'policy.json', + 'policy.d/a.conf', + ]) + def test_load_multiple_directories(self): self.create_config_file('policy.d/a.conf', POLICY_A_CONTENTS) self.create_config_file('policy.d/b.conf', POLICY_B_CONTENTS) @@ -285,6 +338,7 @@ class EnforcerTest(base.PolicyBaseTestCase): use_conf=True) self.enforcer.overwrite = False + self.enforcer._is_directory_updated = lambda x, y: True # Call enforce(), it will load rules from # policy configuration files, to merge with -- cgit v1.2.1