NSX-v: Fix VPNaaS driver

Commit I0095e160481f1d4572e38ad1d3bbc8e183039b84 introduced some
changes to the VPNaaS driver and validator apis.
This patch closses this gap.

Change-Id: I46d2381e3a0b05551518d41a3f9957a1b9d2b834

vmware_nsx/plugins/nsx_v/plugin.py

@@ -3899,7 +3899,7 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
 
             vpn_plugin = directory.get_plugin(plugin_const.VPN)
             if vpn_plugin:
-                vpn_driver = vpn_plugin.ipsec_driver
+                vpn_driver = vpn_plugin.drivers[vpn_plugin.default_provider]
                 vpn_rules = (
                     vpn_driver._generate_ipsecvpn_firewall_rules(edge_id))
                 fw_rules.extend(vpn_rules)

vmware_nsx/services/vpnaas/nsxv/ipsec_driver.py

@@ -14,10 +14,10 @@
 #    under the License.
 
 import netaddr
-from neutron_lib import exceptions as n_exc
 from neutron_lib.plugins import directory
 from neutron_vpnaas.services.vpn import service_drivers
 from oslo_log import log as logging
+from oslo_utils import excutils
 
 from vmware_nsx._i18n import _
 from vmware_nsx.common import exceptions as nsxv_exc
@@ -47,9 +47,6 @@ class NSXvIPsecVpnDriver(service_drivers.VpnDriver):
     def service_type(self):
         return IPSEC
 
-    def _is_shared_router(self, router):
-        return router.get('router_type') == nsxv_constants.SHARED
-
     def _get_router_edge_id(self, context, vpnservice_id):
         vpnservice = self.service_plugin._get_vpnservice(context,
                                                          vpnservice_id)
