From: John Kasperski Date: Fri, 7 Aug 2015 14:15:33 +0000 (-0500) Subject: Validate updated allocation pool before using it X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=e64f5bdd22f7907cc77b9f94f5f6b6676dc57a93;p=openstack-build%2Fneutron-build.git 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 f417c0477..29ecf8065 100644 --- 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 398efa15e..43ef98002 100644 --- 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 dd35ac0b2..17e1371c3 100644 --- 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 7e2538d35..8bdb54bbd 100644 --- 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