diff options
| -rw-r--r-- | doc/source/admin/policy-yaml-file.rst | 14 | ||||
| -rw-r--r-- | oslo_policy/_cache_handler.py | 29 | ||||
| -rw-r--r-- | oslo_policy/tests/test_cache_handler.py | 94 | ||||
| -rw-r--r-- | oslo_policy/tests/test_policy.py | 6 |
4 files changed, 139 insertions, 4 deletions
diff --git a/doc/source/admin/policy-yaml-file.rst b/doc/source/admin/policy-yaml-file.rst index 5757dc7..9d9b85a 100644 --- a/doc/source/admin/policy-yaml-file.rst +++ b/doc/source/admin/policy-yaml-file.rst @@ -239,7 +239,19 @@ Target object attributes are fields from the object description in the database. For example in the case of the ``"compute:start"`` API, the object is the instance to be started. The policy for starting instances could use the ``%(project_id)s`` attribute, that is the project that -owns the instance. The trailing ``s`` indicates this is a string. +owns the instance. The trailing ``s`` indicates this is a string. The same +case would be valid for API attributes like ``%(user_id)s`` and +``%(domain_id)s``. + +During a debug logging phase, it's common to have the target object +attributes retrieved in the API calls. Comparing the API call on the logs +with the policy enforced for the corresponding API, you can check which API +attribute has been used as the target object. For example in the policy.yaml +for the Nova project you can find ``"compute:start"`` API, the policy will show as +``"rule:admin_or_owner"`` which will point for +``"admin_or_owner": "is_admin:True or project_id:%(project_id)s"`` and in this +way you can check that the target object in the debug logging it needs to be a +``project_id`` attribute. ``is_admin`` indicates that administrative privileges are granted via the admin token mechanism (the ``--os-token`` option of the ``keystone`` diff --git a/oslo_policy/_cache_handler.py b/oslo_policy/_cache_handler.py index ab90684..fd34915 100644 --- a/oslo_policy/_cache_handler.py +++ b/oslo_policy/_cache_handler.py @@ -13,9 +13,12 @@ # License for the specific language governing permissions and limitations # under the License. +import errno import logging import os +from oslo_config import cfg + LOG = logging.getLogger(__name__) @@ -33,13 +36,33 @@ def read_cached_file(cache, filename, force_reload=False): delete_cached_file(cache, filename) reloaded = False - mtime = os.path.getmtime(filename) + try: + mtime = os.path.getmtime(filename) + except OSError as err: + msg = err.strerror + LOG.error('Config file not found %(filename)s: %(msg)s', + {'filename': filename, 'msg': msg}) + return True, {} + cache_info = cache.setdefault(filename, {}) if not cache_info or mtime > cache_info.get('mtime', 0): LOG.debug("Reloading cached file %s", filename) - with open(filename) as fap: - cache_info['data'] = fap.read() + try: + with open(filename) as fap: + cache_info['data'] = fap.read() + except IOError as err: + msg = err.strerror + err_code = err.errno + LOG.error('IO error loading %(filename)s: %(msg)s', + {'filename': filename, 'msg': msg}) + if err_code == errno.EACCES: + raise cfg.ConfigFilesPermissionDeniedError((filename,)) + except OSError as err: + msg = err.strerror + LOG.error('Config file not found %(filename)s: %(msg)s', + {'filename': filename, 'msg': msg}) + raise cfg.ConfigFilesNotFoundError((filename,)) cache_info['mtime'] = mtime reloaded = True return (reloaded, cache_info['data']) diff --git a/oslo_policy/tests/test_cache_handler.py b/oslo_policy/tests/test_cache_handler.py new file mode 100644 index 0000000..3251009 --- /dev/null +++ b/oslo_policy/tests/test_cache_handler.py @@ -0,0 +1,94 @@ +# Copyright (c) 2020 OpenStack Foundation. +# All Rights Reserved. + +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Test the cache handler module""" + +import os +from unittest import mock + +import fixtures +import oslo_config +from oslotest import base as test_base + +from oslo_policy import _cache_handler as _ch + + +class CacheHandlerTest(test_base.BaseTestCase): + + def setUp(self): + super().setUp() + self.tmpdir = self.useFixture(fixtures.TempDir()) + + def test_read_cached_file(self): + file_cache = {} + + path = os.path.join(self.tmpdir.path, 'tmpfile') + with open(path, 'w+') as fp: + fp.write('test') + + reloaded, data = _ch.read_cached_file(file_cache, path) + self.assertEqual('test', data) + self.assertTrue(reloaded) + + reloaded, data = _ch.read_cached_file(file_cache, path) + self.assertEqual('test', data) + self.assertFalse(reloaded) + + reloaded, data = _ch.read_cached_file( + file_cache, path, force_reload=True) + self.assertEqual('test', data) + self.assertTrue(reloaded) + + def test_read_cached_file_with_updates(self): + file_cache = {} + + path = os.path.join(self.tmpdir.path, 'tmpfile') + with open(path, 'w+') as fp: + fp.write('test') + + reloaded, data = _ch.read_cached_file(file_cache, path) + + # update the timestamps + times = (os.stat(path).st_atime + 1, os.stat(path).st_mtime + 1) + os.utime(path, times) + + reloaded, data = _ch.read_cached_file(file_cache, path) + self.assertTrue(reloaded) + + @mock.patch.object(_ch, 'LOG') + def test_reloading_cache_with_permission_denied(self, mock_log): + file_cache = {} + + path = os.path.join(self.tmpdir.path, 'tmpfile') + with open(path, 'w+') as fp: + fp.write('test') + + os.chmod(path, 000) + self.assertRaises( + oslo_config.cfg.ConfigFilesPermissionDeniedError, + _ch.read_cached_file, file_cache, path) + mock_log.error.assert_called_once() + + @mock.patch.object(_ch, 'LOG') + def test_reloading_on_removed_file(self, mock_log): + file_cache = {} + + # don't actually create the file + path = os.path.join(self.tmpdir.path, 'tmpfile') + + reloaded, data = _ch.read_cached_file(file_cache, path) + self.assertEqual({}, data) + self.assertTrue(reloaded) + mock_log.error.assert_called_once() diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index 0a8bab5..d5c86c3 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -1802,6 +1802,12 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): self.assertEqual( str(enforcer.rules['foo:create_bar']), str(expected_check)) self.assertEqual(check, rule.check) + # Hacky way to check whether _handle_deprecated_rule was called again. + # If a second call to load_rules doesn't overwrite our dummy rule then + # we know it didn't call the deprecated rule function again. + enforcer.rules['foo:create_bar'] = 'foo:bar' + enforcer.load_rules() + self.assertEqual('foo:bar', enforcer.rules['foo:create_bar']) class DocumentedRuleDefaultTestCase(base.PolicyBaseTestCase): |