diff --git a/docs/installation.md b/docs/installation.md index ca831b3f..f2d302dc 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -11,6 +11,7 @@ - [Namespace-Scoped Watching](#namespace-scoped-watching) - [With CRDs As Subchart](#with-crds-as-subchart) - [Without cert-manager](#without-cert-manager) + - [With an Externally-Managed Webhook Certificate](#with-an-externally-managed-webhook-certificate) - [Slurm Cluster](#slurm-cluster) - [Controller Persistence](#controller-persistence) - [With Accounting](#with-accounting) @@ -98,6 +99,142 @@ helm install slurm-operator oci://ghcr.io/slinkyproject/charts/slurm-operator \ --namespace=slinky --create-namespace ``` +> [!NOTE] +> Without cert-manager, the chart generates a self-signed CA and serving +> certificate via Helm's `genCA` / `genSignedCert` functions at render time. +> The cert has no in-cluster rotation: every `helm upgrade` produces a new +> CA + leaf and rewrites both the Secret and the webhook's `caBundle`, +> which causes Secret churn on each release and a brief window where +> in-flight admission requests may fail until the apiserver re-reads the +> updated `caBundle`. For long-lived clusters prefer cert-manager or +> `externalCertInjection`. + +### With an Externally-Managed Webhook Certificate + +If your organization issues TLS certificates from its own PKI (HashiCorp +Vault PKI, AWS Private CA, an internal CA, [external-secrets], etc.), you +can supply the webhook serving certificate as a pre-existing +`kubernetes.io/tls` Secret. The chart will neither generate a certificate +nor render any cert-manager resources, and rotation becomes the +responsibility of your PKI tooling. + +Create the Secret in the release namespace before installing the chart. It +must contain `tls.crt`, `tls.key`, and `ca.crt`, and the certificate's SANs +must include both `.` and +`..svc` (default service name: +`slurm-operator-webhook`). + +```sh +kubectl create namespace slinky +kubectl create secret tls slurm-operator-webhook-ca \ + --namespace=slinky \ + --cert=tls.crt --key=tls.key +kubectl patch secret slurm-operator-webhook-ca \ + --namespace=slinky \ + --type=merge \ + -p "{\"data\":{\"ca.crt\":\"$(base64 < ca.crt | tr -d '\n')\"}}" +``` + +Then install the chart with `certManager.enabled=false` and +`externalCertInjection.enabled=true`: + +```sh +helm install slurm-operator oci://ghcr.io/slinkyproject/charts/slurm-operator \ + --set 'certManager.enabled=false' \ + --set 'externalCertInjection.enabled=true' \ + --set 'externalCertInjection.secretName=slurm-operator-webhook-ca' \ + --namespace=slinky --create-namespace +``` + +The chart reads `ca.crt` from the Secret at install/upgrade time via Helm +`lookup` and inlines it into every webhook's `clientConfig.caBundle`. When +you rotate the certificate, run `helm upgrade` to refresh the `caBundle`. + +> [!WARNING] +> `certManager.enabled=true` and `externalCertInjection.enabled=true` are +> mutually exclusive (both would manage the same Secret). The chart will +> fail at template time if you set both. + +> [!IMPORTANT] +> `helm template` and `helm install --dry-run=client` cannot contact the +> Kubernetes API, so `lookup` returns empty and the chart fails with +> "Secret … was not found" even when the Secret exists. Use +> `helm install --dry-run=server` to exercise the lookup. For GitOps +> workflows that rely on `helm template` for diffs (helm-diff, ArgoCD, +> Flux), use the webhook annotation pass-through described below instead. + +> [!NOTE] +> When migrating from `certManager.enabled=true`, give the BYO Secret a +> name distinct from the chart-managed one (`certManager.secretName`, +> default `slurm-operator-webhook-ca`). Order of operations: +> +> 1. Create the BYO Secret under the new name in the release namespace. +> 2. `helm upgrade` with `certManager.enabled=false`, +> `externalCertInjection.enabled=true`, and the new `secretName`. +> The webhook Pod rolls; expect a brief admission gap (seconds) +> while the new pod becomes ready — for webhooks with +> `failurePolicy: Fail` this means matched API writes are blocked +> during that window. +> 3. After the rollout completes and the webhook is healthy, delete the +> old cert-manager Secret. +> +> Reusing the cert-manager Secret name is supported but risky: if the BYO +> Secret has not yet been created on a fresh install, the Pod silently +> mounts the stale cert-manager Secret. The chart cannot detect this; the +> distinct-name workflow above avoids the race entirely. + +If you would rather have cert-manager's [cainjector] populate `caBundle` +automatically from a Secret your PKI keeps refreshed (so `helm upgrade` is +not needed for rotation), combine `externalCertInjection` with the webhook +annotation pass-through. `externalCertInjection` makes the webhook pod +mount the BYO Secret; the annotations make cainjector keep `caBundle` in +sync with the same Secret's `ca.crt`. + +> [!IMPORTANT] +> For cainjector to read a `ca.crt` from a `Secret` (the +> `cert-manager.io/inject-ca-from-secret` annotation), the Secret MUST +> carry the annotation `cert-manager.io/allow-direct-injection: "true"`. +> Without it, cainjector silently refuses to inject and the chart-rendered +> `caBundle` becomes the only source of truth. Add the annotation to your +> BYO Secret (either at create time or via `kubectl annotate`) before +> installing the chart. + +```sh +kubectl create namespace slinky +kubectl create secret tls slurm-operator-webhook-byo \ + --namespace=slinky \ + --cert=tls.crt --key=tls.key +kubectl patch secret slurm-operator-webhook-byo \ + --namespace=slinky \ + --type=merge \ + -p "{\"data\":{\"ca.crt\":\"$(base64 < ca.crt | tr -d '\n')\"}}" +kubectl annotate secret slurm-operator-webhook-byo \ + --namespace=slinky \ + cert-manager.io/allow-direct-injection=true + +helm install slurm-operator oci://ghcr.io/slinkyproject/charts/slurm-operator \ + --set 'certManager.enabled=false' \ + --set 'externalCertInjection.enabled=true' \ + --set 'externalCertInjection.secretName=slurm-operator-webhook-byo' \ + --set 'webhook.validatingAnnotations.cert-manager\.io/inject-ca-from-secret=slinky/slurm-operator-webhook-byo' \ + --set 'webhook.mutatingAnnotations.cert-manager\.io/inject-ca-from-secret=slinky/slurm-operator-webhook-byo' \ + --namespace=slinky --create-namespace +``` + +This requires cert-manager (specifically `cainjector`) to be running in +the cluster, but the chart still does not manage the certificate itself. +The chart-rendered `caBundle` (read from the Secret via `lookup` at +install time) and the cainjector-written `caBundle` will be identical on +install; on rotation, cainjector keeps `caBundle` correct without a +`helm upgrade`. + +> [!IMPORTANT] +> Do not omit `externalCertInjection.enabled=true` and rely on the +> annotations alone. Without it, the chart falls back to the self-signed +> `genCA` mode and the webhook pod serves a chart-generated certificate +> whose CA your PKI Secret does not match — the user's PKI is silently +> ignored regardless of the cainjector annotation. + ## Slurm Cluster Install a Slurm cluster via helm chart: @@ -535,11 +672,13 @@ crw-rw-rw- 1 root root 509, 1 Apr 20 17:08 /dev/nvidia-caps-imex-channels/channe [autodetect]: https://slurm.schedmd.com/gres.conf.html#OPT_AutoDetect +[cainjector]: https://cert-manager.io/docs/concepts/ca-injector/ [cert-manager]: https://cert-manager.io/docs/installation/helm/ [default-storageclass]: https://kubernetes.io/docs/concepts/storage/storage-classes/#default-storageclass [device-plugins]: https://kubernetes.io/docs/tasks/manage-gpus/scheduling-gpus/#using-device-plugins [dra]: https://kubernetes.io/docs/concepts/scheduling-eviction/dynamic-resource-allocation/ [dra-driver-nvidia-gpu]: https://github.com/kubernetes-sigs/dra-driver-nvidia-gpu +[external-secrets]: https://external-secrets.io/ [gres]: https://slurm.schedmd.com/gres.html [grestypes]: https://slurm.schedmd.com/slurm.conf.html#OPT_GresTypes [imex]: https://docs.nvidia.com/multi-node-nvlink-systems/imex-guide/overview.html diff --git a/helm/slurm-operator/README.md b/helm/slurm-operator/README.md index f0b89daa..3d6668d0 100644 --- a/helm/slurm-operator/README.md +++ b/helm/slurm-operator/README.md @@ -34,6 +34,8 @@ Kubernetes: `>= 1.29.0-0` | certManager.secretName | string | `"slurm-operator-webhook-ca"` | The secret to be (created and) mounted. | | crds | object | `{"enabled":false}` | Configure Custom Resource Definitions (CRDs). | | crds.enabled | bool | `false` | Whether this helm chart should manage the CRD and its upgrades. | +| externalCertInjection.enabled | bool | `false` | Mount a pre-existing TLS Secret instead of provisioning one. | +| externalCertInjection.secretName | string | `""` | Name of a pre-existing kubernetes.io/tls Secret containing `tls.crt`, `tls.key`, and `ca.crt`. Must be set explicitly when `enabled` is true; intentionally has no default to avoid silently reusing the chart-managed Secret name during migrations. | | extraObjects | list | `[]` | Extra Kubernetes objects to deploy alongside the chart. Each entry is rendered as a standalone Kubernetes object. Supports Helm templating (e.g. {{ .Release.Namespace }}). | | fullnameOverride | string | `""` | Overrides the full name of the release. | | imagePullPolicy | string | `"IfNotPresent"` | Set the default image pull policy. | @@ -85,6 +87,7 @@ Kubernetes: `>= 1.29.0-0` | webhook.mutating.failurePolicy | string | `"Ignore"` | Action taken when the mutating admission webhook is unreachable or returns an error. Ref: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy | | webhook.mutating.matchConditions | list | `[]` | List of MatchConditions, which represents a condition which must by fulfilled for a request to be sent to a webhook. Ref: https://kubernetes.io/docs/reference/kubernetes-api/definitions/match-condition-v1-admissionregistration/ | | webhook.mutating.matchPolicy | string | `"Equivalent"` | How the rules listed in the mutating webhook are matched against incoming requests. Ref: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy | +| webhook.mutatingAnnotations | object | `{}` | Extra annotations on the MutatingWebhookConfiguration. Merged with chart-managed annotations; user keys win on collision. | | webhook.namespaces | string | `""` | Comma-separated list of namespaces the webhook will watch. If empty, all namespaces are watched. | | webhook.nodeSelector | object | `{}` | Node label selector for pod assignment. Ref: https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#nodeselector | | webhook.pdb.enabled | bool | `false` | Enable PodDisruptionBudget. | @@ -105,4 +108,5 @@ Kubernetes: `>= 1.29.0-0` | webhook.validating.matchConditions | list | `[]` | List of MatchConditions, which represents a condition which must by fulfilled for a request to be sent to a webhook. Ref: https://kubernetes.io/docs/reference/kubernetes-api/definitions/match-condition-v1-admissionregistration/ | | webhook.validating.matchPolicy | string | `"Equivalent"` | How the rules listed in the validating webhook are matched against incoming requests. Ref: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy | | webhook.validating.namespaceSelector | object | `{}` | Full override for the validating webhooks' namespaceSelector, rendered verbatim when set. Replaces `webhook.namespaces` and the default kube-system/kube-node-lease exclusions for these webhooks. | +| webhook.validatingAnnotations | object | `{}` | Extra annotations on the ValidatingWebhookConfiguration. Merged with chart-managed annotations; user keys win on collision. | diff --git a/helm/slurm-operator/templates/_webhook.tpl b/helm/slurm-operator/templates/_webhook.tpl index ba6ed9da..567c7a67 100644 --- a/helm/slurm-operator/templates/_webhook.tpl +++ b/helm/slurm-operator/templates/_webhook.tpl @@ -81,3 +81,89 @@ matchExpressions: {{- concat (list "kube-system" "kube-node-lease") $extraExcludes | toYaml | nindent 6 }} {{- end -}} {{- end -}} + +{{/* +Validate cert-provisioning modes. Called from every template that +participates in provisioning so the error surfaces no matter which +template Helm renders first. +*/}} +{{- define "slurm-operator.webhook.validateModes" -}} +{{- if and .Values.certManager.enabled .Values.externalCertInjection.enabled -}} +{{- fail "certManager.enabled and externalCertInjection.enabled are mutually exclusive: both would manage the webhook TLS Secret. Pick one." -}} +{{- end -}} +{{- if and .Values.externalCertInjection.enabled (not .Values.externalCertInjection.secretName) -}} +{{- fail "externalCertInjection.enabled=true but externalCertInjection.secretName is empty." -}} +{{- end -}} +{{- end }} + +{{/* +Name of the Secret that holds the webhook serving cert. +*/}} +{{- define "slurm-operator.webhook.tlsSecretName" -}} +{{- if .Values.externalCertInjection.enabled -}} +{{ .Values.externalCertInjection.secretName }} +{{- else -}} +{{ .Values.certManager.secretName }} +{{- end -}} +{{- end }} + +{{/* +Chart-managed cert-manager annotations. Empty when certManager is off. +*/}} +{{- define "slurm-operator.webhook.certManagerAnnotations" -}} +{{- $ann := dict -}} +{{- if .Values.certManager.enabled -}} +{{- $ref := printf "%s/%s" (include "slurm-operator.namespace" .) .Values.certManager.secretName -}} +{{- $_ := set $ann "certmanager.k8s.io/inject-ca-from" $ref -}} +{{- $_ := set $ann "cert-manager.io/inject-ca-from" $ref -}} +{{- end -}} +{{- toYaml $ann -}} +{{- end }} + +{{/* +ValidatingWebhookConfiguration annotations. User keys win on collision. +*/}} +{{- define "slurm-operator.webhook.validatingAnnotations" -}} +{{- $userAnn := .Values.webhook.validatingAnnotations | default dict -}} +{{- $cmAnn := include "slurm-operator.webhook.certManagerAnnotations" . | fromYaml -}} +{{- $ann := merge dict $userAnn $cmAnn -}} +{{- if $ann -}} +{{- toYaml $ann -}} +{{- end -}} +{{- end }} + +{{/* +MutatingWebhookConfiguration annotations. User keys win on collision. +*/}} +{{- define "slurm-operator.webhook.mutatingAnnotations" -}} +{{- $userAnn := .Values.webhook.mutatingAnnotations | default dict -}} +{{- $cmAnn := include "slurm-operator.webhook.certManagerAnnotations" . | fromYaml -}} +{{- $ann := merge dict $userAnn $cmAnn -}} +{{- if $ann -}} +{{- toYaml $ann -}} +{{- end -}} +{{- end }} + +{{/* +Base64-encoded CA bundle read from the external TLS Secret. +NOTE: `lookup` returns nil during `helm template` and `--dry-run=client`, +so this helper will fail with "not found" in those contexts even when +the Secret exists. Use `--dry-run=server` or the annotation pass-through. +*/}} +{{- define "slurm-operator.webhook.externalCABundle" -}} +{{- $ns := include "slurm-operator.namespace" . -}} +{{- $name := .Values.externalCertInjection.secretName -}} +{{- $secret := lookup "v1" "Secret" $ns $name -}} +{{- if not $secret -}} +{{- fail (printf "externalCertInjection Secret %q in namespace %q was not found (use --dry-run=server; see docs/installation.md)." $name $ns) -}} +{{- end -}} +{{- if ne $secret.type "kubernetes.io/tls" -}} +{{- fail (printf "externalCertInjection Secret %q in namespace %q must be of type kubernetes.io/tls, got %q." $name $ns $secret.type) -}} +{{- end -}} +{{- range $key := list "tls.crt" "tls.key" "ca.crt" -}} +{{- if not (index $secret.data $key) -}} +{{- fail (printf "externalCertInjection Secret %q in namespace %q is missing the %q key." $name $ns $key) -}} +{{- end -}} +{{- end -}} +{{- index $secret.data "ca.crt" -}} +{{- end }} diff --git a/helm/slurm-operator/templates/cert-manager/pki.yaml b/helm/slurm-operator/templates/cert-manager/pki.yaml index 086f7e96..74cee830 100644 --- a/helm/slurm-operator/templates/cert-manager/pki.yaml +++ b/helm/slurm-operator/templates/cert-manager/pki.yaml @@ -3,6 +3,7 @@ SPDX-FileCopyrightText: Copyright (C) SchedMD LLC. SPDX-License-Identifier: Apache-2.0 */}} +{{- include "slurm-operator.webhook.validateModes" . -}} {{- if and .Values.webhook.enabled .Values.certManager.enabled }} {{- $certManager := .Values.certManager | default dict -}} --- diff --git a/helm/slurm-operator/templates/webhook/deployment.yaml b/helm/slurm-operator/templates/webhook/deployment.yaml index d42fe823..00aa2f94 100644 --- a/helm/slurm-operator/templates/webhook/deployment.yaml +++ b/helm/slurm-operator/templates/webhook/deployment.yaml @@ -3,6 +3,7 @@ SPDX-FileCopyrightText: Copyright (C) SchedMD LLC. SPDX-License-Identifier: Apache-2.0 */}} +{{- include "slurm-operator.webhook.validateModes" . -}} {{- if .Values.webhook.enabled }} apiVersion: apps/v1 kind: Deployment @@ -111,5 +112,5 @@ spec: - name: certificates secret: defaultMode: 420 - secretName: {{ .Values.certManager.secretName }} + secretName: {{ include "slurm-operator.webhook.tlsSecretName" . }} {{- end }}{{- /* if .Values.webhook.enabled */}} diff --git a/helm/slurm-operator/templates/webhook/webhook.yaml b/helm/slurm-operator/templates/webhook/webhook.yaml index 6f4cc032..c3e21039 100644 --- a/helm/slurm-operator/templates/webhook/webhook.yaml +++ b/helm/slurm-operator/templates/webhook/webhook.yaml @@ -3,14 +3,18 @@ SPDX-FileCopyrightText: Copyright (C) SchedMD LLC. SPDX-License-Identifier: Apache-2.0 */}} +{{- include "slurm-operator.webhook.validateModes" . -}} {{- if .Values.webhook.enabled }} -{{- $certNamespacedName := printf "%s/%s" (include "slurm-operator.namespace" .) .Values.certManager.secretName -}} +{{- $caBundle := "" -}} +{{- if .Values.externalCertInjection.enabled -}} +{{- $caBundle = include "slurm-operator.webhook.externalCABundle" . -}} +{{- else if not .Values.certManager.enabled -}} {{- $durationDays := int 3650 -}} {{- $ca := genCA .Values.certManager.secretName $durationDays -}} -{{- 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" .)) -}} {{- $cert := genSignedCert $cn nil $dnsList $durationDays $ca -}} +{{- $caBundle = $ca.Cert | b64enc -}} --- apiVersion: v1 kind: Secret @@ -24,17 +28,16 @@ data: tls.crt: {{ $cert.Cert | b64enc | quote }} tls.key: {{ $cert.Key | b64enc | quote }} ca.crt: {{ $ca.Cert | b64enc | quote }} -{{- end }}{{- /* if not .Values.certManager.enabled */}} +{{- end }}{{- /* mode selection */}} --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: name: {{ include "slurm-operator.webhook.name" . }} - {{- if .Values.certManager.enabled }} + {{- with include "slurm-operator.webhook.validatingAnnotations" . }} annotations: - certmanager.k8s.io/inject-ca-from: {{ $certNamespacedName | quote }} - cert-manager.io/inject-ca-from: {{ $certNamespacedName | quote }} - {{- end }}{{- /* if .Values.certManager.enabled */}} + {{- . | nindent 4 }} + {{- end }}{{- /* with include "slurm-operator.webhook.validatingAnnotations" . */}} labels: {{- include "slurm-operator.webhook.labels" . | nindent 4 }} webhooks: @@ -53,9 +56,9 @@ webhooks: - UPDATE scope: Namespaced clientConfig: - {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} - {{- end }}{{- /* if not .Values.certManager.enabled */}} + {{- with $caBundle }} + caBundle: {{ . | quote }} + {{- end }}{{- /* with $caBundle */}} service: namespace: {{ include "slurm-operator.namespace" . }} name: {{ include "slurm-operator.webhook.name" . }} @@ -87,9 +90,9 @@ webhooks: - UPDATE scope: Namespaced clientConfig: - {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} - {{- end }}{{- /* if not .Values.certManager.enabled */}} + {{- with $caBundle }} + caBundle: {{ . | quote }} + {{- end }}{{- /* with $caBundle */}} service: namespace: {{ include "slurm-operator.namespace" . }} name: {{ include "slurm-operator.webhook.name" . }} @@ -121,9 +124,9 @@ webhooks: - UPDATE scope: Namespaced clientConfig: - {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} - {{- end }}{{- /* if not .Values.certManager.enabled */}} + {{- with $caBundle }} + caBundle: {{ . | quote }} + {{- end }}{{- /* with $caBundle */}} service: namespace: {{ include "slurm-operator.namespace" . }} name: {{ include "slurm-operator.webhook.name" . }} @@ -155,9 +158,9 @@ webhooks: - UPDATE scope: Namespaced clientConfig: - {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} - {{- end }}{{- /* if not .Values.certManager.enabled */}} + {{- with $caBundle }} + caBundle: {{ . | quote }} + {{- end }}{{- /* with $caBundle */}} service: namespace: {{ include "slurm-operator.namespace" . }} name: {{ include "slurm-operator.webhook.name" . }} @@ -189,9 +192,9 @@ webhooks: - UPDATE scope: Namespaced clientConfig: - {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} - {{- end }}{{- /* if not .Values.certManager.enabled */}} + {{- with $caBundle }} + caBundle: {{ . | quote }} + {{- end }}{{- /* with $caBundle */}} service: namespace: {{ include "slurm-operator.namespace" . }} name: {{ include "slurm-operator.webhook.name" . }} @@ -223,9 +226,9 @@ webhooks: - UPDATE scope: Namespaced clientConfig: - {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} - {{- end }}{{- /* if not .Values.certManager.enabled */}} + {{- with $caBundle }} + caBundle: {{ . | quote }} + {{- end }}{{- /* with $caBundle */}} service: namespace: {{ include "slurm-operator.namespace" . }} name: {{ include "slurm-operator.webhook.name" . }} @@ -248,11 +251,10 @@ apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: name: {{ include "slurm-operator.webhook.name" . }} - {{- if .Values.certManager.enabled }} + {{- with include "slurm-operator.webhook.mutatingAnnotations" . }} annotations: - certmanager.k8s.io/inject-ca-from: {{ $certNamespacedName | quote }} - cert-manager.io/inject-ca-from: {{ $certNamespacedName | quote }} - {{- end }}{{- /* if .Values.certManager.enabled */}} + {{- . | nindent 4 }} + {{- end }}{{- /* with include "slurm-operator.webhook.mutatingAnnotations" . */}} labels: {{- include "slurm-operator.webhook.labels" . | nindent 4 }} webhooks: @@ -262,9 +264,9 @@ webhooks: admissionReviewVersions: - v1 clientConfig: - {{- if not .Values.certManager.enabled }} - caBundle: {{ $ca.Cert | b64enc | quote }} - {{- end }}{{- /* if not .Values.certManager.enabled */}} + {{- with $caBundle }} + caBundle: {{ . | quote }} + {{- end }}{{- /* with $caBundle */}} service: namespace: {{ include "slurm-operator.namespace" . }} name: {{ include "slurm-operator.webhook.name" . }} diff --git a/helm/slurm-operator/tests/certmanager_pki_test.yaml b/helm/slurm-operator/tests/certmanager_pki_test.yaml index 3298f055..08b9ece1 100644 --- a/helm/slurm-operator/tests/certmanager_pki_test.yaml +++ b/helm/slurm-operator/tests/certmanager_pki_test.yaml @@ -126,3 +126,42 @@ tests: - equal: path: metadata.namespace value: custom-namespace + + - it: should not render when externalCertInjection is enabled + set: + webhook: + enabled: true + certManager: + enabled: false + externalCertInjection: + enabled: true + secretName: my-byo-cert + asserts: + - hasDocuments: + count: 0 + + - it: should fail when both certManager and externalCertInjection are enabled + set: + webhook: + enabled: true + certManager: + enabled: true + externalCertInjection: + enabled: true + secretName: my-byo-cert + asserts: + - failedTemplate: + errorPattern: mutually exclusive + + - it: should fail when externalCertInjection.secretName is empty + set: + webhook: + enabled: true + certManager: + enabled: false + externalCertInjection: + enabled: true + secretName: "" + asserts: + - failedTemplate: + errorPattern: externalCertInjection\.secretName is empty diff --git a/helm/slurm-operator/tests/webhook_webhook_test.yaml b/helm/slurm-operator/tests/webhook_webhook_test.yaml index 0fbc1bae..4c2bda30 100644 --- a/helm/slurm-operator/tests/webhook_webhook_test.yaml +++ b/helm/slurm-operator/tests/webhook_webhook_test.yaml @@ -314,3 +314,285 @@ tests: value: - name: rbac expression: 'request.resource.group != "rbac.authorization.k8s.io"' + + - it: should include cert-manager annotations when certManager is enabled + documentSelector: + path: kind + value: ValidatingWebhookConfiguration + set: + webhook: + enabled: true + certManager: + enabled: true + secretName: slurm-operator-webhook-ca + asserts: + - exists: + path: metadata.annotations["cert-manager.io/inject-ca-from"] + + - it: should pass through validatingAnnotations + documentSelector: + path: kind + value: ValidatingWebhookConfiguration + set: + webhook: + enabled: true + validatingAnnotations: + cert-manager.io/inject-ca-from-secret: vault-pki/webhook-ca + custom.example.com/owner: platform-team + certManager: + enabled: false + asserts: + - equal: + path: metadata.annotations["cert-manager.io/inject-ca-from-secret"] + value: vault-pki/webhook-ca + - equal: + path: metadata.annotations["custom.example.com/owner"] + value: platform-team + + - it: should pass through mutatingAnnotations + documentSelector: + path: kind + value: MutatingWebhookConfiguration + set: + webhook: + enabled: true + podsBinding: true + mutatingAnnotations: + cert-manager.io/inject-ca-from-secret: vault-pki/webhook-ca + certManager: + enabled: false + asserts: + - equal: + path: metadata.annotations["cert-manager.io/inject-ca-from-secret"] + value: vault-pki/webhook-ca + + - it: should let user annotations override chart-managed cert-manager annotations + documentSelector: + path: kind + value: ValidatingWebhookConfiguration + set: + webhook: + enabled: true + validatingAnnotations: + cert-manager.io/inject-ca-from: custom-ns/custom-cert + certManager: + enabled: true + secretName: slurm-operator-webhook-ca + asserts: + - equal: + path: metadata.annotations["cert-manager.io/inject-ca-from"] + value: custom-ns/custom-cert + + - it: should omit annotations block when there are none + documentSelector: + path: kind + value: ValidatingWebhookConfiguration + set: + webhook: + enabled: true + certManager: + enabled: false + asserts: + - notExists: + path: metadata.annotations + + - it: should tolerate validatingAnnotations set to null + documentSelector: + path: kind + value: ValidatingWebhookConfiguration + set: + webhook: + enabled: true + validatingAnnotations: null + certManager: + enabled: true + secretName: slurm-operator-webhook-ca + asserts: + - exists: + path: metadata.annotations["cert-manager.io/inject-ca-from"] + + - it: should tolerate mutatingAnnotations set to null + documentSelector: + path: kind + value: MutatingWebhookConfiguration + set: + webhook: + enabled: true + podsBinding: true + mutatingAnnotations: null + certManager: + enabled: true + secretName: slurm-operator-webhook-ca + asserts: + - exists: + path: metadata.annotations["cert-manager.io/inject-ca-from"] + + - it: should fail when both certManager and externalCertInjection are enabled + set: + webhook: + enabled: true + certManager: + enabled: true + externalCertInjection: + enabled: true + secretName: my-byo-cert + asserts: + - failedTemplate: + errorPattern: mutually exclusive + + - it: should fail when externalCertInjection Secret is missing + set: + webhook: + enabled: true + certManager: + enabled: false + externalCertInjection: + enabled: true + secretName: my-byo-cert + asserts: + - failedTemplate: + errorPattern: was not found + + - it: should fail when externalCertInjection.secretName is empty + set: + webhook: + enabled: true + certManager: + enabled: false + externalCertInjection: + enabled: true + secretName: "" + asserts: + - failedTemplate: + errorPattern: externalCertInjection\.secretName is empty + + - it: should inline caBundle from external Secret when externalCertInjection is enabled + documentSelector: + path: kind + value: ValidatingWebhookConfiguration + set: + webhook: + enabled: true + certManager: + enabled: false + externalCertInjection: + enabled: true + secretName: my-byo-cert + kubernetesProvider: + scheme: + v1/Secret: + gvr: + version: v1 + resource: secrets + namespaced: true + objects: + - apiVersion: v1 + kind: Secret + metadata: + name: my-byo-cert + namespace: test-namespace + type: kubernetes.io/tls + data: + tls.crt: dGxzLWNydA== # "tls-crt" + tls.key: dGxzLWtleQ== # "tls-key" + ca.crt: Y2EtY3J0 # "ca-crt" + asserts: + - notExists: + path: metadata.annotations["cert-manager.io/inject-ca-from"] + - equal: + path: webhooks[0].clientConfig.caBundle + value: Y2EtY3J0 + + - it: should not render TLS secret when externalCertInjection is enabled + documentIndex: 0 + set: + webhook: + enabled: true + certManager: + enabled: false + externalCertInjection: + enabled: true + secretName: my-byo-cert + kubernetesProvider: + scheme: + v1/Secret: + gvr: + version: v1 + resource: secrets + namespaced: true + objects: + - apiVersion: v1 + kind: Secret + metadata: + name: my-byo-cert + namespace: test-namespace + type: kubernetes.io/tls + data: + tls.crt: dGxzLWNydA== + tls.key: dGxzLWtleQ== + ca.crt: Y2EtY3J0 + asserts: + - equal: + path: kind + value: ValidatingWebhookConfiguration + + - it: should fail when externalCertInjection Secret is not kubernetes.io/tls + set: + webhook: + enabled: true + certManager: + enabled: false + externalCertInjection: + enabled: true + secretName: my-byo-cert + kubernetesProvider: + scheme: + v1/Secret: + gvr: + version: v1 + resource: secrets + namespaced: true + objects: + - apiVersion: v1 + kind: Secret + metadata: + name: my-byo-cert + namespace: test-namespace + type: Opaque + data: + tls.crt: dGxzLWNydA== + tls.key: dGxzLWtleQ== + ca.crt: Y2EtY3J0 + asserts: + - failedTemplate: + errorPattern: must be of type kubernetes\.io/tls + + - it: should fail when externalCertInjection Secret is missing tls.key + set: + webhook: + enabled: true + certManager: + enabled: false + externalCertInjection: + enabled: true + secretName: my-byo-cert + kubernetesProvider: + scheme: + v1/Secret: + gvr: + version: v1 + resource: secrets + namespaced: true + objects: + - apiVersion: v1 + kind: Secret + metadata: + name: my-byo-cert + namespace: test-namespace + type: kubernetes.io/tls + data: + tls.crt: dGxzLWNydA== + ca.crt: Y2EtY3J0 + asserts: + - failedTemplate: + errorPattern: is missing the "tls\.key" key diff --git a/helm/slurm-operator/values.yaml b/helm/slurm-operator/values.yaml index a6e7d982..381dd52e 100644 --- a/helm/slurm-operator/values.yaml +++ b/helm/slurm-operator/values.yaml @@ -261,6 +261,12 @@ webhook: # -- Comma-separated list of namespaces the webhook will watch. # If empty, all namespaces are watched. namespaces: "" + # -- Extra annotations on the ValidatingWebhookConfiguration. Merged + # with chart-managed annotations; user keys win on collision. + validatingAnnotations: {} + # -- Extra annotations on the MutatingWebhookConfiguration. Merged + # with chart-managed annotations; user keys win on collision. + mutatingAnnotations: {} # # Cert-Manager certificate configurations. @@ -274,6 +280,17 @@ certManager: # -- Certificate renewal time. Should be before the expiration. renewBefore: 8760h0m0s # 1 year +# +# External webhook certificate configurations. +externalCertInjection: + # -- Mount a pre-existing TLS Secret instead of provisioning one. + enabled: false + # -- Name of a pre-existing kubernetes.io/tls Secret containing + # `tls.crt`, `tls.key`, and `ca.crt`. Must be set explicitly when + # `enabled` is true; intentionally has no default to avoid silently + # reusing the chart-managed Secret name during migrations. + secretName: "" + # -- List of Kubernetes Node Conditions, by type, to propagate to the Slurm node drain reason. # Ref: https://kubernetes.io/docs/reference/node/node-status/#condition propagatedNodeConditions: []