From 3e263ba6c58c43f71c02417f8948300e8cb5c462 Mon Sep 17 00:00:00 2001 From: Yong Sheng Gong Date: Sat, 7 Dec 2013 16:59:18 +0800 Subject: [PATCH] validate if the router has external gateway interface set 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 | 4 +- neutron/extensions/vpnaas.py | 4 + neutron/tests/unit/db/vpn/test_db_vpnaas.py | 120 ++++++++++++++------ 3 files changed, 94 insertions(+), 34 deletions(-) diff --git a/neutron/db/vpn/vpn_db.py b/neutron/db/vpn/vpn_db.py index 729816afc7..46406da722 100644 --- a/neutron/db/vpn/vpn_db.py +++ b/neutron/db/vpn/vpn_db.py @@ -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() diff --git a/neutron/extensions/vpnaas.py b/neutron/extensions/vpnaas.py index 1eaf5ae4f0..5e0560563a 100644 --- a/neutron/extensions/vpnaas.py +++ b/neutron/extensions/vpnaas.py @@ -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'] diff --git a/neutron/tests/unit/db/vpn/test_db_vpnaas.py b/neutron/tests/unit/db/vpn/test_db_vpnaas.py index 214423477e..96c1f08f52 100644 --- a/neutron/tests/unit/db/vpn/test_db_vpnaas.py +++ b/neutron/tests/unit/db/vpn/test_db_vpnaas.py @@ -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,