@@ -122,7 +119,7 @@ class NSXvIPsecVpnDriver(service_drivers.VpnDriver):
                 peer_subnets = site['peerSubnets']['subnets']
                 local_subnets = site['localSubnets']['subnets']
                 ipsec_vpn_fw_rules.append({
-                    'name': 'VPN ' + site['name'],
+                    'name': 'VPN ' + site.get('name', 'rule'),
                     'action': 'allow',
                     'enabled': True,
                     'source_ip_address': peer_subnets,
@@ -155,8 +152,6 @@ class NSXvIPsecVpnDriver(service_drivers.VpnDriver):
     def create_ipsec_site_connection(self, context, ipsec_site_connection):
         LOG.debug('Creating ipsec site connection %(conn_info)s.',
                   {"conn_info": ipsec_site_connection})
-
-        self.validator.validate_ipsec_conn(context, ipsec_site_connection)
         new_ipsec = self._convert_ipsec_conn(context, ipsec_site_connection)
         vpnservice_id = ipsec_site_connection['vpnservice_id']
         edge_id = self._get_router_edge_id(context, vpnservice_id)[1]
@@ -314,16 +309,13 @@ class NSXvIPsecVpnDriver(service_drivers.VpnDriver):
 
     def create_vpnservice(self, context, vpnservice):
         LOG.debug('Creating VPN service %(vpn)s', {'vpn': vpnservice})
-        # Only support distributed and exclusive router type
-        router_id = vpnservice['router_id']
         vpnservice_id = vpnservice['id']
-        router = self._core_plugin.get_router(context, router_id)
+        try:
-        if self._is_shared_router(router):
+            self.validator.validate_vpnservice(context, vpnservice)
-            # Rolling back change on the neutron
+        except Exception:
-            self.service_plugin.delete_vpnservice(context, vpnservice_id)
+            with excutils.save_and_reraise_exception():
-            msg = _("Router type is not supported for VPN service, only "
+                # Rolling back change on the neutron
-                    "support distributed and exclusive router")
+                self.service_plugin.delete_vpnservice(context, vpnservice_id)
-            raise n_exc.InvalidInput(error_message=msg)
 
         vpnservice = self.service_plugin._get_vpnservice(context,
                                                          vpnservice_id)

vmware_nsx/services/vpnaas/nsxv/ipsec_validator.py

@@ -79,15 +79,48 @@ class IPsecValidator(vpn_validator.VpnReferenceValidator):
                             "VSE!")
                 break
 
-    def validate_ipsec_conn(self, context, ipsec_site_conn):
+    def _is_shared_router(self, router):
-        ike_policy_id = ipsec_site_conn['ikepolicy_id']
+        return router.get('router_type') == nsxv_constants.SHARED
-        ipsec_policy_id = ipsec_site_conn['ipsecpolicy_id']
+
-        ipsecpolicy = self.vpn_plugin.get_ipsecpolicy(context,
+    def _validate_router(self, context, router_id):
-                                                      ipsec_policy_id)
+        # Only support distributed and exclusive router type
-        ikepolicy = self.vpn_plugin.get_ikepolicy(context,
+        router = self.core_plugin.get_router(context, router_id)
-                                                  ike_policy_id)
+        if self._is_shared_router(router):
-        self.validate_ikepolicy_version(ikepolicy)
+            msg = _("Router type is not supported for VPN service, only "
-        self.validate_ikepolicy_pfs(ikepolicy)
+                    "support distributed and exclusive router")
-        self.validate_encryption_algorithm(ikepolicy)
+            raise nsxv_exc.NsxVpnValidationError(details=msg)
-        self.validate_ipsec_policy(context, ipsecpolicy)
+
-        self.validate_policies_matching_algorithms(ikepolicy, ipsecpolicy)
+    def validate_vpnservice(self, context, vpnservice):
+        """Called upon create/update of a service"""
+
+        # Call general validations
+        super(IPsecValidator, self).validate_vpnservice(
+            context, vpnservice)
+
+        # Call specific NSX validations
+        self._validate_router(context, vpnservice['router_id'])
+
+        if not vpnservice['subnet_id']:
+            # we currently do not support multiple subnets so a subnet must
+            # be defined
+            msg = _("Subnet must be defined in a service")
+            raise nsxv_exc.NsxVpnValidationError(details=msg)
+
+    def validate_ipsec_site_connection(self, context, ipsec_site_conn):
+        ike_policy_id = ipsec_site_conn.get('ikepolicy_id')
+        if ike_policy_id:
+            ikepolicy = self.vpn_plugin.get_ikepolicy(context,
+                                                      ike_policy_id)
+
+            self.validate_ikepolicy_version(ikepolicy)
+            self.validate_ikepolicy_pfs(ikepolicy)
+            self.validate_encryption_algorithm(ikepolicy)
+
+        ipsec_policy_id = ipsec_site_conn.get('ipsecpolicy_id')
+        if ipsec_policy_id:
+            ipsecpolicy = self.vpn_plugin.get_ipsecpolicy(context,
+                                                          ipsec_policy_id)
+            self.validate_ipsec_policy(context, ipsecpolicy)
+
+        if ike_policy_id and ipsec_policy_id:
+            self.validate_policies_matching_algorithms(ikepolicy, ipsecpolicy)

vmware_nsx/tests/unit/services/vpnaas/test_nsxv_vpnaas.py

@@ -16,10 +16,12 @@
 import contextlib
 
 import mock
+
+from neutron_lib.api.definitions import external_net as extnet_apidef
 from neutron_lib import context
-from neutron_lib import exceptions as n_exc
 from neutron_lib.plugins import directory
 from neutron_vpnaas.db.vpn import vpn_models  # noqa
+from neutron_vpnaas.extensions import vpnaas
 from oslo_utils import uuidutils
 
 from vmware_nsx.common import exceptions as nsxv_exc
@@ -66,7 +68,6 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
         router = self.l3plugin.create_router(self.context, request)
         yield router
 
-    @mock.patch('%s.validate_ipsec_conn' % VALI_PATH)
     @mock.patch('%s._convert_ipsec_conn' % DRIVER_PATH)
     @mock.patch('%s._get_router_edge_id' % DRIVER_PATH)
     @mock.patch('%s._generate_new_sites' % DRIVER_PATH)
@@ -77,15 +78,12 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
                                           mock_update_status,
                                           mock_update_ipsec, mock_gen_new,
                                           mock_get_id,
-                                          mock_conv_ipsec,
+                                          mock_conv_ipsec):
-                                          mock_val_conn):
         mock_get_id.return_value = (FAKE_ROUTER_ID, FAKE_EDGE_ID)
         mock_conv_ipsec.return_value = FAKE_IPSEC_VPN_SITE
         mock_gen_new.return_value = FAKE_IPSEC_VPN_SITE
         self.driver.create_ipsec_site_connection(self.context,
                                                  FAKE_IPSEC_CONNECTION)
