From 1d41ba485f060a4d6dbe0767845391f955247b08 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Fri, 4 Apr 2014 03:09:57 -0700 Subject: [PATCH] Call policy.init() once per API request This patch changes the policy engine behaviour and the API base controller in order to ensure policy.init is invoked only once for each API request. This will avoid issues arising from policy file updates during API processing and speed up response generation for list operations, by about 5%. This patch also removes an obsolete TODO comment. Change-Id: I108ebd26fccdea19cb00959f70d87c3bc1587df9 Closes-Bug: 1302611 --- neutron/api/v2/base.py | 15 +++++++++++---- neutron/policy.py | 3 --- neutron/tests/unit/test_policy.py | 3 +++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index bfd6dcf9db..4bdad18b04 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -175,6 +175,8 @@ class Controller(object): if name in self._member_actions: def _handle_action(request, id, **kwargs): arg_list = [request.context, id] + # Ensure policy engine is initialized + policy.init() # Fetch the resource and verify if the user can access it try: resource = self._item(request, id, True) @@ -185,9 +187,6 @@ class Controller(object): # Explicit comparison with None to distinguish from {} if body is not None: arg_list.append(body) - # TODO(salvatore-orlando): bp/make-authz-ortogonal - # The body of the action request should be included - # in the info passed to the policy engine # It is ok to raise a 403 because accessibility to the # object was checked earlier in this method policy.enforce(request.context, name, resource) @@ -253,7 +252,6 @@ class Controller(object): pagination_links = pagination_helper.get_links(obj_list) if pagination_links: collection[self._collection + "_links"] = pagination_links - return collection def _item(self, request, id, do_authz=False, field_list=None, @@ -284,6 +282,8 @@ class Controller(object): def index(self, request, **kwargs): """Returns a list of the requested entity.""" parent_id = kwargs.get(self._parent_id_name) + # Ensure policy engine is initialized + policy.init() return self._items(request, True, parent_id) def show(self, request, id, **kwargs): @@ -295,6 +295,8 @@ class Controller(object): field_list, added_fields = self._do_field_list( api_common.list_args(request, "fields")) parent_id = kwargs.get(self._parent_id_name) + # Ensure policy engine is initialized + policy.init() return {self._resource: self._view(request.context, self._item(request, @@ -363,6 +365,8 @@ class Controller(object): else: items = [body] bulk = False + # Ensure policy engine is initialized + policy.init() for item in items: self._validate_network_tenant_ownership(request, item[self._resource]) @@ -433,6 +437,7 @@ class Controller(object): action = self._plugin_handlers[self.DELETE] # Check authz + policy.init() parent_id = kwargs.get(self._parent_id_name) obj = self._item(request, id, parent_id=parent_id) try: @@ -484,6 +489,8 @@ class Controller(object): if (value.get('required_by_policy') or value.get('primary_key') or 'default' not in value)] + # Ensure policy engine is initialized + policy.init() orig_obj = self._item(request, id, field_list=field_list, parent_id=parent_id) orig_object_copy = copy.copy(orig_obj) diff --git a/neutron/policy.py b/neutron/policy.py index fbd43fd0f6..e548d3d7ef 100644 --- a/neutron/policy.py +++ b/neutron/policy.py @@ -164,7 +164,6 @@ def _build_match_rule(action, target): action is being executed (e.g.: create_router:external_gateway_info:network_id) """ - match_rule = policy.RuleCheck('rule', action) resource, is_write = get_resource_and_action(action) # Attribute-based checks shall not be enforced on GETs @@ -317,7 +316,6 @@ class FieldCheck(policy.Check): def _prepare_check(context, action, target): """Prepare rule, target, and credentials for the policy engine.""" - init() # Compare with None to distinguish case in which target is {} if target is None: target = {} @@ -374,7 +372,6 @@ def enforce(context, action, target, plugin=None): :raises neutron.exceptions.PolicyNotAuthorized: if verification fails. """ - init() rule, target, credentials = _prepare_check(context, action, target) result = policy.check(rule, target, credentials, action=action) if not result: diff --git a/neutron/tests/unit/test_policy.py b/neutron/tests/unit/test_policy.py index 81fe7dfad5..7c4997630f 100644 --- a/neutron/tests/unit/test_policy.py +++ b/neutron/tests/unit/test_policy.py @@ -53,12 +53,14 @@ class PolicyFileTestCase(base.BaseTestCase): action = "example:test" with open(tmpfilename, "w") as policyfile: policyfile.write("""{"example:test": ""}""") + policy.init() policy.enforce(self.context, action, self.target) with open(tmpfilename, "w") as policyfile: policyfile.write("""{"example:test": "!"}""") # NOTE(vish): reset stored policy cache so we don't have to # sleep(1) policy._POLICY_CACHE = {} + policy.init() self.assertRaises(exceptions.PolicyNotAuthorized, policy.enforce, self.context, @@ -471,6 +473,7 @@ class NeutronPolicyTestCase(base.BaseTestCase): # Trigger a policy with rule admin_or_owner action = "create_network" target = {'tenant_id': 'fake'} + policy.init() self.assertRaises(exceptions.PolicyCheckError, policy.enforce, self.context, action, target)