]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
validate if the router has external gateway interface set
authorYong Sheng Gong <gongysh@unitedstack.com>
Sat, 7 Dec 2013 08:59:18 +0000 (16:59 +0800)
committerYong Sheng Gong <gongysh@unitedstack.com>
Mon, 9 Dec 2013 21:23:56 +0000 (05:23 +0800)
If the router wants to work with vpn service, we must
make sure external gateway interface is set.

This patch cannot prevent user from clearing the gateway interface
of the router after the vpnservice is created.

Change-Id: If0f00def949b31c1e3da7a2cd055454567201e4c
Closes-Bug: #1258379

neutron/db/vpn/vpn_db.py
neutron/extensions/vpnaas.py
neutron/tests/unit/db/vpn/test_db_vpnaas.py

index 729816afc7752f57f40cec10943891214a9e9c0f..46406da72226a0a0631349f7470bb3e4145de895 100644 (file)
@@ -542,7 +542,9 @@ class VPNPluginDb(VPNPluginBase, base_db.CommonDbMixin):
     def _check_router(self, context, router_id):
         l3_plugin = manager.NeutronManager.get_service_plugins().get(
             constants.L3_ROUTER_NAT)
-        l3_plugin.get_router(context, router_id)
+        router = l3_plugin.get_router(context, router_id)
+        if not router.get(l3_db.EXTERNAL_GW_INFO):
+            raise vpnaas.RouterIsNotExternal(router_id=router_id)
 
     def _check_subnet_id(self, context, router_id, subnet_id):
         core_plugin = manager.NeutronManager.get_plugin()
index 1eaf5ae4f0195e2b6d782f98d717d6109cb3cef9..5e0560563abc5d30b8e802473285415cc3dd915f 100644 (file)
@@ -88,6 +88,10 @@ class SubnetIsNotConnectedToRouter(qexception.BadRequest):
                 "connected to Router %(router_id)s")
 
 
+class RouterIsNotExternal(qexception.BadRequest):
+    message = _("Router %(router_id)s has no external network gateway set")
+
+
 vpn_supported_initiators = ['bi-directional', 'response-only']
 vpn_supported_encryption_algorithms = ['3des', 'aes-128',
                                        'aes-192', 'aes-256']
index 214423477eae9ff88faeb80f2a20add9df21c2d1..96c1f08f521d873e8e2d280b7f781b0972df5392 100644 (file)
@@ -243,18 +243,32 @@ class VPNPluginDbTestCase(test_l3_plugin.L3NatTestCaseMixin,
                    router=None,
                    admin_state_up=True,
                    no_delete=False,
-                   plug_subnet=True, **kwargs):
+                   plug_subnet=True,
+                   external_subnet_cidr='192.168.100.0/24',
+                   external_router=True,
+                   **kwargs):
         if not fmt:
             fmt = self.fmt
-        with test_db_plugin.optional_ctx(subnet, self.subnet) as tmp_subnet:
-            with test_db_plugin.optional_ctx(router,
-                                             self.router) as tmp_router:
-                if plug_subnet:
-                    self._router_interface_action(
-                        'add',
-                        tmp_router['router']['id'],
-                        tmp_subnet['subnet']['id'], None)
-
+        with contextlib.nested(
+            test_db_plugin.optional_ctx(subnet, self.subnet),
+            test_db_plugin.optional_ctx(router, self.router),
+            self.subnet(cidr=external_subnet_cidr)) as (tmp_subnet,
+                                                        tmp_router,
+                                                        public_sub):
+            if external_router:
+                self._set_net_external(
+                    public_sub['subnet']['network_id'])
+                self._add_external_gateway_to_router(
+                    tmp_router['router']['id'],
+                    public_sub['subnet']['network_id'])
+                tmp_router['router']['external_gateway_info'] = {
+                    'network_id': public_sub['subnet']['network_id']}
+            if plug_subnet:
+                self._router_interface_action(
+                    'add',
+                    tmp_router['router']['id'],
+                    tmp_subnet['subnet']['id'], None)
+            try:
                 res = self._create_vpnservice(fmt,
                                               name,
                                               admin_state_up,
@@ -263,20 +277,27 @@ class VPNPluginDbTestCase(test_l3_plugin.L3NatTestCaseMixin,
                                               subnet_id=(tmp_subnet['subnet']
                                                          ['id']),
                                               **kwargs)
+                vpnservice = self.deserialize(fmt or self.fmt, res)
                 if res.status_int >= 400:
-                    raise webob.exc.HTTPClientError(code=res.status_int)
-                try:
-                    vpnservice = self.deserialize(fmt or self.fmt, res)
-                    yield vpnservice
-                finally:
-                    if not no_delete:
-                        self._delete('vpnservices',
-                                     vpnservice['vpnservice']['id'])
-                    if plug_subnet:
-                        self._router_interface_action(
-                            'remove',
-                            tmp_router['router']['id'],
-                            tmp_subnet['subnet']['id'], None)
+                    raise webob.exc.HTTPClientError(
+                        code=res.status_int, detail=vpnservice)
+                yield vpnservice
+            finally:
+                if not no_delete and vpnservice.get('vpnservice'):
+                    self._delete('vpnservices',
+                                 vpnservice['vpnservice']['id'])
+                if plug_subnet:
+                    self._router_interface_action(
+                        'remove',
+                        tmp_router['router']['id'],
+                        tmp_subnet['subnet']['id'], None)
+                if external_router:
+                    external_gateway = tmp_router['router'].get(
+                        'external_gateway_info')
+                    if external_gateway:
+                        network_id = external_gateway['network_id']
+                        self._remove_external_gateway_from_router(
+                            tmp_router['router']['id'], network_id)
 
     def _create_ipsec_site_connection(self, fmt, name='test',
                                       peer_address='192.168.1.10',
@@ -813,7 +834,7 @@ class TestVpnaas(VPNPluginDbTestCase):
                                      expected)
 
     def test_create_vpnservice_with_invalid_router(self):
-        """Test case to create a vpnservice with invalid router"""
+        """Test case to create a vpnservice with other tenant's router"""
         with self.network(
             set_context=True,
             tenant_id='tenant_a') as network:
@@ -829,6 +850,24 @@ class TestVpnaas(VPNPluginDbTestCase):
                         expected_res_status=webob.exc.HTTPNotFound.code,
                         set_context=True, tenant_id='tenant_b')
 
