From 2d41fb255f6a8e1e32f0df0e3452e600ee7721d9 Mon Sep 17 00:00:00 2001 From: Sebastian Preisner Date: Wed, 29 Jul 2026 11:58:12 +0200 Subject: [PATCH] helm: refuse empty port and DNS peer lists instead of widening In a NetworkPolicy an empty ports list means every port and a missing peer selector means every pod, so `kubeApiServer.ports: []` silently opened the API server CIDRs on all ports, and nulling a DNS selector rendered `podSelector: null`, which is every pod in kube-system. Both now fail the render, and the DNS rule emits only the selectors that are set. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/helm_ci.yml | 54 +++++++++++++++---- .../templates/shared/networkpolicy.yaml | 20 +++++++ 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/.github/workflows/helm_ci.yml b/.github/workflows/helm_ci.yml index 262d0928a..9ff6705b9 100644 --- a/.github/workflows/helm_ci.yml +++ b/.github/workflows/helm_ci.yml @@ -834,12 +834,25 @@ jobs: chart = sys.argv[1] - def render(values): + def render(values, json_values=None): args = ["helm", "template", "test", chart] for k, v in values.items(): args += ["--set", f"{k}={v}"] + for k, v in (json_values or {}).items(): + args += ["--set-json", f"{k}={v}"] return subprocess.check_output(args, text=True, stderr=subprocess.STDOUT) + def expect_failure(values, needle, label, json_values=None): + try: + render(values, json_values) + except subprocess.CalledProcessError as e: + if needle not in (e.output or ""): + failed.append(f"{label}: unexpected error: {(e.output or '')[:200]}") + else: + print(label) + return + failed.append(f"{label}: render should have failed") + def docs(manifest): return [d for d in yaml.safe_load_all(manifest) if d] @@ -940,19 +953,42 @@ jobs: # Components that need the API server must not silently lose it: rendering # fails with a pointer to the value instead. - try: - render(dict(on, **{"networkPolicy.egress.enabled": "true"})) - failed.append("egress on with empty kubeApiServer.cidrs: render should have failed") - except subprocess.CalledProcessError as e: - if "kubeApiServer.cidrs is empty" not in (e.output or ""): - failed.append(f"egress on with empty cidrs: unexpected error: {(e.output or '')[:200]}") - else: - print("empty kubeApiServer.cidrs fails the render with a pointer to the value") + expect_failure(dict(on, **{"networkPolicy.egress.enabled": "true"}), + "kubeApiServer.cidrs is empty", + "empty kubeApiServer.cidrs fails the render with a pointer to the value") egress_on = dict(on, **{ "networkPolicy.egress.enabled": "true", "networkPolicy.egress.kubeApiServer.cidrs[0]": "10.96.0.1/32", }) + + # An empty port or peer list is "everything" in a NetworkPolicy, not + # "nothing", so the two places that could be emptied have to be refused + # rather than quietly widened. + expect_failure(egress_on, "kubeApiServer.ports is empty", + "empty kubeApiServer.ports is refused instead of allowing every port", + json_values={"networkPolicy.egress.kubeApiServer.ports": "[]"}) + expect_failure(egress_on, "allowDNS is on but both", + "DNS with no selector is refused instead of allowing every pod", + json_values={"networkPolicy.egress.dnsNamespaceSelector": "null", + "networkPolicy.egress.dnsPodSelector": "null"}) + + # Dropping one of the two DNS selectors must leave the key out, not + # render it as null - null namespaceSelector means this namespace and + # null podSelector means every pod in the peer namespace. + for dropped in ("dnsNamespaceSelector", "dnsPodSelector"): + out = render(egress_on, {f"networkPolicy.egress.{dropped}": "null"}) + nulls = [(c, k) for c, p in policies(out).items() + for r in p["spec"]["egress"] + for t in r.get("to") or [] + for k, v in t.items() if v is None] + nulls += [(c, "ports") for c, p in policies(out).items() + for r in p["spec"]["egress"] if "ports" in r and r["ports"] is None] + if nulls: + failed.append(f"{dropped}=null: rendered null keys {sorted(set(nulls))}") + else: + print(f"{dropped}=null: the key is left out rather than rendered as null") + out = render(egress_on) pols = policies(out) diff --git a/k8s/charts/seaweedfs/templates/shared/networkpolicy.yaml b/k8s/charts/seaweedfs/templates/shared/networkpolicy.yaml index ed7a75387..6b4d5b043 100644 --- a/k8s/charts/seaweedfs/templates/shared/networkpolicy.yaml +++ b/k8s/charts/seaweedfs/templates/shared/networkpolicy.yaml @@ -197,11 +197,26 @@ spec: app.kubernetes.io/name: {{ template "seaweedfs.name" $ }} app.kubernetes.io/instance: {{ $.Release.Name }} {{- if $egressCfg.allowDNS }} +{{- /* Emit only the selectors that are set. An empty one has to be left out + rather than rendered as null: a missing podSelector means every pod in + the namespace, and a missing namespaceSelector means this namespace + instead of the one DNS runs in. */}} +{{- if not (or $egressCfg.dnsNamespaceSelector $egressCfg.dnsPodSelector) }} +{{- fail "networkPolicy: egress.allowDNS is on but both egress.dnsNamespaceSelector and egress.dnsPodSelector are empty, which would open port 53 to every pod in the release namespace. Set at least one of them, or turn allowDNS off and name the resolver in egress.extraEgress." }} +{{- end }} - to: +{{- if and $egressCfg.dnsNamespaceSelector $egressCfg.dnsPodSelector }} - namespaceSelector: {{- toYaml $egressCfg.dnsNamespaceSelector | nindent 12 }} podSelector: {{- toYaml $egressCfg.dnsPodSelector | nindent 12 }} +{{- else if $egressCfg.dnsNamespaceSelector }} + - namespaceSelector: + {{- toYaml $egressCfg.dnsNamespaceSelector | nindent 12 }} +{{- else }} + - podSelector: + {{- toYaml $egressCfg.dnsPodSelector | nindent 12 }} +{{- end }} ports: - protocol: UDP port: 53 @@ -212,6 +227,11 @@ spec: {{- if not $egressCfg.kubeApiServer.cidrs }} {{- fail (printf "networkPolicy: %s needs the Kubernetes API server, but networkPolicy.egress.kubeApiServer.cidrs is empty. Set it to your API server address(es), or disable networkPolicy.egress.kubeApiServer and allow it through networkPolicy.egress.extraEgress." $component) }} {{- end }} +{{- /* An empty port list is not "no ports" in a NetworkPolicy, it is every + port, so refuse it rather than quietly opening the CIDR up. */}} +{{- if not $egressCfg.kubeApiServer.ports }} +{{- fail "networkPolicy: egress.kubeApiServer.ports is empty, which allows every port on the API server CIDR(s) rather than none. Set it (the default is [443, 6443]), or turn egress.kubeApiServer off and describe the access in egress.extraEgress." }} +{{- end }} {{- range $cidr := $egressCfg.kubeApiServer.cidrs }} - to: - ipBlock: