fix(scanners): own copied pull-secrets at create time so they GC with the Job - #505
Merged
Conversation
… the Job image-scan pull-secret copies were created without an ownerReference and then patched with one via a post-create Secret Update. The runner ServiceAccount has `create` but not `update`/`delete` on secrets (and setting BlockOwnerDeletion additionally needs `update` on jobs/finalizers, which it also lacks), so the Update was rejected and silently dropped — every copied `imgps-*` secret was left orphaned in the scanner namespace, accumulating forever. Stamp the ownerReference at Secret *create* time instead (needs only `create`, which the agent has). Because the reference needs the Job UID, and the pod must not pull before its credentials exist, create the Job suspended, create the copies owned by it, then resume it (patch spec.suspend=false — `patch` on jobs is already granted). BlockOwnerDeletion is dropped (unnecessary for GC and would require jobs/finalizers). If resume fails, the Job is deleted so its owned copies GC with it rather than lingering. No RBAC/chart change and no expansion of the agent's secret permissions. Verified: unit test asserts the copy is Job-owned and the Job is not left suspended; the real-apiserver integration test (ScheduleJob_AutoCopyPullSecrets) confirms ownership end to end.
There was a problem hiding this comment.
Code Review
This pull request refactors the Kubernetes Job creation flow to safely handle image-pull secrets. Instead of copying secrets beforehand, the Job is created in a suspended state, the copied secrets are created with the Job's UID as the owner reference, and then the Job is resumed. A review comment points out a critical issue in the error handling path: if resuming the Job fails due to context cancellation or timeout, the cleanup Delete call will also fail because it uses the same cancelled context. It is recommended to use a non-cancelled context (such as context.WithoutCancel(ctx)) for the cleanup deletion and log any failures.
resumeJob most often fails because ctx was cancelled/timed out; the cleanup Delete reused that ctx and would fail immediately, leaking the suspended Job. Detach via context.WithoutCancel (bounded to 15s) and log a delete failure. Addresses PR review feedback.
blue4209211
approved these changes
Jul 4, 2026
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.
Description
Copied image-pull secrets (
imgps-*) were leaking: they accumulate in the scanner namespace and never get cleaned up.Root cause
The copy flow created each Secret without an ownerReference, then tried to attach one via a post-create Secret
Update(ownReferencedSecrets). But the runner ServiceAccount hascreateon secrets and notupdate/delete. Additionally, the ownerReference setBlockOwnerDeletion: true, which the GC admission plugin only allows if the caller hasupdateon the owner'sfinalizerssubresource (jobs/finalizers) — which the agent also lacks. So theUpdatewas rejected (403) and, being best-effort, silently dropped. Result: every copied secret was orphaned with no ownerReference, so neither the Job's TTL nor the reaper ever garbage-collected it.(Confirmed on a live cluster:
kubectl auth can-i update secrets→ no,update jobs/finalizers→ no; observedimgps-*secrets with empty ownerReferences.)Fix
Stamp the ownerReference at Secret create time (needs only
create, which the agent has). Since the reference needs the Job UID and the pod must not pull before its credentials exist:patch spec.suspend=false—patchon jobs is already granted).BlockOwnerDeletionis dropped (unnecessary for GC, and avoids thejobs/finalizersrequirement). If resume fails, the Job is deleted so its owned copies GC with it instead of lingering.No RBAC/chart change, and no expansion of the agent's secret permissions (a customer-deployed agent stays at
create-only on secrets).Testing
TestAutoCopyPullSecrets_CopiesRegistrySecretsAndAttaches): asserts each copy is Job-owned and the Job is not left suspended.TestPrimitives_RealAPIServer/ScheduleJob_AutoCopyPullSecrets): confirms the copy is owned by the Job end to end. Both pass;go build ./...,go vet,gofmtclean.Notes
SCANNER_AUTO_COPY_PULL_SECRETSenablement (separate PRs) that make private-image scans work in the first place.