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
This commit is contained in:
parent
f1de09d60b
commit
1d41ba485f
@ -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)
|
||||
|
@ -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:
|
||||
|
@ -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)
|
||||
|
Loading…
Reference in New Issue
Block a user