diff --git a/vmware_nsx/plugins/nsx_v/plugin.py b/vmware_nsx/plugins/nsx_v/plugin.py index d419288c01..7e884b8eb1 100644 --- a/vmware_nsx/plugins/nsx_v/plugin.py +++ b/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) diff --git a/vmware_nsx/services/vpnaas/nsxv/ipsec_driver.py b/vmware_nsx/services/vpnaas/nsxv/ipsec_driver.py index 90f0f5f628..81baa86916 100644 --- a/vmware_nsx/services/vpnaas/nsxv/ipsec_driver.py +++ b/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) - if self._is_shared_router(router): - # Rolling back change on the neutron - self.service_plugin.delete_vpnservice(context, vpnservice_id) - msg = _("Router type is not supported for VPN service, only " - "support distributed and exclusive router") - raise n_exc.InvalidInput(error_message=msg) + try: + self.validator.validate_vpnservice(context, vpnservice) + except Exception: + with excutils.save_and_reraise_exception(): + # Rolling back change on the neutron + self.service_plugin.delete_vpnservice(context, vpnservice_id) vpnservice = self.service_plugin._get_vpnservice(context, vpnservice_id) diff --git a/vmware_nsx/services/vpnaas/nsxv/ipsec_validator.py b/vmware_nsx/services/vpnaas/nsxv/ipsec_validator.py index cc4a825780..259cca161f 100644 --- a/vmware_nsx/services/vpnaas/nsxv/ipsec_validator.py +++ b/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): - ike_policy_id = ipsec_site_conn['ikepolicy_id'] - ipsec_policy_id = ipsec_site_conn['ipsecpolicy_id'] - ipsecpolicy = self.vpn_plugin.get_ipsecpolicy(context, - ipsec_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) - self.validate_ipsec_policy(context, ipsecpolicy) - self.validate_policies_matching_algorithms(ikepolicy, ipsecpolicy) + def _is_shared_router(self, router): + return router.get('router_type') == nsxv_constants.SHARED + + def _validate_router(self, context, router_id): + # Only support distributed and exclusive router type + router = self.core_plugin.get_router(context, router_id) + if self._is_shared_router(router): + msg = _("Router type is not supported for VPN service, only " + "support distributed and exclusive router") + raise nsxv_exc.NsxVpnValidationError(details=msg) + + 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) diff --git a/vmware_nsx/tests/unit/services/vpnaas/test_nsxv_vpnaas.py b/vmware_nsx/tests/unit/services/vpnaas/test_nsxv_vpnaas.py index 0c9ae1e22c..12ce92059e 100644 --- a/vmware_nsx/tests/unit/services/vpnaas/test_nsxv_vpnaas.py +++ b/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_val_conn): + 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 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_val_conn): + 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 @@ -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): - with self.router(router_type='shared') as router, self.subnet(): + def test_create_vpn_service_legal(self): + """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()} - self.assertRaises(n_exc.InvalidInput, + 'id': _uuid(), + '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)