From 1bfd86e1ef7148370798aa99c868d7f931fcbf78 Mon Sep 17 00:00:00 2001 From: Andrew Boik Date: Wed, 25 Mar 2015 16:05:41 -0400 Subject: [PATCH] Limit router gw ports' stateful fixed IPs to one per address family Validate a router's gateway port during a router update by ensuring it has no more than one v4 fixed IP and one v6 (statefully-assigned) fixed IP. Note that there is no limit on v6 addresses from SLAAC and DHCPv6-stateless subnets as they are automatically allocated. Change-Id: I6a328048b99af39ab9497fd9f265d1a9b95b7148 Closes-Bug: 1438819 Partially-implements: blueprint multiple-ipv6-prefixes --- neutron/db/db_base_plugin_v2.py | 27 +++++-- neutron/tests/unit/extensions/test_l3.py | 96 ++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 29edc707d9c..74dabca7ffa 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1143,19 +1143,32 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, pool=pool_range, ip_address=gateway_ip) - def _update_router_gw_ports(self, context, subnet_id, network_id): + def _update_router_gw_ports(self, context, network, subnet): l3plugin = manager.NeutronManager.get_service_plugins().get( service_constants.L3_ROUTER_NAT) if l3plugin: gw_ports = self._get_router_gw_ports_by_network(context, - network_id) + network['id']) router_ids = [p['device_id'] for p in gw_ports] ctx_admin = ctx.get_admin_context() + ext_subnets_dict = {s['id']: s for s in network['subnets']} for id in router_ids: router = l3plugin.get_router(ctx_admin, id) external_gateway_info = router['external_gateway_info'] + # Get all stateful (i.e. non-SLAAC/DHCPv6-stateless) fixed ips + fips = [f for f in external_gateway_info['external_fixed_ips'] + if not ipv6_utils.is_auto_address_subnet( + ext_subnets_dict[f['subnet_id']])] + num_fips = len(fips) + # Don't add the fixed IP to the port if it already + # has a stateful fixed IP of the same IP version + if num_fips > 1: + continue + if num_fips == 1 and netaddr.IPAddress( + fips[0]['ip_address']).version == subnet['ip_version']: + continue external_gateway_info['external_fixed_ips'].append( - {'subnet_id': subnet_id}) + {'subnet_id': subnet['id']}) info = {'router': {'external_gateway_info': external_gateway_info}} l3plugin.update_router(context, id, info) @@ -1295,8 +1308,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, s['allocation_pools']) if hasattr(network, 'external') and network.external: self._update_router_gw_ports(context, - subnet['id'], - subnet['network_id']) + network, + subnet) return self._make_subnet_dict(subnet) def _create_subnet_from_implicit_pool(self, context, subnet): @@ -1321,8 +1334,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, s['allocation_pools']) if hasattr(network, 'external') and network.external: self._update_router_gw_ports(context, - subnet['id'], - subnet['network_id']) + network, + subnet) return self._make_subnet_dict(subnet) def _get_subnetpool_id(self, subnet): diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index 8c824d6c078..b8c2922bb2f 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -883,6 +883,56 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): self.assertNotEqual(fip1['subnet_id'], fip2['subnet_id']) self.assertNotEqual(fip1['ip_address'], fip2['ip_address']) + def test_router_update_gateway_upon_subnet_create_max_ips_ipv6(self): + """Create subnet should not cause excess fixed IPs on router gw + + If a router gateway port has the maximum of one IPv4 and one IPv6 + fixed, create subnet should not add any more IP addresses to the port + (unless this is the subnet is a SLAAC/DHCPv6-stateless subnet in which + case the addresses are added automatically) + + """ + with self.router() as r, self.network() as n: + with self.subnet(cidr='10.0.0.0/24', network=n) as s1, ( + self.subnet(ip_version=6, cidr='2001:db8::/64', + network=n)) as s2: + self._set_net_external(n['network']['id']) + self._add_external_gateway_to_router( + r['router']['id'], + n['network']['id'], + ext_ips=[{'subnet_id': s1['subnet']['id']}, + {'subnet_id': s2['subnet']['id']}], + expected_code=exc.HTTPOk.code) + res1 = self._show('routers', r['router']['id']) + original_fips = (res1['router']['external_gateway_info'] + ['external_fixed_ips']) + # Add another IPv4 subnet - a fip SHOULD NOT be added + # to the external gateway port as it already has a v4 address + self._create_subnet(self.fmt, net_id=n['network']['id'], + cidr='10.0.1.0/24') + res2 = self._show('routers', r['router']['id']) + self.assertEqual(original_fips, + res2['router']['external_gateway_info'] + ['external_fixed_ips']) + # Add a SLAAC subnet - a fip from this subnet SHOULD be added + # to the external gateway port + s3 = self.deserialize(self.fmt, + self._create_subnet(self.fmt, + net_id=n['network']['id'], + ip_version=6, cidr='2001:db8:1::/64', + ipv6_ra_mode=l3_constants.IPV6_SLAAC, + ipv6_address_mode=l3_constants.IPV6_SLAAC)) + res3 = self._show('routers', r['router']['id']) + fips = (res3['router']['external_gateway_info'] + ['external_fixed_ips']) + fip_subnet_ids = [fip['subnet_id'] for fip in fips] + self.assertIn(s1['subnet']['id'], fip_subnet_ids) + self.assertIn(s2['subnet']['id'], fip_subnet_ids) + self.assertIn(s3['subnet']['id'], fip_subnet_ids) + self._remove_external_gateway_from_router( + r['router']['id'], + n['network']['id']) + def _test_router_add_interface_subnet(self, router, subnet, msg=None): exp_notifications = ['router.create.start', 'router.create.end', @@ -1371,6 +1421,52 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): expected_code=exc. HTTPBadRequest.code) + def test_router_add_gateway_multiple_subnets_ipv6(self): + """Ensure external gateway set doesn't add excess IPs on router gw + + Setting the gateway of a router to an external network with more than + one IPv4 and one IPv6 subnet should only add an address from the first + IPv4 subnet, an address from the first IPv6-stateful subnet, and an + address from each IPv6-stateless (SLAAC and DHCPv6-stateless) subnet + + """ + with self.router() as r, self.network() as n: + with self.subnet( + cidr='10.0.0.0/24', network=n) as s1, ( + self.subnet( + cidr='10.0.1.0/24', network=n)) as s2, ( + self.subnet( + cidr='2001:db8::/64', network=n, + ip_version=6, + ipv6_ra_mode=l3_constants.IPV6_SLAAC, + ipv6_address_mode=l3_constants.IPV6_SLAAC)) as s3, ( + self.subnet( + cidr='2001:db8:1::/64', network=n, + ip_version=6, + ipv6_ra_mode=l3_constants.DHCPV6_STATEFUL, + ipv6_address_mode=l3_constants.DHCPV6_STATEFUL)) as s4, ( + self.subnet( + cidr='2001:db8:2::/64', network=n, + ip_version=6, + ipv6_ra_mode=l3_constants.DHCPV6_STATELESS, + ipv6_address_mode=l3_constants.DHCPV6_STATELESS)) as s5: + self._set_net_external(n['network']['id']) + self._add_external_gateway_to_router( + r['router']['id'], + n['network']['id']) + res = self._show('routers', r['router']['id']) + fips = (res['router']['external_gateway_info'] + ['external_fixed_ips']) + fip_subnet_ids = [fip['subnet_id'] for fip in fips] + self.assertIn(s1['subnet']['id'], fip_subnet_ids) + self.assertNotIn(s2['subnet']['id'], fip_subnet_ids) + self.assertIn(s3['subnet']['id'], fip_subnet_ids) + self.assertIn(s4['subnet']['id'], fip_subnet_ids) + self.assertIn(s5['subnet']['id'], fip_subnet_ids) + self._remove_external_gateway_from_router( + r['router']['id'], + n['network']['id']) + def test_router_add_and_remove_gateway(self): with self.router() as r: with self.subnet() as s: