Mirror secrets to workspaces chosen by a label selector by zoroyihan7 · Pull Request #768 · AMD-AGI/Primus-SaFE

zoroyihan7 · 2026-10-10T11:31:34Z

Problem

A Secret in the primus-safe namespace is mirrored by resource-manager only into the workspaces listed in its primus-safe.workspace.ids annotation. That list is hand-kept, and the controller does not watch Workspaces, so a workspace created afterwards never gets its copy. For sandbox.extra_ca_secret, which the sandbox template mounts into every Sandbox Pod, that leaves a new workspace's Sandbox Pods stuck on the volume mount.

Change

primus-safe.workspace.selector (new Secret annotation): a label selector in kubectl -l syntax over Workspace labels, honoured only on a Secret that also carries primus-safe.secret.broadcast: "true". The Secret is mirrored into every matching Workspace that is not being deleted, in union with the ids list. The ids list keeps its meaning, including as the authorisation that image-secret binding and CI/CD proxy validation read; the selector only decides where copies go and is never written back into ids.
Selectable labels: only primus-safe.workspace.scope.sandbox, and only positive operators (=, ==, in, exists). Other labels take their value from what a workspace creator submits; a negative operator is true of a workspace that lacks the label.
Derived label primus-safe.workspace.scope.sandbox=true: set by the workspace mutating webhook from Spec.Scopes (no scopes, or Sandbox among them). The controller matches against the value derived from Spec.Scopes, so a workspace written before the label existed is selected at once; kubectl -l may disagree for such a workspace until its next update.
Workspace watch: the secret controller watches Workspaces (create, delete, deletion start, label or sandbox-scope change) and enqueues the Secrets that carry a selector or name the workspace in their ids. This also fixes the ids case where the workspace appears after the Secret.
Removal: a workspace that stops matching loses the copy, only if it carries the primus-safe/secret-mirror=true label the controller sets. A same-named Secret reached only through the selector and not made by the controller is neither overwritten nor deleted (Warning event MirrorConflict). Listing a workspace by id still takes over such a Secret, as before.
Freeze on an invalid selector: an unparsable or disallowed selector, or one without the broadcast mark, mirrors nothing new and removes nothing. The workspaces a valid selector last selected are recorded by the controller in primus-safe.workspace.selector.applied; while frozen, those copies are kept and kept in sync, and the ids list applies as usual. If that record is missing or unreadable (for example after kubectl replace), it is treated as unknown: every existing copy is left as it is, ids removals included, and only the ids list is synced, until the selector is fixed (which rewrites the record) or removed. The set of existing copies is not rebuilt from the data plane: anyone who can create a Secret in their namespace can put the mirror label on it. The record is widened to old-plus-new before any data plane is touched, and if that write fails nothing is mirrored. The reason is in primus-safe.workspace.selector.error and raised as a Warning InvalidWorkspaceSelector once per new reason.
Finalizer: added to Secrets with mirror targets that lack it (made with kubectl), and released once the Secret has no targets and its copies are gone. On delete, the cross-data-plane cleanup runs only for a Secret that may have been mirrored (ids, selector, broadcast mark or a selector record); other Secrets, such as SSH keys, are released without reaching any data plane, so an offline data plane does not hold them.
Per-target failures: one workspace failing (namespace missing, cluster not ready) does not starve the others; a target that failed is not cleaned up; errors are returned for the default backoff.

Data plane requests

Cleanup used to issue one GET secrets/<name> per workspace on every reconcile (plus a DELETE per stale copy). It now issues one LIST secrets per data plane cluster, across namespaces, filtered by labelSelector=primus-safe/secret-mirror=true and fieldSelector=metadata.name=<name>, so the response holds only this Secret's copies. With W workspaces on C clusters a reconcile goes from W GETs to C LISTs; the DELETEs are unchanged. This needs list secrets across namespaces on the data plane; resource-manager reaches each data plane with the cluster's control-plane client certificate, the same client that creates and deletes workspace namespaces there.

Tests

resource-manager/pkg/resource/secret_selector_test.go (fake controller client plus fake data-plane clientset) covers: matching and non-matching workspaces; workspaces created later (selector and ids); label/scope removal removing only the controller's copy; no take-over of a hand-made Secret; workspaces being deleted excluded; ids ∪ selector; ids-only behaviour unchanged; blank selector; invalid selectors (each rejected form) frozen with the reason recorded; broadcast mark required; ids edits under a frozen selector; unknown record after kubectl replace, a forced server-side apply, or a damaged record (copies kept as they are, only ids synced, nothing added, freeze lifted by fixing the selector); a forged mirror label not trusted under an unknown record; applied record write failure stopping before any data plane write; Warning raised once per reason; delete of a selector-only, a kubectl-made, and an annotation-stripped Secret; delete of an unrelated Secret with an offline data plane; one LIST per cluster in cleanup; finalizer release; per-target failures and cluster-not-ready backoff.

webhooks/pkg/workspace_webhook_test.go: TestWorkspaceMutateSandboxScopeLabel.

Mutation check: reverting each fix of the latest commit (continue after a failed record write, always emit the event, always clean up on delete, drop the record from the delete check, per-workspace GET, missing record treated as empty, copies rebuilt from the data plane) fails the test written for it.

Not in scope: a source Secret deleted with all its mirror annotations removed before the controller reconciled the removal leaves its copies, as on main. A same-named Secret created by someone else between our Get and Create is detected after the AlreadyExists and treated as a conflict; the next event reconciles it.

🤖 Generated with Claude Code

A Secret in the primus-safe namespace could only be mirrored into the workspaces named in its primus-safe.workspace.ids list. A workspace created later never got its copy, and nothing re-read the list when one appeared, so a Secret every sandbox mounts (sandbox.extra_ca_secret) left new workspaces with Pods stuck on the volume. - New annotation primus-safe.workspace.selector: a label selector in kubectl -l syntax over Workspace labels. The Secret is mirrored into every matching workspace that is not being deleted, in union with the ids list, which keeps its meaning (and stays the authorisation that image-secret and CI/CD proxy checks read; the selector grants nothing). - The secret controller now watches Workspaces: create, delete and label changes enqueue the Secrets that carry a selector or list the workspace by id, so a workspace gets or loses its copy without anyone touching the Secret. - A workspace that stops matching loses the copy the controller made; same-named Secrets it did not make are never overwritten through the selector, nor deleted. - An empty selector selects nothing (labels.Parse("") would be Everything). An invalid one raises a Warning event, mirrors nothing beyond the ids, and removes nothing until it is fixed. - The workspace webhook derives primus-safe.workspace.scope.sandbox=true from Spec.Scopes (no scopes, or Sandbox among them), so "primus-safe.workspace.scope.sandbox=true" names every workspace that runs sandboxes. values.yaml and the upgrade doc now say so. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

codecov · 2026-10-10T11:33:42Z

Codecov Report

❌ Patch coverage is 81.85053% with 51 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...resource-manager/pkg/resource/secret_controller.go	81.45%	29 Missing and 22 partials ⚠️

📢 Thoughts on this report? Let us know!

The sandbox scope label was set only by the workspace webhook, so a workspace written before it existed carried none. A selector-only Secret then selected nothing there and its cleanup deleted the copy the workspace's sandboxes already mount. Match selectors against the workspace's labels with the sandbox scope label taken from Spec.Scopes, and add a small controller that writes the label onto workspaces whose label disagrees with their scopes; on start every existing workspace is seen once. Also enqueue on a scope change, cover deleting a selector-only Secret, and document that a selector should use primus-safe.* labels only. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…rget A workspace creator can set any label outside the primus-safe. prefix, so a Secret selector on one would hand the Secret to whoever sets it: treat such a selector as invalid. Also collect per-target errors instead of returning on the first, so one workspace whose namespace is not ready yet no longer starves the others or skips cleanup, and return them for default backoff rather than a 1s requeue. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

