Restrict subnet create/update to avoid DHCP resync

As we know, IPs in subnet CIDR are used for
1) Broadcast port
2) Gateway port
3) DHCP port if enable_dhcp is True, or update to True
4) Others go into allocation_pools
Above 1) to 3) are created by default, which means if CIDR doesn't
have that much of IPs, subnet create/update will cause a DHCP resync.

This fix is to add some restricts to the issue:
A) When subnet create, if enable_dhcp is True, /31 and /32
   cidrs are forbidden for IPv4 subnets while /127 and /128 cidrs are
   forbidden for IPv6 subnets.
B) When subnet update, if enable_dhcp is changing to True and there are no
   more IPs in allocation_pools, the request should be denied.

Change-Id: I2e4a4d5841b9ad908f02b7d0795cba07596c023d
Co-authored-by: Andrew Boik <dboik@cisco.com>
Closes-Bug: #1443798

diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py
index 714d189e81fb84966f248c6fec5c5ca46caf1253..2abdb948debcf0e7b3f29af5c910952da90ba1be 100644 (file)
--- a/neutron/db/db_base_plugin_v2.py
+++ b/neutron/db/db_base_plugin_v2.py
@@ -1052,6 +1052,30 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
         if attributes.is_attr_set(s.get('cidr')):
             self._validate_ip_version(ip_ver, s['cidr'], 'cidr')
 
+        # TODO(watanabe.isao): After we found a way to avoid the re-sync
+        # from the agent side, this restriction could be removed.
+        if cur_subnet:
+            dhcp_was_enabled = cur_subnet.enable_dhcp
+        else:
+            dhcp_was_enabled = False
+        if s.get('enable_dhcp') and not dhcp_was_enabled:
+            subnet_prefixlen = netaddr.IPNetwork(s['cidr']).prefixlen
+            error_message = _("Subnet has a prefix length that is "
+                              "incompatible with DHCP service enabled.")
+            if ((ip_ver == 4 and subnet_prefixlen > 30) or
+                (ip_ver == 6 and subnet_prefixlen > 126)):
+                    raise n_exc.InvalidInput(error_message=error_message)
+            else:
+                # NOTE(watanabe.isao): The following restriction is necessary
+                # only when updating subnet.
+                if cur_subnet:
+                    range_qry = context.session.query(models_v2.
+                        IPAvailabilityRange).join(models_v2.IPAllocationPool)
+                    ip_range = range_qry.filter_by(subnet_id=s['id']).first()
+                    if not ip_range:
+                        raise n_exc.IpAddressGenerationFailure(
+                            net_id=cur_subnet.network_id)
+
         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

diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py
index e8ef97e8fdb353e6dc107a49f152ed9e2462c57e..0804e9047d9da12a1e3dc9dc3bb8d87cbf7c132f 100644 (file)
--- a/neutron/tests/unit/db/test_db_base_plugin_v2.py
+++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py
@@ -1339,7 +1339,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', gateway_ip=None) as subnet:
+        with self.subnet(cidr='10.0.0.0/32', enable_dhcp=False) as subnet:
             id = subnet['subnet']['network_id']
             res = self._create_port(self.fmt, id)
             data = self.deserialize(self.fmt, res)
@@ -3223,7 +3223,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', gateway_ip=None)
+            self.subnet(cidr='17.129.122.5/32', enable_dhcp=False)
         ) as subs:
             # the API should accept and correct these for users
             self.assertEqual(subs[0]['subnet']['cidr'], '10.0.0.0/8')
@@ -3235,6 +3235,24 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
             self.assertEqual(subs[6]['subnet']['cidr'], '16.129.122.0/28')
             self.assertEqual(subs[7]['subnet']['cidr'], '17.129.122.5/32')
 
+    def _test_create_subnet_with_invalid_netmask_returns_400(self, *args):
+        with self.network() as network:
+            for cidr in args:
+                ip_version = netaddr.IPNetwork(cidr).version
+                self._create_subnet(self.fmt,
+                                    network['network']['id'],
+                                    cidr,
+                                    webob.exc.HTTPClientError.code,
+                                    ip_version=ip_version)
+
+    def test_create_subnet_with_invalid_netmask_returns_400_ipv4(self):
+        self._test_create_subnet_with_invalid_netmask_returns_400(
+                '10.0.0.0/31', '10.0.0.0/32')
+
+    def test_create_subnet_with_invalid_netmask_returns_400_ipv6(self):
+        self._test_create_subnet_with_invalid_netmask_returns_400(
+                'cafe:cafe::/127', 'cafe:cafe::/128')
+
     def test_create_subnet_bad_ip_version(self):
         with self.network() as network:
             # Check bad IP version
@@ -4153,6 +4171,36 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                 self.assertEqual(res.status_int,
                                  webob.exc.HTTPClientError.code)
 
+    def _test_subnet_update_enable_dhcp_no_ip_available_returns_409(
+            self, allocation_pools, cidr):
+        ip_version = netaddr.IPNetwork(cidr).version
+        with self.network() as network:
+            with self.subnet(network=network,
+                             allocation_pools=allocation_pools,
+                             enable_dhcp=False,
+                             cidr=cidr,
+                             ip_version=ip_version) as subnet:
+                id = subnet['subnet']['network_id']
+                self._create_port(self.fmt, id)
+                data = {'subnet': {'enable_dhcp': True}}
+                req = self.new_update_request('subnets', data,
+                                              subnet['subnet']['id'])
+                res = req.get_response(self.api)
+                self.assertEqual(res.status_int,
+                                 webob.exc.HTTPConflict.code)
+
+    def test_subnet_update_enable_dhcp_no_ip_available_returns_409_ipv4(self):
+        allocation_pools = [{'start': '10.0.0.2', 'end': '10.0.0.2'}]
+        cidr = '10.0.0.0/30'
+        self._test_subnet_update_enable_dhcp_no_ip_available_returns_409(
+                allocation_pools, cidr)
+
+    def test_subnet_update_enable_dhcp_no_ip_available_returns_409_ipv6(self):
+        allocation_pools = [{'start': '2001:db8::2', 'end': '2001:db8::2'}]
+        cidr = '2001:db8::/126'
+        self._test_subnet_update_enable_dhcp_no_ip_available_returns_409(
+                allocation_pools, cidr)
+
     def test_show_subnet(self):
         with self.network() as network:
             with self.subnet(network=network) as subnet: