Add check for subnet update with conflict gateway and allocation_pools

Fixes: bug 1062061

The patch will raise exception 'GatewayConflictWithAllocationPools' when
subnet update with conflict gateway and allocation_pools.

Because before validate gateway ip with conflict allocation pools, we need
validate allocation pools first. Move the validation of allocation pools
into _validate_subnet. Then_allocate_pools_for_subnet is only responsible
for pools allocation, and_validate_subnet is responsible for most validate
for subnet.

Change-Id: I8dffe45ce5c02e8e6c317bad470054564538bcf2

quantum/common/exceptions.py

@@ -249,3 +249,8 @@ class TooManyExternalNetworks(QuantumException):
 class InvalidConfigurationOption(QuantumException):
     message = _("An invalid value was provided for %(opt_name)s: "
                 "%(opt_value)s")
+
+
+class GatewayConflictWithAllocationPools(InUse):
+    message = _("Gateway ip %(ip_address)s conflicts with "
+                "allocation pool %(pool)s")

quantum/db/db_base_plugin_v2.py

@@ -706,14 +706,13 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
                            'cidr': subnet.cidr})
                 raise q_exc.InvalidInput(error_message=err_msg)
 
-    def _validate_allocation_pools(self, ip_pools, gateway_ip, 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,
         ie: constituted by valid and appropriately ordered IP addresses.
-        Also, verify pools do not overlap among themselves and with the
+        Also, verify pools do not overlap among themselves.
-        gateway IP. Finally, verify that each range, and the gateway IP,
+        Finally, verify that each range fall within the subnet's CIDR.
-        fall within the subnet's CIDR.
 
         """
 
@@ -760,10 +759,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
         LOG.debug(_("Checking for overlaps among allocation pools "
                     "and gateway ip"))
         ip_ranges = ip_pools[:]
-        # Treat gw as IPset as well
+
-        if gateway_ip:
-            ip_ranges.append(gateway_ip)
-            ip_sets.append(netaddr.IPSet([gateway_ip]))
         # Use integer cursors as an efficient way for implementing
         # comparison and avoiding comparing the same pair twice
         for l_cursor in range(len(ip_sets)):
@@ -803,7 +799,6 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
         """
 
         pools = []
-        if subnet['allocation_pools'] is attributes.ATTR_NOT_SPECIFIED:
         # Auto allocate the pool around gateway_ip
         net = netaddr.IPNetwork(subnet['cidr'])
         first_ip = net.first + 1
@@ -821,12 +816,6 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
         # return auto-generated pools
         # no need to check for their validity
         return pools
-        else:
-            pools = subnet['allocation_pools']
-            self._validate_allocation_pools(pools,
-                                            subnet['gateway_ip'],
-                                            subnet['cidr'])
-            return pools
 
     def _validate_shared_update(self, context, id, original, updated):
         # The only case that needs to be validated is when 'shared'
@@ -997,14 +986,18 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
     def _validate_subnet(self, s):
         """Validate a subnet spec"""
 
+        # This method will validate attributes which may change during
+        # create_subnet() and update_subnet().
         # The method requires the subnet spec 's' has 'ip_version' field.
         # If 's' dict does not have 'ip_version' field in an API call
         # (e.g., update_subnet()), you need to set 'ip_version' field
         # before calling this method.
+
         ip_ver = s['ip_version']
 
         if 'cidr' in s:
             self._validate_ip_version(ip_ver, s['cidr'], 'cidr')
+
         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
@@ -1036,14 +1029,34 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
             for rt in s['host_routes']:
                 self._validate_host_route(rt, ip_ver)
 
-    def create_subnet(self, context, subnet):
+    def _validate_gw_out_of_pools(self, gateway_ip, pools):
-        s = subnet['subnet']
+        for allocation_pool in pools:
-        self._validate_subnet(s)
+            pool_range = netaddr.IPRange(
+                allocation_pool['start'],
+                allocation_pool['end'])
+            if netaddr.IPAddress(gateway_ip) in pool_range:
+                raise q_exc.GatewayConflictWithAllocationPools(
+                    pool=pool_range,
+                    ip_address=gateway_ip)
 
+    def create_subnet(self, context, subnet):
+
+        s = subnet['subnet']
         net = netaddr.IPNetwork(s['cidr'])
+
         if s['gateway_ip'] is attributes.ATTR_NOT_SPECIFIED:
             s['gateway_ip'] = str(netaddr.IPAddress(net.first + 1))
 
+        if s['allocation_pools'] == attributes.ATTR_NOT_SPECIFIED:
+            s['allocation_pools'] = self._allocate_pools_for_subnet(context, s)
+        else:
+            self._validate_allocation_pools(s['allocation_pools'], s['cidr'])
+            if s['gateway_ip'] is not None:
+                self._validate_gw_out_of_pools(s['gateway_ip'],
+                                               s['allocation_pools'])
+
+        self._validate_subnet(s)
+
         tenant_id = self._get_tenant_id_for_create(context, s)
         with context.session.begin(subtransactions=True):
             network = self._get_network(context, s["network_id"])
@@ -1061,9 +1074,6 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
                     'shared': network.shared}
             subnet = models_v2.Subnet(**args)
 
