Validate updated allocation pool before using it

The allocation pool specified during subnet-update is being
used in gateway validation checks before the allocation pool
is ever validated.  Errors indicating that the gateway IP is
invalid are returned when it is the allocation pool that is
actually in error.

Additional testing of the allocation pool is also being added
to the subnet-update unit tests.

Closes-Bug: #1479948
Change-Id: I854333d571bad2550ea5a0b93ff64ce44d0bd010

diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py
index f417c04775cbc2bddf91ce64d20cd910b951f97e..29ecf806525ffceba8ae19340126dc996ceb3259 100644 (file)
--- a/neutron/db/db_base_plugin_v2.py
+++ b/neutron/db/db_base_plugin_v2.py
@@ -623,6 +623,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
         if s.get('allocation_pools') is not None:
             # Convert allocation pools to IPRange to simplify future checks
             range_pools = self.ipam.pools_to_ip_range(s['allocation_pools'])
+            self.ipam.validate_allocation_pools(range_pools, s['cidr'])
             s['allocation_pools'] = range_pools
 
         update_ports_needed = False

diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py
index 398efa15ee39119811b5cd0180efdce517f03b6f..43ef9800206e8c555e27f4a13449db43905922b5 100644 (file)
--- a/neutron/db/ipam_backend_mixin.py
+++ b/neutron/db/ipam_backend_mixin.py
@@ -186,8 +186,6 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
                 context, subnet_id, s)
 
         if "allocation_pools" in s:
-            self._validate_allocation_pools(s['allocation_pools'],
-                                            s['cidr'])
             changes['allocation_pools'] = (
                 self._update_subnet_allocation_pools(context, subnet_id, s))
 
@@ -245,7 +243,7 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
                     new_subnetpool_id != subnet.subnetpool_id):
                 raise n_exc.NetworkSubnetPoolAffinityError()
 
-    def _validate_allocation_pools(self, ip_pools, subnet_cidr):
+    def validate_allocation_pools(self, ip_pools, subnet_cidr):
         """Validate IP allocation pools.
 
         Verify start and end address for each allocation pool are valid,
@@ -342,7 +340,7 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
             return self.generate_pools(cidr, gateway_ip)
 
         ip_range_pools = self.pools_to_ip_range(allocation_pools)
-        self._validate_allocation_pools(ip_range_pools, cidr)
+        self.validate_allocation_pools(ip_range_pools, cidr)
         if gateway_ip:
             self.validate_gw_out_of_pools(gateway_ip, ip_range_pools)
         return ip_range_pools

diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py
index dd35ac0b2719b8acf00b47f2f6134689119928d6..17e1371c375d81743e1d1f6751895dc2c3d4f090 100644 (file)
--- a/neutron/db/ipam_pluggable_backend.py
+++ b/neutron/db/ipam_pluggable_backend.py
@@ -137,8 +137,8 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
         return allocated
 
     def _ipam_update_allocation_pools(self, context, ipam_driver, subnet):
-        self._validate_allocation_pools(subnet['allocation_pools'],
-                                        subnet['cidr'])
+        self.validate_allocation_pools(subnet['allocation_pools'],
+                                       subnet['cidr'])
 
         factory = ipam_driver.get_subnet_request_factory()
         subnet_request = factory.get_request(context, subnet, None)

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 7e2538d352ae4750cc9126f758765484ef703358..8bdb54bbd0458095611379a577da82ef3b90ced5 100644 (file)
--- a/neutron/tests/unit/db/test_db_base_plugin_v2.py
+++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py
@@ -4245,6 +4245,47 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                 self.assertEqual(res.status_int,
                                  webob.exc.HTTPConflict.code)
 
+    def test_update_subnet_allocation_pools_invalid_returns_400(self):
+        allocation_pools = [{'start': '10.0.0.2', 'end': '10.0.0.254'}]
+        with self.network() as network:
+            with self.subnet(network=network,
+                             allocation_pools=allocation_pools,
+                             cidr='10.0.0.0/24') as subnet:
+                # Check allocation pools
+                invalid_pools = [[{'end': '10.0.0.254'}],
+                                 [{'start': '10.0.0.254'}],
+                                 [{'start': '1000.0.0.254'}],
+                                 [{'start': '10.0.0.2', 'end': '10.0.0.254'},
+                                  {'end': '10.0.0.254'}],
+                                 None,
+                                 [{'start': '10.0.0.200', 'end': '10.0.3.20'}],
+                                 [{'start': '10.0.2.250', 'end': '10.0.3.5'}],
+                                 [{'start': '10.0.0.0', 'end': '10.0.0.50'}],
+                                 [{'start': '10.0.2.10', 'end': '10.0.2.5'}],
+                                 [{'start': 'fe80::2', 'end': 'fe80::ffff'}]]
+                for pool in invalid_pools:
+                    data = {'subnet': {'allocation_pools': pool}}
+                    req = self.new_update_request('subnets', data,
+                                                  subnet['subnet']['id'])
+                    res = req.get_response(self.api)
+                    self.assertEqual(res.status_int,
+                                     webob.exc.HTTPClientError.code)
+
+    def test_update_subnet_allocation_pools_overlapping_returns_409(self):
+        allocation_pools = [{'start': '10.0.0.2', 'end': '10.0.0.254'}]
+        with self.network() as network:
+            with self.subnet(network=network,
+                             allocation_pools=allocation_pools,
+                             cidr='10.0.0.0/24') as subnet:
+                data = {'subnet': {'allocation_pools': [
+                        {'start': '10.0.0.20', 'end': '10.0.0.40'},
+                        {'start': '10.0.0.30', 'end': '10.0.0.50'}]}}
+                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(
             self, allocation_pools, cidr):
         ip_version = netaddr.IPNetwork(cidr).version