From 2a7994851cc0767e6a7b192a4101e8b43681ae6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89douard=20Thuleau?= Date: Wed, 21 Nov 2018 16:56:07 +0100 Subject: [PATCH] Move port validation support into the driver Each firewall driver have specific checks to do on port validation (like checks if the VIF port type corresponds to a type supported by the driver (aka the SDN controller)). This patch adds two methods to the driver interface to validate if the VM or the router port is supported (just have to return a boolean). Change-Id: I8fdf0956ac5428558aae413e610d13c4a4a56273 Closes-Bug: #1803723 --- neutron_fwaas/common/exceptions.py | 4 +- .../services/firewall/fwaas_plugin_v2.py | 44 ++++++++----------- .../firewall/service_drivers/agents/agents.py | 17 +++++++ .../firewall/service_drivers/driver_api.py | 6 +++ .../db/firewall/v2/test_firewall_db_v2.py | 6 +-- .../services/firewall/test_fwaas_plugin_v2.py | 14 +++++- 6 files changed, 57 insertions(+), 34 deletions(-) diff --git a/neutron_fwaas/common/exceptions.py b/neutron_fwaas/common/exceptions.py index 9ecfe0903..261a9d476 100644 --- a/neutron_fwaas/common/exceptions.py +++ b/neutron_fwaas/common/exceptions.py @@ -20,5 +20,5 @@ from neutron_fwaas._i18n import _ # TODO(annp): migrate to neutron-lib after Queen release class FirewallGroupPortNotSupported(n_exc.Conflict): - message = _("Port %(port_id)s is not supported by firewall L2 driver. " - "This may happen due to incompatible driver combination.") + message = _("Port %(port_id)s is not supported by firewall driver " + "'%(driver_name)s'.") diff --git a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py index 1477f212e..1fa2dba72 100644 --- a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py +++ b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py @@ -66,6 +66,7 @@ class FirewallPluginV2(Firewallv2PluginBase): "although running multiple drivers in parallel is " "not yet supported") + self.driver_name = default_provider self.driver = drivers[default_provider] # start rpc listener if driver required @@ -151,7 +152,8 @@ class FirewallPluginV2(Firewallv2PluginBase): """Validate firewall group associated ports Check if the firewall group associated ports have the same project - owner and is router interface type or a compute layer 2. + owner and is router interface type or a compute layer 2 and supported + by the firewall driver :param context: neutron context :param tenant_id: firewall group project ID :param fwg_ports: firewall group associated ports @@ -164,19 +166,20 @@ class FirewallPluginV2(Firewallv2PluginBase): raise f_exc.FirewallGroupPortInvalidProject( port_id=port_id, project_id=port['tenant_id']) device_owner = port.get('device_owner', '') - if (device_owner not in nl_constants.ROUTER_INTERFACE_OWNERS and - not device_owner.startswith( - nl_constants.DEVICE_OWNER_COMPUTE_PREFIX)): + if device_owner in nl_constants.ROUTER_INTERFACE_OWNERS: + if not self.driver.is_supported_l3_port(port): + raise exceptions.FirewallGroupPortNotSupported( + driver_name=self.driver_name, port_id=port_id) + elif device_owner.startswith( + nl_constants.DEVICE_OWNER_COMPUTE_PREFIX): + if not self._is_supported_l2_port(context, port_id): + raise exceptions.FirewallGroupPortNotSupported( + driver_name=self.driver_name, port_id=port_id) + else: raise f_exc.FirewallGroupPortInvalid(port_id=port_id) - if (device_owner.startswith( - nl_constants.DEVICE_OWNER_COMPUTE_PREFIX) and not - self._is_supported_by_fw_l2_driver(context, port_id)): - raise exceptions.FirewallGroupPortNotSupported(port_id=port_id) - # TODO(ethuleau): move that check in the driver. Each driver can have - # different support - def _is_supported_by_fw_l2_driver(self, context, port_id): - """Whether this port is supported by firewall l2 driver""" + def _is_supported_l2_port(self, context, port_id): + """Whether this l2 port is supported""" # Re-fetch to get up-to-date data from db port = self._core_plugin.get_port(context, id=port_id) @@ -186,18 +189,7 @@ class FirewallPluginV2(Firewallv2PluginBase): pb_def.VIF_TYPE_BINDING_FAILED]: return False - if not port['port_security_enabled']: - return True - - if port[pb_def.VIF_TYPE] == pb_def.VIF_TYPE_OVS: - # TODO(annp): remove these lines after we fully support for hybrid - # port - if not port[pb_def.VIF_DETAILS][pb_def.OVS_HYBRID_PLUG]: - return True - LOG.warning("Doesn't support hybrid port at the moment") - else: - LOG.warning("Doesn't support vif type %s", port[pb_def.VIF_TYPE]) - return False + return self.driver.is_supported_l2_port(port) def _validate_if_firewall_group_on_ports(self, context, firewall_group, id=None): @@ -288,8 +280,8 @@ class FirewallPluginV2(Firewallv2PluginBase): context = kwargs['context'] port_id = updated_port['id'] - # Check port is supported by firewall l2 driver or not - if not self._is_supported_by_fw_l2_driver(context, port_id): + # Check port is supported by firewall driver + if not self._is_supported_l2_port(context, port_id): return project_id = updated_port['project_id'] diff --git a/neutron_fwaas/services/firewall/service_drivers/agents/agents.py b/neutron_fwaas/services/firewall/service_drivers/agents/agents.py index 8ae52b9c7..e4e2d24c7 100644 --- a/neutron_fwaas/services/firewall/service_drivers/agents/agents.py +++ b/neutron_fwaas/services/firewall/service_drivers/agents/agents.py @@ -166,6 +166,23 @@ class FirewallAgentDriver(driver_api.FirewallDriverDB, super(FirewallAgentDriver, self).__init__(service_plugin) self.agent_rpc = FirewallAgentApi(constants.FW_AGENT, cfg.CONF.host) + def is_supported_l2_port(self, port): + if port[pb_def.VIF_TYPE] == pb_def.VIF_TYPE_OVS: + if not port['port_security_enabled']: + return True + + # TODO(annp): remove these lines after we fully support for hybrid + # port + if not port[pb_def.VIF_DETAILS][pb_def.OVS_HYBRID_PLUG]: + return True + LOG.warning("Doesn't support hybrid port at the moment") + else: + LOG.warning("Doesn't support vif type %s", port[pb_def.VIF_TYPE]) + return False + + def is_supported_l3_port(self, port): + return True + def start_rpc_listener(self): self.endpoints = [FirewallAgentCallbacks(self.firewall_db)] self.rpc_connection = n_rpc.Connection() diff --git a/neutron_fwaas/services/firewall/service_drivers/driver_api.py b/neutron_fwaas/services/firewall/service_drivers/driver_api.py index dbb9e6345..eae09177f 100644 --- a/neutron_fwaas/services/firewall/service_drivers/driver_api.py +++ b/neutron_fwaas/services/firewall/service_drivers/driver_api.py @@ -46,6 +46,12 @@ class FirewallDriver(object): def _core_plugin(self): return directory.get_plugin() + def is_supported_l2_port(self, port): + return False + + def is_supported_l3_port(self, port): + return False + # Firewall Group @abc.abstractmethod def create_firewall_group(self, context, firewall_group): diff --git a/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py b/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py index a0b40ea7f..ad4402af2 100644 --- a/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py +++ b/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py @@ -29,9 +29,7 @@ from neutron_fwaas.tests.unit.services.firewall import test_fwaas_plugin_v2 class TestFirewallDBPluginV2(test_fwaas_plugin_v2.FirewallPluginV2TestCase): def setUp(self): - provider = ('neutron_fwaas.services.firewall.service_drivers.' - 'driver_api.FirewallDriverDB') - super(TestFirewallDBPluginV2, self).setUp(service_provider=provider) + super(TestFirewallDBPluginV2, self).setUp() self.db = self.plugin.driver.firewall_db def test_get_policy_ordered_rules(self): @@ -1614,7 +1612,7 @@ class TestFirewallDBPluginV2(test_fwaas_plugin_v2.FirewallPluginV2TestCase): 'device_owner': 'compute:nova', 'binding:vif_type': 'ovs', } - self.plugin._is_supported_by_fw_l2_driver = mock.Mock( + self.plugin._is_supported_l2_port = mock.Mock( return_value=True) with self.port(**port_args) as port1, self.port(**port_args) as port2: port1_id = port1['port']['id'] diff --git a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py index 6e759bd8d..3939c1090 100644 --- a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py +++ b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py @@ -31,6 +31,8 @@ from oslo_utils import importutils from neutron_fwaas.common import fwaas_constants from neutron_fwaas import extensions from neutron_fwaas.services.firewall import fwaas_plugin_v2 +from neutron_fwaas.services.firewall.service_drivers.driver_api import \ + FirewallDriverDB from neutron_fwaas.tests import base @@ -41,6 +43,14 @@ def http_client_error(req, res): explanation=explanation) +class DummyDriverDB(FirewallDriverDB): + def is_supported_l2_port(self, port): + return True + + def is_supported_l3_port(self, port): + return True + + class FirewallPluginV2TestCase(base.NeutronDbPluginV2TestCase): DESCRIPTION = 'default description' PROTOCOL = 'tcp' @@ -64,8 +74,8 @@ class FirewallPluginV2TestCase(base.NeutronDbPluginV2TestCase): extra_service_plugins=None, extra_extension_paths=None): provider = fwaas_constants.FIREWALL_V2 if not service_provider: - provider += (':dummy:neutron_fwaas.services.firewall.' - 'service_drivers.driver_api.FirewallDriverDB:default') + provider += (':dummy:neutron_fwaas.tests.unit.services.firewall.' + 'test_fwaas_plugin_v2.DummyDriverDB:default') else: provider += ':test:' + service_provider + ':default'