-            # perform allocate pools first, since it might raise an error
-            pools = self._allocate_pools_for_subnet(context, s)
-
             context.session.add(subnet)
             if s['dns_nameservers'] is not attributes.ATTR_NOT_SPECIFIED:
                 for addr in s['dns_nameservers']:
@@ -1077,7 +1087,8 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
                                             destination=rt['destination'],
                                             nexthop=rt['nexthop'])
                     context.session.add(route)
-            for pool in pools:
+
+            for pool in s['allocation_pools']:
                 ip_pool = models_v2.IPAllocationPool(subnet=subnet,
                                                      first_ip=pool['start'],
                                                      last_ip=pool['end'])
@@ -1096,15 +1107,22 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
            gratuitous DHCP offers"""
 
         s = subnet['subnet']
-        # Fill 'ip_version' field with the current value since
+        db_subnet = self._get_subnet(context, id)
-        # _validate_subnet() expects subnet spec has 'ip_version' field.
+        # Fill 'ip_version' and 'allocation_pools' fields with the current
-        s['ip_version'] = self._get_subnet(context, id).ip_version
+        # value since _validate_subnet() expects subnet spec has 'ip_version'
+        # and 'allocation_pools' fields.
+        s['ip_version'] = db_subnet.ip_version
+        s['cidr'] = db_subnet.cidr
         self._validate_subnet(s)
 
+        if 'gateway_ip' in s:
+            allocation_pools = [{'start': p['first_ip'], 'end': p['last_ip']}
+                                for p in db_subnet.allocation_pools]
+            self._validate_gw_out_of_pools(s["gateway_ip"], allocation_pools)
+
         with context.session.begin(subtransactions=True):
             if "dns_nameservers" in s:
                 old_dns_list = self._get_dns_by_subnet(context, id)
-
                 new_dns_addr_set = set(s["dns_nameservers"])
                 old_dns_addr_set = set([dns['address']
                                         for dns in old_dns_list])

quantum/tests/unit/test_db_plugin.py

@@ -2540,6 +2540,18 @@ class TestSubnetsV2(QuantumDbPluginV2TestCase):
                 res = req.get_response(self.api)
                 self.assertEqual(res.status_int, 400)
 
+    def test_update_subnet_gateway_in_allocation_pool_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': {'gateway_ip': '10.0.0.50'}}
+                req = self.new_update_request('subnets', data,
+                                              subnet['subnet']['id'])
+                res = req.get_response(self.api)
+                self.assertEquals(res.status_int, 409)
+
     def test_show_subnet(self):
         with self.network() as network:
             with self.subnet(network=network) as subnet: