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

diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py
index bfd6dcf9db7473e58ce360c091875120e606b2bc..4bdad18b04e7b4dfd150e28941ad480105fb43db 100644 (file)
--- 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 fbd43fd0f6bd8d394008128b637733e13fdc0dc4..e548d3d7ef821f454eb8ff372fe4c23df1ebfffc 100644 (file)
--- 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 81fe7dfad563d0ea2b5e77f7b0947a79b0a35703..7c4997630fec2e3a5e209f46fa2e69a0af494db5 100644 (file)
--- 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)