Make sure that gateway is in CIDR range by default

Git commit c3706fa2 introduced the force_gateway_on_subnet
option that verified that the defined gateway is in the CIDR
range of a newly created or updated subnet. However, the default
value was False for backwards compatability reasons. The default
will change to True and the option will be marked as deprecated.

For IPv6, the gateway must be in the CIDR only if the gateway
is not a link local address.

DocImpact
Change-Id: I04fd1caec6da5dceee3f736b3f91f2468150ba2a
Closes-Bug: #1304181

diff --git a/etc/neutron.conf b/etc/neutron.conf
index ec77905e165efc4a7ef55cee461119a21df25fa1..456d52c89772cb05b0cabf41680a76a923eacde6 100644 (file)
--- a/etc/neutron.conf
+++ b/etc/neutron.conf
@@ -97,8 +97,10 @@ lock_path = $state_path/lock
 # Attention: the following parameter MUST be set to False if Neutron is
 # being used in conjunction with nova security groups
 # allow_overlapping_ips = False
-# Ensure that configured gateway is on subnet
-# force_gateway_on_subnet = False
+# Ensure that configured gateway is on subnet. For IPv6, validate only if
+# gateway is not a link local address. Deprecated, to be removed during the
+# K release, at which point the check will be mandatory.
+# force_gateway_on_subnet = True
 
 # Default maximum number of items returned in a single response,
 # value == infinite and value < 0 means no max limit, and value must

diff --git a/neutron/common/config.py b/neutron/common/config.py
index 0a8232fa02befab12e0344c6a3696076e3673cef..78e6a9f54d9608b8fb739242fa6de037a38c136a 100644 (file)
--- a/neutron/common/config.py
+++ b/neutron/common/config.py
@@ -80,8 +80,12 @@ core_opts = [
                 help=_("Allow overlapping IP support in Neutron")),
     cfg.StrOpt('host', default=utils.get_hostname(),
                help=_("The hostname Neutron is running on")),
-    cfg.BoolOpt('force_gateway_on_subnet', default=False,
-                help=_("Ensure that configured gateway is on subnet")),
+    cfg.BoolOpt('force_gateway_on_subnet', default=True,
+                help=_("Ensure that configured gateway is on subnet. "
+                       "For IPv6, validate only if gateway is not a link "
+                       "local address. Deprecated, to be removed during the "
+                       "K release, at which point the check will be "
+                       "mandatory.")),
     cfg.BoolOpt('notify_nova_on_port_status_changes', default=True,
                 help=_("Send notification to nova when port status changes")),
     cfg.BoolOpt('notify_nova_on_port_data_changes', default=True,

diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py
index 4d804f559d896f9063fb43401d7e8fb6c003001f..6ad0f27e6c98b48e97d5170e9956f60a71b0ec28 100644 (file)
--- a/neutron/db/db_base_plugin_v2.py
+++ b/neutron/db/db_base_plugin_v2.py
@@ -491,8 +491,16 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
             return True
         return False
 
-    @staticmethod
-    def _check_subnet_ip(cidr, ip_address):
+    @classmethod
+    def _check_gateway_in_subnet(cls, cidr, gateway):
+        """Validate that the gateway is on the subnet."""
+        ip = netaddr.IPAddress(gateway)
+        if ip.version == 4 or (ip.version == 6 and not ip.is_link_local()):
+            return cls._check_subnet_ip(cidr, gateway)
+        return True
+
+    @classmethod
+    def _check_subnet_ip(cls, cidr, ip_address):
         """Validate that the IP address is on the subnet."""
         ip = netaddr.IPAddress(ip_address)
         net = netaddr.IPNetwork(cidr)
@@ -549,8 +557,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
                 filter = {'network_id': [network_id]}
                 subnets = self.get_subnets(context, filters=filter)
                 for subnet in subnets:
-                    if NeutronDbPluginV2._check_subnet_ip(subnet['cidr'],
-                                                          fixed['ip_address']):
+                    if self._check_subnet_ip(subnet['cidr'],
+                                             fixed['ip_address']):
                         found = True
                         subnet_id = subnet['id']
                         break
@@ -579,8 +587,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
 
                 # Ensure that the IP is valid on the subnet
                 if (not found and
-                    not NeutronDbPluginV2._check_subnet_ip(
-                        subnet['cidr'], fixed['ip_address'])):
+                    not self._check_subnet_ip(subnet['cidr'],
+                                              fixed['ip_address'])):
                     msg = _('IP address %s is not a valid IP for the defined '
                             'subnet') % fixed['ip_address']
                     raise n_exc.InvalidInput(error_message=msg)
