Fence the culler, and refuse a release that leaves a pod unfenced - #277
Conversation
bc279ec to
66c404f
Compare
|
Rebased onto Confirmed, and it reaches a shipped target: Three NetworkPolicies render there — postgresql, server, computer — and the CronJob carries The second commit adds that policy. It is narrower than the culler's, because the pod holds less: DNS, the database, and the API server on its service port, which is where After it, every values file renders and the gate passes:
The gate was written for the culler and caught this on its own, which is the case it was written for. If you would rather the routines policy landed with #283 instead, say so and I will drop the second commit — this PR just happened to own the file. |
66c404f to
768fac8
Compare
|
Rebased onto v0.0.5. Three commits now. The third one touches your Its carrier lookup matches any resource with the component label, and the culler now has a NetworkPolicy carrying that label too — so it counted two and failed on a CronJob field that had not changed: The message already says "workloads", so I narrowed the filter to the kinds that carry a pod rather than relabelling the policy — every other policy in that file carries its component label and mine would have been the odd one out. Twelve lines, no behaviour change otherwise. Checked it still catches what it is for: drifting the culler's helm lint passes on all five, typecheck and biome are clean, server suite is 1604 pass 0 fail. If you would rather that file stayed yours, say so and I will drop the label from my policy instead. |
|
Correcting something I claimed earlier in this PR. I wrote that the culler's policy gives it no rule to the internet. That is not what renders. It is the same rule the server's policy already carries, for the reason that one gives — the API server answers on a ClusterIP from a range no chart knows at template time — and the culler ends up with strictly fewer rules than the server, which also has 80 and 443 to everything outside the private ranges. So the shape is the chart's existing trade rather than something new here. But "no rule to the internet" was wrong, and it should not need a render to discover, so the template now says what the rule permits and The routines policy has no such rule: DNS aside, its database and API-server egress are both bound by podSelector. I also cut my comments back to single lines while I was in there. The three block comments I had added were out of proportion to what they explained. Nothing changed in what renders — comments and one values line. All five targets still render and pass both checks, helm lint passes on all five, and the server suite is 1604 pass 0 fail. |
Closes #276
What this changes
networkPolicy.enabledrendered two policies, selectingcomponent: serverandcomponent: computer. The culler CronJob's pod carriescomponent: cullerand was selected by neither. A NetworkPolicy applies to the pods its selector matches, and a pod nothing selects keeps the cluster default rather than being denied — so turning the switch on fenced the API and the computers and left open the one pod that mounts a service account token bound tocreate, patch, deleteon Sandboxes and carries the API's whole environment,KEY_ENCRYPTION_KEYandBETTER_AUTH_SECRETincluded. It wakes every five minutes.This adds the third policy. It is narrower than the API's because the culler does less: DNS, the database, and the Kubernetes API server. No rule to the internet — it never calls Intelligence, an identity provider or a Bot, and the exception-list egress the API needs is exactly what this pod should not have. No ingress at all, since nothing connects to a CronJob.
The database rule needed care. The bundled-database egress is behind
postgresql.enabled, and both shipped targets that turn policy on use an external one — so a policy handling only the bundled case would have left the culler unable to reach its own database and turned a silent gap into a broken sweep. The chart already refusesnetworkPolicy.enabledwith an external database unlessnetworkPolicy.extraEgressnames it, so that list is where the address already is.cullerExtraEgressfalls back to it, which keeps an operator from writing the same CIDR twice and keeps an upgrade from breaking the sweep. Set it to give the culler a narrower list than the API's.The check that would have caught it
scripts/check-rendered-chart.tsgains one: on a release that has NetworkPolicies at all, every standing workload must be selected by one. The migrations Job is skipped, because it runs once at install and is torn down.This is asked of the rendered objects rather than the templates, because the question is which pods came out, not which conditionals were written. It is the same reason the existing 443 check is written that way, and it generalises: the next component added to this chart cannot quietly arrive without a policy.
Where it runs
Boundary and audit
Changelog
No
CHANGELOG.mdentry proposed, but this is the one of my recent changes where an entry has the best case, so say the word. A deployment withnetworkPolicy.enableddoes behave differently afterwards: the culler's egress is restricted where it was unrestricted. On a cluster whose CNI enforces policy and whose database sits somewhereextraEgressdoes not name, that is a sweep that starts failing — which is the correct outcome, but not a silent one.Proof
The gates
Rendered against
charts/openbot/ci/eks-sandbox-values.yaml, which setsnetworkPolicy.enabled: true:The culler policy is selected by the culler pod and by nothing else. Parsing the rendered output and comparing every policy's
podSelector.matchLabelsagainst the CronJob's pod labels:Both database shapes
External database, inheriting
extraEgress, which is what the two policy-enabled targets use:Bundled database:
The check fails on main
Rendering
mainin a detached worktree and running this branch's checker against that output:Against this branch's render:
Everything else
All five CI values files render and pass the checker:
helm lintpasses on all five.bun run typecheckpasses acrossapp,serverandworker.biome lintreports nothing on 446 files andbiome formatleaves the tree clean. The server suite is 1293 pass, 0 fail.Not done: no live cluster. Everything here is
helm templateoutput and a check over it, so it proves what the chart renders and not that a CNI enforces it. The claim that an unselected pod is unrestricted rather than denied is the Kubernetes rule this rests on, and it is the same rule the computers' policy in this file was written for.