…selector The reserved-prefix check looked at the label key only, so a negated requirement such as !<misspelt reserved key> or <reserved key>!=false passed, and matched every workspace that lacks the label. Only =, ==, in and exists are accepted now. An invalid selector used to skip cleanup and return nil, leaving every mirrored copy in place with no visible record. It now behaves like an empty selector: copies are reclaimed, and the reason is written to the primus-safe.workspace.selector.error annotation on the Secret, which is removed once the selector is valid again. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…lizer to hand-made Secrets An invalid selector used to reclaim every copy, which stops every sandbox that mounts the extra CA. It now freezes: nothing new is mirrored by the selector, nothing is removed, the reason goes to primus-safe.workspace.selector.error and a Warning event. The extra-ca volume is optional so a missing copy no longer blocks Pod admission. A selector is honoured only on a Secret carrying primus-safe.secret.broadcast =true: scope.sandbox follows Spec.Scopes, which a workspace's editors can change, so without the mark they could pull a credential into their own namespace. Secrets made with kubectl carry no finalizer, so deleting one left its copies behind; the reconcile now adds it when the Secret has mirror targets. Document that rolling back to a release without selector support drops copies reached only by the selector. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… mark, restrict selectable labels - The Secret update predicate now compares the broadcast annotation, so adding the mark after a freeze reconciles the Secret. - An invalid selector freezes only the copies the selector made, recorded in primus-safe.workspace.selector.applied while it was valid; ids-list additions and removals (copy and workspace reference) keep working. With no record nothing is removed. - A selector may only use the system-derived sandbox scope label; other keys, including primus-safe.* ones filled from user input, are invalid. - Revert optional: true on the extra-ca volume: with restartPolicy Never a Pod that starts without the copy would die for good instead of waiting. - Document the finalizer on hand-made Secrets for helm uninstall. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…only Secrets - With no selector.applied record the selector never mirrored anything, so a frozen selector no longer holds back the ids list: copies and workspace references of workspaces removed from ids are reclaimed. An unreadable record still freezes everything. - The Secret Create predicate also passes Secrets that carry our finalizer or are being deleted, so a Secret whose annotations were removed while the controller was down can still be cleaned up and released. - Document that removing or falsifying the broadcast annotation freezes the selector rather than removing copies; removing the selector annotation does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…kspace; sync frozen copies - Once a Secret has no mirror targets and its copies are cleaned, the finalizer added for them is removed (kept while a cluster references the Secret, since the delete path clears that reference). - cleanupMirroredSecrets now continues past a workspace whose cluster is gone or whose data plane client is not ready, and returns the errors together. - A frozen selector keeps syncing the copies it made (the applied record), so a rotated CA still reaches them; nothing is added or removed. - A workspace holding a Secret resource-manager did not make is not recorded as mirrored into; the record is written after the mirror loop. - Document the frozen semantics and the scale limit of the applied record. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…applied record - delete() cleans up mirrored copies unconditionally (the mirror label guards what it removes), so annotations removed before the delete cannot leave copies behind. - Remove the workspace scope label backfill controller: its patch went through the workspace webhook, which rejects workspaces whose NodeFlavor is gone and can reset Spec.Replica of legacy workspaces with no NodeFlavor. The mirror derives the label in memory and does not need it persisted. - Widen the applied record to old plus selected before the mirror loop, so a crash mid-loop cannot leave an incomplete record. - A blank selector is no mirror target, so it no longer holds the finalizer. - copySecret re-reads on AlreadyExists and reports a conflict for a Secret not carrying the mirror label. - Parse the applied record before testing it. - Document that scope editors can match the selector, so broadcast must never go on a credential, and how a damaged applied record is repaired. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…s per cluster - Stop before touching any data plane when the widened applied record cannot be written; a copy made without it could later be removed by a frozen selector. - Treat a missing or unreadable applied record under an invalid selector as unknown: keep and sync every existing copy, ids removals included, until the selector is fixed or removed. The docs no longer say to delete the record. - On delete, clean up across data planes only for a Secret that may have been mirrored (ids, selector, broadcast mark or a selector record), so an SSH key is not held by an offline data plane. - Find copies with one List per data plane cluster (mirror label and name) instead of one Get per workspace. - Raise InvalidWorkspaceSelector only when the recorded reason changes. - Move the SECURITY note after the extra CA paragraph in values.yaml. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

With the applied record missing or unreadable, the set of existing copies was rebuilt from the data plane by the mirror label and then synced. Anyone who can create a Secret in their own namespace can put that label on it, so a credential whose selector was never valid could be written into it. Keep every copy as it is instead and sync only the ids list until the selector is fixed or removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

zoroyihan7 · 2026-10-13T03:31:56Z

Review round on f78289c, addressed in d803a7b and 256345e:

