From 6af59e06bf6bb40496ce09f327557c8a317eb405 Mon Sep 17 00:00:00 2001 From: Giulio Calzolari Date: Fri, 26 Jun 2026 14:00:17 +0200 Subject: [PATCH] fix(webhook): reuse Helm-native serving cert on upgrade 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 --- .../templates/webhook/webhook.yaml | 53 +++++--- .../tests/webhook_webhook_test.yaml | 118 ++++++++++++++++++ 2 files changed, 155 insertions(+), 16 deletions(-) diff --git a/helm/slurm-operator/templates/webhook/webhook.yaml b/helm/slurm-operator/templates/webhook/webhook.yaml index 2cb13839..198a12a1 100644 --- a/helm/slurm-operator/templates/webhook/webhook.yaml +++ b/helm/slurm-operator/templates/webhook/webhook.yaml @@ -5,25 +5,46 @@ SPDX-License-Identifier: Apache-2.0 {{- if .Values.webhook.enabled }} {{- $certNamespacedName := printf "%s/%s" (include "slurm-operator.namespace" .) .Values.certManager.secretName -}} -{{- $durationDays := int 3650 -}} -{{- $ca := genCA .Values.certManager.secretName $durationDays -}} +{{- $caBundle := "" -}} {{- if not .Values.certManager.enabled }} -{{- $cn := (include "slurm-operator.webhook.name" .) -}} -{{- $dnsList := list (printf "%s.%s" (include "slurm-operator.webhook.name" .) (include "slurm-operator.namespace" .)) (printf "%s.%s.svc" (include "slurm-operator.webhook.name" .) (include "slurm-operator.namespace" .)) -}} +{{- $namespace := include "slurm-operator.namespace" . -}} +{{- $secretName := .Values.certManager.secretName -}} +{{- $durationDays := int 3650 -}} +{{- /* Reuse the existing serving certificate on upgrade to avoid rotating it + (and the injected caBundle) on every render. Regenerate only when the + Secret is missing/incomplete (e.g. first install). lookup returns an + empty map during `helm template`/`--dry-run`, so a cert is generated then. */ -}} +{{- $existing := lookup "v1" "Secret" $namespace $secretName -}} +{{- $existingData := dict -}} +{{- if $existing -}} +{{- $existingData = ($existing.data | default dict) -}} +{{- end -}} +{{- $tlsCrt := index $existingData "tls.crt" | default "" -}} +{{- $tlsKey := index $existingData "tls.key" | default "" -}} +{{- if and $tlsCrt $tlsKey (index $existingData "ca.crt") -}} +{{- $caBundle = index $existingData "ca.crt" -}} +{{- else -}} +{{- $cn := include "slurm-operator.webhook.name" . -}} +{{- $dnsList := list (printf "%s.%s" (include "slurm-operator.webhook.name" .) $namespace) (printf "%s.%s.svc" (include "slurm-operator.webhook.name" .) $namespace) -}} +{{- $ca := genCA $secretName $durationDays -}} {{- $cert := genSignedCert $cn nil $dnsList $durationDays $ca -}} +{{- $tlsCrt = $cert.Cert | b64enc -}} +{{- $tlsKey = $cert.Key | b64enc -}} +{{- $caBundle = $ca.Cert | b64enc -}} +{{- end -}} --- apiVersion: v1 kind: Secret metadata: - name: {{ .Values.certManager.secretName }} - namespace: {{ include "slurm-operator.namespace" . }} + name: {{ $secretName }} + namespace: {{ $namespace }} labels: {{- include "slurm-operator.webhook.labels" . | nindent 4 }} type: kubernetes.io/tls data: - tls.crt: {{ $cert.Cert | b64enc | quote }} - tls.key: {{ $cert.Key | b64enc | quote }} - ca.crt: {{ $ca.Cert | b64enc | quote }} + tls.crt: {{ $tlsCrt | quote }} + tls.key: {{ $tlsKey | quote }} + ca.crt: {{ $caBundle | quote }} {{- end }}{{- /* if not .Values.certManager.enabled */}} --- apiVersion: admissionregistration.k8s.io/v1 @@ -65,7 +86,7 @@ webhooks: scope: Namespaced clientConfig: {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} + caBundle: {{ $caBundle | quote }} {{- end }}{{- /* if not .Values.certManager.enabled */}} service: namespace: {{ include "slurm-operator.namespace" . }} @@ -106,7 +127,7 @@ webhooks: scope: Namespaced clientConfig: {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} + caBundle: {{ $caBundle | quote }} {{- end }}{{- /* if not .Values.certManager.enabled */}} service: namespace: {{ include "slurm-operator.namespace" . }} @@ -147,7 +168,7 @@ webhooks: scope: Namespaced clientConfig: {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} + caBundle: {{ $caBundle | quote }} {{- end }}{{- /* if not .Values.certManager.enabled */}} service: namespace: {{ include "slurm-operator.namespace" . }} @@ -188,7 +209,7 @@ webhooks: scope: Namespaced clientConfig: {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} + caBundle: {{ $caBundle | quote }} {{- end }}{{- /* if not .Values.certManager.enabled */}} service: namespace: {{ include "slurm-operator.namespace" . }} @@ -229,7 +250,7 @@ webhooks: scope: Namespaced clientConfig: {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} + caBundle: {{ $caBundle | quote }} {{- end }}{{- /* if not .Values.certManager.enabled */}} service: namespace: {{ include "slurm-operator.namespace" . }} @@ -270,7 +291,7 @@ webhooks: scope: Namespaced clientConfig: {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} + caBundle: {{ $caBundle | quote }} {{- end }}{{- /* if not .Values.certManager.enabled */}} service: namespace: {{ include "slurm-operator.namespace" . }} @@ -316,7 +337,7 @@ webhooks: - v1 clientConfig: {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} + caBundle: {{ $caBundle | quote }} {{- end }}{{- /* if not .Values.certManager.enabled */}} service: namespace: {{ include "slurm-operator.namespace" . }} diff --git a/helm/slurm-operator/tests/webhook_webhook_test.yaml b/helm/slurm-operator/tests/webhook_webhook_test.yaml index daff70cf..af601868 100644 --- a/helm/slurm-operator/tests/webhook_webhook_test.yaml +++ b/helm/slurm-operator/tests/webhook_webhook_test.yaml @@ -169,3 +169,121 @@ tests: value: Secret exists: path: data["ca.crt"] + + - it: should generate a self-signed cert and inject the caBundle into every webhook when certManager is disabled + set: + certManager: + enabled: false + secretName: slurm-operator-webhook-ca + asserts: + # caBundle must be present on all validating webhooks, not just the first. + - documentSelector: + path: kind + value: ValidatingWebhookConfiguration + notExists: + path: webhooks[?(@.clientConfig.caBundle == "")] + - documentSelector: + path: kind + value: ValidatingWebhookConfiguration + exists: + path: webhooks[5].clientConfig.caBundle + # The generated material must be base64-encoded PEM. + - documentSelector: + path: kind + value: Secret + matchRegex: + path: data["tls.crt"] + pattern: ^LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0t + - documentSelector: + path: kind + value: Secret + matchRegex: + path: data["ca.crt"] + pattern: ^LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0t + - documentSelector: + path: kind + value: Secret + matchRegex: + path: data["tls.key"] + pattern: ^LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVkt + # cert-manager annotations must not be present in the native path. + - documentSelector: + path: kind + value: ValidatingWebhookConfiguration + notExists: + path: metadata.annotations["cert-manager.io/inject-ca-from"] + + - it: should not render the native TLS secret nor caBundle when certManager is enabled + set: + certManager: + enabled: true + secretName: slurm-operator-webhook-ca + asserts: + - notExists: + path: webhooks[*].clientConfig.caBundle + documentSelector: + path: kind + value: ValidatingWebhookConfiguration + - documentSelector: + path: kind + value: ValidatingWebhookConfiguration + exists: + path: metadata.annotations["cert-manager.io/inject-ca-from"] + - hasDocuments: + count: 2 + + - it: should reuse the existing serving certificate instead of regenerating when certManager is disabled + set: + certManager: + enabled: false + secretName: slurm-operator-webhook-ca + kubernetesProvider: + scheme: + "v1/Secret": + gvr: + version: v1 + resource: secrets + namespaced: true + objects: + - kind: Secret + apiVersion: v1 + metadata: + name: slurm-operator-webhook-ca + namespace: test-namespace + data: + tls.crt: cmV1c2VkLXRscy1jcnQ= + tls.key: cmV1c2VkLXRscy1rZXk= + ca.crt: cmV1c2VkLWNhLWNydA== + asserts: + # The Secret data must be carried over verbatim from the existing Secret. + - documentSelector: + path: kind + value: Secret + equal: + path: data["tls.crt"] + value: cmV1c2VkLXRscy1jcnQ= + - documentSelector: + path: kind + value: Secret + equal: + path: data["tls.key"] + value: cmV1c2VkLXRscy1rZXk= + - documentSelector: + path: kind + value: Secret + equal: + path: data["ca.crt"] + value: cmV1c2VkLWNhLWNydA== + # The injected caBundle must match the reused CA, not a freshly generated one. + - documentSelector: + path: kind + value: ValidatingWebhookConfiguration + equal: + path: webhooks[0].clientConfig.caBundle + value: cmV1c2VkLWNhLWNydA== + - documentSelector: + path: kind + value: MutatingWebhookConfiguration + equal: + path: webhooks[0].clientConfig.caBundle + value: cmV1c2VkLWNhLWNydA==