Repository navigation
fix(deploy): kates deploy puts the cert-manager webhook back to failurePolicy Fail even when the deploy group is cancelled - #392
Merged
Conversation
…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.
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.
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:failurePolicy: Ignoreon the mutating and validating configs, applies the issuer, and patchesFailback.gCtx, and the restore's errors are dropped.gCtxis already cancelled, so the restore'skubectl patchnever runs, and nothing says so.Ignore.Changes
cli/cmd/deploy_components.go:restoreCertMgrWebhookPolicy, deferred straight after the bypass message and before the firstIgnorepatch. It runs oncontext.WithoutCancel(gCtx)with a 30 s timeout, and prints a warning naming the config when a patch fails.Tests
Added
TestRestoreCertMgrWebhookPolicyOutlivesCancel. It calls the restore with an already-cancelled context, and the stubkubectlrefuses a cancelled context asexec.CommandContextwould. BothFailpatches are still issued, mutating then validating, each on a context with a deadline. A restore ongCtx, 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.shandscripts/check-cli-compat.sh: OK.cli/**triggers (Build & Test): this PR touches nothing underkates/../mvnw teston main'skates/tree ran 1,254 tests with 0 failures and 0 errors, once for all of Wave 1.Skipped locally:
govulncheck, asgo.modis unchanged.CI only:
./mvnw verifywith the*ITtests, the Kind and Docker jobs, CodeQL and Trivy.