Skip to content

Commit

Permalink
salt/checks: Only check routing tables
Browse files Browse the repository at this point in the history
In the previous commit, we introduce a validation of the full Service
CIDR by looking at all routes defined in the system routing tables
(obtained with `network.routes`).
This change makes the previous approach (using `network.get_route`,
similar to an `ip route get` invocation) unneeded, as we are already
comparing networks inclusion-wise.

Cherry-picked from: 1c347fc
  • Loading branch information
gdemonet committed Feb 26, 2021
1 parent 4f03ff1 commit f819bae
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 48 deletions.
44 changes: 18 additions & 26 deletions salt/_modules/metalk8s_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,37 +212,29 @@ def route_exists(destination, raises=True):
raises (bool): the method will raise if there is no route for this
destination.
"""
network = ipaddress.IPv4Network(destination)

dest_net = ipaddress.IPv4Network(destination)
error = None
route_info = None
try:
route_info = __salt__["network.get_route"](network.network_address)
except Exception as exc:
# NOTE: if no route exists, this module may fail with an
# AttributeError. To be on the safe side, we catch any Exception.
error = "No route exists for {}".format(destination)

if route_info is not None:
# We found a route for the first network address in the provided
# `destination`. Let's verify that the whole network can be routed.
all_routes = __salt__["network.routes"](family="inet")

for route in all_routes:
if route["gateway"] == route_info["gateway"] \
and route["interface"] == route_info["interface"]:
# The route matches the one we found, let's check if our
# destination is fully included in this route.
route_net = ipaddress.IPv4Network(
"{r[destination]}/{r[netmask]}".format(r=route)
)
if _is_subnet_of(network, route_net):
break
route_exists = False

all_routes = __salt__["network.routes"](family="inet")

for route in all_routes:
# Check if our destination network is fully included in this route.
route_net = ipaddress.IPv4Network(
"{r[destination]}/{r[netmask]}".format(r=route)
)
if _is_subnet_of(dest_net, route_net):
break
else:
route_exists |= dest_net.network_address in route_net
else:
if route_exists:
error = (
"A route was found for {n.network_address}, but it does not "
"match the full destination network {n.compressed}"
).format(n=network)
).format(n=dest_net)
else:
error = "No route exists for {}".format(dest_net.compressed)

if error and raises:
raise CheckError(error)
Expand Down
18 changes: 4 additions & 14 deletions salt/tests/unit/modules/files/test_metalk8s_checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -310,36 +310,26 @@ route_exists:
# 1. Success: Nominal (default route)
- &route_exists_nominal
destination: 10.96.0.0/12
get_route_ret:
gateway: &default_gw "10.10.0.1"
interface: &default_iface eth0
destination: 10.96.0.0/32
source: "10.10.1.2"
routes_ret:
- &default_route
addr_family: inet
destination: "0.0.0.0"
flags: UG
gateway: *default_gw
interface: *default_iface
gateway: "10.10.0.1"
interface: eth0
netmask: "0.0.0.0"

# 2. Success: Nominal (dedicated dummy route)
- <<: *route_exists_nominal
get_route_ret:
gateway: *default_gw
interface: dummy0
destination: 10.96.0.0/32
source: "10.10.1.2"
routes_ret:
- <<: *default_route
interface: dummy0
destination: "10.96.0.0"
netmask: "255.240.0.0"

# 3. Error: Failure to get route
# 3. Error: No route exists
- <<: *route_exists_nominal
get_route_ret: Some error when getting route
routes_ret: []
error: No route exists for 10.96.0.0/12

# 4. Error: Route found is too small
Expand Down
8 changes: 0 additions & 8 deletions salt/tests/unit/modules/test_metalk8s_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,22 +162,14 @@ def test_route_exists(
self,
destination,
error=None,
get_route_ret=None,
routes_ret=None,
):
"""
Tests the return of `route_exists` function
"""
def _network_get_route(dest):
if isinstance(get_route_ret, dict):
return get_route_ret
raise AttributeError(get_route_ret)

network_get_route_mock = MagicMock(side_effect=_network_get_route)
network_routes_mock = MagicMock(return_value=routes_ret)

patch_dict = {
'network.get_route': network_get_route_mock,
'network.routes': network_routes_mock,
}

Expand Down

0 comments on commit f819bae

Please sign in to comment.