-        mock_val_conn.assert_called_with(self.context,
-                                         FAKE_IPSEC_CONNECTION)
         mock_conv_ipsec.assert_called_with(self.context,
                                            FAKE_IPSEC_CONNECTION)
         mock_get_id.assert_called_with(self.context, FAKE_VPNSERVICE_ID)
@@ -100,7 +98,6 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
             FAKE_IPSEC_CONNECTION["id"],
             "ACTIVE")
 
-    @mock.patch('%s.validate_ipsec_conn' % VALI_PATH)
     @mock.patch('%s._convert_ipsec_conn' % DRIVER_PATH)
     @mock.patch('%s._get_router_edge_id' % DRIVER_PATH)
     @mock.patch('%s._generate_new_sites' % DRIVER_PATH)
@@ -110,8 +107,7 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
                                                mock_update_status,
                                                mock_update_ipsec,
                                                mock_gen_new, mock_get_id,
-                                               mock_conv_ipsec,
+                                               mock_conv_ipsec):
-                                               mock_val_conn):
         mock_get_id.return_value = (FAKE_ROUTER_ID, FAKE_EDGE_ID)
         mock_conv_ipsec.return_value = FAKE_IPSEC_VPN_SITE
         mock_gen_new.return_value = FAKE_IPSEC_VPN_SITE
@@ -120,7 +116,6 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
         self.assertRaises(nsxv_exc.NsxPluginException,
                           self.driver.create_ipsec_site_connection,
                           self.context, FAKE_IPSEC_CONNECTION)
-        mock_val_conn.assert_called_with(self.context, FAKE_IPSEC_CONNECTION)
         mock_conv_ipsec.assert_called_with(self.context, FAKE_IPSEC_CONNECTION)
         mock_get_id.assert_called_with(self.context, FAKE_VPNSERVICE_ID)
         mock_gen_new.assert_called_with(FAKE_EDGE_ID, FAKE_IPSEC_VPN_SITE)
@@ -133,7 +128,6 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
             FAKE_IPSEC_CONNECTION["id"],
             "ERROR")
 
-    @mock.patch('%s.validate_ipsec_conn' % VALI_PATH)
     @mock.patch('%s._convert_ipsec_conn' % DRIVER_PATH)
     @mock.patch('%s._get_router_edge_id' % DRIVER_PATH)
     @mock.patch('%s._generate_new_sites' % DRIVER_PATH)
@@ -142,7 +136,7 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
     @mock.patch('%s._update_firewall_rules' % DRIVER_PATH)
     def test_update_fw_fail(self, mock_update_fw, mock_update_status,
                             mock_update_ipsec, mock_gen_new,
-                            mock_get_id, mock_conv_ipsec, mock_val_conn):
+                            mock_get_id, mock_conv_ipsec):
         mock_get_id.return_value = (FAKE_ROUTER_ID, FAKE_EDGE_ID)
         mock_conv_ipsec.return_value = FAKE_IPSEC_VPN_SITE
         mock_gen_new.return_value = FAKE_IPSEC_VPN_SITE
@@ -151,7 +145,6 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
         self.assertRaises(nsxv_exc.NsxPluginException,
                           self.driver.create_ipsec_site_connection,
                           self.context, FAKE_IPSEC_CONNECTION)
-        mock_val_conn.assert_called_with(self.context, FAKE_IPSEC_CONNECTION)
         mock_conv_ipsec.assert_called_with(self.context, FAKE_IPSEC_CONNECTION)
         mock_get_id.assert_called_with(self.context, FAKE_VPNSERVICE_ID)
         mock_gen_new.assert_called_with(FAKE_EDGE_ID, FAKE_IPSEC_VPN_SITE)
@@ -246,10 +239,97 @@ class TestVpnaasDriver(test_plugin.NsxVPluginV2TestCase):
             FAKE_IPSEC_CONNECTION["id"],
             "ERROR")
 
