Skip to content

fix(webhook): reuse Helm-native serving cert on upgrade #218

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.

Sign up for GitHub

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

Open
giuliocalzo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(webhook): reuse Helm-native serving cert on upgrade #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 37 additions & 16 deletions helm/slurm-operator/templates/webhook/webhook.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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" . }}
Expand Down Expand Up @@ -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" . }}
Expand Down Expand Up @@ -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" . }}
Expand Down Expand Up @@ -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" . }}
Expand Down Expand Up @@ -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" . }}
Expand Down Expand Up @@ -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" . }}
Expand Down Expand Up @@ -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" . }}
Expand Down
118 changes: 118 additions & 0 deletions helm/slurm-operator/tests/webhook_webhook_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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==