From 8541a95cb7aa6b48ad3f7ca6e0753388cd88812f Mon Sep 17 00:00:00 2001 From: Ivan Okhotnikov Date: Thu, 2 Jul 2026 12:33:45 +0300 Subject: [PATCH] feat(vertical-pod-autoscaler): cert-manager TLS for admission-controller webhook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy path used genCA/genSignedCert inside a Helm template. Those functions are not idempotent across renders — every Flux HelmRelease reconcile produces a fresh CA + cert. The deployment's checksum/tls-secret podAnnotation is a sha256 of that ever-changing template, so each reconcile flips the PodTemplate and materialises a new ReplicaSet. Under load — e.g. the E2E install phase where cert-manager / CDI / LINSTOR cascade races cause repeated VPA HR reconciles — the churn produces 10+ admission-controller ReplicaSets in minutes and blocks the entire cozystack-platform HR because VPA never converges to Ready. Replace the source of the TLS Secret with a cert-manager Certificate plus a namespace-scoped selfSigned Issuer, gated on a new `admissionController.tls.certManagerEnabled` value that defaults to `true`. cert-manager only writes the Secret when the certificate is close to expiry (renewBefore), so reconcile-driven churn stops. * New charts/vertical-pod-autoscaler/templates/admission-controller/ certificate.yaml renders the Issuer and the Certificate. The Certificate spec exactly mirrors the legacy genSignedCert shape — single DNS SAN `vpa-webhook..svc`, 365d validity — so the admission-controller sees the same certificate topology it always saw. The Secret name resolves via the existing `admissionController.tls.secretName` helper, which means cert-manager writes into the same Secret the legacy template used to own (`-admission-controller-tls`); a separate pre-upgrade hook (next commit) strips Helm/Flux ownership from that existing Secret on upgrade so cert-manager can adopt it without racing Helm's garbage collector. * The legacy Secret template stays in the chart, wrapped in an additional `{{- if not .certManagerEnabled }}` guard, so operators who explicitly opt out keep the old behaviour. * The deployment's `checksum/tls-secret` podAnnotation is gated behind the same flag. Under cert-manager it would either be misleading (Secret content is not driven by that template anymore) or trigger a pointless rollout on every unrelated chart change; both worse than not emitting it. The `existingSecret` path is untouched — operators supplying an external TLS Secret continue to bypass both cert-manager and the legacy Secret template. Verified by `helm template`: - certManagerEnabled=true (default): Certificate+Issuer rendered, no legacy Secret, no checksum/tls-secret annotation. - certManagerEnabled=false: legacy Secret rendered, no Certificate, checksum/tls-secret annotation restored. - existingSecret=: neither path renders, deployment mounts the external Secret. Signed-off-by: Ivan Okhotnikov --- .../admission-controller/certificate.yaml | 96 +++++++++++++++++++ .../admission-controller/deployment.yaml | 15 ++- .../admission-controller/tls-secret.yaml | 12 +++ .../vertical-pod-autoscaler/values.yaml | 3 + 4 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/certificate.yaml diff --git a/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/certificate.yaml b/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/certificate.yaml new file mode 100644 index 000000000..5eace3dfb --- /dev/null +++ b/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/certificate.yaml @@ -0,0 +1,96 @@ +{{- /* + cert-manager-managed TLS for the VPA admission-controller webhook. + + Replaces the legacy `tls-secret.yaml` genCA/genSignedCert Secret. That + approach re-generates a fresh CA on every `helm template` invocation and + therefore on every Flux HelmRelease reconcile — the reconcile-driven cert + churn cascades into a fresh Secret content, a new `checksum/tls-secret` + podAnnotation, a new ReplicaSet, and a rolling admission-controller pod + every retry. Under load (e.g. the E2E sandbox first-install phase where + cert-manager / CDI / LINSTOR races repeatedly re-fire the VPA + HelmRelease), the churn produces 10+ ReplicaSets in minutes and the + admission-controller pod livelocks in startup: it waits on its own + election lease which its previous replicaset is still holding, hits + livenessProbe timeout (periodSeconds=10, failureThreshold=3, no + initialDelaySeconds), and CrashLoopBackOffs — blocking the whole + cozystack-platform HR reconcile. + + cert-manager only rotates when the certificate is close to expiring + (renewBefore), so the reconcile-driven churn stops. + + The webhook itself (`vpa-webhook-config` MutatingWebhookConfiguration) + is registered by the admission-controller pod itself via + `--register-webhook=true`, so no `cert-manager.io/inject-ca-from` + annotation is needed on webhooks; each pod restart picks up the current + CA from the mounted Secret and re-registers. + + Gated on `admissionController.tls.certManagerEnabled`, defaulting to + `true` in this chart. Existing users who supply an external cert via + `admissionController.tls.existingSecret` continue to bypass both this + and the legacy Secret template unchanged. +*/}} +{{- if and .Values.admissionController.enabled .Values.admissionController.tls.certManagerEnabled (not .Values.admissionController.tls.existingSecret) }} +apiVersion: cert-manager.io/v1 +kind: Issuer +metadata: + name: {{ include "vertical-pod-autoscaler.admissionController.fullname" . }}-tls-issuer + namespace: {{ include "common.names.namespace" . | quote }} + labels: + {{- include "vertical-pod-autoscaler.admissionController.labels" . | nindent 4 }} + {{- if .Values.commonLabels }} + {{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" $ ) | nindent 4 }} + {{- end }} + {{- if .Values.commonAnnotations }} + annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }} + {{- end }} +spec: + selfSigned: {} +--- +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: {{ include "vertical-pod-autoscaler.admissionController.fullname" . }}-tls + namespace: {{ include "common.names.namespace" . | quote }} + labels: + {{- include "vertical-pod-autoscaler.admissionController.labels" . | nindent 4 }} + {{- if .Values.commonLabels }} + {{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" $ ) | nindent 4 }} + {{- end }} + {{- if .Values.commonAnnotations }} + annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }} + {{- end }} +spec: + {{- /* + secretName MUST match the volume mount name in + admission-controller/deployment.yaml, which is driven by + `vertical-pod-autoscaler.admissionController.tls.secretName`. In the + default path this resolves to + `-admission-controller-tls`, which is the exact same name the + legacy genCA Secret uses — so on an upgrade from a pre-cert-manager + chart version cert-manager adopts the existing Secret (see the + pre-upgrade tls-migrator hook that strips Helm/Flux ownership) and + the Deployment volume mount picks up the refreshed content on next + rollout. + */}} + secretName: {{ include "vertical-pod-autoscaler.admissionController.tls.secretName" . }} + {{- /* + Match the legacy genSignedCert shape exactly: + $cn := printf "%s.%s.svc" "vpa-webhook" (include "common.names.namespace" .) + $cert := genSignedCert $cn nil (list $cn) 365 $ca + That is: CN and single DNS SAN both `vpa-webhook..svc`, validity + 365 days. Keeping identical spec means an in-place upgrade rotates + the cert content once (issuer changes from ephemeral in-chart CA to + cert-manager selfSigned Issuer) and thereafter stays stable. + */}} + commonName: {{ printf "vpa-webhook.%s.svc" (include "common.names.namespace" .) }} + dnsNames: + - {{ printf "vpa-webhook.%s.svc" (include "common.names.namespace" .) }} + duration: 8760h + renewBefore: 360h + issuerRef: + name: {{ include "vertical-pod-autoscaler.admissionController.fullname" . }}-tls-issuer + kind: Issuer + secretTemplate: + labels: + {{- include "vertical-pod-autoscaler.admissionController.labels" . | nindent 6 }} +{{- end }} diff --git a/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/deployment.yaml b/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/deployment.yaml index b7a923721..5c56cf01e 100644 --- a/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/deployment.yaml +++ b/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/deployment.yaml @@ -30,7 +30,20 @@ spec: {{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" $ ) | nindent 8 }} {{- end }} annotations: - {{- if not .Values.admissionController.tls.existingSecret }} + {{- /* + checksum/tls-secret trips pod rotation whenever tls-secret.yaml + renders a different sha. Under the legacy path that Secret's + content depends on `genCA`/`genSignedCert` which produce a + fresh CA on every render — so this annotation was the amplifier + that turned a single-chart-race into a rolling-ReplicaSet + storm. When cert-manager owns the TLS Secret it's stable + across renders (only rotates near `renewBefore`), and the + `checksum` would either be misleading (secret content isn't + driven by this template anymore) or trigger a needless rollout + on every unrelated chart change. Gate on the same flag as the + legacy template so exactly one code path emits it. + */}} + {{- if and (not .Values.admissionController.tls.existingSecret) (not .Values.admissionController.tls.certManagerEnabled) }} checksum/tls-secret: {{ include (print $.Template.BasePath "/admission-controller/tls-secret.yaml") . | sha256sum }} {{- end }} {{- if .Values.admissionController.podAnnotations }} diff --git a/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/tls-secret.yaml b/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/tls-secret.yaml index f36b4d833..0bab86ed9 100644 --- a/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/tls-secret.yaml +++ b/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller/tls-secret.yaml @@ -1,5 +1,16 @@ +{{- /* + Legacy in-chart genCA/genSignedCert path — kept for the escape hatch + `admissionController.tls.certManagerEnabled=false`. Superseded by + `certificate.yaml` (cert-manager Certificate + Issuer). See the + header comment in that file for the reconcile-driven cert churn this + path used to cause. The check on `.certManagerEnabled` intentionally + wraps the ENTIRE template rather than being folded into the earlier + `if`s so a single Helm render only emits ONE Secret owner (either + cert-manager via Certificate, or this chart directly). +*/ -}} {{- if .Values.admissionController.enabled }} {{- if not .Values.admissionController.tls.existingSecret }} +{{- if not .Values.admissionController.tls.certManagerEnabled }} {{- $ca := genCA (include "vertical-pod-autoscaler.admissionController.fullname" .) 365 }} {{- $cn := printf "%s.%s.svc" "vpa-webhook" (include "common.names.namespace" .) }} {{- $cert := genSignedCert $cn nil (list $cn) 365 $ca }} @@ -23,3 +34,4 @@ data: tls.key: {{ default $cert.Key .Values.admissionController.tls.key | b64enc | quote }} {{- end }} {{- end }} +{{- end }} diff --git a/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/values.yaml b/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/values.yaml index 7d9ac33be..ba202b4c8 100644 --- a/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/values.yaml +++ b/packages/system/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/values.yaml @@ -323,6 +323,9 @@ admissionController: relabelings: [] tls: + ## @param admissionController.tls.certManagerEnabled Provision the admission-controller webhook TLS via cert-manager Issuer + Certificate. When true, `caCert` / `cert` / `key` below are ignored, no in-chart `genCA` Secret is rendered, and the `checksum/tls-secret` podAnnotation that used to rotate the ReplicaSet on every reconcile is dropped. Requires cert-manager installed in the cluster. See `certificate.yaml` header for rationale. + certManagerEnabled: true + ## @param admissionController.tls.caCert TLS CA certificate (Generated using the `genCA` function if not set) caCert: ""