-    def test_create_vpn_service_on_shared_router(self):
+    def test_create_vpn_service_legal(self):
-        with self.router(router_type='shared') as router, self.subnet():
+        """Create a legal vpn service"""
+        # create an external network with a subnet, and an exclusive router
+        providernet_args = {extnet_apidef.EXTERNAL: True}
+        with self.network(name='ext-net',
+                          providernet_args=providernet_args,
+                          arg_list=(extnet_apidef.EXTERNAL, )) as ext_net,\
+            self.subnet(ext_net),\
+            self.router(router_type='exclusive',
+                        external_gateway_info={'network_id':
+                            ext_net['network']['id']}) as router,\
+            self.subnet() as sub:
+            # add an interface to the router
+            self.l3plugin.add_router_interface(
+                self.context,
+                router['id'],
+                {'subnet_id': sub['subnet']['id']})
+            # create the service
             vpnservice = {'router_id': router['id'],
-                          'id': _uuid()}
+                          'id': _uuid(),
-            self.assertRaises(n_exc.InvalidInput,
+                          'subnet_id': sub['subnet']['id']}
+            with mock.patch.object(self.driver, '_get_gateway_ips',
+                                   return_value=(None, None)):
+                self.driver.create_vpnservice(self.context, vpnservice)
+
+    def test_create_vpn_service_on_shared_router(self):
+        """Creating a service with shared router is not allowed"""
+        # create an external network with a subnet, and a shared router
+        providernet_args = {extnet_apidef.EXTERNAL: True}
+        with self.network(name='ext-net',
+                          providernet_args=providernet_args,
+                          arg_list=(extnet_apidef.EXTERNAL, )) as ext_net,\
+            self.subnet(ext_net),\
+            self.router(router_type='shared',
+                        external_gateway_info={'network_id':
+                            ext_net['network']['id']}) as router,\
+            self.subnet() as sub:
+            # add an interface to the router
+            self.l3plugin.add_router_interface(
+                self.context,
+                router['id'],
+                {'subnet_id': sub['subnet']['id']})
+            # create the service
+            vpnservice = {'router_id': router['id'],
+                          'id': _uuid(),
+                          'subnet_id': sub['subnet']['id']}
+            self.assertRaises(nsxv_exc.NsxPluginException,
+                              self.driver.create_vpnservice,
+                              self.context, vpnservice)
+
+    def test_create_vpn_service_on_router_without_if(self):
+        """Creating a service with unattached subnet is not allowed"""
+        # create an external network with a subnet, and an exclusive router
+        providernet_args = {extnet_apidef.EXTERNAL: True}
+        with self.network(name='ext-net',
+                          providernet_args=providernet_args,
+                          arg_list=(extnet_apidef.EXTERNAL, )) as ext_net,\
+            self.subnet(ext_net),\
+            self.router(router_type='exclusive',
+                        external_gateway_info={'network_id':
+                            ext_net['network']['id']}) as router,\
+            self.subnet() as sub:
+            # create the service
+            vpnservice = {'router_id': router['id'],
+                          'id': _uuid(),
+                          'subnet_id': sub['subnet']['id']}
+            self.assertRaises(vpnaas.SubnetIsNotConnectedToRouter,
+                              self.driver.create_vpnservice,
+                              self.context, vpnservice)
+
+    def test_create_vpn_service_without_subnet(self):
+        """Creating a service without a subnet is not allowed"""
+        # create an external network with a subnet, and an exclusive router
+        providernet_args = {extnet_apidef.EXTERNAL: True}
+        with self.network(name='ext-net',
+                          providernet_args=providernet_args,
+                          arg_list=(extnet_apidef.EXTERNAL, )) as ext_net,\
+            self.subnet(ext_net),\
+            self.router(router_type='exclusive',
+                        external_gateway_info={'network_id':
+                            ext_net['network']['id']}) as router,\
+            self.subnet() as sub:
+            # add an interface to the router
+            self.l3plugin.add_router_interface(
+                self.context,
+                router['id'],
+                {'subnet_id': sub['subnet']['id']})
+            # create the service without the subnet
+            vpnservice = {'router_id': router['id'],
+                          'id': _uuid(),
+                          'subnet_id': None}
+            self.assertRaises(nsxv_exc.NsxPluginException,
                               self.driver.create_vpnservice,
                               self.context, vpnservice)