feat(ci): assert every rendered NetworkPolicy against its own render

The part that carries the rollout of Gitea #5. 29 charts get a policy that is
on by default, and every way one of them can be wrong is SILENT: the API server
accepts a policy that filters nothing just as readily as one that filters
everything, and neither is a render error, so neither the build nor a rollout
sees it.

ci/run-networkpolicy-test.sh holds every rendered NetworkPolicy against the pod
specs, probes and Services of the SAME render:

  1. the podSelector really matches a pod of that render - otherwise the object
     sits in the namespace, is listed by kubectl, reads as a boundary and
     filters nothing, usually because a label was renamed in another file;
  2. every INGRESS port is a containerPort of the selected pods, which catches
     the NodePort mistake mechanically. Egress ports are deliberately excluded:
     an egress port belongs to the DESTINATION, and comparing it would fail
     every chart that reaches a database;
  3. no ingress rule allows every source by accident. "from: []" always fails -
     it allows everything while reading as a restriction. A MISSING "from" is
     the documented open form and is accepted only where the render justifies
     it: every port of the rule is addressed by a network probe, or published by
     a NodePort Service. That is stricter than the plan's wording, which banned
     both forms, and it had to be, because the precedents use the second one on
     purpose;
  4. the probe path. For every port a NETWORK probe addresses, the policy either
     leaves it unrestricted or the chart offers networkPolicy.probeSources AND
     says in NOTES.txt that a list missing the node's IP leaves the pod
     permanently un-Ready. exec probes are excluded by design - they run inside
     the container and put no packet on the network, which is exactly why
     valkey and minecraft-server can restrict their port by source with no
     probeSources at all.

Two guards beyond the four: a chart that renders NO policy fails, because that
is how this issue decayed in the first place - garage and keycloak were both
created after #5 and both arrived without one. And a "#" comment inside a
rendered policy fails, because it is part of the object and is shipped to the
cluster.

Helm is the parser through ci/yaml2json, as in the other render tests: the
runners have neither pyyaml nor yq, and a hand-written parser could read
nothing, find nothing and leave this green over a chart it never looked at.

Every failure path was run, not just the green ones, and one of them earned its
keep immediately: the probeSources check was a substring test, so a chart whose
key had been deleted while the comment above it still discussed the value
passed. It now looks for the key.

Part of #5
This commit is contained in:
jklein committed 2026-10-14 13:56:06 +00:00
1 parent 1001de620a
commit 36bcfd3b11
3 files changed
+918

No files matched your search

+30
View File
@@ -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
+842
View File
@@ -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/<chart>.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 "<unnamed %s>" % 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