fix(webhook): reuse Helm-native serving cert on upgrade - #218
Open
giuliocalzo wants to merge 1 commit into
Open
Conversation
When cert-manager is disabled, the chart generates the webhook serving certificate with genCA/genSignedCert. These regenerate on every render, so each `helm upgrade` rotated the CA/cert and the injected caBundle while the running webhook pod kept serving the old cert, breaking admission until the pod was restarted. Look up the existing TLS Secret and reuse its cert/CA when present, regenerating only when the Secret is missing or incomplete (first install, or `helm template`/`--dry-run` where lookup returns empty). Add helm-unittest coverage for both branches: PEM generation on first install, and verbatim reuse via a kubernetesProvider-mocked Secret. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
giuliocalzo
requested review from
SkylerMalinowski,
alanmutsch,
catblade,
vivian-hafener and
wickberg
as code owners
June 26, 2026 12:00
Contributor
|
Good afternoon @giuliocalzo, I will look into this. Best, |
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.
Summary
When
certManager.enabled=false, the operator chart generates the webhook serving certificate in Helm using Sprig'sgenCA/genSignedCert. Because those functions produce fresh material on every render, eachhelm upgraderotated the CA, the serving cert, and the injectedcaBundle. The running webhook pod is not restarted on upgrade (no checksum annotation), so it kept serving the old cert while the API server was updated to trust only the new CA — breaking admission until the pod was manually rolled.This PR makes the Helm-native path reuse the existing certificate:
lookupthe existing TLSSecret; if it has a complete cert (tls.crt/tls.key/ca.crt), reuse it verbatim for both theSecretand the injectedcaBundle.Secretis missing/incomplete — i.e. first install, orhelm template/--dry-runwherelookupreturns an empty map.Secretand the CA injected ascaBundlecome from the same source, so they stay consistent within a render.cert-manager mode (
certManager.enabled=true, the default) is unchanged.Checklist
Breaking Changes
None. cert-manager mode (the default) is unchanged, and the Helm-native path now preserves existing certificate material across upgrades instead of rotating it.
Testing Notes
helm templatewithcertManager.enabled=false: single TLSSecret, one uniquecaBundle,Secret.ca.crt == caBundle.helm templatewithcertManager.enabled=true: no nativeSecret/caBundle, cert-managerIssuer/Certificate+ inject annotations present (default snapshot unchanged).helm unittest ./helm/slurm-operator: 105 tests / 12 snapshots pass, including:caBundleon every webhook, no cert-manager annotations),Secretmocked viakubernetesProvider,Secret/caBundle.helm lintpasses with cert-manager both enabled and disabled.Additional Context
Fixes broken admission after
helm upgradein the Helm-native (non cert-manager) certificate mode.