Merge branch 'improvement/check-conflicting-cidrs' into q/126.0

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Release 126.0.3 (in development)
 
+### Enhancements
+
+- Add a Check to ensure pods/services CIDRs don't overlap
+  Workload Plane/Control Plane CIDRs.
+  (PR[#4217](https://github.com/scality/metalk8s/pull/4217))
+
 ## Release 126.0.2
 
 ### Bug fixes

salt/_modules/metalk8s_checks.py

@@ -16,6 +16,56 @@ def __virtual__():
     return __virtualname__
 
 
+def bootstrap_file():
+    """Check sanity of the bootstrap config file."""
+
+    networks = __pillar__.get("networks", {})
+    errors = []
+
+    # check if pod and svc CIDRs don't intersect
+    # WP and CP CIDRs
+
+    # each entry should be a list of 1 or more CIDR
+    # if not, the error is detected upon pillar generation
+    wp = networks.get("workload_plane").get("cidr")
+    cp = networks.get("control_plane").get("cidr")
+    node_networks = [
+        (
+            f"Workload Plane ({', '.join(wp)})",
+            [ipaddress.ip_network(cidr) for cidr in wp],
+        ),
+        (
+            f"Control Plane ({', '.join(cp)})",
+            [ipaddress.ip_network(cidr) for cidr in cp],
+        ),
+    ]
+
+    # These should always be defined in the pillar
+    # They are each a CIDR address
+    pod = networks.get("pod")
+    service = networks.get("service")
+    object_networks = [
+        (f"Pods ({pod})", ipaddress.ip_network(pod)),
+        (f"Services ({service})", ipaddress.ip_network(service)),
+    ]
+
+    if object_networks[0][1].overlaps(object_networks[1][1]):
+        errors.append(f" - {object_networks[0][0]} and {object_networks[1][0]}")
+
+    for o in object_networks:
+        for n in node_networks:
+            if any(o[1].overlaps(net) for net in n[1]):
+                errors.append(f" - {o[0]} and {n[0]}")
+
+    if errors:
+        msg = (
+            "The following CIDRs must not overlap in bootstrap configuration:\n{}"
+        ).format("\n".join(errors))
+        raise CheckError(msg)
+
+    return True
+
+
 def node(raises=True, **kwargs):
     """Check if the current Salt-minion match some requirements so that it can
     be used as a MetalK8s node.

salt/tests/unit/modules/files/test_metalk8s_checks.yaml

@@ -1,3 +1,71 @@
+bootstrap_file:
+  # 1. Success: Nominal
+  - &bstrapf_nominal
+    cp_cidrs: [10.1.0.0/16]
+    wp_cidrs: [10.2.0.0/16]
+    pods_cidr: 10.233.0.0/16
+    svc_cidr: 10.96.0.0/12
+
+  # 2. Fail: pods intersect fully
+  - <<: *bstrapf_nominal
+    pods_cidr: 10.1.0.0/16
+    expected_raise: |-
+      The following CIDRs must not overlap in bootstrap configuration:
+       - Pods \(10.1.0.0/16\) and Control Plane \(10.1.0.0/16\)
+
+  # 3. Fail: pods intersect partially
+  - <<: *bstrapf_nominal
+    cp_cidrs: [10.0.0.0/16, 10.1.0.0/24]
+    wp_cidrs: [10.2.0.0/16]
+    pods_cidr: 10.1.0.0/16
+    expected_raise: |-
+      The following CIDRs must not overlap in bootstrap configuration:
+       - Pods \(10.1.0.0/16\) and Control Plane \(10.0.0.0/16, 10.1.0.0/24\)
+
+  # 4. Fail: svc intersect fully
+  - <<: *bstrapf_nominal
+    svc_cidr: 10.2.0.0/16
+    expected_raise: |-
+      The following CIDRs must not overlap in bootstrap configuration:
+       - Services \(10.2.0.0/16\) and Workload Plane \(10.2.0.0/16\)
+
+  # 5. Fail: services intersect partially
+  - <<: *bstrapf_nominal
+    cp_cidrs: [10.1.0.0/16]
+    wp_cidrs: [10.0.0.0/16, 10.2.0.0/24]
+    svc_cidr: 10.2.0.0/16
+    expected_raise: |-
+      The following CIDRs must not overlap in bootstrap configuration:
+       - Services \(10.2.0.0/16\) and Workload Plane \(10.0.0.0/16, 10.2.0.0/24\)
+
+  # 6. Fail: pods and services intersect
+  - <<: *bstrapf_nominal
+    svc_cidr: 10.233.0.0/16
+    expected_raise: |-
+      The following CIDRs must not overlap in bootstrap configuration:
+       - Pods \(10.233.0.0/16\) and Services \(10.233.0.0/16\)
+
+  # 7. Fail: all intersect
+  - <<: *bstrapf_nominal
+    cp_cidrs: [10.0.0.0/16]
+    wp_cidrs: [10.0.0.0/16]
+    pods_cidr: 10.0.0.0/16
+    svc_cidr: 10.0.0.0/16
+    expected_raise: >-
+      The following CIDRs must not overlap in bootstrap configuration:
+       - Pods \(10.0.0.0/16\) and Services \(10.0.0.0/16\)
+       - Pods \(10.0.0.0/16\) and Workload Plane \(10.0.0.0/16\)
+       - Pods \(10.0.0.0/16\) and Control Plane \(10.0.0.0/16\)
+       - Services \(10.0.0.0/16\) and Workload Plane \(10.0.0.0/16\)
+       - Services \(10.0.0.0/16\) and Control Plane \(10.0.0.0/16\)
+
+# 4. Fail: svc intersect partially - non obvious
+  - <<: *bstrapf_nominal
+    cp_cidrs: [10.100.0.0/16]
+    expected_raise: |-
+      The following CIDRs must not overlap in bootstrap configuration:
+       - Services \(10.96.0.0/12\) and Control Plane \(10.100.0.0/16\)
+
 node:
   # 1. Success: Nominal
   - &node_nominal

salt/tests/unit/modules/test_metalk8s_checks.py

@@ -32,6 +32,27 @@ def test_virtual(self):
         """
         self.assertEqual(metalk8s_checks.__virtual__(), "metalk8s_checks")
 
+    @utils.parameterized_from_cases(YAML_TESTS_CASES["bootstrap_file"])
+    def test_bootstrap_file(
+        self, cp_cidrs, wp_cidrs, pods_cidr, svc_cidr, expected_raise=None
+    ):
+        """
+        Tests the return of the `bootstrap_file` function
+        """
+        networks = {
+            "workload_plane": {"cidr": wp_cidrs},
+            "control_plane": {"cidr": cp_cidrs},
+            "pod": pods_cidr,
+            "service": svc_cidr,
+        }
+        with patch.dict(metalk8s_checks.__pillar__, {"networks": networks}):
+            if expected_raise is not None:
+                self.assertRaisesRegex(
+                    CheckError, expected_raise, metalk8s_checks.bootstrap_file
+                )
+            else:
+                self.assertEqual(metalk8s_checks.bootstrap_file(), True)
+
     @utils.parameterized_from_cases(YAML_TESTS_CASES["node"])
     def test_node(
         self,

scripts/bootstrap.sh.in

@@ -155,6 +155,10 @@ bootstrap_file_is_present() {
     fi
 }
 
+bootstrap_file_is_sane() {
+    "$SALT_CALL" --local --retcode-passthrough metalk8s_checks.bootstrap_file
+}
+
 check_ca_minion() {
     # NOTE: Today using bootstrap script, bootstrap node is also the CA
     # minion, so check for minion id equal to CA minion
@@ -184,6 +188,7 @@ main() {
     run "Checking Salt minion ID" check_minion_id
     run "Configuring Salt minion to run in local mode" configure_salt_minion_local_mode
     run "Ensure archive is available" ensure_archives_mounted
+    run "Checking that BootstrapConfiguration is sane" bootstrap_file_is_sane
     run "Calculating Salt grains in local mode" set_local_grains
     run "Checking CA minion" check_ca_minion
     run "Checking local node" check_local_node