From 16687e39b698bd2d95d343b403fa861de0b6648c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Thu, 27 Feb 2020 02:38:13 +0100 Subject: [PATCH] Filter subnets on fixed_ips segment For v6_stateless IP addresses for all stateless subnets within a network are implicitly included. When using segments implicitly allocating addresses across subnets on different segments is incorrect. IPs from subnets on differnt segments was allocated when no host binding information was available but a subnet_id in fixed_ips request was present. This change adds filtering based on segment_id when fixed_ips are used. If fixed_ips are not all on the same segment exception FixedIpsSubnetsNotOnSameSegment is raised. Related: rhbz#1803989 Related-Bug: #1864333 Related-Bug: #1865138 Closes-Bug: #1864225 Change-Id: I336ae76283f29dd226344fb454aaa0e4aac030ea (cherry picked from commit 7e09e72661b0b3a0f898c20d451e204aa7a17194) --- neutron/db/ipam_backend_mixin.py | 5 +- neutron/db/ipam_pluggable_backend.py | 7 +- neutron/objects/subnet.py | 46 +++++++++- neutron/services/segments/exceptions.py | 4 + .../unit/db/test_ipam_pluggable_backend.py | 3 +- neutron/tests/unit/extensions/test_segment.py | 91 +++++++++++++++++++ 6 files changed, 149 insertions(+), 7 deletions(-) diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index 15656d1880c..b8b4ee4945b 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -642,14 +642,15 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): return fixed_ip_list def _ipam_get_subnets(self, context, network_id, host, service_type=None, - fixed_configured=False): + fixed_configured=False, fixed_ips=None): """Return eligible subnets If no eligible subnets are found, determine why and potentially raise an appropriate error. """ subnets = subnet_obj.Subnet.find_candidate_subnets( - context, network_id, host, service_type, fixed_configured) + context, network_id, host, service_type, fixed_configured, + fixed_ips) if subnets: subnet_dicts = [self._make_subnet_dict(subnet, context=context) for subnet in subnets] diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py index 7282ef80ed2..533b335019c 100644 --- a/neutron/db/ipam_pluggable_backend.py +++ b/neutron/db/ipam_pluggable_backend.py @@ -230,11 +230,13 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): """ p = port['port'] fixed_configured = p['fixed_ips'] is not constants.ATTR_NOT_SPECIFIED + fixed_ips = p['fixed_ips'] if fixed_configured else [] subnets = self._ipam_get_subnets(context, network_id=p['network_id'], host=p.get(portbindings.HOST_ID), service_type=p.get('device_owner'), - fixed_configured=fixed_configured) + fixed_configured=fixed_configured, + fixed_ips=fixed_ips) v4, v6_stateful, v6_stateless = self._classify_subnets( context, subnets) @@ -345,7 +347,8 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): try: subnets = self._ipam_get_subnets( context, network_id=port['network_id'], host=host, - service_type=port.get('device_owner'), fixed_configured=True) + service_type=port.get('device_owner'), fixed_configured=True, + fixed_ips=changes.add + changes.original) except ipam_exc.DeferIpam: subnets = [] diff --git a/neutron/objects/subnet.py b/neutron/objects/subnet.py index a29113fb14a..e195841bef3 100644 --- a/neutron/objects/subnet.py +++ b/neutron/objects/subnet.py @@ -298,7 +298,7 @@ class Subnet(base.NeutronDbObject): @classmethod def find_candidate_subnets(cls, context, network_id, host, service_type, - fixed_configured): + fixed_configured, fixed_ips): """Find canditate subnets for the network, host, and service_type""" query = cls.query_subnets_on_network(context, network_id) query = SubnetServiceType.query_filter_service_subnets( @@ -311,7 +311,8 @@ class Subnet(base.NeutronDbObject): # the network are candidates. Host/Segment will be validated # on port update with binding:host_id set. Allocation _cannot_ # be deferred as requested fixed_ips would then be lost. - return query.all() + return cls._query_filter_by_fixed_ips_segment( + query, fixed_ips).all() # If the host isn't known, we can't allocate on a routed network. # So, exclude any subnets attached to segments. return cls._query_exclude_subnets_on_segments(query).all() @@ -332,6 +333,47 @@ class Subnet(base.NeutronDbObject): return [subnet for subnet, _mapping in results] + @classmethod + def _query_filter_by_fixed_ips_segment(cls, query, fixed_ips): + """Excludes subnets not on the same segment as fixed_ips + + :raises: FixedIpsSubnetsNotOnSameSegment + """ + segment_ids = [] + for fixed_ip in fixed_ips: + subnet = None + if 'subnet_id' in fixed_ip: + try: + subnet = query.filter( + cls.db_model.id == fixed_ip['subnet_id']).all()[0] + except IndexError: + # NOTE(hjensas): The subnet is invalid for the network, + # return all subnets. This will be detected in following + # IPAM code and some exception will be raised. + return query + elif 'ip_address' in fixed_ip: + ip = netaddr.IPNetwork(fixed_ip['ip_address']) + + for s in query.all(): + if ip in netaddr.IPNetwork(s.cidr): + subnet = s + break + if not subnet: + # NOTE(hjensas): The ip address is invalid, return all + # subnets. This will be detected in following IPAM code + # and some exception will be raised. + return query + + if subnet and subnet.segment_id not in segment_ids: + segment_ids.append(subnet.segment_id) + + if 1 < len(segment_ids): + raise segment_exc.FixedIpsSubnetsNotOnSameSegment() + + segment_id = False if not segment_ids else segment_ids[0] + + return query.filter(cls.db_model.segment_id == segment_id) + @classmethod def _query_filter_by_segment_host_mapping(cls, query, host): # TODO(tuanvu): find OVO-like solution for handling "join queries" and diff --git a/neutron/services/segments/exceptions.py b/neutron/services/segments/exceptions.py index b49fa4aed54..4b8a43464c2 100644 --- a/neutron/services/segments/exceptions.py +++ b/neutron/services/segments/exceptions.py @@ -69,3 +69,7 @@ class HostNotCompatibleWithFixedIps(exceptions.Conflict): class SegmentInUse(exceptions.InUse): message = _("Segment '%(segment_id)s' cannot be deleted: %(reason)s.") + + +class FixedIpsSubnetsNotOnSameSegment(exceptions.BadRequest): + message = _("Cannot allocate addresses from different segments.") diff --git a/neutron/tests/unit/db/test_ipam_pluggable_backend.py b/neutron/tests/unit/db/test_ipam_pluggable_backend.py index c013176e471..3151671e0cc 100644 --- a/neutron/tests/unit/db/test_ipam_pluggable_backend.py +++ b/neutron/tests/unit/db/test_ipam_pluggable_backend.py @@ -689,7 +689,8 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): mocks['driver'].get_address_request_factory.assert_called_once_with() mocks['ipam']._ipam_get_subnets.assert_called_once_with( context, network_id=port_dict['network_id'], fixed_configured=True, - host=None, service_type=port_dict['device_owner']) + fixed_ips=[ip_dict], host=None, + service_type=port_dict['device_owner']) # Validate port_dict is passed into address_factory address_factory.get_request.assert_called_once_with(context, port_dict, diff --git a/neutron/tests/unit/extensions/test_segment.py b/neutron/tests/unit/extensions/test_segment.py index db16dd4818f..08c31f4ab8a 100644 --- a/neutron/tests/unit/extensions/test_segment.py +++ b/neutron/tests/unit/extensions/test_segment.py @@ -1007,6 +1007,19 @@ class SegmentAwareIpamTestCase(SegmentTestCase): is_adjacent=False) return subnet + def _create_test_slaac_subnet_with_segment( + self, network, segment, cidr='2001:db8:0:0::/64'): + with self.subnet(network=network, + segment_id=segment['segment']['id'], + ip_version=constants.IP_VERSION_6, + ipv6_ra_mode=constants.IPV6_SLAAC, + ipv6_address_mode=constants.IPV6_SLAAC, + cidr=cidr, + allocation_pools=None) as subnet: + self._validate_l2_adjacency(network['network']['id'], + is_adjacent=False) + return subnet + def _validate_l2_adjacency(self, network_id, is_adjacent): request = self.new_show_request('networks', network_id) response = self.deserialize(self.fmt, request.get_response(self.api)) @@ -1029,6 +1042,27 @@ class TestSegmentAwareIpam(SegmentAwareIpamTestCase): subnets.append(subnet) return network, segments, subnets + def _create_net_two_segments_four_slaac_subnets(self): + with self.network() as network: + segment_a = self._test_create_segment( + network_id=network['network']['id'], + physical_network='physnet_a', + network_type=constants.TYPE_FLAT) + segment_b = self._test_create_segment( + network_id=network['network']['id'], + physical_network='physnet_b', + network_type=constants.TYPE_FLAT) + subnet_a0 = self._create_test_slaac_subnet_with_segment( + network, segment_a, '2001:db8:a:0::/64') + subnet_a1 = self._create_test_slaac_subnet_with_segment( + network, segment_a, '2001:db8:a:1::/64') + subnet_b0 = self._create_test_slaac_subnet_with_segment( + network, segment_b, '2001:db8:b:0::/64') + subnet_b1 = self._create_test_slaac_subnet_with_segment( + network, segment_b, '2001:db8:b:1::/64') + return (network, segment_a, segment_b, subnet_a0, subnet_a1, + subnet_b0, subnet_b1) + def test_port_create_with_segment_subnets(self): """No binding information is provided, defer IP allocation""" network, segment, subnet = self._create_test_segment_with_subnet() @@ -1628,6 +1662,63 @@ class TestSegmentAwareIpam(SegmentAwareIpamTestCase): self.assertEqual(webob.exc.HTTPOk.code, response.status_int) self._assert_one_ip_in_subnet(response, subnet['subnet']['cidr']) + def test_slaac_segment_aware_no_binding_info(self): + (network, segment_a, segment_b, subnet_a0, subnet_a1, subnet_b0, + subnet_b1) = self._create_net_two_segments_four_slaac_subnets() + + # Create a port with no IP address (since there is no subnet) + port_deferred = self._create_deferred_ip_port(network) + self._validate_deferred_ip_allocation(port_deferred['port']['id']) + + def test_slaac_segment_aware_immediate_fixed_ips_no_binding_info_(self): + (network, segment_a, segment_b, subnet_a0, subnet_a1, subnet_b0, + subnet_b1) = self._create_net_two_segments_four_slaac_subnets() + + # Create two ports, port_a with subnet_a0 in fixed_ips and port_b + # with subnet_b0 in fixed_ips + port_a = self._create_port_and_show( + network, fixed_ips=[{'subnet_id': subnet_a0['subnet']['id']}]) + port_b = self._create_port_and_show( + network, fixed_ips=[{'subnet_id': subnet_b0['subnet']['id']}]) + self._validate_immediate_ip_allocation(port_a['port']['id']) + self._validate_immediate_ip_allocation(port_b['port']['id']) + self.assertEqual(2, len(port_a['port']['fixed_ips'])) + self.assertEqual(2, len(port_b['port']['fixed_ips'])) + port_a_snet_ids = [f['subnet_id'] for f in port_a['port']['fixed_ips']] + port_b_snet_ids = [f['subnet_id'] for f in port_b['port']['fixed_ips']] + self.assertIn(subnet_a0['subnet']['id'], port_a_snet_ids) + self.assertIn(subnet_a1['subnet']['id'], port_a_snet_ids) + self.assertIn(subnet_b0['subnet']['id'], port_b_snet_ids) + self.assertIn(subnet_b1['subnet']['id'], port_b_snet_ids) + self.assertNotIn(subnet_a0['subnet']['id'], port_b_snet_ids) + self.assertNotIn(subnet_a1['subnet']['id'], port_b_snet_ids) + self.assertNotIn(subnet_b0['subnet']['id'], port_a_snet_ids) + self.assertNotIn(subnet_b1['subnet']['id'], port_a_snet_ids) + + def test_slaac_segment_aware_immediate_with_binding_info(self): + (network, segment_a, segment_b, subnet_a0, subnet_a1, subnet_b0, + subnet_b1) = self._create_net_two_segments_four_slaac_subnets() + + self._setup_host_mappings([(segment_a['segment']['id'], 'fakehost_a')]) + + # Create a port with host ID, validate immediate allocation on subnets + # with correct segment_id. + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + arg_list=(portbindings.HOST_ID,), + **{portbindings.HOST_ID: 'fakehost_a'}) + res = self.deserialize(self.fmt, response) + self._validate_immediate_ip_allocation(res['port']['id']) + # Since host mapped to segment_a, IP's must come from subnets: + # subnet_a0 and subnet_a1 + self.assertEqual(2, len(res['port']['fixed_ips'])) + res_subnet_ids = [f['subnet_id'] for f in res['port']['fixed_ips']] + self.assertIn(subnet_a0['subnet']['id'], res_subnet_ids) + self.assertIn(subnet_a1['subnet']['id'], res_subnet_ids) + self.assertNotIn(subnet_b0['subnet']['id'], res_subnet_ids) + self.assertNotIn(subnet_b1['subnet']['id'], res_subnet_ids) + class TestSegmentAwareIpamML2(TestSegmentAwareIpam):