+    def test_create_vpnservice_with_router_no_external_gateway(self):
+        """Test case to create a vpnservice with inner router"""
+        error_code = 0
+        with self.subnet(cidr='10.2.0.0/24') as subnet:
+            with self.router() as router:
+                router_id = router['router']['id']
+                try:
+                    with self.vpnservice(subnet=subnet,
+                                         router=router,
+                                         external_router=False):
+                        pass
+                except webob.exc.HTTPClientError as e:
+                    error_code, error_detail = (
+                        e.status_code, e.detail['NeutronError']['message'])
+        self.assertEqual(400, error_code)
+        msg = str(vpnaas.RouterIsNotExternal(router_id=router_id))
+        self.assertEqual(msg, error_detail)
+
     def test_create_vpnservice_with_nonconnected_subnet(self):
         """Test case to create a vpnservice with nonconnected subnet."""
         with self.network() as network:
@@ -959,15 +998,20 @@ class TestVpnaas(VPNPluginDbTestCase):
                 with contextlib.nested(
                     self.vpnservice(name='vpnservice1',
                                     subnet=subnet,
-                                    router=router),
+                                    router=router,
+                                    external_subnet_cidr='192.168.10.0/24',),
                     self.vpnservice(name='vpnservice2',
                                     subnet=subnet,
                                     router=router,
-                                    plug_subnet=False),
+                                    plug_subnet=False,
+                                    external_router=False,
+                                    external_subnet_cidr='192.168.11.0/24',),
                     self.vpnservice(name='vpnservice3',
                                     subnet=subnet,
                                     router=router,
-                                    plug_subnet=False)
+                                    plug_subnet=False,
+                                    external_router=False,
+                                    external_subnet_cidr='192.168.13.0/24',)
                 ) as(vpnservice1, vpnservice2, vpnservice3):
                     self._test_list_with_sort('vpnservice', (vpnservice3,
                                                              vpnservice2,
@@ -981,15 +1025,20 @@ class TestVpnaas(VPNPluginDbTestCase):
                 with contextlib.nested(
                     self.vpnservice(name='vpnservice1',
                                     subnet=subnet,
-                                    router=router),
+                                    router=router,
+                                    external_subnet_cidr='192.168.10.0/24'),
                     self.vpnservice(name='vpnservice2',
                                     subnet=subnet,
                                     router=router,
-                                    plug_subnet=False),
+                                    plug_subnet=False,
+                                    external_subnet_cidr='192.168.20.0/24',
+                                    external_router=False),
                     self.vpnservice(name='vpnservice3',
                                     subnet=subnet,
                                     router=router,
-                                    plug_subnet=False)
+                                    plug_subnet=False,
+                                    external_subnet_cidr='192.168.30.0/24',
+                                    external_router=False)
                 ) as(vpnservice1, vpnservice2, vpnservice3):
                     self._test_list_with_pagination('vpnservice',
                                                     (vpnservice1,
@@ -1004,15 +1053,20 @@ class TestVpnaas(VPNPluginDbTestCase):
                 with contextlib.nested(
                     self.vpnservice(name='vpnservice1',
                                     subnet=subnet,
-                                    router=router),
+                                    router=router,
+                                    external_subnet_cidr='192.168.10.0/24'),
                     self.vpnservice(name='vpnservice2',
                                     subnet=subnet,
                                     router=router,
-                                    plug_subnet=False),
+                                    plug_subnet=False,
+                                    external_subnet_cidr='192.168.11.0/24',
+                                    external_router=False),
                     self.vpnservice(name='vpnservice3',
                                     subnet=subnet,
                                     router=router,
-                                    plug_subnet=False)
+                                    plug_subnet=False,
+                                    external_subnet_cidr='192.168.12.0/24',
+                                    external_router=False)
                 ) as(vpnservice1, vpnservice2, vpnservice3):
                     self._test_list_with_pagination_reverse('vpnservice',
                                                             (vpnservice1,