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'