NSX|V raise error when FWaaS uses unsupported routers

When attaching a firewall to an unsupported router type, we should
raise an exception, causing the firewall to become inactive.

Change-Id: Ia32ac4e7092138794825b9692d98073745dbb426

vmware_nsx/services/fwaas/nsx_v/edge_fwaas_driver.py

@@ -49,22 +49,27 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
         """Return True if the firewall rules should be added the router
 
         Return False in those cases:
+        - router without an external gateway (rule may be added later when
+                                              there is a gateway)
+
+        Raise an exception if the router is unsupported:
         - shared router (not supported)
-        - router without an external gateway
-        - md proxy router
+        - md proxy router (not supported)
+
         """
-        if not router_data.get('external_gateway_info'):
-            LOG.info("Cannot apply firewall to router %s with no gateway",
-                     router_data['id'])
-            return False
         if (not router_data.get('distributed') and
             router_data.get('router_type') == 'shared'):
-            LOG.info("Cannot apply firewall to shared router %s",
-                     router_data['id'])
-            return False
+            LOG.error("Cannot apply firewall to shared router %s",
+                      router_data['id'])
+            raise fw_ext.FirewallInternalDriverError(driver=FWAAS_DRIVER_NAME)
 
         if router_data.get('name', '').startswith('metadata_proxy_router'):
-            LOG.info("Cannot apply firewall to the metadata proxy router %s",
+            LOG.error("Cannot apply firewall to the metadata proxy router %s",
+                      router_data['id'])
+            raise fw_ext.FirewallInternalDriverError(driver=FWAAS_DRIVER_NAME)
+
+        if not router_data.get('external_gateway_info'):
+            LOG.info("Cannot apply firewall to router %s with no gateway",
                      router_data['id'])
             return False
 

vmware_nsx/services/fwaas/nsx_v/fwaas_callbacks.py

@@ -78,18 +78,18 @@ class NsxvFwaasCallbacks(firewall_l3_agent.L3WithFWaaS):
         if not self.fwaas_enabled:
             return False
 
+        ctx_elevated = context.elevated()
+        if not self._get_router_firewall_id(ctx_elevated, router_id):
+            # No FWaas Firewall was assigned to this router
+            return False
+
         # get all the relevant router info
         # ("router" does not have all the fields)
-        ctx_elevated = context.elevated()
         router_data = self.core_plugin.get_router(ctx_elevated, router['id'])
         if not router_data:
             LOG.error("Couldn't read router %s data", router['id'])
             return False
 
-        # Check if the FWaaS driver supports this router
-        if not self.fwaas_driver.should_apply_firewall_to_router(router_data):
-            return False
-
         if router_data.get('distributed'):
             # in case of a distributed-router:
             # router['id'] is the id of the neutron router (=tlr)
@@ -98,8 +98,8 @@ class NsxvFwaasCallbacks(firewall_l3_agent.L3WithFWaaS):
                 # Do not add firewall rules on the tlr router.
                 return False
 
-        if not self._get_router_firewall_id(ctx_elevated, router_id):
-            # No FWaas Firewall was assigned to this router
+        # Check if the FWaaS driver supports this router
+        if not self.fwaas_driver.should_apply_firewall_to_router(router_data):
             return False
 
         return True

vmware_nsx/tests/unit/nsx_v/test_fwaas_driver.py

@@ -16,6 +16,8 @@
 import copy
 import mock
 
+from neutron_fwaas.extensions import firewall as fw_ext
+
 from vmware_nsx.services.fwaas.nsx_v import edge_fwaas_driver
 from vmware_nsx.tests.unit.nsx_v import test_plugin as test_v_plugin
 
@@ -197,9 +199,17 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
         # not for shared router:
         router['router_type'] = 'shared'
         router['distributed'] = False
-        self.assertFalse(self.firewall.should_apply_firewall_to_router(router))
+        self.assertRaises(fw_ext.FirewallInternalDriverError,
+                          self.firewall.should_apply_firewall_to_router,
+                          router)
 
         # should work for distributed router
         router['router_type'] = 'exclusive'
         router['distributed'] = True
         self.assertTrue(self.firewall.should_apply_firewall_to_router(router))
+
+        # not for mdproxy router:
+        router['name'] = 'metadata_proxy_router'
+        self.assertRaises(fw_ext.FirewallInternalDriverError,
+                          self.firewall.should_apply_firewall_to_router,
+                          router)