diff --git a/neutron/extensions/l3.py b/neutron/extensions/l3.py index 1c4f0ff13c2..ff98efdf364 100644 --- a/neutron/extensions/l3.py +++ b/neutron/extensions/l3.py @@ -22,6 +22,7 @@ from neutron.api import extensions from neutron.api.v2 import attributes as attr from neutron.api.v2 import resource_helper from neutron.common import exceptions as nexception +from neutron.pecan_wsgi import controllers from neutron.plugins.common import constants @@ -78,6 +79,8 @@ class RouterExternalGatewayInUseByFloatingIp(nexception.InUse): "more floating IPs.") ROUTERS = 'routers' +FLOATINGIP = 'floatingip' +FLOATINGIPS = '%ss' % FLOATINGIP EXTERNAL_GW_INFO = 'external_gateway_info' FLOATINGIPS = 'floatingips' @@ -204,6 +207,12 @@ class L3(extensions.ExtensionDescriptor): super(L3, self).update_attributes_map( attributes, extension_attrs_map=RESOURCE_ATTRIBUTE_MAP) + @classmethod + def get_pecan_controllers(cls): + return ((ROUTERS, controllers.RoutersController()), + (FLOATINGIPS, controllers.CollectionsController(FLOATINGIPS, + FLOATINGIP))) + def get_extended_resources(self, version): if version == "2.0": return RESOURCE_ATTRIBUTE_MAP diff --git a/neutron/pecan_wsgi/app.py b/neutron/pecan_wsgi/app.py index b9ff394e5e9..0dd30fcd99b 100644 --- a/neutron/pecan_wsgi/app.py +++ b/neutron/pecan_wsgi/app.py @@ -45,7 +45,6 @@ def setup_app(*args, **kwargs): app_hooks = [ hooks.ExceptionTranslationHook(), # priority 100 hooks.ContextHook(), # priority 95 - hooks.MemberActionHook(), # piority 95 hooks.BodyValidationHook(), # priority 120 hooks.OwnershipValidationHook(), # priority 125 hooks.QuotaEnforcementHook(), # priority 130 diff --git a/neutron/pecan_wsgi/controllers/__init__.py b/neutron/pecan_wsgi/controllers/__init__.py index a25c1ad67c6..1f840d77223 100644 --- a/neutron/pecan_wsgi/controllers/__init__.py +++ b/neutron/pecan_wsgi/controllers/__init__.py @@ -11,6 +11,10 @@ # under the License. from neutron.pecan_wsgi.controllers import quota +from neutron.pecan_wsgi.controllers import resource +from neutron.pecan_wsgi.controllers import router +CollectionsController = resource.CollectionsController QuotasController = quota.QuotasController +RoutersController = router.RoutersController diff --git a/neutron/pecan_wsgi/controllers/resource.py b/neutron/pecan_wsgi/controllers/resource.py index 31330dd8f7c..725bafc79e8 100644 --- a/neutron/pecan_wsgi/controllers/resource.py +++ b/neutron/pecan_wsgi/controllers/resource.py @@ -18,13 +18,56 @@ from pecan import request from neutron.pecan_wsgi.controllers import utils +class ItemController(utils.NeutronPecanController): + + def __init__(self, resource, item): + super(ItemController, self).__init__(None, resource) + self.item = item + + @utils.expose(generic=True) + def index(self, *args, **kwargs): + return self.get() + + def get(self, *args, **kwargs): + getter = getattr(self.plugin, 'get_%s' % self.resource) + neutron_context = request.context['neutron_context'] + return {self.resource: getter(neutron_context, self.item)} + + @utils.when(index, method='HEAD') + @utils.when(index, method='POST') + @utils.when(index, method='PATCH') + def not_supported(self): + pecan.abort(405) + + @utils.when(index, method='PUT') + def put(self, *args, **kwargs): + neutron_context = request.context['neutron_context'] + resources = request.context['resources'] + # TODO(kevinbenton): bulk? + updater = getattr(self.plugin, 'update_%s' % self.resource) + # Bulk update is not supported, 'resources' always contains a single + # elemenet + data = {self.resource: resources[0]} + return {self.resource: updater(neutron_context, self.item, data)} + + @utils.when(index, method='DELETE') + def delete(self): + # TODO(kevinbenton): setting code could be in a decorator + pecan.response.status = 204 + neutron_context = request.context['neutron_context'] + deleter = getattr(self.plugin, 'delete_%s' % self.resource) + return deleter(neutron_context, self.item) + + class CollectionsController(utils.NeutronPecanController): + item_controller_class = ItemController + @utils.expose() def _lookup(self, item, *remainder): # Store resource identifier in request context request.context['resource_id'] = item - return ItemController(self.resource, item), remainder + return self.item_controller_class(self.resource, item), remainder @utils.expose(generic=True) def index(self, *args, **kwargs): @@ -69,49 +112,3 @@ class CollectionsController(utils.NeutronPecanController): creator = getattr(self.plugin, method) neutron_context = request.context['neutron_context'] return {key: creator(neutron_context, data)} - - -class ItemController(utils.NeutronPecanController): - - def __init__(self, resource, item): - super(ItemController, self).__init__(None, resource) - self.item = item - - @utils.expose(generic=True) - def index(self, *args, **kwargs): - return self.get() - - def get(self, *args, **kwargs): - getter = getattr(self.plugin, 'get_%s' % self.resource) - neutron_context = request.context['neutron_context'] - return {self.resource: getter(neutron_context, self.item)} - - @utils.when(index, method='HEAD') - @utils.when(index, method='POST') - @utils.when(index, method='PATCH') - def not_supported(self): - pecan.abort(405) - - @utils.when(index, method='PUT') - def put(self, *args, **kwargs): - neutron_context = request.context['neutron_context'] - if request.member_action: - member_action_method = getattr(self.plugin, - request.member_action) - return member_action_method(neutron_context, self.item, - request.prepared_data) - # TODO(kevinbenton): bulk? - updater = getattr(self.plugin, 'update_%s' % self.resource) - resources = request.context['resources'] - # Bulk update is not supported, 'resources' always contains a single - # elemenet - data = {self.resource: resources[0]} - return {self.resource: updater(neutron_context, self.item, data)} - - @utils.when(index, method='DELETE') - def delete(self): - # TODO(kevinbenton): setting code could be in a decorator - pecan.response.status = 204 - neutron_context = request.context['neutron_context'] - deleter = getattr(self.plugin, 'delete_%s' % self.resource) - return deleter(neutron_context, self.item) diff --git a/neutron/pecan_wsgi/controllers/router.py b/neutron/pecan_wsgi/controllers/router.py new file mode 100644 index 00000000000..84b26df88b6 --- /dev/null +++ b/neutron/pecan_wsgi/controllers/router.py @@ -0,0 +1,82 @@ +# Copyright (c) 2015 Taturiello Consulting, Meh. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutron._i18n import _LE +from oslo_log import log +import pecan +from pecan import request + +from neutron.pecan_wsgi.controllers import resource +from neutron.pecan_wsgi.controllers import utils + +LOG = log.getLogger(__name__) + + +class RouterController(resource.ItemController): + """Customize ResourceController for member actions""" + + ### Pecan generic controllers don't work very well with inheritance + + @utils.expose(generic=True) + def index(self, *args, **kwargs): + return super(RouterController, self).index(*args, **kwargs) + + @utils.when(index, method='HEAD') + @utils.when(index, method='POST') + @utils.when(index, method='PATCH') + def not_supported(self): + return super(RouterController, self).not_supported() + + @utils.when(index, method='PUT') + def put(self, *args, **kwargs): + neutron_context = request.context['neutron_context'] + if args: + # There is a member action to process + member_action = args[0] + LOG.debug("Processing member action %(action)s for resource " + "%(resource)s identified by %(item)s", + {'action': member_action, + 'resource': self.resource, + 'item': self.item}) + # NOTE(salv-orlando): The following simply verify that the plugin + # has a method for a given action. It therefore enables plugins to + # implement actions which are not part of the API specification. + # Unfortunately the API extension descriptor does not do a good job + # of sanctioning which actions are available on a given resource. + # TODO(salv-orlando): prevent plugins from implementing actions + # which are not part of the Neutron API spec + try: + member_action_method = getattr(self.plugin, member_action) + return member_action_method(neutron_context, self.item, + request.context['request_data']) + except AttributeError: + LOG.error(_LE("Action %(action)s is not defined on resource " + "%(resource)s"), + {'action': member_action, 'resource': self.resource}) + pecan.abort(404) + # Do standard PUT processing + return super(RouterController, self).put(*args, **kwargs) + + @utils.when(index, method='DELETE') + def delete(self): + return super(RouterController, self).delete() + + +class RoutersController(resource.CollectionsController): + + item_controller_class = RouterController + + def __init__(self): + super(RoutersController, self).__init__('routers', 'router') diff --git a/neutron/pecan_wsgi/hooks/__init__.py b/neutron/pecan_wsgi/hooks/__init__.py index a14a810e892..899cccabe93 100644 --- a/neutron/pecan_wsgi/hooks/__init__.py +++ b/neutron/pecan_wsgi/hooks/__init__.py @@ -15,7 +15,6 @@ from neutron.pecan_wsgi.hooks import body_validation from neutron.pecan_wsgi.hooks import context -from neutron.pecan_wsgi.hooks import member_action from neutron.pecan_wsgi.hooks import notifier from neutron.pecan_wsgi.hooks import ownership_validation from neutron.pecan_wsgi.hooks import policy_enforcement @@ -25,7 +24,6 @@ from neutron.pecan_wsgi.hooks import translation ExceptionTranslationHook = translation.ExceptionTranslationHook ContextHook = context.ContextHook -MemberActionHook = member_action.MemberActionHook BodyValidationHook = body_validation.BodyValidationHook OwnershipValidationHook = ownership_validation.OwnershipValidationHook PolicyHook = policy_enforcement.PolicyHook diff --git a/neutron/pecan_wsgi/hooks/body_validation.py b/neutron/pecan_wsgi/hooks/body_validation.py index 6e957301a0b..a5a8cb9a8bb 100644 --- a/neutron/pecan_wsgi/hooks/body_validation.py +++ b/neutron/pecan_wsgi/hooks/body_validation.py @@ -13,11 +13,15 @@ # License for the specific language governing permissions and limitations # under the License. +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 +LOG = log.getLogger(__name__) + class BodyValidationHook(hooks.PecanHook): @@ -32,10 +36,24 @@ class BodyValidationHook(hooks.PecanHook): is_create = state.request.method == 'POST' if not resource: return + + try: + json_data = jsonutils.loads(state.request.body) + except ValueError: + LOG.debug("No JSON Data in %(method)s request for %(collection)s", + {'method': state.request.method, + 'collections': collection}) + return + # Raw data are consumed by member actions such as add_router_interface + state.request.context['request_data'] = json_data + if not (resource in json_data or collection in json_data): + # there is no resource in the request. This can happen when a + # member action is being processed. + return # Prepare data to be passed to the plugin from request body data = v2_base.Controller.prepare_request_body( neutron_context, - state.request.json, + json_data, is_create, resource, v2_attributes.get_collection_info(collection), diff --git a/neutron/pecan_wsgi/hooks/member_action.py b/neutron/pecan_wsgi/hooks/member_action.py deleted file mode 100644 index 42d3418cb81..00000000000 --- a/neutron/pecan_wsgi/hooks/member_action.py +++ /dev/null @@ -1,69 +0,0 @@ -# Copyright (c) 2015 Mirantis, Inc. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from pecan import abort -from pecan import hooks - -from neutron.api import extensions -from neutron.api.v2 import attributes - - -class MemberActionHook(hooks.PecanHook): - - priority = 95 - - def before(self, state): - # TODO(salv-orlando): This hook must go. Handling actions like this is - # shameful - state.request.member_action = None - resource = state.request.context.get('resource') - if not resource: - return - try: - # Remove the format suffix if any - uri = state.request.path.rsplit('.', 1)[0].split('/')[2:] - if not uri: - # there's nothing to process in the URI - return - except IndexError: - return - collection = None - for (collection, res) in attributes.PLURALS.items(): - if res == resource: - break - else: - return - state.request.member_action = self._parse_action( - resource, collection, uri[1:]) - - def _parse_action(self, resource, collection, remainder): - # NOTE(salv-orlando): This check is revolting and makes me - # puke, but avoids silly failures when dealing with API actions - # such as "add_router_interface". - if len(remainder) > 1: - action = remainder[1] - else: - return - ext_mgr = extensions.PluginAwareExtensionManager.get_instance() - resource_exts = ext_mgr.get_resources() - for ext in resource_exts: - if (ext.collection == collection and action in ext.member_actions): - return action - # Action or resource extension not found - if action: - abort(404, detail="Action %(action)s for resource " - "%(resource)s undefined" % - {'action': action, - 'resource': resource}) diff --git a/neutron/pecan_wsgi/hooks/notifier.py b/neutron/pecan_wsgi/hooks/notifier.py index 18c9d544633..7bd2d6506b0 100644 --- a/neutron/pecan_wsgi/hooks/notifier.py +++ b/neutron/pecan_wsgi/hooks/notifier.py @@ -74,15 +74,16 @@ class NotifierHook(hooks.PecanHook): except ValueError: if not state.response.body: data = {} - - if cfg.CONF.dhcp_agent_notification: - if data: - if resource_name in data: - resources = [data[resource_name]] - elif collection_name in data: - # This was a bulk request - resources = data[collection_name] - else: - resources = [] - self._notify_dhcp_agent(neutron_context, resource_name, - action, resources) + # Send a notification only if a resource can be identified in the + # response. This means that for operations such as add_router_interface + # no notification will be sent + if cfg.CONF.dhcp_agent_notification and data: + resources = [] + if resource_name in data: + resources = [data[resource_name]] + elif collection_name in data: + # This was a bulk request + resources = data[collection_name] + self._notify_dhcp_agent( + neutron_context, resource_name, + action, resources) diff --git a/neutron/tests/functional/pecan_wsgi/test_controllers.py b/neutron/tests/functional/pecan_wsgi/test_controllers.py index 70939d601b1..194e8a008c8 100644 --- a/neutron/tests/functional/pecan_wsgi/test_controllers.py +++ b/neutron/tests/functional/pecan_wsgi/test_controllers.py @@ -13,6 +13,7 @@ from collections import namedtuple import mock +from oslo_config import cfg from oslo_serialization import jsonutils import pecan from pecan import request @@ -21,6 +22,7 @@ from neutron.api.v2 import attributes from neutron import context from neutron import manager from neutron.pecan_wsgi.controllers import root as controllers +from neutron.plugins.common import constants from neutron.tests.functional.pecan_wsgi import test_functional _SERVICE_PLUGIN_RESOURCE = 'serviceplugin' @@ -394,3 +396,69 @@ class TestRequestProcessing(TestResourceController): response = self.do_request('/v2.0/dummy/serviceplugins.json') self.assertEqual(200, response.status_int) self.assertEqual(_SERVICE_PLUGIN_INDEX_BODY, response.json_body) + + +class TestRouterController(TestResourceController): + """Specialized tests for the router resource controller + + This test class adds tests specific for the router controller in + order to verify the 'member_action' functionality, which this + controller uses for adding and removing router interfaces. + """ + + def setUp(self): + cfg.CONF.set_override( + 'service_plugins', + ['neutron.services.l3_router.l3_router_plugin.L3RouterPlugin']) + + super(TestRouterController, self).setUp() + + # Create a network, a subnet, and a router + pl = manager.NeutronManager.get_plugin() + service_plugins = manager.NeutronManager.get_service_plugins() + l3_plugin = service_plugins[constants.L3_ROUTER_NAT] + ctx = context.get_admin_context() + network_id = pl.create_network( + ctx, + {'network': + {'name': 'pecannet', + 'tenant_id': 'tenid', + 'shared': False, + 'admin_state_up': True, + 'status': 'ACTIVE'}})['id'] + self.subnet = pl.create_subnet( + ctx, + {'subnet': + {'tenant_id': 'tenid', + 'network_id': network_id, + 'name': 'pecansub', + 'ip_version': 4, + 'cidr': '10.20.30.0/24', + 'gateway_ip': '10.20.30.1', + 'enable_dhcp': True, + 'allocation_pools': [ + {'start': '10.20.30.2', + 'end': '10.20.30.254'}], + 'dns_nameservers': [], + 'host_routes': []}}) + self.router = l3_plugin.create_router( + ctx, + {'router': + {'name': 'pecanrtr', + 'tenant_id': 'tenid', + 'admin_state_up': True}}) + + def test_member_actions_processing(self): + response = self.app.put_json( + '/v2.0/routers/%s/add_router_interface.json' % self.router['id'], + params={'subnet_id': self.subnet['id']}, + headers={'X-Project-Id': 'tenid'}) + self.assertEqual(200, response.status_int) + + def test_non_existing_member_action_returns_404(self): + response = self.app.put_json( + '/v2.0/routers/%s/do_meh.json' % self.router['id'], + params={'subnet_id': 'doesitevenmatter'}, + headers={'X-Project-Id': 'tenid'}, + expect_errors=True) + self.assertEqual(404, response.status_int)