Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(webhook) exclude Helm Secrets #500

Merged
merged 1 commit into from
Nov 24, 2021
Merged

fix(webhook) exclude Helm Secrets #500

merged 1 commit into from
Nov 24, 2021

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Nov 23, 2021

What this PR does / why we need it:

Hurray! We added Secret validation! Oh no! We added Secret validation!

Helm charts with webhooks that handle Secrets run into an issue that
prevents changes after an action that enables the webhook:
helm/helm#10023

Because Helm's Secret for release information is subject to the webhook,
Kubernetes will attempt to validate it, likely before the webhook
service comes online (because Helm just created the Pod that will
provide it). If the service is not online, validation fails, and Helm
cannot update its Secret to mark the release status, usually leaving it
stuck in a pending state that blocks future interactions.

This change excludes Helm Secrets from our validation, because we have
no need to validate them.

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

Helm charts with webhooks that handle Secrets run into an issue that
prevents changes after an action that enables the webhook:
helm/helm#10023

Because Helm's Secret for release information is subject to the webhook,
Kubernetes will attempt to validate it, likely before the webhook
service comes online (because Helm just created the Pod that will
provide it). If the service is not online, validation fails, and Helm
cannot update its Secret to mark the release status, usually leaving it
stuck in a pending state that blocks future interactions.

This change excludes Helm Secrets from our validation, because we have
no need to validate them.
@rainest rainest requested a review from a team as a code owner November 23, 2021 20:43
@rainest rainest changed the base branch from main to next November 23, 2021 20:44
@mflendrich
Copy link
Contributor

I'm wondering whether this commands any change to the deploy/kustomize manifests we have in the KIC repo, but I don't see an obvious reason that it would.

@rainest
Copy link
Contributor Author

rainest commented Nov 24, 2021

The risk of hitting this with plain manifests should be dramatically less: you'd need to intentionally deploy the manifest and then install or upgrade some unrelated Helm release immediately after.

The proper solution is that we require labels on credential Secrets so we can use a selector for them only and get rid of all unnecessary Secret checks, but that's a breaking change.

@rainest rainest merged commit 4ebfe8d into next Nov 24, 2021
@rainest rainest deleted the fix/exclude-helm branch November 24, 2021 18:49
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.

None yet

2 participants