[AIM] Prevent overlapping CIDRs in routed VRF
Reject adding or removing a router interface if the operation would
result in overlapping AIM Subnets within a routed AIM VRF. This can be
temporarily disabled, if necessary, by setting the
allow_routed_vrf_subnet_overlap config variable to True. Setting this
will also result in a warning being logged at startup.
A followup patch will add similar checking to the gbp-validate tool,
which should be run immediately after upgrading an existing deployment
to a new version with this feature, to make sure no overlap already
exists. If it does, it may be necessary to disable this checking until
existing overlap can be cleaned up.
Change-Id: I2de7fee0d31719293654dbe6fff032cfe1f91b9d
(cherry picked from commit 1bb8958101)
This commit is contained in:
Robert Kukura
parent
300f0ff1b5
commit
a7647168e0
|
|
@ -62,6 +62,15 @@ apic_opts = [
|
|||
help=("How many seconds for the polling thread on each "
|
||||
"controller should wait before it updates the nova vm "
|
||||
"name cache again.")),
|
||||
cfg.BoolOpt('allow_routed_vrf_subnet_overlap',
|
||||
default=False,
|
||||
help=("Set to True to turn off checking for overlapping "
|
||||
"subnets within a routed VRF when adding or removing "
|
||||
"router interfaces. Overlapping subnets in a routed VRF "
|
||||
"will result in ACI faults and lost connectivity, so "
|
||||
"this should only be used temporarily to enable "
|
||||
"cleaning up overlapping routed subnets created before "
|
||||
"overlap checking was implemented.")),
|
||||
]
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -85,3 +85,8 @@ class ExternalSubnetNotAllowed(exceptions.BadRequest):
|
|||
message = _("Connecting port or subnet which is on external network "
|
||||
"%(network_id)s as a router interface is not allowed. "
|
||||
"External networks can only be used as router gateways.")
|
||||
|
||||
|
||||
class SubnetOverlapInRoutedVRF(exceptions.BadRequest):
|
||||
message = _("Subnets %(id1)s (%(cidr1)s) and %(id2)s (%(cidr2)s) mapped "
|
||||
"to %(vrf)s overlap.")
|
||||
|
|
|
|||
|
|
@ -240,6 +240,15 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
|
|||
self.l3_domain_dn = cfg.CONF.ml2_apic_aim.l3_domain_dn
|
||||
self.apic_nova_vm_name_cache_update_interval = (cfg.CONF.ml2_apic_aim.
|
||||
apic_nova_vm_name_cache_update_interval)
|
||||
self.allow_routed_vrf_subnet_overlap = (
|
||||
cfg.CONF.ml2_apic_aim.allow_routed_vrf_subnet_overlap)
|
||||
if self.allow_routed_vrf_subnet_overlap:
|
||||
LOG.warning("The allow_routed_vrf_subnet_overlap config option is "
|
||||
"True, disabling checking for overlapping CIDRs "
|
||||
"within a routed VRF. Overlapping CIDRs result in ACI "
|
||||
"faults and loss of connectivity, so please eliminate "
|
||||
"any existing overlap and set this option to False "
|
||||
"(the default) as soon as possible.")
|
||||
self._setup_nova_vm_update()
|
||||
local_api.QUEUE_OUT_OF_PROCESS_NOTIFICATIONS = True
|
||||
self._ensure_static_resources()
|
||||
|
|
@ -1853,6 +1862,12 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
|
|||
epg = self._get_network_l3out_ext_net(
|
||||
network_db.aim_mapping)
|
||||
|
||||
# If we are using an unscoped VRF, raise exception if it has
|
||||
# overlapping subnets, which could be due to this new
|
||||
# interface itself or to movement of either topology.
|
||||
if scope_id == NO_ADDR_SCOPE:
|
||||
self._check_vrf_for_overlap(session, vrf, subnets)
|
||||
|
||||
if network_db.aim_mapping.epg_name:
|
||||
# Create AIM Subnet(s) for each added Neutron subnet.
|
||||
for subnet in subnets:
|
||||
|
|
@ -2019,6 +2034,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
|
|||
self._move_topology(
|
||||
context, aim_ctx, intf_topology, old_vrf, intf_vrf,
|
||||
nets_to_notify, vrfs_to_notify)
|
||||
self._check_vrf_for_overlap(session, intf_vrf)
|
||||
|
||||
# See if the router's topology must be moved.
|
||||
router_topology = self._router_topology(session, router_db.id)
|
||||
|
|
@ -2032,6 +2048,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
|
|||
self._move_topology(
|
||||
context, aim_ctx, router_topology, old_vrf, router_vrf,
|
||||
nets_to_notify, vrfs_to_notify)
|
||||
self._check_vrf_for_overlap(session, router_vrf)
|
||||
router_topo_moved = True
|
||||
|
||||
# If network is no longer connected to any router, make the
|
||||
|
|
@ -2091,6 +2108,48 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
|
|||
if vrfs_to_notify:
|
||||
self._notify_vrf_update(context, vrfs_to_notify)
|
||||
|
||||
def _check_vrf_for_overlap(self, session, vrf, new_subnets=None):
|
||||
if self.allow_routed_vrf_subnet_overlap:
|
||||
return
|
||||
|
||||
query = BAKERY(lambda s: s.query(
|
||||
models_v2.Subnet.id,
|
||||
models_v2.Subnet.cidr))
|
||||
query += lambda q: q.join(
|
||||
models_v2.IPAllocation,
|
||||
models_v2.IPAllocation.subnet_id ==
|
||||
models_v2.Subnet.id)
|
||||
query += lambda q: q.join(
|
||||
l3_db.RouterPort,
|
||||
l3_db.RouterPort.port_id ==
|
||||
models_v2.IPAllocation.port_id)
|
||||
query += lambda q: q.join(
|
||||
db.NetworkMapping,
|
||||
db.NetworkMapping.network_id ==
|
||||
models_v2.Subnet.network_id)
|
||||
query += lambda q: q.filter(
|
||||
l3_db.RouterPort.port_type ==
|
||||
n_constants.DEVICE_OWNER_ROUTER_INTF,
|
||||
db.NetworkMapping.vrf_name ==
|
||||
sa.bindparam('vrf_name'),
|
||||
db.NetworkMapping.vrf_tenant_name ==
|
||||
sa.bindparam('vrf_tenant_name'))
|
||||
subnets = [(id, netaddr.IPNetwork(cidr))
|
||||
for id, cidr in query(session).params(
|
||||
vrf_name=vrf.name,
|
||||
vrf_tenant_name=vrf.tenant_name)]
|
||||
|
||||
if new_subnets:
|
||||
subnets.extend(
|
||||
[(s['id'], netaddr.IPNetwork(s['cidr']))
|
||||
for s in new_subnets])
|
||||
|
||||
subnets.sort(key=lambda s: s[1])
|
||||
for (id1, cidr1), (id2, cidr2) in zip(subnets[:-1], subnets[1:]):
|
||||
if id2 != id1 and cidr2 in cidr1:
|
||||
raise exceptions.SubnetOverlapInRoutedVRF(
|
||||
id1=id1, cidr1=cidr1, id2=id2, cidr2=cidr2, vrf=vrf)
|
||||
|
||||
def bind_port(self, context):
|
||||
port = context.current
|
||||
LOG.debug("Attempting to bind port %(port)s on network %(net)s",
|
||||
|
|
|
|||
|
|
@ -4103,6 +4103,212 @@ class TestTopology(ApicAimTestCase):
|
|||
{'port_id': port_id,
|
||||
l3_ext.OVERRIDE_NETWORK_ROUTING_TOPOLOGY_VALIDATION: True})
|
||||
|
||||
def test_reject_subnet_overlap(self):
|
||||
# Create two routers.
|
||||
router1_id = self._make_router(
|
||||
self.fmt, 'test-tenant', 'router1')['router']['id']
|
||||
router2_id = self._make_router(
|
||||
self.fmt, 'test-tenant', 'router2')['router']['id']
|
||||
|
||||
# Create two networks.
|
||||
net1_resp = self._make_network(self.fmt, 'net1', True)
|
||||
net2_resp = self._make_network(self.fmt, 'net2', True)
|
||||
|
||||
# Create 4 subnets on 1st network and add to 1st router.
|
||||
subnet1a_id = self._make_subnet(
|
||||
self.fmt, net1_resp, '10.1.0.1', '10.1.0.0/24')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router1_id,
|
||||
{'subnet_id': subnet1a_id})
|
||||
subnet1b_id = self._make_subnet(
|
||||
self.fmt, net1_resp, '10.2.0.1', '10.2.0.0/24')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router1_id,
|
||||
{'subnet_id': subnet1b_id})
|
||||
subnet1c_id = self._make_subnet(
|
||||
self.fmt, net1_resp, '10.3.0.1', '10.3.0.0/24')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router1_id,
|
||||
{'subnet_id': subnet1c_id})
|
||||
subnet1d_id = self._make_subnet(
|
||||
self.fmt, net1_resp, '10.4.0.1', '10.4.0.0/24')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router1_id,
|
||||
{'subnet_id': subnet1d_id})
|
||||
|
||||
# Create non-overlapping subnet on 2nd network and add to 2nd router.
|
||||
subnet2a_id = self._make_subnet(
|
||||
self.fmt, net2_resp, '10.1.1.2', '10.1.1.0/24')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2a_id})
|
||||
|
||||
# Verify that adding larger overlapping subnet on 2nd network
|
||||
# to 2nd router is rejected due to overlapping CIDR within the
|
||||
# project's default VRF.
|
||||
subnet2b_id = self._make_subnet(
|
||||
self.fmt, net2_resp, '10.2.0.2', '10.2.0.0/16')['subnet']['id']
|
||||
self.assertRaises(
|
||||
exceptions.SubnetOverlapInRoutedVRF,
|
||||
self.l3_plugin.add_router_interface,
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2b_id})
|
||||
|
||||
# Verify that adding identical overlapping subnet on 2nd
|
||||
# network to 2nd router is rejected due to overlapping CIDR
|
||||
# within the project's default VRF.
|
||||
subnet2c_id = self._make_subnet(
|
||||
self.fmt, net2_resp, '10.3.0.2', '10.3.0.0/24')['subnet']['id']
|
||||
self.assertRaises(
|
||||
exceptions.SubnetOverlapInRoutedVRF,
|
||||
self.l3_plugin.add_router_interface,
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2c_id})
|
||||
|
||||
# Verify that adding smaller overlapping subnet on 2nd network
|
||||
# to 2nd router is rejected due to overlapping CIDR within the
|
||||
# project's default VRF.
|
||||
subnet2d_id = self._make_subnet(
|
||||
self.fmt, net2_resp, '10.4.0.2', '10.4.0.0/26')['subnet']['id']
|
||||
self.assertRaises(
|
||||
exceptions.SubnetOverlapInRoutedVRF,
|
||||
self.l3_plugin.add_router_interface,
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2d_id})
|
||||
|
||||
# Create another non-overlapping subnet on 2nd network and add
|
||||
# to 2nd router, ensuring unrouted subnets are not considered
|
||||
# overlapping.
|
||||
subnet2e_id = self._make_subnet(
|
||||
self.fmt, net2_resp, '10.5.0.2', '10.5.0.0/24')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2e_id})
|
||||
|
||||
def test_reject_moved_subnet_overlap(self):
|
||||
# Create isolated routed topology as tenant_1.
|
||||
router1_id = self._make_router(
|
||||
self.fmt, 'tenant_1', 'router1')['router']['id']
|
||||
net1_resp = self._make_network(
|
||||
self.fmt, 'net1', True, tenant_id='tenant_1')
|
||||
subnet1_id = self._make_subnet(
|
||||
self.fmt, net1_resp, '10.0.1.1', '10.0.1.0/24',
|
||||
tenant_id='tenant_1')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router1_id,
|
||||
{'subnet_id': subnet1_id})
|
||||
|
||||
# Create overlapping isolated routed topology as tenant_2.
|
||||
router2_id = self._make_router(
|
||||
self.fmt, 'tenant_2', 'router2')['router']['id']
|
||||
net2_resp = self._make_network(
|
||||
self.fmt, 'net2', True, tenant_id='tenant_2')
|
||||
subnet2_id = self._make_subnet(
|
||||
self.fmt, net2_resp, '10.0.1.1', '10.0.1.0/24',
|
||||
tenant_id='tenant_2')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2_id})
|
||||
|
||||
# Create shared network and non-overlapping subnet as tenant_2.
|
||||
net3_resp = self._make_network(
|
||||
self.fmt, 'net3', True, tenant_id='tenant_2', shared=True)
|
||||
subnet3_id = self._make_subnet(
|
||||
self.fmt, net3_resp, '10.0.3.1', '10.0.3.0/24',
|
||||
tenant_id='tenant_2')['subnet']['id']
|
||||
|
||||
# Verify that adding tenant_2's shared subnet to tenant_1's
|
||||
# topology is rejected due to tenant_1's overlapping CIDR
|
||||
# being moved to tenant_2's default VRF.
|
||||
self.assertRaises(
|
||||
exceptions.SubnetOverlapInRoutedVRF,
|
||||
self.l3_plugin.add_router_interface,
|
||||
n_context.get_admin_context(), router1_id,
|
||||
{'subnet_id': subnet3_id})
|
||||
|
||||
def test_reject_moved_back_subnet_overlap(self):
|
||||
# Create isolated routed topology as tenant_1.
|
||||
router1_id = self._make_router(
|
||||
self.fmt, 'tenant_1', 'router1')['router']['id']
|
||||
net1_resp = self._make_network(
|
||||
self.fmt, 'net1', True, tenant_id='tenant_1')
|
||||
subnet1_id = self._make_subnet(
|
||||
self.fmt, net1_resp, '10.0.1.1', '10.0.1.0/24',
|
||||
tenant_id='tenant_1')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router1_id,
|
||||
{'subnet_id': subnet1_id})
|
||||
|
||||
# Create shared network and non-overlapping subnet as tenant_2.
|
||||
net2_resp = self._make_network(
|
||||
self.fmt, 'net', True, tenant_id='tenant_2', shared=True)
|
||||
subnet2_id = self._make_subnet(
|
||||
self.fmt, net2_resp, '10.0.2.1', '10.0.2.0/24',
|
||||
tenant_id='tenant_2')['subnet']['id']
|
||||
|
||||
# Create another router as tenant_1 and add subnet shared by tenant_2.
|
||||
router2_id = self._make_router(
|
||||
self.fmt, 'tenant_1', 'router2')['router']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2_id})
|
||||
|
||||
# Create and add subnet that overlaps isolated topology. This
|
||||
# is allowed because router2's topology is using tenant_2's
|
||||
# routed VRF.
|
||||
net3_resp = self._make_network(
|
||||
self.fmt, 'net3', True, tenant_id='tenant_1')
|
||||
subnet3_id = self._make_subnet(
|
||||
self.fmt, net3_resp, '10.0.1.1', '10.0.1.0/24',
|
||||
tenant_id='tenant_1')['subnet']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet3_id})
|
||||
|
||||
# Create yet another router and add the overlapping subnet to
|
||||
# this router too, so the network does not become unrouted
|
||||
# when the subnet is later removed from router2.
|
||||
router3_id = self._make_router(
|
||||
self.fmt, 'tenant_1', 'router3')['router']['id']
|
||||
port_id = self._make_port(
|
||||
self.fmt, net3_resp['network']['id'],
|
||||
fixed_ips=[{'subnet_id': subnet3_id, 'ip_address': '10.0.1.2'}]
|
||||
)['port']['id']
|
||||
self.l3_plugin.add_router_interface(
|
||||
n_context.get_admin_context(), router3_id,
|
||||
{'port_id': port_id})
|
||||
|
||||
# Verify that removing shared subnet from router2 is rejected
|
||||
# due to overlapping CIDR in router topology being moved back
|
||||
# to tenant_1's default VRF.
|
||||
self.assertRaises(
|
||||
exceptions.SubnetOverlapInRoutedVRF,
|
||||
self.l3_plugin.remove_router_interface,
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2_id})
|
||||
|
||||
# Verify that removing overlapping subnet from router2 is
|
||||
# rejected due to overlapping CIDR in interface topology being
|
||||
# moved back to tenant_1's default VRF.
|
||||
self.assertRaises(
|
||||
exceptions.SubnetOverlapInRoutedVRF,
|
||||
self.l3_plugin.remove_router_interface,
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet3_id})
|
||||
|
||||
# Remove the overlapping subnet from router1's isolated
|
||||
# topology.
|
||||
self.l3_plugin.remove_router_interface(
|
||||
n_context.get_admin_context(), router1_id,
|
||||
{'subnet_id': subnet1_id})
|
||||
|
||||
# Remove the shared subnet from router2, which is no longer
|
||||
# should result in overlapping CIDRs in tenant_1's default
|
||||
# VRF.
|
||||
self.l3_plugin.remove_router_interface(
|
||||
n_context.get_admin_context(), router2_id,
|
||||
{'subnet_id': subnet2_id})
|
||||
|
||||
def test_reject_routing_shared_networks_from_different_projects(self):
|
||||
# Create router as tenant_1.
|
||||
router_id = self._make_router(
|
||||
|
|
|
|||
Loading…
Reference in New Issue