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

@@ -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. "
+    message = _("Port %(port_id)s is not supported by firewall driver "
-                "This may happen due to incompatible driver combination.")
+                "'%(driver_name)s'.")

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
+            if device_owner in nl_constants.ROUTER_INTERFACE_OWNERS:
-                not device_owner.startswith(
+                if not self.driver.is_supported_l3_port(port):
-                    nl_constants.DEVICE_OWNER_COMPUTE_PREFIX)):
+                    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
+    def _is_supported_l2_port(self, context, port_id):
-    #                 different support
+        """Whether this l2 port is supported"""
-    def _is_supported_by_fw_l2_driver(self, context, port_id):
-        """Whether this port is supported by firewall l2 driver"""
 
         # 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 self.driver.is_supported_l2_port(port)
-            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
 
     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
+        # Check port is supported by firewall driver
-        if not self._is_supported_by_fw_l2_driver(context, port_id):
+        if not self._is_supported_l2_port(context, port_id):
             return
 
         project_id = updated_port['project_id']

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()

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):

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.'
+        super(TestFirewallDBPluginV2, self).setUp()
-                    'driver_api.FirewallDriverDB')
-        super(TestFirewallDBPluginV2, self).setUp(service_provider=provider)
         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']

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.'
+            provider += (':dummy:neutron_fwaas.tests.unit.services.firewall.'
-                         'service_drivers.driver_api.FirewallDriverDB:default')
+                         'test_fwaas_plugin_v2.DummyDriverDB:default')
         else:
             provider += ':test:' + service_provider + ':default'