From 433228dd78098ca84a13c75b9dc5ce40f43c7f9d Mon Sep 17 00:00:00 2001 From: LIU Yulong Date: Mon, 8 Oct 2018 14:52:16 +0800 Subject: [PATCH] Prevent bind fip to port has port forwarding If one port has port forwarding and the port is under a dvr router, then binding floating IP to this port will not be allowed. Change-Id: Ia014e18264b43cf751a5bc0e82bc55d106582620 Closes-Bug: #1799138 --- neutron/db/l3_db.py | 17 +++++ .../portforwarding/common/exceptions.py | 6 ++ neutron/services/portforwarding/pf_plugin.py | 22 ++++++ .../portforwarding/test_port_forwarding.py | 23 +++++- .../test_expose_port_forwarding_in_fip.py | 70 +++++++++++++++++-- neutron/tests/unit/extensions/test_l3.py | 2 + ...p-binding-limitation-1d2509950847b085.yaml | 7 ++ 7 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/fip-binding-limitation-1d2509950847b085.yaml diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 52777a4cb4e..dbec9759c18 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -1336,6 +1336,14 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, def _create_floatingip(self, context, floatingip, initial_status=constants.FLOATINGIP_STATUS_ACTIVE): + try: + registry.publish(resources.FLOATING_IP, events.BEFORE_CREATE, + self, payload=events.DBEventPayload( + context, request_body=floatingip)) + except exceptions.CallbackFailure as e: + # raise the underlying exception + raise e.errors[0].error + fip = floatingip['floatingip'] fip_id = uuidutils.generate_uuid() @@ -1441,6 +1449,15 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, return self._create_floatingip(context, floatingip, initial_status) def _update_floatingip(self, context, id, floatingip): + try: + registry.publish(resources.FLOATING_IP, events.BEFORE_UPDATE, + self, payload=events.DBEventPayload( + context, request_body=floatingip, + resource_id=id)) + except exceptions.CallbackFailure as e: + # raise the underlying exception + raise e.errors[0].error + fip = floatingip['floatingip'] with context.session.begin(subtransactions=True): floatingip_obj = self._get_floatingip(context, id) diff --git a/neutron/services/portforwarding/common/exceptions.py b/neutron/services/portforwarding/common/exceptions.py index 56af6e895e2..a17cb382190 100644 --- a/neutron/services/portforwarding/common/exceptions.py +++ b/neutron/services/portforwarding/common/exceptions.py @@ -25,6 +25,12 @@ class PortForwardingNotSupportFilterField(n_exc.BadRequest): message = _("Port Forwarding filter %(filter)s is not supported.") +class PortHasPortForwarding(n_exc.BadRequest): + message = _("Cannot associate floating IP to port " + "%(port_id)s because it already has a " + "Port Forwarding binding.") + + class FipInUseByPortForwarding(n_exc.InUse): message = _("Floating IP %(id)s in use by Port Forwarding resources.") diff --git a/neutron/services/portforwarding/pf_plugin.py b/neutron/services/portforwarding/pf_plugin.py index 2fcd4b5d555..96f3e3bb962 100644 --- a/neutron/services/portforwarding/pf_plugin.py +++ b/neutron/services/portforwarding/pf_plugin.py @@ -38,6 +38,7 @@ from neutron.api.rpc.handlers import resources_rpc from neutron.common import utils from neutron.db import _resource_extend as resource_extend from neutron.db import db_base_plugin_common +from neutron.db import l3_dvr_db from neutron.extensions import floating_ip_port_forwarding as fip_pf from neutron.objects import base as base_obj from neutron.objects import port_forwarding as pf @@ -103,6 +104,27 @@ class PortForwardingPlugin(fip_pf.PortForwardingPluginBase): result_dict[apidef.COLLECTION_NAME] = port_forwarding_result return result_dict + @registry.receives(resources.FLOATING_IP, [events.BEFORE_CREATE, + events.BEFORE_UPDATE]) + def _check_port_has_port_forwarding(self, resource, event, + trigger, payload=None): + port_id = payload.request_body['floatingip'].get('port_id') + if not port_id: + return + + pf_objs = pf.PortForwarding.get_objects( + payload.context, internal_port_id=port_id) + if not pf_objs: + return + # Port may not bind to host yet, or port may migrate from one + # dvr_no_external host to one dvr host. So we just do not allow + # all dvr router's floating IP to be binded to a port which + # already has port forwarding. + router = self.l3_plugin.get_router(payload.context.elevated(), + pf_objs[0].router_id) + if l3_dvr_db.is_distributed_router(router): + raise pf_exc.PortHasPortForwarding(port_id=port_id) + @registry.receives(resources.FLOATING_IP, [events.PRECOMMIT_UPDATE, events.PRECOMMIT_DELETE]) def _check_floatingip_request(self, resource, event, trigger, context, diff --git a/neutron/tests/functional/services/portforwarding/test_port_forwarding.py b/neutron/tests/functional/services/portforwarding/test_port_forwarding.py index 1c94f931344..ae187f3da5b 100644 --- a/neutron/tests/functional/services/portforwarding/test_port_forwarding.py +++ b/neutron/tests/functional/services/portforwarding/test_port_forwarding.py @@ -86,7 +86,7 @@ class PortForwardingTestCase(PortForwardingTestCaseBase): self._prepare_env() def _prepare_env(self): - self.router = self._create_router() + self.router = self._create_router(distributed=True) self.ext_net = self._create_network( self.fmt, 'ext-net', True, arg_list=("router:external",), **{"router:external": True}).json['network'] @@ -461,3 +461,24 @@ class PortForwardingTestCase(PortForwardingTestCaseBase): self._simulate_concurrent_requests_process_and_raise(funcs, args_list) self.assertEqual([], self.pf_plugin.get_floatingip_port_forwardings( self.context, floatingip_id=self.fip['id'])) + + def test_create_floatingip_port_forwarding_port_in_use(self): + res = self.pf_plugin.create_floatingip_port_forwarding( + self.context, self.fip['id'], self.port_forwarding) + expected = { + "external_port": 2225, + "internal_port": 25, + "internal_port_id": self.port['id'], + "protocol": "tcp", + "internal_ip_address": self.port['fixed_ips'][0]['ip_address'], + 'id': mock.ANY, + 'router_id': self.router['id'], + 'floating_ip_address': self.fip['floating_ip_address'], + 'floatingip_id': self.fip['id']} + self.assertEqual(expected, res) + + fip_2 = self._create_floatingip(self.ext_net['id']) + self.assertRaises( + pf_exc.PortHasPortForwarding, + self._update_floatingip, + fip_2['id'], {'port_id': self.port['id']}) diff --git a/neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py b/neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py index 9b44087a2f8..76e04387d57 100644 --- a/neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py +++ b/neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py @@ -16,18 +16,22 @@ from neutron_lib.api.definitions import external_net as extnet_apidef from neutron_lib.api.definitions import floating_ip_port_forwarding as apidef from neutron_lib import constants from neutron_lib import context +from neutron_lib.plugins import constants as plugin_constants +from neutron_lib.plugins import directory from oslo_utils import uuidutils +from webob import exc +from neutron.db import l3_dvr_db from neutron.db import l3_fip_qos from neutron.extensions import floating_ip_port_forwarding as pf_ext from neutron.extensions import l3 from neutron.objects.qos import policy -from neutron.services.portforwarding import pf_plugin from neutron.tests.unit.db import test_db_base_plugin_v2 from neutron.tests.unit.extensions import test_l3 -PLUGIN_NAME = 'port_forwarding' +PF_PLUGIN_NAME = ('neutron.services.portforwarding.' + 'pf_plugin.PortForwardingPlugin') L3_PLUGIN = ('neutron.tests.unit.extensions.' 'test_expose_port_forwarding_in_fip.' 'TestL3PorForwardingServicePlugin') @@ -70,14 +74,13 @@ class TestExtendFipPortForwardingExtension( def setUp(self): mock.patch('neutron.api.rpc.handlers.resources_rpc.' 'ResourcesPushRpcApi').start() - svc_plugins = {'l3_plugin_name': L3_PLUGIN, - 'port_forwarding_plugin_name': PLUGIN_NAME, - 'qos': 'neutron.services.qos.qos_plugin.QoSPlugin'} + svc_plugins = (L3_PLUGIN, PF_PLUGIN_NAME, + 'neutron.services.qos.qos_plugin.QoSPlugin') ext_mgr = ExtendFipPortForwardingExtensionManager() super(TestExtendFipPortForwardingExtension, self).setUp( plugin=CORE_PLUGIN, ext_mgr=ext_mgr, service_plugins=svc_plugins) - self.pf_plugin = pf_plugin.PortForwardingPlugin() - self.l3_plugin = TestL3PorForwardingServicePlugin() + self.l3_plugin = directory.get_plugin(plugin_constants.L3) + self.pf_plugin = directory.get_plugin(plugin_constants.PORTFORWARDING) ctx = context.get_admin_context() self.policy_1 = policy.QosPolicy(ctx, @@ -291,3 +294,56 @@ class TestExtendFipPortForwardingExtension( self._router_interface_action( 'remove', router['router']['id'], insub3['subnet']['id'], None) + + @mock.patch.object(l3_dvr_db.L3_NAT_with_dvr_db_mixin, + '_notify_floating_ip_change') + @mock.patch.object(l3_dvr_db.DVRResourceOperationHandler, + '_create_dvr_floating_gw_port') + def test_port_in_used_by_port_forwarding(self, mock_gw_port, mock_notify): + port_forwarding = { + apidef.RESOURCE_NAME: + {apidef.EXTERNAL_PORT: 2225, + apidef.INTERNAL_PORT: 25, + apidef.INTERNAL_PORT_ID: None, + apidef.PROTOCOL: "tcp", + apidef.INTERNAL_IP_ADDRESS: None}} + ctx = context.get_admin_context() + kwargs = {'arg_list': (extnet_apidef.EXTERNAL,), + extnet_apidef.EXTERNAL: True} + with self.network(**kwargs) as extnet, self.network() as innet: + with self.subnet(network=extnet, cidr='200.0.0.0/22'),\ + self.subnet(network=innet, cidr='10.0.0.0/24') as insub,\ + self.router(distributed=True) as router: + fip = self._make_floatingip(self.fmt, extnet['network']['id']) + # check the floatingip response contains port_forwarding field + self.assertIn(apidef.COLLECTION_NAME, fip['floatingip']) + self._add_external_gateway_to_router(router['router']['id'], + extnet['network']['id']) + self._router_interface_action('add', router['router']['id'], + insub['subnet']['id'], None) + + with self.port(subnet=insub) as port1: + update_dict1 = { + apidef.INTERNAL_PORT_ID: port1['port']['id'], + apidef.INTERNAL_IP_ADDRESS: + port1['port']['fixed_ips'][0]['ip_address']} + port_forwarding[apidef.RESOURCE_NAME].update(update_dict1) + self.pf_plugin.create_floatingip_port_forwarding( + ctx, fip['floatingip']['id'], port_forwarding) + + body = self._show('floatingips', fip['floatingip']['id']) + self.assertEqual( + 1, len(body['floatingip'][apidef.COLLECTION_NAME])) + + self._make_floatingip( + self.fmt, + extnet['network']['id'], + port_id=port1['port']['id'], + http_status=exc.HTTPBadRequest.code) + fip_2 = self._make_floatingip(self.fmt, + extnet['network']['id']) + self._update( + 'floatingips', + fip_2['floatingip']['id'], + {'floatingip': {'port_id': port1['port']['id']}}, + expected_code=exc.HTTPBadRequest.code) diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index 738a0cd41d0..71d95fac7e1 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -388,6 +388,8 @@ class L3NatTestCaseMixin(object): # Arg must be present and not empty if arg in kwargs: data['router'][arg] = kwargs[arg] + if 'distributed' in kwargs: + data['router']['distributed'] = bool(kwargs['distributed']) router_req = self.new_create_request('routers', data, fmt) if set_context and tenant_id: # create a specific auth context for this request diff --git a/releasenotes/notes/fip-binding-limitation-1d2509950847b085.yaml b/releasenotes/notes/fip-binding-limitation-1d2509950847b085.yaml new file mode 100644 index 00000000000..76733d5b51e --- /dev/null +++ b/releasenotes/notes/fip-binding-limitation-1d2509950847b085.yaml @@ -0,0 +1,7 @@ +--- +other: + - | + If an instance port is under a dvr router, and the port already has + binding port forwarding(s). Neutron will no longer allow binding a + floating IP to that port again, because dvr floating IP traffic rules + will break the existing port forwarding functionality.