fix(security+k8s): retry cosign sign on Rekor 409 conflict; honor cloudExtras startupProbe + periodSeconds#316
Merged
Merged
Conversation
…udExtras startupProbe + periodSeconds Two production-surfaced fixes: 1) Image signing: parallel CI deploys of stacks sharing one image can fail with Rekor "[POST /api/v1/log/entries][409] createLogEntryConflict" — an identical entry already exists, typically because cosign re-uploaded an entry whose first attempt timed out client-side but succeeded server-side. The signature is valid and present, yet the deploy aborts. Retry the full cosign invocation (fresh ephemeral / randomized-ECDSA signature cannot conflict with itself) up to 3 attempts; any non-conflict error still fails fast. Applied to both keyless and key-based signers via a shared helper. 2) Probes: cloudExtras.startupProbe was silently dropped — CloudExtras and DeploymentConfig had no StartupProbe field, so slow-booting services were stuck with the readiness probe (or its 60s default window) as their cold- start budget and got SIGTERM-killed mid-boot on fresh nodes. Add the field and thread it through gke_autopilot -> DeploymentConfig -> DeploymentArgs -> container spec with the same ingress-container fallback semantics as readiness/liveness. Also add the k8s-native periodSeconds spelling to CloudRunProbe (precedence over the legacy duration-typed interval, which does not survive YAML conversion of plain integers). Tests: Rekor-conflict detection + retry/no-retry/give-up behavior via an exec seam; ConvertDescriptor regression tests proving startupProbe/periodSeconds now survive the yaml mapping; toProbeArgs periodSeconds precedence table. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Cre-eD
requested review from
Laboratory,
smecsia and
universe-ops
as code owners
June 10, 2026 09:18
Semgrep Scan ResultsRepository:
Scanned at 2026-06-10 13:57 UTC |
Security Scan ResultsRepository:
Scanned at 2026-06-10 13:57 UTC |
…onds and Rekor-409 self-heal - kube_run_probe_test.go converted from testify to gomega per docs/TESTING.md (gomega is the mandated framework; the neighboring testify file predates the policy). - GKE Autopilot guide: startupProbe added to the Complete CloudExtras Reference yaml + field table. The reference already promised periodSeconds on probes before the code supported it — now both are true. - Troubleshooting: createLogEntryConflict (Rekor 409) entry documenting the cause and the automatic fresh-signature retry. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
…honor explicit probes on multi-port containers; DI exec seam Codex + Gemini review findings: - gke_autopilot_stack.go forwards readiness/liveness into DeploymentArgs but not the new StartupProbe — the GKE Autopilot path (the primary consumer) silently dropped it, reintroducing the original bug one layer up. Wired. - deployment.go required a single port / mainPort even for an EXPLICIT startup probe, dropping it on multi-port containers; readiness applies explicit probes unconditionally and toProbeArgs resolves the port from httpGet.port / mainPort / first port. Mirrored that behavior. - Replaced the package-global execCommand test seam with an exec field on both signers (data race if tests ever parallelize; proper DI). - Retry warning now goes to stderr (stdout can be piped/parsed) and no longer claims a "fresh signature" — key-based signing with deterministic keys (ed25519) reproduces the same signature; documented why exhausting the loop and failing is the correct outcome there (a tlog entry existing does not prove the signature reached the registry). - IntervaSeconds typo field marked Deprecated. - Tests: key-based 409 persistent-conflict (exhausts loop, surfaces error) and transient-conflict (one retry then success) cases added. Declined (with rationale): applying the global startupProbe to non-ingress containers — global probes are deliberately ingress-only, mirroring the readiness/liveness semantics; HTTP probes on worker/sidecar containers is the failure mode that guard exists to prevent. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Contributor
Author
Preview validation — PASSPreview build startupProbe / periodSeconds wiring (the silent-drop fix):
Pod went 2/2 Ready, app serving 200 end-to-end after the deploy (one transient 521 during the normal Caddy aggregator rollout window). Signing: the deploy's scan→sign→attest→verify chain completed successfully with the retry-wrapped sign path (no Rekor conflict occurred in this run; conflict behavior is covered by the unit suite — keyless retry-then-succeed, non-conflict fail-fast, bounded exhaustion, and the key-based deterministic-conflict cases). |
smecsia
previously approved these changes
Jun 10, 2026
universe-ops
previously approved these changes
Jun 10, 2026
govulncheck fails on GO-2026-5039 (net/textproto) + GO-2026-5037 (crypto/x509), both fixed in go1.26.4 — pre-existing on main (red since 2026-06-04), surfaced here because the gate blocks this PR. setup-go reads go-version-file: go.mod, so the directive bump moves CI to the patched toolchain. No toolchain line, no dependency changes. Also trims multi-line comment blocks added by this PR down to one-liners. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
smecsia
approved these changes
Jun 10, 2026
universe-ops
approved these changes
Jun 10, 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
3 participants
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.
Two production-surfaced fixes, bundled per repo convention.
1. Signing: tolerate Rekor
409 createLogEntryConflictby retrying with a fresh signatureWhen several CI deploys run in parallel for stacks that share one image (one compose file, several
runs:stacks),sc image signcan abort with:An identical entry already being in the tlog means the signature upload effectively succeeded — keyless ephemeral keys (and randomized-ECDSA key-based signatures) can't produce identical entries across independent invocations, so the typical cause is cosign re-uploading an entry whose first attempt timed out client-side but landed server-side. The image ends up signed, yet the deploy fails. Observed breaking every push-fanout deploy in a multi-stack repo; single-stack dispatches passed.
Fix:
runCosignSignretries the fullcosign signinvocation (up to 3 attempts) when — and only when — the failure is a Rekor entry conflict. A fresh invocation mints a fresh signature, so a retry cannot conflict with itself. Any other error still fails fast. Shared by the keyless and key-based signers.2. Probes:
cloudExtras.startupProbewas silently dropped;periodSecondsunsupportedk8s.CloudExtras/DeploymentConfighadreadinessProbe+livenessProbebut nostartupProbefield, so a user-supplied startup budget never reached the cluster — the container fell back to the readiness probe (or its 60s default TCP window) as its cold-start budget. On a freshly-provisioned Autopilot node (image pull + migrations + worker boot) that gets the pod SIGTERM-killed mid-boot, which converts an ordinary node move into an outage for single-replica services.Additionally
CloudRunProbeonly understood the duration-typedintervalkey; configs written with the k8s-nativeperiodSecondsspelling silently lost their probe period (plain integers don't survive the YAML→time.Durationconversion).Fix:
CloudRunProbe.PeriodSecondsadded; takes precedence overintervalintoProbeArgs.StartupProbeadded toCloudExtras+DeploymentConfig+DeploymentArgs, threadedgke_autopilot → kube_run → deploymentwith the same ingress-container fallback semantics as readiness/liveness. Existing behavior (startup := readiness when nothing is specified) is unchanged.Tests
TestIsRekorConflict(real cosign stderr shapes, incl. non-conflict 409s that must NOT match),TestKeylessSigner_Sign_RetriesOnRekorConflict,…_NoRetryOnOtherErrors,…_GivesUpAfterMaxConflictAttempts— via anexecCommandseam, asserting exact call counts.TestStartupProbeExtraction/TestProbePeriodSecondsExtraction/TestStartupProbeAbsent—ConvertDescriptorregression tests proving the yaml keys now survive the mapping (the original bug class).TestToProbeArgs_PeriodSeconds— precedence table (periodSeconds>interval> unset).go test ./pkg/security/signing/ ./pkg/clouds/k8s/ ./pkg/clouds/pulumi/kubernetes/ ./pkg/clouds/gcloud/— all pass.