From 093bd96b9f90eaf9f099993f939ea71cda63d32b Mon Sep 17 00:00:00 2001 From: Brandon Logan Date: Wed, 25 May 2016 21:52:40 -0500 Subject: [PATCH] Pecan: bind attribute map to controllers The legacy controllers used their own references to the resource attribute info while pecan was looking for it in the attributes.RESOURCE_ATTRIBUTE_MAP. This aligns pecan controllers with that of the legacy controllers, in that the pecan controllers now store the their own resource attribute info. This also fixes a bug that was unnoticed until the above change was made. The ItemController's index method was not passing the *args and **kwargs to the get method. Change-Id: I1cc85daecfb19f73092e52678fd19ff1a2912195 --- neutron/api/v2/base.py | 4 ++++ neutron/manager.py | 1 + neutron/pecan_wsgi/controllers/resource.py | 13 ++++++++----- neutron/pecan_wsgi/controllers/utils.py | 16 +++++++++++----- neutron/pecan_wsgi/hooks/body_validation.py | 6 ++++-- neutron/pecan_wsgi/hooks/policy_enforcement.py | 14 ++++++++------ neutron/pecan_wsgi/startup.py | 3 ++- .../tests/functional/pecan_wsgi/test_hooks.py | 4 ++-- 8 files changed, 40 insertions(+), 21 deletions(-) diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index b1359b2930f..093a45bec98 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -68,6 +68,10 @@ class Controller(object): def resource(self): return self._resource + @property + def attr_info(self): + return self._attr_info + def __init__(self, plugin, collection, resource, attr_info, allow_bulk=False, member_actions=None, parent=None, allow_pagination=False, allow_sorting=False): diff --git a/neutron/manager.py b/neutron/manager.py index a9242ec43fa..45e656c9765 100644 --- a/neutron/manager.py +++ b/neutron/manager.py @@ -269,6 +269,7 @@ class NeutronManager(object): @classmethod def get_controller_for_resource(cls, resource): + resource = resource.replace('_', '-') res_ctrl_mappings = cls.get_instance().resource_controller_mappings # If no controller is found for resource, try replacing dashes with # underscores diff --git a/neutron/pecan_wsgi/controllers/resource.py b/neutron/pecan_wsgi/controllers/resource.py index 31492ca084c..20b287bd5c6 100644 --- a/neutron/pecan_wsgi/controllers/resource.py +++ b/neutron/pecan_wsgi/controllers/resource.py @@ -27,13 +27,14 @@ LOG = logging.getLogger(__name__) class ItemController(utils.NeutronPecanController): - def __init__(self, resource, item): - super(ItemController, self).__init__(None, resource) + def __init__(self, resource, item, plugin=None, resource_info=None): + super(ItemController, self).__init__(None, resource, plugin=plugin, + resource_info=resource_info) self.item = item @utils.expose(generic=True) def index(self, *args, **kwargs): - return self.get() + return self.get(*args, **kwargs) def get(self, *args, **kwargs): getter = getattr(self.plugin, 'get_%s' % self.resource) @@ -89,7 +90,9 @@ class CollectionsController(utils.NeutronPecanController): request.context['resource_id'] = item uri_identifier = '%s_id' % self.resource request.context['uri_identifiers'][uri_identifier] = item - return self.item_controller_class(self.resource, item), remainder + return (self.item_controller_class( + self.resource, item, resource_info=self.resource_info), + remainder) @utils.expose(generic=True) def index(self, *args, **kwargs): @@ -101,7 +104,7 @@ class CollectionsController(utils.NeutronPecanController): _listify = lambda x: x if isinstance(x, list) else [x] filters = api_common.get_filters_from_dict( {k: _listify(v) for k, v in kwargs.items()}, - self._resource_info, + self.resource_info, skips=['fields', 'sort_key', 'sort_dir', 'limit', 'marker', 'page_reverse']) lister = getattr(self.plugin, 'get_%s' % self.collection) diff --git a/neutron/pecan_wsgi/controllers/utils.py b/neutron/pecan_wsgi/controllers/utils.py index 6b1bb83c9c5..70b690f8c14 100644 --- a/neutron/pecan_wsgi/controllers/utils.py +++ b/neutron/pecan_wsgi/controllers/utils.py @@ -87,18 +87,17 @@ def when(index, *args, **kwargs): class NeutronPecanController(object): - def __init__(self, collection, resource, plugin=None): + def __init__(self, collection, resource, plugin=None, resource_info=None): # Ensure dashes are always replaced with underscores self.collection = collection and collection.replace('-', '_') self.resource = resource and resource.replace('-', '_') - self._resource_info = api_attributes.get_collection_info( - self.collection) + self._resource_info = resource_info self._plugin = plugin # Controllers for some resources that are not mapped to anything in # RESOURCE_ATTRIBUTE_MAP will not have anything in _resource_info - if self._resource_info: + if self.resource_info: self._mandatory_fields = set([field for (field, data) in - self._resource_info.items() if + self.resource_info.items() if data.get('required_by_policy')]) else: self._mandatory_fields = set() @@ -113,6 +112,13 @@ class NeutronPecanController(object): self.resource) return self._plugin + @property + def resource_info(self): + if not self._resource_info: + self._resource_info = api_attributes.get_collection_info( + self.collection) + return self._resource_info + class ShimRequest(object): diff --git a/neutron/pecan_wsgi/hooks/body_validation.py b/neutron/pecan_wsgi/hooks/body_validation.py index 89e6f045253..1b50abb63e4 100644 --- a/neutron/pecan_wsgi/hooks/body_validation.py +++ b/neutron/pecan_wsgi/hooks/body_validation.py @@ -17,8 +17,8 @@ from oslo_log import log from oslo_serialization import jsonutils from pecan import hooks -from neutron.api.v2 import attributes as v2_attributes from neutron.api.v2 import base as v2_base +from neutron import manager LOG = log.getLogger(__name__) @@ -51,12 +51,14 @@ class BodyValidationHook(hooks.PecanHook): # member action is being processed or on agent scheduler operations return # Prepare data to be passed to the plugin from request body + controller = manager.NeutronManager.get_controller_for_resource( + collection) data = v2_base.Controller.prepare_request_body( neutron_context, json_data, is_create, resource, - v2_attributes.get_collection_info(collection), + controller.resource_info, allow_bulk=is_create) if collection in data: state.request.context['resources'] = [item[resource] for item in diff --git a/neutron/pecan_wsgi/hooks/policy_enforcement.py b/neutron/pecan_wsgi/hooks/policy_enforcement.py index b30ab5b3eda..72a9668b039 100644 --- a/neutron/pecan_wsgi/hooks/policy_enforcement.py +++ b/neutron/pecan_wsgi/hooks/policy_enforcement.py @@ -21,7 +21,6 @@ from pecan import hooks import webob from neutron._i18n import _ -from neutron.api.v2 import attributes as v2_attributes from neutron.common import constants as const from neutron.extensions import quotasv2 from neutron import manager @@ -36,8 +35,10 @@ def _custom_getter(resource, resource_id): return quota.get_tenant_quotas(resource_id)[quotasv2.RESOURCE_NAME] -def fetch_resource(neutron_context, resource, resource_id): - attrs = v2_attributes.get_resource_info(resource) +def fetch_resource(neutron_context, collection, resource, resource_id): + controller = manager.NeutronManager.get_controller_for_resource( + collection) + attrs = controller.resource_info if not attrs: # this isn't a request for a normal resource. it could be # an action like removing a network from a dhcp agent. @@ -96,7 +97,7 @@ class PolicyHook(hooks.PecanHook): # Ops... this was a delete after all! item = {} resource_id = state.request.context.get('resource_id') - resource_obj = fetch_resource(neutron_context, + resource_obj = fetch_resource(neutron_context, collection, resource, resource_id) if resource_obj: original_resources.append(resource_obj) @@ -196,8 +197,9 @@ class PolicyHook(hooks.PecanHook): """ attributes_to_exclude = [] for attr_name in data.keys(): - attr_data = v2_attributes.get_resource_info( - resource).get(attr_name) + controller = manager.NeutronManager.get_controller_for_resource( + collection) + attr_data = controller.resource_info.get(attr_name) if attr_data and attr_data['is_visible']: if policy.check( context, diff --git a/neutron/pecan_wsgi/startup.py b/neutron/pecan_wsgi/startup.py index 060f2827ad0..48bbcc39d14 100644 --- a/neutron/pecan_wsgi/startup.py +++ b/neutron/pecan_wsgi/startup.py @@ -68,8 +68,9 @@ def initialize_all(): if isinstance(legacy_controller, base.Controller): resource = legacy_controller.resource plugin = legacy_controller.plugin + attr_info = legacy_controller.attr_info new_controller = res_ctrl.CollectionsController( - collection, resource) + collection, resource, resource_info=attr_info) manager.NeutronManager.set_plugin_for_resource(resource, plugin) if path_prefix: manager.NeutronManager.add_resource_for_path_prefix( diff --git a/neutron/tests/functional/pecan_wsgi/test_hooks.py b/neutron/tests/functional/pecan_wsgi/test_hooks.py index bda5994ce7f..10499ff39b2 100644 --- a/neutron/tests/functional/pecan_wsgi/test_hooks.py +++ b/neutron/tests/functional/pecan_wsgi/test_hooks.py @@ -335,7 +335,7 @@ class TestNovaNotifierHook(test_functional.PecanFunctionalTest): # NOTE(kevinbenton): the original passed into the notifier does # not contain all of the fields of the object. Only those required # by the policy engine are included. - orig = pe.fetch_resource(context.get_admin_context(), + orig = pe.fetch_resource(context.get_admin_context(), 'networks', 'network', network_id) response = self.app.put_json( '/v2.0/networks/%s.json' % network_id, @@ -347,7 +347,7 @@ class TestNovaNotifierHook(test_functional.PecanFunctionalTest): orig, json_body) self.mock_notifier.reset_mock() - orig = pe.fetch_resource(context.get_admin_context(), + orig = pe.fetch_resource(context.get_admin_context(), 'networks', 'network', network_id) response = self.app.delete( '/v2.0/networks/%s.json' % network_id, headers=req_headers)