]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Limit router gw ports' stateful fixed IPs to one per address family
authorAndrew Boik <dboik@cisco.com>
Wed, 25 Mar 2015 20:05:41 +0000 (16:05 -0400)
committerAndrew Boik <dboik@cisco.com>
Thu, 7 May 2015 20:06:16 +0000 (20:06 +0000)
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
neutron/tests/unit/extensions/test_l3.py

index 29edc707d9cc38e20ca89ce36c1ebcba9ad07ea0..74dabca7ffaef7b5bcccd2ad9b38622c966da6c8 100644 (file)
@@ -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):
index 8c824d6c07831049220a95ee7c9569ada500c07d..b8c2922bb2f3d1e51317959330b0d7168ba1a614 100644 (file)
@@ -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: