Skip to content

fix(deploy): kates deploy puts the cert-manager webhook back to failurePolicy Fail even when the deploy group is cancelled - #392

Merged
bmscomp merged 1 commit into
mainfrom
fix/deploy-restores-webhook-policy
Oct 18, 2026
Merged

bmscomp merged 1 commit into
mainfrom
fix/deploy-restores-webhook-policy

Conversation

@bmscomp

@bmscomp bmscomp commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Item 1.5 of the CLI performance and stability plan (finding F10). Independent of the other Wave 1 PRs.

Why

Deploy can leave the cert-manager webhook fail-open. On main (6f4cf61), cli/cmd/deploy_components.go:886-904, inside the Group A errgroup:

  • When the ClusterIssuer cannot get past the webhook, the code sets failurePolicy: Ignore on the mutating and validating configs, applies the issuer, and patches Fail back.
  • All three run on gCtx, and the restore's errors are dropped.
  • When a sibling branch (Strimzi, monitoring, Kyverno) fails meanwhile, gCtx is already cancelled, so the restore's kubectl patch never runs, and nothing says so.
  • Later deploys skip cert-manager because its release exists, so the webhook stays Ignore.
  • The comment says cert-manager reconciles the configs back. It does not; only a helm upgrade of cert-manager does.

Changes

cli/cmd/deploy_components.go:

  • The restore is the new restoreCertMgrWebhookPolicy, deferred straight after the bypass message and before the first Ignore patch. It runs on context.WithoutCancel(gCtx) with a 30 s timeout, and prints a warning naming the config when a patch fails.
  • The inline restore loop and the false comment sentence are gone.
  • The success line reads "ClusterIssuer created. Restoring the webhook policy.", since the restore now runs as the branch returns.

Tests

Added TestRestoreCertMgrWebhookPolicyOutlivesCancel. It calls the restore with an already-cancelled context, and the stub kubectl refuses a cancelled context as exec.CommandContext would. Both Fail patches are still issued, mutating then validating, each on a context with a deadline. A restore on gCtx, as before, issues none.

Run locally, passing:

  • cd cli && go test ./..., go test -race ./cmd, go vet ./....
  • golangci-lint run --new-from-rev=origin/main ./...: 0 issues.
  • scripts/check-cli-style.sh and scripts/check-cli-compat.sh: OK.
  • The backend suite, which cli/** triggers (Build & Test): this PR touches nothing under kates/. ./mvnw test on main's kates/ tree ran 1,254 tests with 0 failures and 0 errors, once for all of Wave 1.

Skipped locally: govulncheck, as go.mod is unchanged.

CI only: ./mvnw verify with the *IT tests, the Kind and Docker jobs, CodeQL and Trivy.

…rePolicy Fail even when the deploy group is cancelled

When the ClusterIssuer could not get past the cert-manager webhook, deploy set
failurePolicy: Ignore on both webhook configs, applied the issuer, and set Fail
again, all on the Group A errgroup's context. When a sibling branch failed
meanwhile, that context was already cancelled, the restore patches never ran
and their errors were dropped, and the webhook stayed fail-open. Later deploys
skip cert-manager because its release exists, and the comment's claim that
cert-manager reconciles the configs back is wrong: only a helm upgrade of it
does.

The restore is now deferred straight after the bypass, on
context.WithoutCancel(gCtx) with a 30 s timeout, and warns on a patch that
fails. TestRestoreCertMgrWebhookPolicyOutlivesCancel issues it on a cancelled
context and gets both Fail patches, each with a deadline.
@bmscomp
bmscomp merged commit 3aaa515 into main Oct 18, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant