Merge pull request #2134 from rackerlabs/svi_external_gateway

feat(neutron): Prevent SVI routers from using external gateways PUC-1754

Author: cardoe

python/neutron-understack/neutron_understack/l3_router/svi.py

@@ -335,6 +335,39 @@ def _is_svi_flavor(self, context, router):
         # flavor -> provider -> driver lookup.
         return _is_svi_router(context, router)
 
+    @registry.receives(resources.ROUTER_GATEWAY, [events.BEFORE_CREATE])
+    def _reject_svi_external_gateway(self, _resource, _event, _trigger, payload):
+        context = payload.context
+        router_id = payload.resource_id
+        router = self.l3plugin.get_router(context, router_id)
+
+        if not self._is_svi_flavor(context, router):
+            LOG.debug(
+                "SVI gateway check skipped: router %(router)s is not SVI "
+                "(name=%(name)s flavor=%(flavor)s)",
+                {
+                    "router": router_id,
+                    "name": router.get("name"),
+                    "flavor": router.get("flavor_id"),
+                },
+            )
+            return
+
+        network_id = (payload.metadata or {}).get("network_id")
+        LOG.warning(
+            "SVI gateway check FAILED: router %(router)s (%(name)s) cannot "
+            "have an external gateway network %(network)s",
+            {
+                "router": router_id,
+                "name": router.get("name"),
+                "network": network_id,
+            },
+        )
+        raise n_exc.BadRequest(
+            resource="router",
+            msg="SVI routers cannot have an external gateway.",
+        )
+
     @registry.receives(resources.ROUTER_INTERFACE, [events.BEFORE_CREATE])
     def _validate_svi_router_interface(self, _resource, _event, _trigger, payload):
         router = payload.states[0]

python/neutron-understack/neutron_understack/tests/test_svi.py

@@ -283,3 +283,67 @@ def test_rejects_svi_router_interface_without_scope(self, caplog, mocker):
 
         assert "must belong to an address scope" in str(exc_info.value)
         assert "attach_mode=port" in caplog.text
+
+
+class TestRejectSviExternalGatewayCallback:
+    def test_rejects_svi_router_external_gateway(self, caplog, mocker):
+        router = {"id": "router-a", "name": "svi-router", "flavor_id": "flavor-a"}
+        l3_plugin = FakeL3Plugin(router)
+        flavor_plugin = FakeFlavorPlugin(_svi_driver())
+        _patch_plugins(
+            mocker,
+            core_plugin=FakeCorePlugin({}),
+            l3_plugin=l3_plugin,
+            flavor_plugin=flavor_plugin,
+        )
+        provider = svi.Svi(l3_plugin)
+        payload = SimpleNamespace(
+            context="context",
+            resource_id="router-a",
+            metadata={"network_id": "external-net"},
+        )
+
+        caplog.set_level(logging.WARNING, logger=svi.LOG.name)
+        with pytest.raises(n_exc.BadRequest) as exc_info:
+            provider._reject_svi_external_gateway(None, None, None, payload)
+
+        assert "cannot have an external gateway" in str(exc_info.value)
+        assert "SVI gateway check FAILED" in caplog.text
+        assert "external-net" in caplog.text
+
+    def test_allows_non_svi_router_external_gateway(self, mocker):
+        router = {"id": "router-a", "name": "vrf-router", "flavor_id": "flavor-a"}
+        l3_plugin = FakeL3Plugin(router)
+        flavor_plugin = FakeFlavorPlugin("neutron_understack.l3_router.vrf.Vrf")
+        _patch_plugins(
+            mocker,
+            core_plugin=FakeCorePlugin({}),
+            l3_plugin=l3_plugin,
+            flavor_plugin=flavor_plugin,
+        )
+        provider = svi.Svi(l3_plugin)
+        payload = SimpleNamespace(
+            context="context",
+            resource_id="router-a",
+            metadata={"network_id": "external-net"},
+        )
+
+        provider._reject_svi_external_gateway(None, None, None, payload)
+
+    def test_allows_router_without_flavor(self, mocker):
+        router = {"id": "router-a", "name": "plain-router", "flavor_id": None}
+        l3_plugin = FakeL3Plugin(router)
+        _patch_plugins(
+            mocker,
+            core_plugin=FakeCorePlugin({}),
+            l3_plugin=l3_plugin,
+            flavor_plugin=FakeFlavorPlugin(_svi_driver()),
+        )
+        provider = svi.Svi(l3_plugin)
+        payload = SimpleNamespace(
+            context="context",
+            resource_id="router-a",
+            metadata={"network_id": "external-net"},
+        )
+
+        provider._reject_svi_external_gateway(None, None, None, payload)