secret_controller.go:287 (widened applied record write fails): fixed in d803a7b. The reconcile now returns before any data plane is touched. TestAppliedRecordWriteFailureStopsMirroring (Patch fails as too large: error returned, the data plane is only read).
secret_controller.go:340 (invalid selector, record missing): fixed in d803a7b and 256345e. A missing or unreadable record is unknown: no copy is added or removed, ids removals included, and only the ids list is synced, until the selector is fixed (the controller then rewrites the record) or removed. The set of copies is deliberately not rebuilt from the data plane, since a namespace owner can put the mirror label on a Secret of their own. upgrade.md, values.yaml and the constant's comment no longer say to delete the annotation. Tests: TestInvalidSelectorWithoutRecordFreezesAsUnknown (kubectl replace, forced server-side apply, damaged record), TestNeverValidSelectorFreezesIdsRemovalUntilRemoved, TestUnknownRecordDoesNotTrustTheMirrorLabel.
secret_controller.go:180 (delete of unrelated Secrets): fixed in d803a7b. The cross-data-plane cleanup on delete runs only for a Secret with ids, a selector, the broadcast mark or a selector record; others are released without reaching a data plane. TestDeleteUnrelatedSecretIgnoresOfflineDataPlane. Remaining edge, as on main: all mirror annotations removed and the Secret deleted before the controller reconciles the removal leaves its copies.
secret_controller.go:522 / cleanup fan-out: fixed in d803a7b. Copies are found with one LIST per data plane cluster (mirror label plus metadata.name field selector) instead of one GET per workspace: W GETs become C LISTs per reconcile; DELETEs unchanged. Described in the PR body. TestCleanupListsOncePerCluster.
secret_controller.go:206 (event noise): fixed in d803a7b. InvalidWorkspaceSelector is raised only when the recorded reason changes. TestInvalidSelectorEventOnlyWhenReasonChanges.
values.yaml:372: fixed in d803a7b. The SECURITY note now follows the whole paragraph, which reads as one sentence again.

Not changed: secret_controller.go:311/816 (a Secret created between our Get and Create). The window is one round trip; copySecret re-reads after AlreadyExists and treats an unlabelled Secret as a conflict, and the next Secret or Workspace event reconciles it. Tracked as a follow-up.

Each new test fails with its fix reverted. go vet and the resource-manager, apis and webhooks tests pass.

zoroyihan7 · 2026-10-13T03:41:41Z

Security verification round on 256345e: CLEAN.

With an invalid selector and a missing or unreadable applied record, no data plane create, update or delete, and no control-plane write, takes its target set from the mirror label: only the ids list is written, and cleanup is skipped.
Cleanup deletes a copy only if it carries the mirror label, has the source's name, sits in the namespace of a workspace on that cluster, and that workspace is not kept. There is no further ownership check, as on main. A Secret someone labels by hand can be deleted only in a namespace they can already write to, so this is self-inflicted, not reachable against another tenant.

go vet and the package tests pass at this head.

zoroyihan7 and others added 2 commits October 10, 2026 11:26

Drop a selector guard the blank check already covers

cb4667d

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

zoroyihan7 requested a review from a team as a code owner October 10, 2026 11:31

zoroyihan7 added the enhancement New feature or request label Oct 10, 2026

zoroyihan7 and others added 10 commits October 10, 2026 11:43

weilei0120 approved these changes Oct 13, 2026

View reviewed changes

weilei0120 merged commit 9a06ce6 into main Oct 13, 2026
18 checks passed

weilei0120 deleted the l720-secret-mirror-selector branch October 13, 2026 06:08

zoroyihan7 mentioned this pull request Oct 13, 2026

Per-workspace sandbox environment Secret, writable only by admins #771

Merged

Navigation Menu

Repository navigation

Mirror secrets to workspaces chosen by a label selector - #768

Mirror secrets to workspaces chosen by a label selector#768
weilei0120 merged 12 commits into
mainfrom
l720-secret-mirror-selector

zoroyihan7 commented Oct 10, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

zoroyihan7 commented Oct 13, 2026

Uh oh!

zoroyihan7 commented Oct 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zoroyihan7 commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Data plane requests

Tests

Uh oh!

codecov Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zoroyihan7 commented Oct 13, 2026

Uh oh!

zoroyihan7 commented Oct 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zoroyihan7 commented Oct 10, 2026 •

edited

Loading

codecov Bot commented Oct 10, 2026 •

edited

Loading