Merge "Improve the CIDRs overlap check method for router add interface" into stable/stein

neutron/db/l3_db.py

@@ -488,8 +488,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
 
             self._check_for_dup_router_subnets(context, router,
                                                new_network_id,
-                                               subnets,
-                                               include_gateway=True)
+                                               subnets)
             self._create_router_gw_port(context, router,
                                         new_network_id, ext_ips)
 
@@ -613,8 +612,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
                                                 filters=filters)
 
     def _check_for_dup_router_subnets(self, context, router,
-                                      network_id, new_subnets,
-                                      include_gateway=False):
+                                      network_id, new_subnets):
         # It's possible these ports are on the same network, but
         # different subnets.
         new_subnet_ids = {s['id'] for s in new_subnets}
@@ -625,8 +623,13 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
                     msg = (_("Router already has a port on subnet %s")
                            % ip['subnet_id'])
                     raise n_exc.BadRequest(resource='router', msg=msg)
-                gw_owner = (p.get('device_owner') == DEVICE_OWNER_ROUTER_GW)
-                if include_gateway == gw_owner:
+                if p.get('device_owner') == DEVICE_OWNER_ROUTER_GW:
+                    ext_subts = self._core_plugin.get_subnets(
+                        context.elevated(),
+                        filters={'network_id': p['network_id']})
+                    for sub in ext_subts:
+                        router_subnets.append(sub['id'])
+                else:
                     router_subnets.append(ip['subnet_id'])
 
         # Ignore temporary Prefix Delegation CIDRs

neutron/tests/unit/extensions/test_l3.py

@@ -1753,18 +1753,61 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                                                   HTTPBadRequest.code)
 
     def test_router_add_interface_cidr_overlapped_with_gateway(self):
-        with self.router() as r:
+        with self.router() as r, self.network() as ext_net:
             with self.subnet(cidr='10.0.1.0/24') as s1, self.subnet(
-                    cidr='10.0.0.0/16') as s2:
-                self._set_net_external(s2['subnet']['network_id'])
+                    network=ext_net, cidr='10.0.0.0/16') as s2:
+                ext_net_id = ext_net['network']['id']
+                self._set_net_external(ext_net_id)
                 self._add_external_gateway_to_router(
-                    r['router']['id'],
-                    s2['subnet']['network_id'])
-                res = self._router_interface_action('add',
-                                                    r['router']['id'],
-                                                    s1['subnet']['id'],
-                                                    None)
-                self.assertIn('port_id', res)
+                    r['router']['id'], ext_net_id)
+                res = self._router_interface_action(
+                    'add', r['router']['id'], s1['subnet']['id'], None,
+                    expected_code=exc.HTTPBadRequest.code)
+                expected_msg = ("Bad router request: Cidr 10.0.1.0/24 of "
+                                "subnet %(internal_subnet_id)s overlaps with "
+                                "cidr 10.0.0.0/16 of subnet "
+                                "%(external_subnet_id)s.") % {
+                                    "external_subnet_id": s2['subnet']['id'],
+                                    "internal_subnet_id": s1['subnet']['id']}
+                self.assertEqual(expected_msg, res['NeutronError']['message'])
+
+                # External network have multiple subnets.
+                with self.subnet(network=ext_net,
+                                 cidr='192.168.1.0/24') as s3, \
+                        self.subnet(cidr='192.168.1.0/24') as s4:
+                    res = self._router_interface_action(
+                        'add', r['router']['id'], s4['subnet']['id'], None,
+                        expected_code=exc.HTTPBadRequest.code)
+                    expected_msg = (
+                        "Bad router request: Cidr 192.168.1.0/24 of subnet "
+                        "%(internal_subnet_id)s overlaps with cidr "
+                        "192.168.1.0/24 of subnet %(external_subnet_id)s.") % {
+                            "external_subnet_id": s3['subnet']['id'],
+                            "internal_subnet_id": s4['subnet']['id']}
+                    self.assertEqual(expected_msg,
+                                     res['NeutronError']['message'])
+
+    def test_router_set_gateway_cidr_overlapped_with_subnets(self):
+        with self.router() as r, self.network() as ext_net:
+            with self.subnet(network=ext_net, cidr='10.0.1.0/24') as s1, \
+                    self.subnet(network=ext_net, cidr='10.0.2.0/24') as s2, \
+                    self.subnet(cidr='10.0.2.0/24') as s3:
+                ext_net_id = ext_net['network']['id']
+                self._set_net_external(ext_net_id)
+                self._router_interface_action(
+                    'add', r['router']['id'],
+                    s3['subnet']['id'], None)
+                res = self._add_external_gateway_to_router(
+                    r['router']['id'], ext_net_id,
+                    ext_ips=[{'subnet_id': s1['subnet']['id']}],
+                    expected_code=exc.HTTPBadRequest.code)
+                expected_msg = (
+                    "Bad router request: Cidr 10.0.2.0/24 of subnet "
+                    "%(external_subnet_id)s overlaps with cidr 10.0.2.0/24 of "
+                    "subnet %(internal_subnet_id)s.") % {
+                        "external_subnet_id": s2["subnet"]["id"],
+                        "internal_subnet_id": s3["subnet"]["id"]}
+                self.assertEqual(expected_msg, res['NeutronError']['message'])
 
     def test_router_add_interface_by_port_cidr_overlapped_with_gateway(self):
         with self.router() as r:
@@ -1776,11 +1819,17 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                         r['router']['id'],
                         s2['subnet']['network_id'])
 
-                    res = self._router_interface_action('add',
-                                                        r['router']['id'],
-                                                        None,
-                                                        p['port']['id'])
-                    self.assertIn('port_id', res)
+                    res = self._router_interface_action(
+                        'add', r['router']['id'], None, p['port']['id'],
+                        expected_code=exc.HTTPBadRequest.code)
+                    expected_msg = (
+                        "Bad router request: Cidr 10.0.1.0/24 of subnet "
+                        "%(internal_subnet_id)s overlaps with cidr "
+                        "10.0.0.0/16 of subnet %(external_subnet_id)s.") % {
+                            "external_subnet_id": s2['subnet']['id'],
+                            "internal_subnet_id": s1['subnet']['id']}
+                    self.assertEqual(expected_msg,
+                                     res['NeutronError']['message'])
 
     def test_router_add_gateway_dup_subnet1_returns_400(self):
         with self.router() as r: