Skip to content

feat(webhook): support external TLS certificate - #200

Open
faganihajizada wants to merge 1 commit into
SlinkyProject:mainfrom
faganihajizada:externalCertInjection
Open

feat(webhook): support external TLS certificate#200
faganihajizada wants to merge 1 commit into
SlinkyProject:mainfrom
faganihajizada:externalCertInjection

Conversation

@faganihajizada

Copy link
Copy Markdown
Contributor

Summary

Adds a third webhook-TLS provisioning mode, externalCertInjection, to the slurm-operator Helm chart, allowing organizations to issue the webhook serving certificate from their own PKI (HashiCorp Vault, AWS Private CA, external-secrets-operator, an internal DigiCert intermediate, etc.). Instead of relying on cert-manager or the chart's Helm-generated self-signed (genCA) fallback. This closes the only remaining "we can't ship slurm-operator into our cluster as-is" objection from users who require corporate-CA-signed leaves on every in-cluster TLS endpoint.

What the user gets

A new top-level values block:

externalCertInjection:
  enabled: false
  secretName: ""

And two annotation pass-through fields on the webhook configurations, for GitOps users who can't depend on Helm lookup:

webhook:
  validatingAnnotations: {}
  mutatingAnnotations: {}

The three TLS modes are mutually exclusive at template-render time

cert-manager (default, unchanged)certManager.enabled: true, externalCertInjection.enabled: false. The chart renders an Issuer + Certificate; cert-manager creates the Secret; cainjector populates caBundle on both webhook configurations via a chart-injected annotation. Cert rotation owner: cert-manager.

Helm self-signed (unchanged)certManager.enabled: false, externalCertInjection.enabled: false. The chart renders a chart-owned Opaque Secret containing genCA + genSignedCert material and inlines its base64 into caBundle at render time. Cert rotation owner: Helm, which re-rolls the CA on every helm upgrade.

External (new)certManager.enabled: false, externalCertInjection.enabled: true, externalCertInjection.secretName: <user-set>. The chart renders no Secret and no cert-manager resources. The webhook Deployment mounts the user's pre-existing kubernetes.io/tls Secret, and caBundle is populated by Helm lookup reading the Secret's ca.crt. Cert rotation owner: the user's external PKI.

A misconfigured combination (certManager.enabled=true and externalCertInjection.enabled=true, or externalCertInjection.enabled=true with an empty secretName) is rejected with a clear message during template rendering, before the user can apply a half-broken manifest set to the cluster.

Checklist

  • I have read CONTRIBUTING.md and the Code of Conduct.
  • New or existing tests cover these changes (where applicable).
  • Documentation is updated if user-visible behavior changes.

Breaking Changes

N/A

Testing Notes

Automated checks

Check Result
helm lint helm/slurm-operator Pass
helm unittest helm/slurm-operator Pass
make helm-docs Pass

New unit tests added

In certmanager_pki_test.yaml:

  • externalCertInjection.enabled=true — no Issuer, no Certificate rendered.
  • Mode conflict (certManager.enabled=true + externalCertInjection.enabled=true) — template render fails with the documented message.
  • Empty secretName (externalCertInjection.enabled=true, secretName="") — template render fails with the documented message.

In webhook_webhook_test.yaml:

  • External mode, mocked kubernetesProvider SecretcaBundle equals b64enc(ca.crt) byte-exact on both Validating and Mutating configs.
  • External mode + wrong Secret type (Opaque) — template render fails.
  • External mode + missing key (tls.crt / tls.key / ca.crt each) — template render fails on each.
  • Annotation pass-throughwebhook.validatingAnnotations and webhook.mutatingAnnotations keys land on the right *WebhookConfiguration.
  • User vs. chart-managed precedence — when a user annotation collides with a chart-managed cert-manager annotation, the user key wins.
  • null-tolerancewebhook.validatingAnnotations: null does not panic the template.
  • No-annotations case — when both maps are empty and certManager.enabled=false, no annotations: block is emitted at all.

End-to-end on a kind v1.34 cluster

Verified against the released ghcr.io/slinkyproject/slurm-operator:1.1.0 and slurm-operator-webhook:1.1.0 images, with the chart pointed at the branch in this PR.

Each scenario was verified end-to-end: chart install → kubectl get *WebhookConfiguration -o jsonpath='{.webhooks[0].clientConfig.caBundle}' → submit a real NodeSet admission request → observe the webhook log the validation decision.

Reproducing

kind create cluster --name slurm-byo

openssl genrsa -out ca.key 2048
openssl req -x509 -new -nodes -key ca.key -sha256 -days 365 -out ca.crt \
  -subj "/CN=my-org-pki-root"

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')\"}}"

helm install slurm-operator helm/slurm-operator \
  --namespace=slinky --create-namespace \
  --set 'certManager.enabled=false' \
  --set 'externalCertInjection.enabled=true' \
  --set 'externalCertInjection.secretName=slurm-operator-webhook-byo'

for cfg in validatingwebhookconfiguration mutatingwebhookconfiguration; do
  diff <(base64 < ca.crt | tr -d '\n') \
       <(kubectl get $cfg slurm-operator-webhook \
           -o jsonpath='{.webhooks[0].clientConfig.caBundle}') \
       && echo "$cfg: OK"
done

kubectl apply -f config/samples/slinky_v1alpha1_nodeset.yaml

Additional Context

Why the strict validateModes template-render failure?

Kyverno and Gatekeeper handle conflicting cert-provisioning flags either by documentation ("cert-manager takes precedence") or by trusting the user. Both have runtime cert-controllers that gracefully recover from inconsistent state. We don't — so a misconfiguration like certManager.enabled=true + externalCertInjection.enabled=true would render two Secrets with the same name and confuse the webhook.

Why the cainjector annotation pass-through was promoted to a first-class field

Live testing surfaced a non-obvious blocker: cainjector refuses to read a Secret unless that Secret carries cert-manager.io/allow-direct-injection: "true". Without it, the user ends up with the BYO mode silently disabled and the chart's self-signed mode silently re-enabled, producing a TLS-handshake mismatch that's diagnosable only by reading both pod logs and cert-manager logs side by side.

@vivian-hafener

Copy link
Copy Markdown
Contributor

Good afternoon @faganihajizada,

I will look into this.

Best,
Vivian Hafener

@vivian-hafener

Copy link
Copy Markdown
Contributor

Good afternoon Fagani,

When you have the chance, could you please rebase this against main? Once you do so, I can begin review on this.

Thanks!

Best,
Vivian Hafener

Adds a third webhook-TLS mode, `externalCertInjection`, mutually
exclusive with cert-manager and the existing self-signed `genCA` path.
When enabled, the chart renders neither a Secret nor any cert-manager
resources; the webhook Deployment mounts a pre-existing
`kubernetes.io/tls` Secret named by the user, and the webhook
configurations' `caBundle` is populated via Helm `lookup` at install
time. This lets organizations issue the webhook certificate from their
own PKI (Vault, AWS Private CA, external-secrets, an internal CA)
without the chart managing it

Changelog: Added - Helm chart values `externalCertInjection.{enabled,
 secretName}` for the slurm-operator chart, allowing the webhook
 serving certificate to be supplied via a pre-existing
 `kubernetes.io/tls` Secret. Added `webhook.validatingAnnotations` and
 `webhook.mutatingAnnotations` for cainjector-based `caBundle`
 population.
@faganihajizada
faganihajizada force-pushed the externalCertInjection branch from 23d6567 to c414eb5 Compare August 6, 2026 06:50
@faganihajizada

Copy link
Copy Markdown
Contributor Author

Hi @vivian-hafener,

thanks! Done

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.

3 participants