Merge "NSX|v3: Enforce address scopes for no-NAT routers"

vmware_nsx/plugins/common/plugin.py

@@ -20,6 +20,7 @@ from neutron.db import address_scope_db
 from neutron.db import api as db_api
 from neutron.db import db_base_plugin_v2
 from neutron.db import l3_db
+from neutron.extensions import address_scope as ext_address_scope
 from neutron_lib.api.definitions import network as net_def
 from neutron_lib.api.definitions import port as port_def
 from neutron_lib.api.definitions import subnet as subnet_def
@@ -28,6 +29,7 @@ from neutron_lib import exceptions as n_exc
 from neutron_lib.plugins import directory
 
 from vmware_nsx._i18n import _
+from vmware_nsx.common import exceptions as nsx_exc
 
 LOG = logging.getLogger(__name__)
 
@@ -93,3 +95,34 @@ class NsxPluginBase(db_base_plugin_v2.NeutronDbPluginV2,
                         "has SNAT disabled")
                 raise n_exc.InvalidInput(error_message=msg)
         return router_id
+
+    def _get_network_address_scope(self, context, net_id):
+        network = self.get_network(context, net_id)
+        return network.get(ext_address_scope.IPV4_ADDRESS_SCOPE)
+
+    def _get_subnet_address_scope(self, context, subnet_id):
+        subnet = self.get_subnet(context, subnet_id)
+        if not subnet['subnetpool_id']:
+            return
+        subnetpool = self.get_subnetpool(context, subnet['subnetpool_id'])
+        return subnetpool.get('address_scope_id', '')
+
+    # TODO(asarfaty): the NSX-V3 needs a very similar code too
+    def _validate_address_scope_for_router_interface(self, context, router_id,
+                                                     gw_network_id, subnet_id):
+        """Validate that the GW address scope is the same as the interface"""
+        gw_address_scope = self._get_network_address_scope(context,
+                                                           gw_network_id)
+        if not gw_address_scope:
+            return
+        subnet_address_scope = self._get_subnet_address_scope(context,
+                                                              subnet_id)
+        if (not subnet_address_scope or
+            subnet_address_scope != gw_address_scope):
+            raise nsx_exc.NsxRouterInterfaceDoesNotMatchAddressScope(
+                router_id=router_id, address_scope_id=gw_address_scope)
+
+    def _get_router_interfaces(self, context, router_id):
+        port_filters = {'device_id': [router_id],
+                        'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF]}
+        return self.get_ports(context, filters=port_filters)

vmware_nsx/plugins/nsx_v/plugin.py

@@ -65,7 +65,6 @@ from neutron.db import portsecurity_db
 from neutron.db import quota_db  # noqa
 from neutron.db import securitygroups_db
 from neutron.db import vlantransparent_db
-from neutron.extensions import address_scope as ext_address_scope
 from neutron.extensions import allowedaddresspairs as addr_pair
 from neutron.extensions import availability_zone as az_ext
 from neutron.extensions import external_net as ext_net_extn
@@ -3234,11 +3233,6 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
         intf_net_ids = list(set([port['network_id'] for port in intf_ports]))
         return intf_net_ids
 
-    def _get_router_interfaces(self, context, router_id):
-        port_filters = {'device_id': [router_id],
-                        'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF]}
-        return self.get_ports(context, filters=port_filters)
-
     def _get_address_groups(self, context, router_id, network_id):
         address_groups = []
         ports = self._get_router_interface_ports_by_network(
@@ -3422,31 +3416,6 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                 context, subnet_id)['network_id']
         return net_id, subnet_id
 
-    def _get_network_address_scope(self, context, net_id):
-        network = self.get_network(context, net_id)
-        return network.get(ext_address_scope.IPV4_ADDRESS_SCOPE)
-
-    def _get_subnet_address_scope(self, context, subnet_id):
-        subnet = self.get_subnet(context, subnet_id)
-        if not subnet['subnetpool_id']:
-            return
-        subnetpool = self.get_subnetpool(context, subnet['subnetpool_id'])
-        return subnetpool.get('address_scope_id', '')
-
-    # TODO(asarfaty): the NSX-V3 needs a very similar code too
-    def _validate_address_scope_for_router_interface(self, context, router_id,
-                                                     gw_network_id, subnet_id):
-        gw_address_scope = self._get_network_address_scope(context,
-                                                           gw_network_id)
-        if not gw_address_scope:
-            return
-        subnet_address_scope = self._get_subnet_address_scope(context,
-                                                              subnet_id)
-        if (not subnet_address_scope or
-            subnet_address_scope != gw_address_scope):
-            raise nsx_exc.NsxRouterInterfaceDoesNotMatchAddressScope(
-                router_id=router_id, address_scope_id=gw_address_scope)
-
     def add_router_interface(self, context, router_id, interface_info):
         router = self.get_router(context, router_id)
         net_id, subnet_id = self._get_interface_info(context, interface_info)

vmware_nsx/plugins/nsx_v3/plugin.py

@@ -2890,6 +2890,18 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
         # TODO(berlin): admin_state_up support
         gw_info = self._extract_external_gw(context, router, is_extract=False)
         router_data = router['router']
+
+        # if setting this router as no-snat, make sure gw address scope match
+        # those of the subnets
+        if (validators.is_attr_set(gw_info) and
+            not gw_info.get('enable_snat', cfg.CONF.enable_snat_by_default)):
+            router_ports = self._get_router_interfaces(context, router_id)
+            for port in router_ports:
+                for fip in port['fixed_ips']:
+                    self._validate_address_scope_for_router_interface(
+                        context, router_id,
+                        gw_info['network_id'], fip['subnet_id'])
+
         nsx_router_id = None
         routes_added = []
         routes_removed = []
@@ -3050,6 +3062,14 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
             network_id = subnet['network_id']
             nsx_net_id, nsx_port_id = nsx_db.get_nsx_switch_and_port_id(
                 context.session, port['id'])
+            router_db = self._get_router(context, router_id)
+
+            # If it is a no-snat router, interface address scope must be the
+            # same as the gateways
+            if not router_db.enable_snat:
+                gw_network_id = router_db.gw_port.network_id
+                self._validate_address_scope_for_router_interface(
+                    context, router_id, gw_network_id, subnet['id'])
 
             nsx_router_id = nsx_db.get_nsx_router_id(context.session,
                                                      router_id)
@@ -3069,7 +3089,6 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                 logical_switch_port_id=nsx_port_id,
                 address_groups=address_groups)
 
-            router_db = self._get_router(context, router_id)
             if router_db.gw_port and not router_db.enable_snat:
                 # TODO(berlin): Announce the subnet on tier0 if enable_snat
                 # is False

vmware_nsx/tests/unit/nsx_v3/test_plugin.py

@@ -14,11 +14,13 @@
 # limitations under the License.
 
 import mock
+import netaddr
 import six
 from webob import exc
 
 from neutron.api.v2 import attributes
 from neutron.db import models_v2
+from neutron.extensions import address_scope
 from neutron.extensions import external_net
 from neutron.extensions import extraroute
 from neutron.extensions import l3
@@ -26,6 +28,7 @@ from neutron.extensions import l3_ext_gw_mode
 from neutron.extensions import securitygroup as secgrp
 from neutron.tests.unit import _test_extension_portbindings as test_bindings
 from neutron.tests.unit.db import test_db_base_plugin_v2 as test_plugin
+from neutron.tests.unit.extensions import test_address_scope
 from neutron.tests.unit.extensions import test_extra_dhcp_opt as test_dhcpopts
 from neutron.tests.unit.extensions import test_extraroute as test_ext_route
 from neutron.tests.unit.extensions import test_l3 as test_l3_plugin
@@ -535,7 +538,10 @@ class TestL3ExtensionManager(object):
         # Finally add l3 resources to the global attribute map
         attributes.RESOURCE_ATTRIBUTE_MAP.update(
             l3.RESOURCE_ATTRIBUTE_MAP)
-        return l3.L3.get_resources()
+        attributes.RESOURCE_ATTRIBUTE_MAP.update(
+            address_scope.RESOURCE_ATTRIBUTE_MAP)
+        return (l3.L3.get_resources() +
+                address_scope.Address_scope.get_resources())
 
     def get_actions(self):
         return []
@@ -555,7 +561,8 @@ def restore_l3_attribute_map(map_to_restore):
     l3.RESOURCE_ATTRIBUTE_MAP = map_to_restore
 
 
-class L3NatTest(test_l3_plugin.L3BaseForIntTests, NsxV3PluginTestCaseMixin):
+class L3NatTest(test_l3_plugin.L3BaseForIntTests, NsxV3PluginTestCaseMixin,
+                test_address_scope.AddressScopeTestCase):
 
     def _restore_l3_attribute_map(self):
         l3.RESOURCE_ATTRIBUTE_MAP = self._l3_attribute_map_bk
@@ -759,6 +766,131 @@ class TestL3NatTestCase(L3NatTest,
     def test_floatingip_update_to_same_port_id_twice(self):
         self.skipTest('Plugin changes floating port status')
 
+    def _test_create_subnetpool(self, prefixes, expected=None,
+                                admin=False, **kwargs):
+        keys = kwargs.copy()
+        keys.setdefault('tenant_id', self._tenant_id)
+        with self.subnetpool(prefixes, admin, **keys) as subnetpool:
+            self._validate_resource(subnetpool, keys, 'subnetpool')
+            if expected:
+                self._compare_resource(subnetpool, expected, 'subnetpool')
+        return subnetpool
+
+    def _update_router_enable_snat(self, router_id, network_id, enable_snat):
+        return self._update('routers', router_id,
+                            {'router': {'external_gateway_info':
+                                        {'network_id': network_id,
+                                         'enable_snat': enable_snat}}})
+
+    def test_router_no_snat_with_different_address_scope(self):
+        """Test that if the router has no snat, you cannot add an interface
+        from a different address scope than the gateway.
+        """
+        # create an external network on one address scope
+        with self.address_scope(name='as1') as addr_scope, \
+            self.network() as ext_net:
+            self._set_net_external(ext_net['network']['id'])
+            as_id = addr_scope['address_scope']['id']
+            subnet = netaddr.IPNetwork('10.10.10.0/24')
+            subnetpool = self._test_create_subnetpool(
+                [subnet.cidr], name='sp1',
+                min_prefixlen='24', address_scope_id=as_id)
+            subnetpool_id = subnetpool['subnetpool']['id']
+            data = {'subnet': {
+                    'network_id': ext_net['network']['id'],
+                    'subnetpool_id': subnetpool_id,
+                    'ip_version': 4,
+                    'enable_dhcp': False,
+                    'tenant_id': ext_net['network']['tenant_id']}}
+            req = self.new_create_request('subnets', data)
+            ext_subnet = self.deserialize(self.fmt, req.get_response(self.api))
+
+            # create a regular network on another address scope
+            with self.address_scope(name='as2') as addr_scope2, \
+                self.network() as net:
+                as_id2 = addr_scope2['address_scope']['id']
+                subnet2 = netaddr.IPNetwork('20.10.10.0/24')
+                subnetpool2 = self._test_create_subnetpool(
+                    [subnet2.cidr], name='sp2',
+                    min_prefixlen='24', address_scope_id=as_id2)
+                subnetpool_id2 = subnetpool2['subnetpool']['id']
+                data = {'subnet': {
+                        'network_id': net['network']['id'],
+                        'subnetpool_id': subnetpool_id2,
+                        'ip_version': 4,
+                        'tenant_id': net['network']['tenant_id']}}
+                req = self.new_create_request('subnets', data)
+                int_subnet = self.deserialize(
+                    self.fmt, req.get_response(self.api))
+
+                # create a no snat router with this gateway
+                with self.router() as r:
+                    self._add_external_gateway_to_router(
+                        r['router']['id'],
+                        ext_subnet['subnet']['network_id'])
+                    self._update_router_enable_snat(
+                        r['router']['id'],
+                        ext_subnet['subnet']['network_id'],
+                        False)
+
+                    # should fail adding the interface to the router
+                    err_code = exc.HTTPBadRequest.code
+                    self._router_interface_action('add',
+                                                  r['router']['id'],
+                                                  int_subnet['subnet']['id'],
+                                                  None,
+                                                  err_code)
+
+    def test_router_no_snat_with_same_address_scope(self):
+        """Test that if the router has no snat, you can add an interface
+        from the same address scope as the gateway.
+        """
+        # create an external network on one address scope
+        with self.address_scope(name='as1') as addr_scope, \
+            self.network() as ext_net:
+            self._set_net_external(ext_net['network']['id'])
+            as_id = addr_scope['address_scope']['id']
+            subnet = netaddr.IPNetwork('10.10.10.0/21')
+            subnetpool = self._test_create_subnetpool(
+                [subnet.cidr], name='sp1',
+                min_prefixlen='24', address_scope_id=as_id)
+            subnetpool_id = subnetpool['subnetpool']['id']
+            data = {'subnet': {
+                    'network_id': ext_net['network']['id'],
+                    'subnetpool_id': subnetpool_id,
+                    'ip_version': 4,
+                    'enable_dhcp': False,
+                    'tenant_id': ext_net['network']['tenant_id']}}
+            req = self.new_create_request('subnets', data)
+            ext_subnet = self.deserialize(self.fmt, req.get_response(self.api))
+
+            # create a regular network on the same address scope
+            with self.network() as net:
+                data = {'subnet': {
+                        'network_id': net['network']['id'],
+                        'subnetpool_id': subnetpool_id,
+                        'ip_version': 4,
+                        'tenant_id': net['network']['tenant_id']}}
+                req = self.new_create_request('subnets', data)
+                int_subnet = self.deserialize(
+                    self.fmt, req.get_response(self.api))
+
+                # create a no snat router with this gateway
+                with self.router() as r:
+                    self._add_external_gateway_to_router(
+                        r['router']['id'],
+                        ext_subnet['subnet']['network_id'])
+                    self._update_router_enable_snat(
+                        r['router']['id'],
+                        ext_subnet['subnet']['network_id'],
+                        False)
+
+                    # should succeed adding the interface to the router
+                    self._router_interface_action('add',
+                                                  r['router']['id'],
+                                                  int_subnet['subnet']['id'],
+                                                  None)
+
 
 class ExtGwModeTestCase(test_ext_gw_mode.ExtGwModeIntTestCase,
                         L3NatTest):