From 1902e2adf68e983ba96e088e767161424684813a Mon Sep 17 00:00:00 2001 From: Armando Migliaccio Date: Wed, 11 Jan 2017 18:31:15 -0800 Subject: [PATCH] Revisit exported policy module The primary goal of the library policy module is to support the context module during the enforcement of service and admin rules, and as such an enforcer is needed. Incidentally the enforcer is stored in a global variable whose name is the same as the enforcer's used by neutron's policy engine. To avoid confusion, this patch revises some parts of the library's policy module to make sure the cut of responsibilities is better defined. It finally makes the policy module private to avoid any danger of mixing up the enforcer instances. Change-Id: Ie55d557aa3e24678aed2fb3b5c590485f54fe792 --- neutron_lib/{policy.py => _policy.py} | 42 ++++--------------- neutron_lib/context.py | 2 +- .../unit/{test_policy.py => test__policy.py} | 16 +++---- .../notes/policy-redux-25c26836219fd02d.yaml | 7 ++++ 4 files changed, 21 insertions(+), 46 deletions(-) rename neutron_lib/{policy.py => _policy.py} (66%) rename neutron_lib/tests/unit/{test_policy.py => test__policy.py} (84%) create mode 100644 releasenotes/notes/policy-redux-25c26836219fd02d.yaml diff --git a/neutron_lib/policy.py b/neutron_lib/_policy.py similarity index 66% rename from neutron_lib/policy.py rename to neutron_lib/_policy.py index 379afa2a2..1bd16cc12 100644 --- a/neutron_lib/policy.py +++ b/neutron_lib/_policy.py @@ -14,26 +14,11 @@ from oslo_config import cfg from oslo_policy import policy -_ENFORCER = None +_ROLE_ENFORCER = None _ADMIN_CTX_POLICY = 'context_is_admin' _ADVSVC_CTX_POLICY = 'context_is_advsvc' -def reset(): - """Reset the global enforcer. - - Resets the global enforcer thereby deleting any rules and state associated - with it. Subsequent calls to this modules API will trigger a - re-initialization of the global enforcer as necessary. - - :returns: None. - """ - global _ENFORCER - if _ENFORCER: - _ENFORCER.clear() - _ENFORCER = None - - def init(conf=cfg.CONF, policy_file=None): """Initialize the global enforcer if not already initialized. @@ -47,32 +32,19 @@ def init(conf=cfg.CONF, policy_file=None): :returns: None. """ - global _ENFORCER - if not _ENFORCER: - _ENFORCER = policy.Enforcer(conf, policy_file=policy_file) - _ENFORCER.load_rules(True) - - -def refresh(policy_file=None): - """Reset the global enforcer and re-initialize it. - - Reset the global policy and re-initialize it optionally using the said - policy file. - - :param policy_file: The policy file to initialize the global enforcer with. - :returns: None. - """ - reset() - init(policy_file=policy_file) + global _ROLE_ENFORCER + if not _ROLE_ENFORCER: + _ROLE_ENFORCER = policy.Enforcer(conf, policy_file=policy_file) + _ROLE_ENFORCER.load_rules(True) def _check_rule(context, rule): init() # the target is user-self credentials = context.to_policy_values() - if rule not in _ENFORCER.rules: + if rule not in _ROLE_ENFORCER.rules: return False - return _ENFORCER.enforce(rule, credentials, credentials) + return _ROLE_ENFORCER.enforce(rule, credentials, credentials) def check_is_admin(context): diff --git a/neutron_lib/context.py b/neutron_lib/context.py index d955ee6aa..14ce5a05f 100644 --- a/neutron_lib/context.py +++ b/neutron_lib/context.py @@ -19,8 +19,8 @@ from oslo_context import context as oslo_context from oslo_db.sqlalchemy import enginefacade # TODO(HenryG): replace db/_api.py with the real db/api.py +from neutron_lib import _policy as policy from neutron_lib.db import _api as db_api -from neutron_lib import policy class ContextBase(oslo_context.RequestContext): diff --git a/neutron_lib/tests/unit/test_policy.py b/neutron_lib/tests/unit/test__policy.py similarity index 84% rename from neutron_lib/tests/unit/test_policy.py rename to neutron_lib/tests/unit/test__policy.py index 99e9e6732..4c49f910f 100644 --- a/neutron_lib/tests/unit/test_policy.py +++ b/neutron_lib/tests/unit/test__policy.py @@ -13,8 +13,8 @@ import mock +from neutron_lib import _policy as policy from neutron_lib import context -from neutron_lib import policy from neutron_lib.tests import _base as base @@ -22,17 +22,13 @@ from neutron_lib.tests import _base as base class TestPolicyEnforcer(base.BaseTestCase): def setUp(self): super(TestPolicyEnforcer, self).setUp() - # Isolate one _ENFORCER per test case - mock.patch.object(policy, '_ENFORCER', None).start() + # Isolate one _ROLE_ENFORCER per test case + mock.patch.object(policy, '_ROLE_ENFORCER', None).start() - def test_init_reset_refresh(self): - self.assertIsNone(policy._ENFORCER) + def test_init_reset(self): + self.assertIsNone(policy._ROLE_ENFORCER) policy.init() - self.assertIsNotNone(policy._ENFORCER) - policy.reset() - self.assertIsNone(policy._ENFORCER) - policy.refresh() - self.assertIsNotNone(policy._ENFORCER) + self.assertIsNotNone(policy._ROLE_ENFORCER) def test_check_user_is_not_admin(self): ctx = context.Context('me', 'my_project') diff --git a/releasenotes/notes/policy-redux-25c26836219fd02d.yaml b/releasenotes/notes/policy-redux-25c26836219fd02d.yaml new file mode 100644 index 000000000..e3668f82d --- /dev/null +++ b/releasenotes/notes/policy-redux-25c26836219fd02d.yaml @@ -0,0 +1,7 @@ +--- +deprecations: + - | + ``policy.refresh()`` and ``policy.reset()`` have been removed. The library + policy module is not meant for public consumption, and it should be + considered in practice a private component of the library. If you use it, + you will do so at your own risk, as it has been marked as a private module.