Fence the culler, and refuse a release that leaves a pod unfenced - #277
Open
zopeVaibhav wants to merge 1 commit into
Open
Fence the culler, and refuse a release that leaves a pod unfenced#277zopeVaibhav wants to merge 1 commit into
zopeVaibhav wants to merge 1 commit into
Conversation
zopeVaibhav
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
August 27, 2026 14:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.