@@ -1099,8 +1107,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
         if attributes.is_attr_set(s.get('gateway_ip')):
             self._validate_ip_version(ip_ver, s['gateway_ip'], 'gateway_ip')
             if (cfg.CONF.force_gateway_on_subnet and
-                not NeutronDbPluginV2._check_subnet_ip(s['cidr'],
-                                                       s['gateway_ip'])):
+                not self._check_gateway_in_subnet(
+                    s['cidr'], s['gateway_ip'])):
                 error_message = _("Gateway is not valid on subnet")
                 raise n_exc.InvalidInput(error_message=error_message)
             # Ensure the gateway IP is not assigned to any port

diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py
index 9bab5382c8b3c5899b4bf7b9a1ece2e274ab82e0..9dd5a800b7d50adf5c0a830236c710b86f2bb5af 100644 (file)
--- a/neutron/tests/unit/ml2/test_ml2_plugin.py
+++ b/neutron/tests/unit/ml2/test_ml2_plugin.py
@@ -461,7 +461,7 @@ class TestFaultyMechansimDriver(Ml2PluginV2FaultyDriverTestCase):
                                    'name': 'subnet1',
                                    'tenant_id':
                                    network['network']['tenant_id'],
-                                   'gateway_ip': '10.0.2.1'}}
+                                   'gateway_ip': '10.0.20.1'}}
                 req = self.new_create_request('subnets', data)
                 res = req.get_response(self.api)
                 self.assertEqual(500, res.status_int)
@@ -488,7 +488,7 @@ class TestFaultyMechansimDriver(Ml2PluginV2FaultyDriverTestCase):
                                        'name': 'subnet1',
                                        'tenant_id':
                                        network['network']['tenant_id'],
-                                       'gateway_ip': '10.0.2.1'}}
+                                       'gateway_ip': '10.0.20.1'}}
                     subnet_req = self.new_create_request('subnets', data)
                     subnet_res = subnet_req.get_response(self.api)
                     self.assertEqual(201, subnet_res.status_int)
@@ -519,7 +519,7 @@ class TestFaultyMechansimDriver(Ml2PluginV2FaultyDriverTestCase):
                                        'name': 'subnet1',
                                        'tenant_id':
                                        network['network']['tenant_id'],
-                                       'gateway_ip': '10.0.2.1'}}
+                                       'gateway_ip': '10.0.20.1'}}
                     subnet_req = self.new_create_request('subnets', data)
                     subnet_res = subnet_req.get_response(self.api)
                     self.assertEqual(201, subnet_res.status_int)

