diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index f32aab414f0..40401a54bc5 100644 --- a/neutron/db/l3_db.py +++ b/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 diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index 10f9190090c..11e096c3a28 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/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: