diff --git a/.github/workflows/validate-charts.yml b/.github/workflows/validate-charts.yml index 4979c78..49c3f1d 100644 --- a/.github/workflows/validate-charts.yml +++ b/.github/workflows/validate-charts.yml @@ -303,6 +303,52 @@ jobs: - name: "Check every rendered scalar that must be a string" run: ci/run-yaml-scalar-render-test.sh + networkpolicy: + name: "NetworkPolicy" + runs-on: ubuntu-latest + # The sixth job that ignores the changed-chart matrix and the fifth that runs + # over *every* chart (Gitea #5), for the same reason as the render tests + # above: the mistake arrives by copy into a chart whose own diff nobody reads + # for it - and since #5 this convention is in all 29 installable charts, so + # there is a lot of copy. + # + # What it asserts, on every NetworkPolicy a chart renders, against the pod + # specs, probes and Services of the SAME render: the podSelector really + # matches a pod; every ingress port is a containerPort of those pods; no + # ingress rule allows every source by accident; and every port a NETWORK + # probe addresses is either left unrestricted or comes with + # networkPolicy.probeSources and the consequence written into NOTES.txt. + # Plus two guards of its own: a chart that renders NO policy fails, and a + # rendered "#" comment fails. + # + # Every one of those failures is SILENT. The API server accepts a policy + # whose selector matches nothing - it then sits in the namespace, is listed + # by kubectl, reads as a boundary and filters exactly nothing - just as + # readily as it accepts one naming a NodePort number, which matches no + # traffic at all because the policy is evaluated at the pod's interface + # after kube-proxy has already translated the port. Neither is a render + # error and neither denies anything, so neither the build nor a rollout can + # see it. + # + # It does NOT replace the install-test, which is the real net under this + # convention: that one proves ENFORCEMENT on a live k3s with its policy + # controller running, and it is what makes "all charts at once, default on" + # defensible. This job is the cheap half, and it covers the two charts the + # install-test cannot reach at all - satisfactory and template-chart, both + # in ci/excluded-charts.txt. + # + # No cluster and no Trivy; a few seconds for the whole repository, depends + # on no other job. The YAML is parsed by Helm through ci/yaml2json, because + # the runners have neither pyyaml nor yq and a hand-written parser could read + # nothing and report nothing - see the script's header. + steps: + - name: "Checkout Repository" + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + - name: "Install Helm" + uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5 + - name: "Check every rendered NetworkPolicy against the render it belongs to" + run: ci/run-networkpolicy-test.sh + trivy-config: name: "Trivy config ${{ matrix.chart }}" needs: detect-changed-charts diff --git a/ci/networkpolicy/template-chart.yaml b/ci/networkpolicy/template-chart.yaml new file mode 100644 index 0000000..514a6d7 --- /dev/null +++ b/ci/networkpolicy/template-chart.yaml @@ -0,0 +1,30 @@ +# Overlay for the NetworkPolicy render test (Gitea #5), applied last. +# +# Its only job is to render a code path the chart's own defaults never reach: +# the SOURCE-RESTRICTED ingress rule. With probeSources empty - the default, and +# deliberately so - the probe port's rule carries no "from" at all, so the whole +# restricted branch of the template, the one containing the ingress-controller +# AND, the namespace list and the ipBlock peers, is never rendered and never +# checked. That branch is also the one with the expensive mistake in it: a +# namespaceSelector and a podSelector written as two list entries instead of one +# would be an OR, and would open the port to every pod in kube-system. +# +# ci/run-networkpolicy-test.sh FAILS a chart that has an overlay here and still +# renders no restricted rule. That guard is the point: without it the overlay +# could silently stop reaching the branch, and the test would stay green over a +# code path nobody entered. +# +# template-chart carries it rather than a real chart because it is installed +# nowhere (ci/excluded-charts.txt) - so the restricted form is exercised on +# every pull request with no release depending on these values. +networkPolicy: + # Not the real node addresses of any cluster; a documentation CIDR + # (RFC 5737 TEST-NET-1) so nobody can mistake these for something to copy. + probeSources: + - 192.0.2.11/32 + - 192.0.2.12/32 + allowedNamespaces: + - monitoring + egress: + extraPorts: + - 5432 diff --git a/ci/run-networkpolicy-test.sh b/ci/run-networkpolicy-test.sh new file mode 100755 index 0000000..53f58b1 --- /dev/null +++ b/ci/run-networkpolicy-test.sh @@ -0,0 +1,842 @@ +#!/usr/bin/env bash +# NetworkPolicy render test: every NetworkPolicy a chart renders is held against +# the pod specs, probes and Services of the SAME render (Gitea #5). +# +# Usage: ci/run-networkpolicy-test.sh [chart ...] (no argument = all charts) +# +# WHY THIS EXISTS, AND WHY A GREEN INSTALL-TEST DOES NOT COVER IT +# +# Since #5 every chart in this repository ships a NetworkPolicy, on by default. +# That is 29 objects whose failure modes are all SILENT in the same way: the API +# server accepts a policy that filters nothing, and it accepts a policy that +# filters everything. Four mistakes are reachable by copy, and each one has a +# shape a render can decide: +# +# A SELECTOR THAT MATCHES NO POD. The object sits in the namespace, is listed +# by kubectl, reads as a boundary, and filters nothing whatsoever. The usual +# cause is a label renamed in the workload template - a different file, a +# different diff. Nothing in a rollout notices, because nothing is denied. +# A NODEPORT NUMBER WHERE A POD PORT BELONGS. A NetworkPolicy is evaluated at +# the POD's network interface, after kube-proxy has already translated the +# NodePort to the targetPort. A rule naming 30025 where the pod listens on +# 25 matches nothing: accepted, strict-looking, and - being the only rule +# for that port - blocking every inbound connection instead. Four charts +# here publish NodePorts, so this is not a hypothetical. +# AN EMPTY "from: []". Allows every source, exactly like no "from" at all, +# while reading in a diff and in kubectl output as a restriction. The one +# mistake in this list that makes a policy weaker while looking stricter. +# A RESTRICTED PROBE PORT. The kubelet's liveness/readiness/startup probes +# come from the IP of the WORKER NODE running the pod, an address no chart +# can know. A rule that restricts a probed port by source without that +# address drops every probe: the pod never becomes Ready, the rollout times +# out, and the symptom reads as a broken application rather than as a +# policy. This is the one the install-test does catch - after minutes, at +# the end of a 29-chart matrix, with its cause three steps away. +# +# The install-test is the real net under the rollout and it is not replaced +# here: it proves ENFORCEMENT, on a live k3s whose policy controller is +# running. What it cannot do is cover satisfactory and template-chart, which +# have no install-test at all (ci/excluded-charts.txt), and it cannot run on a +# pull request that touches no chart. This test is the cheap half: no cluster, +# no Docker, a few seconds for the whole repository. +# +# Over ALL charts and not the changed ones, for the same reason as +# ci/run-selector-match-test.sh and ci/run-quantity-render-test.sh: the mistake +# arrives by copy into a NEW chart, whose own diff nobody reads for it, and +# running over everything means there is no list to maintain - adding a chart +# enrols it. That property is what the first assertion below leans on, and it +# is not theoretical: garage (#81) and keycloak (#83) were both created AFTER +# #5 was filed and both arrived with no policy at all, keycloak with an +# unauthenticated management port on 9000. +# +# HOW THE YAML IS PARSED, AND WHY NOT WITH A PARSER WRITTEN HERE +# +# The runners have neither pyyaml nor yq - measured, the same constraint +# ci/run-trivy-config.sh, ci/run-quantity-render-test.sh and +# ci/run-selector-match-test.sh work under. So HELM is the parser: ci/yaml2json +# is a one-template chart that prints its values back as JSON, the rendered +# manifest stream is handed to it as a VALUES FILE, and python3 reads the result +# with its standard library. What IS done textually is only the document split, +# on a line that is exactly "---" in column one, and the consequence of getting +# that wrong is not a false pass: the fragment stops being valid YAML and helm +# exits 1. +# +# Assertion 5 is the exception and works on the raw rendered YAML on purpose: +# comments are gone by the time yaml2json has read the stream, and a comment is +# exactly what it looks for. +# +# WHAT IS DELIBERATELY NOT CHECKED, named so the gaps are not mistaken for +# coverage: +# +# - THAT TRAFFIC STILL FLOWS. Nothing here, and nothing in CI, proves the path +# from Traefik to a pod: no ingress controller runs in the install-test +# cluster. What is proven is the probe path, acceptance by the API server, +# and the render-side assertions below. +# - EGRESS DESTINATIONS. A NetworkPolicy matches CIDRs and selectors and never +# hostnames, so "reaches its database" is not expressible in the object and +# not checkable here. Egress ports are deliberately NOT held against the +# pod's containerPorts either - an egress port is a port on the DESTINATION, +# and that comparison would fail every chart that reaches a database. +# - SELECTORS EXPRESSED ONLY THROUGH matchExpressions. Evaluating a set-based +# requirement means reimplementing label-selector semantics, i.e. a second +# parser. Those are REPORTED as "not checked" and never counted as passing, +# because a green tick for a code path nobody entered is worse than an +# honest gap (README, "Two rules for every check added here"). +# - PORT RANGES (endPort). Same treatment: reported, never counted. +# +# AND THE ANTI-VACUUM GUARDS, because this test could be green over nothing: +# +# 1. A chart that renders NO NetworkPolicy fails. Since #5 that is a +# regression, not a style question - see the garage/keycloak note above. +# 2. ci/networkpolicy/.yaml is an overlay applied last, whose only job +# is to render a code path the chart's own defaults do not reach - above +# all the RESTRICTED probe rule, which is only rendered once probeSources +# is non-empty. A chart that HAS an overlay and still renders no restricted +# rule FAILS, because the overlay exists for exactly that. +# 3. If no assertion at all was evaluated across the whole run, the run fails. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +CI_DIR="$ROOT/ci" +YAML2JSON="$CI_DIR/yaml2json" +OVERLAY_DIR="$CI_DIR/networkpolicy" +RELEASE="netpol" + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +# --- Which charts ----------------------------------------------------------- +CHARTS=() +if [ "$#" -gt 0 ]; then + CHARTS=("$@") +else + for DIR in "$ROOT"/*/; do + [ -f "$DIR/Chart.yaml" ] || continue + CHARTS+=("$(basename "$DIR")") + done +fi +if [ "${#CHARTS[@]}" -eq 0 ]; then + echo "ERROR: no chart directories found under $ROOT" >&2 + exit 1 +fi + +FAILED="" +CHECKED="" +WITH_GAPS="" +TOTAL_ASSERTIONS=0 +SUMMARY_ROWS="" + +for CHART in "${CHARTS[@]}"; do + if [ ! -f "$ROOT/$CHART/Chart.yaml" ]; then + echo "ERROR: '$CHART' is not a chart directory" >&2 + exit 1 + fi + + # Same reason as in ci/run-selector-match-test.sh: a chart with a declared + # dependency cannot be rendered at all while charts/ is empty, and this job + # runs over EVERY chart on EVERY pull request - so without this line a PR + # touching nothing but a README would go red on the charts that have one. + "$ROOT/ci/helm-dependencies.sh" "$ROOT/$CHART" + + # Values: the same precedence the other render tests use, so a chart's CI + # values are maintained in one place and satisfy its required() values. + VALUES_ARGS=() + VALUES_LABEL="chart defaults" + if [ -f "$CI_DIR/$CHART/test-values.yaml" ]; then + VALUES_ARGS=(-f "$CI_DIR/$CHART/test-values.yaml") + VALUES_LABEL="ci/$CHART/test-values.yaml" + elif [ -f "$CI_DIR/trivy/values/$CHART.yaml" ]; then + VALUES_ARGS=(-f "$CI_DIR/trivy/values/$CHART.yaml") + VALUES_LABEL="ci/trivy/values/$CHART.yaml" + fi + HAS_OVERLAY="" + if [ -f "$OVERLAY_DIR/$CHART.yaml" ]; then + # Last, so it wins over the install-test values. + VALUES_ARGS+=(-f "$OVERLAY_DIR/$CHART.yaml") + VALUES_LABEL="$VALUES_LABEL + ci/networkpolicy/$CHART.yaml" + HAS_OVERLAY=1 + fi + + echo "::group::$CHART ($VALUES_LABEL)" + + RENDER="$WORK/$CHART.manifest.yaml" + if ! helm template "$RELEASE" "$ROOT/$CHART" \ + ${VALUES_ARGS[@]+"${VALUES_ARGS[@]}"} >"$RENDER" 2>"$WORK/err"; then + sed 's/^/ /' "$WORK/err" + echo "::endgroup::" + echo "::error title=networkpolicy $CHART::$CHART does not render with its CI values, so no policy could be checked. Reproduce: helm template $RELEASE $CHART $(printf '%s ' "${VALUES_ARGS[@]+${VALUES_ARGS[@]}}")" + FAILED="$FAILED $CHART" + continue + fi + + # --- The document split, and the only textual step before the parser ------- + # One YAML sequence item per rendered document, content indented by four and + # the item marker on a line of its own - so a document whose first line is a + # comment ("# Source: ...") needs no special case. A wrong split does not + # produce a false pass: the result stops being parsable and helm exits 1. + DOCS="$WORK/$CHART.docs.yaml" + awk ' + BEGIN { print "docs:"; started = 0 } + /^---[ \t]*$/ { print " -"; started = 1; next } + { + if (!started) { print " -"; started = 1 } + if (length($0) == 0) print ""; else print " " $0 + } + ' "$RENDER" >"$DOCS" + + JSON="$WORK/$CHART.json" + if ! helm template y2j "$YAML2JSON" -f "$DOCS" 2>"$WORK/err" \ + | awk '/^\{/ { print; exit }' >"$JSON"; then + sed 's/^/ /' "$WORK/err" + echo "::endgroup::" + echo "::error title=networkpolicy $CHART::the rendered manifests of $CHART could not be re-read as YAML by ci/yaml2json. That is a parse failure, not a policy finding: either the render contains a document this script's '---' split cut in the wrong place, or the manifest is not valid YAML. Helm's own message is in the job log." + FAILED="$FAILED $CHART" + continue + fi + if [ ! -s "$JSON" ]; then + echo "::endgroup::" + echo "::error title=networkpolicy $CHART::ci/yaml2json produced no JSON for $CHART - the manifests were not re-read, so nothing was checked." + FAILED="$FAILED $CHART" + continue + fi + + # --- The assertions -------------------------------------------------------- + # Inline and on the standard library only, as in ci/run-selector-match-test.sh + # and ci/run-quantity-render-test.sh: json, because the YAML is already gone + # by this point, plus re for the two textual checks that need the raw render. + # It writes one "VERDICT|detail" line per assertion and never decides the exit + # code itself - the loop below does, so a verdict can never be lost to a + # swallowed status. + set +e + python3 - "$CHART" "$JSON" "$RENDER" "$ROOT/$CHART" >"$WORK/$CHART.out" 2>"$WORK/$CHART.pyerr" <<'PY' +import json, re, sys + +chart, json_path, yaml_path, chart_dir = sys.argv[1:5] + +lines = [] +def ok(msg): lines.append("OK|%s: %s" % (chart, msg)) +def skip(msg): lines.append("SKIP|%s: %s" % (chart, msg)) +def fail(msg): lines.append("FAIL|%s: %s" % (chart, msg)) + +docs = [d for d in (json.load(open(json_path)).get("docs") or []) if isinstance(d, dict)] + + +def pod_templates(node, path="$"): + """Every pod template of the render, found by walking rather than by + enumerating kinds - so a CronJob's spec.jobTemplate.spec.template is covered + without a special case, and so is whatever workload kind a chart adds next. + Same shape as ci/run-selector-match-test.sh.""" + out = [] + if isinstance(node, dict): + for key, value in node.items(): + here = "%s.%s" % (path, key) + if (key == "template" and isinstance(value, dict) + and isinstance(value.get("spec"), dict) + and "containers" in value["spec"]): + out.append((here, value)) + out.extend(pod_templates(value, here)) + elif isinstance(node, list): + for i, value in enumerate(node): + out.extend(pod_templates(value, "%s[%d]" % (path, i))) + return out + + +def name_of(doc): + meta = doc.get("metadata") + if isinstance(meta, dict) and meta.get("name"): + return str(meta["name"]) + return "" % doc.get("kind", "object") + + +def labels_of(template): + meta = template.get("metadata") + labels = meta.get("labels") if isinstance(meta, dict) else None + return labels if isinstance(labels, dict) else {} + + +def matches(selector, labels): + """matchLabels only. Every key must be present with the same value; the + comparison is on the string form, because YAML true and the string "true" + are the same label value to Kubernetes.""" + for key, value in selector.items(): + if key not in labels or str(labels[key]) != str(value): + return False + return True + + +all_templates = [] +for doc in docs: + all_templates.extend(pod_templates(doc, "%s/%s" % (doc.get("kind", "?"), name_of(doc)))) + +policies = [d for d in docs if d.get("kind") == "NetworkPolicy"] +services = [d for d in docs if d.get("kind") == "Service"] + + +def container_ports(template): + """[(number, protocol, name)] over containers AND initContainers. Init + containers are included because a sidecar-style one (restartPolicy: Always) + keeps listening for the life of the pod, and its port is as real as any + other.""" + out = [] + spec = template.get("spec") or {} + for key in ("containers", "initContainers"): + for container in spec.get(key) or []: + if not isinstance(container, dict): + continue + for port in container.get("ports") or []: + if not isinstance(port, dict) or port.get("containerPort") is None: + continue + out.append((int(port["containerPort"]), + str(port.get("protocol") or "TCP"), port.get("name"))) + return out + + +def probe_ports(template): + """Ports a NETWORK probe addresses, as {number: [who]}. + + exec probes are deliberately absent, and that distinction is the whole + reason this is a function rather than a grep for "Probe": an exec probe runs + inside the container through the kubelet's runtime and puts no packet on the + network at all. A chart with exec-only probes (valkey, minecraft-server) can + therefore restrict its port by source with no probeSources whatsoever, and + treating its probe as network traffic would demand a value it does not need + and would weaken the policy it can have.""" + found = {} + by_name = dict((n, num) for num, _p, n in container_ports(template) if n) + spec = template.get("spec") or {} + for key in ("containers", "initContainers"): + for container in spec.get(key) or []: + if not isinstance(container, dict): + continue + for probe_key in ("livenessProbe", "readinessProbe", "startupProbe"): + probe = container.get(probe_key) + if not isinstance(probe, dict): + continue + for handler in ("httpGet", "tcpSocket", "grpc"): + handler_spec = probe.get(handler) + if not isinstance(handler_spec, dict): + continue + raw = handler_spec.get("port") + number = None + if isinstance(raw, int): + number = raw + elif isinstance(raw, str) and raw.isdigit(): + number = int(raw) + elif isinstance(raw, str) and raw in by_name: + number = by_name[raw] + elif isinstance(raw, str): + skip("%s of container %s names port %r, which is no " + "containerPort of the same pod spec - the probe port " + "could not be resolved, so the probe path was not " + "checked for it. Reported, not counted as passing." + % (probe_key, container.get("name"), raw)) + continue + if number is not None: + found.setdefault(number, []).append( + "%s.%s/%s" % (container.get("name"), probe_key, handler)) + return found + + +def nodeport_pod_ports(): + """Pod ports published through a NodePort Service of this render, as + {pod port: [service:nodePort]}. Traffic on these arrives from OUTSIDE the + cluster and is already masqueraded by the time it reaches the pod, so the + source address is a node and not the real client - which is why a rule for + such a port legitimately carries no source selector (assertion 3).""" + out = {} + for svc in services: + spec = svc.get("spec") or {} + if spec.get("type") != "NodePort" or not isinstance(spec.get("selector"), dict): + continue + hits = [t for _w, t in all_templates if matches(spec["selector"], labels_of(t))] + by_name = dict((n, num) for t in hits for num, _p, n in container_ports(t) if n) + for port in spec.get("ports") or []: + if not isinstance(port, dict) or not port.get("nodePort"): + continue + target = port.get("targetPort") + if target is None: + target = port.get("port") + number = None + if isinstance(target, int): + number = target + elif isinstance(target, str): + number = int(target) if target.isdigit() else by_name.get(target) + if number is not None: + out.setdefault(number, []).append( + "%s:%s" % (name_of(svc), port.get("nodePort"))) + return out + + +nodeports = nodeport_pod_ports() +raw = open(yaml_path, encoding="utf-8").read() + + +def read(path): + try: + return open(path, encoding="utf-8").read() + except IOError: + return "" + + +notes = read("%s/templates/NOTES.txt" % chart_dir) +values_yaml = read("%s/values.yaml" % chart_dir) +# The KEY, not the word. A substring test here passed a chart whose +# probeSources key had been deleted while the long comment above it still +# talked about the value - found by running this assertion's failure path, which +# is the reason the repository runs them (README, "Two rules for every check +# added here"). +offers_probe_sources = bool(re.search(r"^\s*probeSources\s*:", values_yaml, re.M)) +# The phrase the convention requires NOTES.txt to carry, so that an operator who +# narrows probeSources has been told what a wrong CIDR costs. A phrase and not a +# vague "mentions probes", because the sentence that matters is the consequence. +notes_warns = bool(re.search(r"never becomes? ready", notes, re.I)) and "probeSources" in notes + +RESTRICTED_RULES = 0 + +# --- Assertion 0: the chart ships a policy at all --------------------------- +if not policies: + fail("renders no NetworkPolicy at all. Since Gitea #5 every chart in this " + "repository ships one, on by default, and this is the guard that keeps a " + "NEW chart from arriving without one - which is exactly how garage and " + "keycloak came to have none, both created after #5 was filed. Copy " + "templates/xxx.networkpolicy.yaml and the networkPolicy block from " + "template-chart, or change this test on purpose if the chart truly has " + "nothing to restrict.") + +for policy in policies: + tag = "NetworkPolicy %s" % name_of(policy) + spec = policy.get("spec") if isinstance(policy.get("spec"), dict) else {} + policy_types = spec.get("policyTypes") + if not isinstance(policy_types, list): + policy_types = [] + + # --- Assertion 1: the podSelector hits a pod spec of the SAME render ---- + selector = spec.get("podSelector") + selected = [] + if selector is None: + fail("%s has no podSelector. That field is required by the API - a policy " + "without it is rejected - and 'podSelector: {}' is the spelling that " + "selects EVERY pod in the namespace." % tag) + elif not isinstance(selector, dict): + fail("%s podSelector is not a mapping (%r)" % (tag, selector)) + elif selector.get("matchExpressions") and not selector.get("matchLabels"): + skip("%s selects only through matchExpressions, which this test does not " + "evaluate - doing so would mean reimplementing label-selector " + "semantics, i.e. a second parser. Reported, not counted as passing." + % tag) + selected = [t for _w, t in all_templates] + else: + match_labels = selector.get("matchLabels") or {} + hits = [(w, t) for w, t in all_templates if matches(match_labels, labels_of(t))] + selected = [t for _w, t in hits] + if not match_labels: + ok("%s has an empty podSelector, which selects every pod in the " + "namespace - legitimate for a namespace-wide policy, as long as " + "that is what was meant." % tag) + selected = [t for _w, t in all_templates] + elif hits: + ok("%s podSelector %s matches %d pod template(s): %s" + % (tag, ", ".join("%s=%s" % kv for kv in sorted(match_labels.items())), + len(hits), ", ".join(w for w, _t in hits))) + else: + known = sorted(set("%s=%s" % kv for _w, t in all_templates + for kv in labels_of(t).items())) + fail("%s podSelector selects %s, which matches NONE of the %d pod " + "template(s) in the same render. The object is then accepted, is " + "listed by kubectl, reads as a boundary and filters nothing at " + "all - and nothing else in this pipeline notices, because nothing " + "is denied. The usual cause is a label renamed in the workload " + "template. Labels actually present on the pod templates: %s" + % (tag, ", ".join("%s=%s" % kv for kv in sorted(match_labels.items())), + len(all_templates), "; ".join(known) or "none")) + + declared = {} + for t in selected: + for number, proto, pname in container_ports(t): + declared.setdefault(number, set()).add(proto) + if pname: + declared.setdefault(pname, set()).add(proto) + probed = {} + for t in selected: + for number, who in probe_ports(t).items(): + probed.setdefault(number, []).extend(who) + + def rule_port_numbers(rule): + """The pod ports a rule names, resolved through the selected pods' port + names. Returns (numbers, unresolved_names).""" + numbers, unresolved = set(), [] + for port in rule.get("ports") or []: + if not isinstance(port, dict): + continue + value = port.get("port") + if isinstance(value, int): + numbers.add(value) + elif isinstance(value, str) and value.isdigit(): + numbers.add(int(value)) + elif isinstance(value, str): + hit = [num for t in selected for num, _p, n in container_ports(t) + if n == value] + if hit: + numbers.update(hit) + else: + unresolved.append(value) + return numbers, unresolved + + # --- Assertion 2: every INGRESS port is a containerPort of those pods --- + # Ingress only, and that restriction is not an oversight: an egress rule's + # port is a port on the DESTINATION and has nothing to do with this pod's + # containerPorts, so holding egress against them would fail every chart that + # reaches a database. + for i, rule in enumerate(spec.get("ingress") or []): + if not isinstance(rule, dict): + fail("%s ingress[%d] is not a mapping" % (tag, i)) + continue + for j, port in enumerate(rule.get("ports") or []): + if not isinstance(port, dict): + fail("%s ingress[%d].ports[%d] is not a mapping" % (tag, i, j)) + continue + if port.get("endPort") is not None: + skip("%s ingress[%d].ports[%d] is a port RANGE (endPort %s), which " + "this test does not expand. Reported, not counted as passing." + % (tag, i, j, port.get("endPort"))) + continue + value = port.get("port") + if value is None: + skip("%s ingress[%d].ports[%d] names no port, which allows EVERY " + "port for the given protocol. Reported, not counted as passing " + "- and worth a second look, because it is a far wider rule " + "than it looks." % (tag, i, j)) + continue + key = int(value) if isinstance(value, int) or ( + isinstance(value, str) and value.isdigit()) else value + if not selected: + fail("%s ingress[%d].ports[%d] names port %r, but the policy selects " + "no pod template at all, so no containerPort could be checked." + % (tag, i, j, value)) + continue + if key in declared: + proto = str(port.get("protocol") or "TCP") + if proto in declared[key]: + ok("%s ingress[%d] port %s/%s is a containerPort of the selected " + "pod spec" % (tag, i, value, proto)) + else: + fail("%s ingress[%d].ports[%d] allows %s/%s, but the selected pod " + "spec declares that port only as %s. A policy written for " + "the wrong protocol filters none of the traffic that " + "actually arrives." + % (tag, i, j, value, proto, "/".join(sorted(declared[key])))) + else: + numeric = sorted(k for k in declared if isinstance(k, int)) + named = sorted(str(k) for k in declared if not isinstance(k, int)) + hint = "" + if isinstance(key, int) and 30000 <= key <= 32767: + hint = (" THAT NUMBER IS IN THE NODEPORT RANGE, which makes this " + "almost certainly the NodePort mistake: a NetworkPolicy " + "is evaluated at the POD's interface, after kube-proxy " + "has already translated the NodePort to the targetPort. " + "The rule therefore matches nothing while looking strict " + "- and being the only rule for that port, it blocks every " + "inbound connection instead. Use the containerPort.") + fail("%s ingress[%d].ports[%d] allows port %r, which is NOT a " + "containerPort of any pod spec this policy selects.%s " + "containerPorts actually declared: %s%s" + % (tag, i, j, value, hint, + ", ".join(str(n) for n in numeric) or "none", + (" (names: %s)" % ", ".join(named)) if named else "")) + + # --- Assertion 3: no ingress rule that silently allows every source ----- + # The plan's wording was "no rule with a missing or empty from". Implemented + # in the two halves it really has, because the precedents deliberately use + # one of them: + # + # from: [] - present but EMPTY. Always a failure. It allows every source, + # exactly like no key at all, while reading as a restriction. + # There is no case where it is the right way to write either + # intention. + # no "from" - the documented open form; valkey and stalwart both use it on + # purpose. Accepted ONLY where the render itself justifies + # being open: every port of the rule is addressed by a network + # probe of the selected pods (the probe comes from the node's + # IP, which no chart can know), or is published by a NodePort + # Service of the same render (the source is outside the cluster + # and already masqueraded). Anything else is an allow-all + # nobody chose, which is the case this assertion exists for. + if "Ingress" in policy_types or (spec.get("ingress") and not policy_types): + for i, rule in enumerate(spec.get("ingress") or []): + if not isinstance(rule, dict): + continue + if "from" in rule and rule["from"] == []: + fail("%s ingress[%d] has an EMPTY \"from: []\". That allows every " + "source - identically to having no \"from\" at all - while " + "reading in a diff and in kubectl output as a restriction. It " + "is the one mistake in this class that makes a policy weaker " + "while looking stricter. Write no \"from\" key where open is " + "meant, and say so in a template comment, or spell the sources " + "out." % (tag, i)) + continue + if "from" in rule: + sources = rule["from"] + if not isinstance(sources, list): + fail("%s ingress[%d].from is not a list (%r)" % (tag, i, sources)) + continue + empty = [k for k, s in enumerate(sources) if isinstance(s, dict) and not s] + if empty: + fail("%s ingress[%d].from contains the empty peer {} at index %s, " + "which matches every pod in every namespace and makes the " + "whole rule open while the surrounding list looks specific." + % (tag, i, ", ".join(str(k) for k in empty))) + else: + RESTRICTED_RULES += 1 + ok("%s ingress[%d] restricts by source (%d peer(s))" + % (tag, i, len(sources))) + continue + numbers, unresolved = rule_port_numbers(rule) + if unresolved: + skip("%s ingress[%d] names port(s) %s that resolve to no " + "containerPort, so this rule's openness could not be judged. " + "Reported, not counted as passing." + % (tag, i, ", ".join(unresolved))) + continue + if not numbers: + fail("%s ingress[%d] has neither \"from\" nor \"ports\". That is the " + "widest rule the object can express - every port from every " + "source - and it makes every other rule in the policy " + "decorative." % (tag, i)) + continue + unjustified = sorted(n for n in numbers + if n not in probed and n not in nodeports) + if unjustified: + fail("%s ingress[%d] carries no \"from\", so it is open to every " + "source the CNI routes to, and port(s) %s justify neither: no " + "network probe of the selected pods addresses them, and no " + "NodePort Service of this render publishes them. Those are the " + "only two reasons this convention leaves a rule open - the " + "kubelet probes from the node's own IP, which a chart cannot " + "know, and NodePort traffic arrives already masqueraded. " + "Restrict this rule by source, or split the genuinely open " + "ports into their own rule and say in a template comment why " + "they are open." % (tag, i, ", ".join(str(n) for n in unjustified))) + else: + why = [] + for n in sorted(numbers): + if n in probed: + why.append("%d (probe path: %s)" % (n, ", ".join(probed[n]))) + else: + why.append("%d (NodePort %s)" % (n, ", ".join(nodeports[n]))) + ok("%s ingress[%d] is deliberately open, and the render justifies " + "it: %s" % (tag, i, "; ".join(why))) + + # --- Assertion 4: the probe path ----------------------------------------- + # For every port a NETWORK probe addresses, the policy either does not + # restrict it by source, or the chart offers probeSources AND its NOTES.txt + # says what a wrong CIDR costs. The most dangerous trap over 29 charts, and + # the only one the install-test shows only after minutes - as a rollout + # timeout whose cause is three steps away. + if selected and not probed: + ok("%s selects pods that have no network probe at all (exec probes, or " + "none), so no probe traffic crosses the network and every port may be " + "restricted by source without probeSources." % tag) + for port_number, who in sorted(probed.items()): + restricted, covered = [], False + for i, rule in enumerate(spec.get("ingress") or []): + if not isinstance(rule, dict): + continue + numbers, _unresolved = rule_port_numbers(rule) + if not rule.get("ports"): + numbers = set([port_number]) # a rule without ports covers all of them + if port_number not in numbers: + continue + covered = True + if rule.get("from"): + restricted.append(i) + if "Ingress" not in policy_types and not (spec.get("ingress") and not policy_types): + ok("%s does not restrict the Ingress direction at all, so the probe on " + "port %d (%s) is unaffected." % (tag, port_number, ", ".join(who))) + continue + if not covered: + fail("%s selects a pod whose probes address port %d (%s), but NO ingress " + "rule of this policy allows that port. With policyTypes Ingress the " + "kubelet's probe is then dropped and THE POD NEVER BECOMES READY - " + "the rollout times out and the symptom reads as a broken " + "application rather than as a policy." + % (tag, port_number, ", ".join(who))) + continue + if not restricted: + ok("%s leaves probe port %d (%s) unrestricted by source, which is the " + "safe default: the kubelet probes from the IP of the node running " + "the pod, and no chart can know it." + % (tag, port_number, ", ".join(who))) + continue + if offers_probe_sources and notes_warns: + ok("%s restricts probe port %d (%s) by source in ingress[%s], and the " + "chart carries what the convention requires with it: " + "networkPolicy.probeSources in values.yaml and the consequence " + "spelled out in NOTES.txt." + % (tag, port_number, ", ".join(who), + ", ".join(str(r) for r in restricted))) + continue + missing = [] + if not offers_probe_sources: + missing.append("values.yaml offers no networkPolicy.probeSources") + if not notes_warns: + missing.append("templates/NOTES.txt does not say that a probeSources list " + "missing the node's IP leaves the pod permanently un-Ready " + "(this test looks for the phrase \"never becomes Ready\" " + "together with the value's name)") + fail("%s restricts probe port %d (%s) by source in ingress[%s] while %s. A " + "restricted probe port is only defensible when the operator can widen " + "it AND has been told what happens if the CIDR is wrong: the kubelet " + "probes from the IP of the WORKER NODE running the pod, so a list that " + "misses it drops every probe and the pod never becomes Ready." + % (tag, port_number, ", ".join(who), + ", ".join(str(r) for r in restricted), " and ".join(missing))) + +# --- Assertion 5: no reasoning left in the rendered manifest ---------------- +# Not one of the original four, and added because it hit several rounds of this +# rollout: a "#" comment is part of the RENDERED object. It travels into the +# cluster, shows up in `helm get manifest` and `kubectl get -o yaml`, and puts +# the chart's reasoning in front of everyone who diffs a release. Go template +# comments render nothing and are the place for it. It can only be checked on +# the raw YAML - the comments are gone once ci/yaml2json has read the stream. +if policies: + current, offenders = [], [] + for line in raw.splitlines() + ["---"]: + if line.strip() == "---": + if re.search(r"^kind:\s*NetworkPolicy\s*$", "\n".join(current), re.M): + for raw_line in current: + stripped = raw_line.strip() + if stripped.startswith("#") and not stripped.startswith("# Source:"): + offenders.append(stripped) + current = [] + else: + current.append(line) + if offenders: + fail("the rendered NetworkPolicy manifest(s) contain %d \"#\" comment line(s), " + "which are part of the OBJECT and are shipped to the cluster: they appear " + "in `helm get manifest` and in `kubectl get -o yaml`, and they put the " + "chart's reasoning into every release diff. Move them into Go template " + "comments, which render nothing. First offender: %s" + % (len(offenders), offenders[0][:160])) + else: + ok("the rendered NetworkPolicy manifest(s) carry no \"#\" comment - all " + "reasoning stayed in template comments") + +print("RESTRICTED|%d" % RESTRICTED_RULES) +for line in lines: + print(line) +PY + RC=$? + set -e + if [ "$RC" -gt 1 ]; then + sed 's/^/ /' "$WORK/$CHART.pyerr" + echo "::endgroup::" + echo "::error title=networkpolicy $CHART::the policy comparison itself failed for $CHART (python exit $RC). Treated as a failure, never as a pass." + FAILED="$FAILED $CHART" + continue + fi + + CHART_FAIL="" + CHART_OK=0 + CHART_SKIP=0 + CHART_RESTRICTED=0 + while IFS='|' read -r VERDICT DETAIL; do + case "$VERDICT" in + OK) + CHART_OK=$((CHART_OK + 1)) + echo " ok $DETAIL" + ;; + SKIP) + CHART_SKIP=$((CHART_SKIP + 1)) + echo " gap $DETAIL" + ;; + RESTRICTED) + CHART_RESTRICTED="$DETAIL" + ;; + FAIL) + CHART_FAIL=1 + echo " FAIL $DETAIL" + echo "::error title=networkpolicy $CHART::$DETAIL" + ;; + *) + [ -n "$VERDICT" ] && echo " $VERDICT $DETAIL" + ;; + esac + done <"$WORK/$CHART.out" + echo "::endgroup::" + + TOTAL_ASSERTIONS=$((TOTAL_ASSERTIONS + CHART_OK)) + + if [ -n "$CHART_FAIL" ]; then + FAILED="$FAILED $CHART" + SUMMARY_ROWS="${SUMMARY_ROWS}| \`$CHART\` | **failed** | $CHART_OK | $CHART_SKIP |"$'\n' + echo "$CHART: FAILED" + elif [ -n "$HAS_OVERLAY" ] && [ "$CHART_RESTRICTED" -eq 0 ]; then + # The overlay's ONE job is to render a code path the chart's defaults do not + # reach - above all the RESTRICTED probe rule, which only appears once + # probeSources is non-empty. If no restricted rule came out, this test is + # vacuum-green for the very chart the overlay was written for. + FAILED="$FAILED $CHART" + SUMMARY_ROWS="${SUMMARY_ROWS}| \`$CHART\` | **failed** | $CHART_OK | $CHART_SKIP |"$'\n' + echo "::error title=networkpolicy $CHART::$CHART has an overlay at ci/networkpolicy/$CHART.yaml but rendered no source-restricted ingress rule, so the restricted branch - the one the chart's own defaults never reach - was not checked. That overlay exists for exactly this; either it no longer sets networkPolicy.probeSources where the chart reads it, or the template stopped rendering the restricted form. A green tick here would be a tick for a check that ran over nothing." + echo "$CHART: FAILED (overlay present, no restricted rule rendered)" + else + CHECKED="$CHECKED $CHART" + [ "$CHART_SKIP" -gt 0 ] && WITH_GAPS="$WITH_GAPS $CHART" + SUMMARY_ROWS="${SUMMARY_ROWS}| \`$CHART\` | ok | $CHART_OK | $CHART_SKIP |"$'\n' + echo "$CHART: ok ($CHART_OK assertion(s) held, $CHART_SKIP not checked, $CHART_RESTRICTED source-restricted rule(s))" + fi +done + +# --- Report ------------------------------------------------------------------ +count() { set -- $1; echo "$#"; } +listing() { set -- $1; [ "$#" -eq 0 ] && { echo "–"; return; }; printf '`%s`' "$1"; shift; for C in "$@"; do printf ', `%s`' "$C"; done; echo; } +SUMMARY="${GITHUB_STEP_SUMMARY:-/dev/null}" +{ + echo "### NetworkPolicy render test (Gitea #5)" + echo + echo "Every \`NetworkPolicy\` a chart renders, held against the pod specs, probes and" + echo "Services of the **same** render. All of these failures are silent: the API server" + echo "accepts a policy that filters nothing just as readily as one that filters" + echo "everything." + echo + echo "| Result | Count | Charts |" + echo "|---|---|---|" + echo "| Policies checked | $(count "$CHECKED") | $(listing "$CHECKED") |" + echo "| Failed | $(count "$FAILED") | $(listing "$FAILED") |" + echo + echo "$TOTAL_ASSERTIONS assertion(s) held in total." + echo + echo "**Not covered here, and not by anything else in CI:** that traffic still flows." + echo "No ingress controller runs in the install-test cluster, so the path from Traefik" + echo "to a pod is proven nowhere. What *is* proven is the probe path (install-test)," + echo "acceptance by the API server, and the render-side assertions above." + if [ -n "$WITH_GAPS" ]; then + echo + echo "Charts with code paths this test does **not** evaluate (\`matchExpressions\`-only selectors, port ranges, rules naming no port — reported, never counted as passing): $(listing "$WITH_GAPS")." + fi + if [ -n "$SUMMARY_ROWS" ]; then + echo + echo "| Chart | Result | Held | Not checked |" + echo "| --- | --- | --- | --- |" + printf '%s' "$SUMMARY_ROWS" + fi +} >>"$SUMMARY" + +echo "===================== networkpolicy test =======================" +echo "checked: $(count "$CHECKED")${CHECKED:+ -$CHECKED}" +echo "failed: $(count "$FAILED")${FAILED:+ -$FAILED}" +echo "assertions held: $TOTAL_ASSERTIONS" +echo "================================================================" + +if [ -n "$FAILED" ]; then + echo "ERROR: NetworkPolicy findings in:$FAILED" >&2 + exit 1 +fi + +# The third anti-vacuum guard. Reaching zero means no chart rendered a policy +# this script could say anything about, which is indistinguishable from a broken +# harness - and it is the state this test would silently decay into. +if [ "$TOTAL_ASSERTIONS" -eq 0 ]; then + echo "ERROR: not a single assertion was evaluated in this run. Either no chart renders a NetworkPolicy any more, or the harness stopped reading the manifests. A pass over zero assertions is not a pass." >&2 + exit 1 +fi