diff --git a/neutron/tests/unit/nuage/test_nuage_plugin.py b/neutron/tests/unit/nuage/test_nuage_plugin.py
index afbc91bb9a63f85d346423b70f069bf147fa2620..426122912ecc551b5b4a49e18d3df08df432d520 100644 (file)
--- a/neutron/tests/unit/nuage/test_nuage_plugin.py
+++ b/neutron/tests/unit/nuage/test_nuage_plugin.py
@@ -216,6 +216,10 @@ class TestNuageSubnetsV2(NuagePluginV2TestCase,
         self.skipTest("Plugin does not support "
                       "Neutron Subnet no-gateway option")
 
+    def test_create_subnet_nonzero_cidr(self):
+        self.skipTest("Plugin does not support "
+                      "Neutron Subnet no-gateway option")
+
     def test_create_subnet_with_none_gateway_fully_allocated(self):
         self.skipTest("Plugin does not support Neutron "
                       "Subnet no-gateway option")
@@ -235,7 +239,9 @@ class TestNuagePluginPortBinding(NuagePluginV2TestCase,
 
 class TestNuagePortsV2(NuagePluginV2TestCase,
                        test_db_plugin.TestPortsV2):
-    pass
+    def test_no_more_port_exception(self):
+        self.skipTest("Plugin does not support "
+                      "Neutron Subnet no-gateway option")
 
 
 class TestNuageL3NatTestCase(NuagePluginV2TestCase,

diff --git a/neutron/tests/unit/oneconvergence/test_nvsd_plugin.py b/neutron/tests/unit/oneconvergence/test_nvsd_plugin.py
index 564b8c56a81d4cab21ac1dd1486a3dacfc54d974..d186f1a4d0c210567bf4501234c9022e58797e68 100644 (file)
--- a/neutron/tests/unit/oneconvergence/test_nvsd_plugin.py
+++ b/neutron/tests/unit/oneconvergence/test_nvsd_plugin.py
@@ -87,6 +87,12 @@ class TestOneConvergencePluginSubnetsV2(test_plugin.TestSubnetsV2,
     def test_update_subnet_ipv6_inconsistent_address_attribute(self):
         self.skipTest("NVSD Plugin does not support IPV6.")
 
+    def test_create_subnet_ipv6_out_of_cidr_global(self):
+        self.skipTest("NVSD Plugin does not support IPV6.")
+
+    def test_create_subnet_ipv6_out_of_cidr_lla(self):
+        self.skipTest("NVSD Plugin does not support IPV6.")
+
 
 class TestOneConvergencePluginPortsV2(test_plugin.TestPortsV2,
                                       test_bindings.PortBindingsTestCase,

diff --git a/neutron/tests/unit/test_db_plugin.py b/neutron/tests/unit/test_db_plugin.py
index c196b8afbd0b9d178da56665f15b65bd8b295c5a..50d73ff4137369508bece8e95f0b6fe0e490e020 100644 (file)
--- a/neutron/tests/unit/test_db_plugin.py
+++ b/neutron/tests/unit/test_db_plugin.py
@@ -1144,7 +1144,7 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s
                                  data['port']['fixed_ips'])
 
     def test_no_more_port_exception(self):
-        with self.subnet(cidr='10.0.0.0/32') as subnet:
+        with self.subnet(cidr='10.0.0.0/32', gateway_ip=None) as subnet:
             id = subnet['subnet']['network_id']
             res = self._create_port(self.fmt, id)
             data = self.deserialize(self.fmt, res)
@@ -2583,7 +2583,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
             self.subnet(cidr='14.129.122.5/22'),
             self.subnet(cidr='15.129.122.5/24'),
             self.subnet(cidr='16.129.122.5/28'),
-            self.subnet(cidr='17.129.122.5/32')
+            self.subnet(cidr='17.129.122.5/32', gateway_ip=None)
         ) as subs:
             # the API should accept and correct these for users
             self.assertEqual(subs[0]['subnet']['cidr'], '10.0.0.0/8')
@@ -2724,15 +2724,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                          allocation_pools)
 
     def test_create_subnet_gw_values(self):
-        # Gateway not in subnet
-        gateway = '100.0.0.1'
         cidr = '10.0.0.0/24'
-        allocation_pools = [{'start': '10.0.0.1',
-                             'end': '10.0.0.254'}]
-        expected = {'gateway_ip': gateway,
-                    'cidr': cidr,
-                    'allocation_pools': allocation_pools}
-        self._test_create_subnet(expected=expected, gateway_ip=gateway)
         # Gateway is last IP in range
         gateway = '10.0.0.254'
         allocation_pools = [{'start': '10.0.0.1',
@@ -2751,8 +2743,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
         self._test_create_subnet(expected=expected,
                                  gateway_ip=gateway)
 
-    def test_create_subnet_gw_outside_cidr_force_on_returns_400(self):
-        cfg.CONF.set_override('force_gateway_on_subnet', True)
+    def test_create_subnet_gw_outside_cidr_returns_400(self):
         with self.network() as network:
             self._create_subnet(self.fmt,
                                 network['network']['id'],
@@ -2760,8 +2751,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                                 webob.exc.HTTPClientError.code,
                                 gateway_ip='100.0.0.1')
 
-    def test_create_subnet_gw_of_network_force_on_returns_400(self):
-        cfg.CONF.set_override('force_gateway_on_subnet', True)
+    def test_create_subnet_gw_of_network_returns_400(self):
         with self.network() as network:
             self._create_subnet(self.fmt,
                                 network['network']['id'],
@@ -2769,8 +2759,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                                 webob.exc.HTTPClientError.code,
                                 gateway_ip='10.0.0.0')
 
-    def test_create_subnet_gw_bcast_force_on_returns_400(self):
-        cfg.CONF.set_override('force_gateway_on_subnet', True)
+    def test_create_subnet_gw_bcast_returns_400(self):
         with self.network() as network:
             self._create_subnet(self.fmt,
                                 network['network']['id'],
@@ -3038,6 +3027,28 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                                      ipv6_ra_mode=mode,
                                      ipv6_address_mode=mode)
 
+    def test_create_subnet_ipv6_out_of_cidr_global(self):
+        gateway_ip = '2000::1'
+        cidr = '2001::/64'
+
+        with testlib_api.ExpectedException(
+            webob.exc.HTTPClientError) as ctx_manager:
+            self._test_create_subnet(
+                gateway_ip=gateway_ip, cidr=cidr, ip_version=6,
+                ipv6_ra_mode=constants.DHCPV6_STATEFUL,
+                ipv6_address_mode=constants.DHCPV6_STATEFUL)
+        self.assertEqual(ctx_manager.exception.code,
+                         webob.exc.HTTPClientError.code)
+
+    def test_create_subnet_ipv6_out_of_cidr_lla(self):
+        gateway_ip = 'fe80::1'
+        cidr = '2001::/64'
+
+        self._test_create_subnet(
+            gateway_ip=gateway_ip, cidr=cidr, ip_version=6,
+            ipv6_ra_mode=constants.IPV6_SLAAC,
+            ipv6_address_mode=constants.IPV6_SLAAC)
+
     def test_create_subnet_ipv6_attributes_no_dhcp_enabled(self):
         gateway_ip = 'fe80::1'
         cidr = 'fe80::/80'
@@ -3122,7 +3133,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
 
     def test_update_subnet_no_gateway(self):
         with self.subnet() as subnet:
-            data = {'subnet': {'gateway_ip': '11.0.0.1'}}
+            data = {'subnet': {'gateway_ip': '10.0.0.1'}}
             req = self.new_update_request('subnets', data,
                                           subnet['subnet']['id'])
             res = self.deserialize(self.fmt, req.get_response(self.api))
@@ -3136,7 +3147,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
 
     def test_update_subnet(self):
         with self.subnet() as subnet:
-            data = {'subnet': {'gateway_ip': '11.0.0.1'}}
+            data = {'subnet': {'gateway_ip': '10.0.0.1'}}
             req = self.new_update_request('subnets', data,
                                           subnet['subnet']['id'])
             res = self.deserialize(self.fmt, req.get_response(self.api))
@@ -3182,8 +3193,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                 self.assertEqual(res.status_int,
                                  webob.exc.HTTPClientError.code)
 
-    def test_update_subnet_gw_outside_cidr_force_on_returns_400(self):
-        cfg.CONF.set_override('force_gateway_on_subnet', True)
+    def test_update_subnet_gw_outside_cidr_returns_400(self):
         with self.network() as network:
             with self.subnet(network=network) as subnet:
                 data = {'subnet': {'gateway_ip': '100.0.0.1'}}

diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py
index 4eb80d0d33dfb3d6941b29e834804ddf8fed5632..324f8bf8ade9a1168622de4a3ec781666700d670 100644 (file)
--- a/neutron/tests/unit/test_l3_plugin.py
+++ b/neutron/tests/unit/test_l3_plugin.py
@@ -880,8 +880,6 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                 try_overlapped_cidr('10.0.1.0/24')
                 # another subnet with overlapped cidr including s1
                 try_overlapped_cidr('10.0.0.0/16')
-                # another subnet with overlapped cidr included by s1
-                try_overlapped_cidr('10.0.1.1/32')
                 # clean-up
                 self._router_interface_action('remove',
                                               r['router']['id'],