Improve the CIDRs overlap check method for router add interface

If an external network have multiple subnets, the router whose
external gateway at the network will have all routes about those
subnets. So, when a internal subnet was added to the router, all
subnets of the the network which the router's external gateway
belong to need to be joined to check whether the CIDRs is
overlapped.

Also, this patch revert the patch [1]
[1] https://review.opendev.org/#/c/473356/

Change-Id: Id5d8ac09a38c656619f88a6f87b8f384fe4c55a8
Closes-Bug: #1903433
Depends-On: https://review.opendev.org/763621
Depends-On: https://review.opendev.org/763626
This commit is contained in:
Yang JianFeng 2020-11-08 07:06:10 +00:00
parent e59a7c9aca
commit 8ebd54d1bc
2 changed files with 73 additions and 21 deletions

View File

@ -427,8 +427,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)
@ -553,8 +552,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}
@ -565,8 +563,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

View File

@ -1752,18 +1752,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:
@ -1775,11 +1818,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: