فهرست منبع

fix(charts): truncate component names to stay within 63-char DNS limit (#6428)

* fix(charts): truncate component names to stay within 63-char DNS limit

`external-secrets.fullname` is already truncated to 63 characters, but
suffixes such as `-cert-controller-metrics` (24 chars) were appended
without re-truncating. This caused Kubernetes to reject resource names
with `metadata.name: must be no more than 63 characters`.

Introduces a `external-secrets.componentName` helper that truncates the
base fullname to `63 - len(suffix)` chars before appending the suffix,
guaranteeing the total never exceeds the DNS label limit.

Existing working installs are unaffected: when the existing name is
already within 63 chars the truncation is a no-op and the helper returns
the same string as the old bare-concat pattern.

Fixes: external-secrets/external-secrets#1997
Signed-off-by: Alexander Chernov <alexander@chernov.it>

* fix(charts): correct sub operand order in componentName helper

Sprig sub(a, b) = a - b, so the previous expression
int (sub (len $suffix) 63) produced a negative $maxLen.
Helm trunc with a negative argument takes chars from the END of
the string rather than the beginning, so long release names were
truncated from the left (e.g. "y-very-long-...") instead of
the right (e.g. "my-very-long-...testin-webhook").

Fix: swap operands to int (sub 63 (len $suffix)), which produces
the correct positive length so trunc preserves the start of the
release name.

Fixes: external-secrets/external-secrets#1997
Signed-off-by: Alexander Chernov <alexander@chernov.it>

* test(charts): add helm-unittest cases for componentName DNS truncation

Four tests in name_truncation_test.yaml cover the componentName helper
introduced to fix issue #1997:

- short name: confirms trunc is a no-op when the base is well under 63 chars
- long name + short suffix (-webhook, 8 chars): verifies truncation happens
  from the right, preserving the start of the release name
- long name + longest suffix (-cert-controller-metrics, 24 chars): the primary
  failure mode from #1997; result is exactly 63 chars
- trailing-dash trim: confirms trimSuffix "-" fires when the cut point is a
  hyphen, preventing a double-dash in the final name

Fixes: external-secrets/external-secrets#1997
Signed-off-by: Alexander Chernov <alexander@chernov.it>

* test(charts): add explicit flags and length assertions to truncation tests

Add matchRegex ^.{1,63}$ assertions to all four name_truncation_test cases
so the 63-char DNS limit is verified directly rather than implicitly via the
equal assertion alone. Also set webhook.create, webhook.service.enabled, and
certController.create explicitly instead of relying on chart defaults, which
makes the tests resilient to future default changes.

Refs: external-secrets/external-secrets#1997
Signed-off-by: Alexander Chernov <alexander@chernov.it>

* fix(charts): narrow DNS-label truncation to Service names only

`componentName` now applies only where Kubernetes enforces the 63-char
DNS label limit: Service metadata.name fields and the values that must
match those names for TLS routing to work (Certificate dnsNames/commonName,
ValidatingWebhook clientConfig.service.name, --dns-name and --service-name
args in the cert-controller Deployment).

All other resources (Deployment, ClusterRole, ClusterRoleBinding, Secret,
Certificate metadata.name, ConfigMap, PDB, ServiceMonitor) revert to plain
fullname + suffix. Those resources accept up to 253-char names; truncating
them would silently rename existing resources on upgrade, which is a
breaking change for any installation with a release name long enough to
push non-Service names past 63 chars.

All 221 helm unit tests pass.

Refs: external-secrets/external-secrets#1997
Signed-off-by: Alexander Chernov <alexander@chernov.it>

* test(charts): extend truncation tests to all 3 Services and their downstream refs

Add test cases for service.yaml (controller metrics Service, -metrics suffix),
validatingwebhook.yaml clientConfig.service.name, and webhook-certificate.yaml
commonName/dnsNames to ensure all three Services and the downstream values that
must match them for TLS routing are covered by name_truncation_test.yaml.

Total test count: 221 -> 224. All suites pass.

Refs: external-secrets/external-secrets#1997
Signed-off-by: Alexander Chernov <alexander@chernov.it>

* fix(charts): add suffix-length guard and cross-consistency tests

Add a fail guard to componentName so an overly long suffix produces an
explicit render error instead of a silently negative maxLen. Also add
two helm-unittest cases that assert cert-controller --service-name and
webhook Service metadata.name produce identical values under truncation,
catching any future helper divergence that would break TLS routing.

Refs: external-secrets/external-secrets#1997
Signed-off-by: Alexander Chernov <alexander@chernov.it>

---------

Signed-off-by: Alexander Chernov <alexander@chernov.it>
Co-authored-by: Gergely Bräutigam <gergely.brautigam@sap.com>
Alexander Chernov 1 هفته پیش
والد
کامیت
9a953d907f

+ 14 - 0
deploy/charts/external-secrets/templates/_helpers.tpl

@@ -23,6 +23,20 @@ If release name contains chart name it will be used as a full name.
 {{- end }}
 {{- end }}
 
+{{/*
+Build a resource name with a suffix, ensuring the total length stays within the
+63-character DNS label limit. The fullname is truncated to (63 - len(suffix))
+chars so that appending the suffix never exceeds the limit.
+Usage: {{ include "external-secrets.componentName" (list . "-webhook") }}
+*/}}
+{{- define "external-secrets.componentName" -}}
+{{- $ctx    := index . 0 -}}
+{{- $suffix := index . 1 -}}
+{{- $maxLen := int (sub 63 (len $suffix)) -}}
+{{- if le $maxLen 0 }}{{- fail (printf "suffix '%s' is too long to fit in a 63-char DNS label" $suffix) }}{{- end -}}
+{{- printf "%s%s" (include "external-secrets.fullname" $ctx | trunc $maxLen | trimSuffix "-") $suffix -}}
+{{- end -}}
+
 {{/*
 Define namespace of chart, useful for multi-namespace deployments
 */}}

+ 1 - 1
deploy/charts/external-secrets/templates/cert-controller-deployment.yaml

@@ -71,7 +71,7 @@ spec:
           args:
           - certcontroller
           - --crd-requeue-interval={{ .Values.certController.requeueInterval }}
-          - --service-name={{ include "external-secrets.fullname" . }}-webhook
+          - --service-name={{ include "external-secrets.componentName" (list . "-webhook") }}
           - --service-namespace={{ template "external-secrets.namespace" . }}
           - --secret-name={{ include "external-secrets.fullname" . }}-webhook
           - --secret-namespace={{ template "external-secrets.namespace" . }}

+ 1 - 1
deploy/charts/external-secrets/templates/cert-controller-service.yaml

@@ -7,7 +7,7 @@
 apiVersion: v1
 kind: Service
 metadata:
-  name: {{ include "external-secrets.fullname" . }}-cert-controller-metrics
+  name: {{ include "external-secrets.componentName" (list . "-cert-controller-metrics") }}
   namespace: {{ template "external-secrets.namespace" . }}
   labels:
     {{- include "external-secrets-cert-controller.labels" . | nindent 4 }}

+ 1 - 1
deploy/charts/external-secrets/templates/service.yaml

@@ -3,7 +3,7 @@
 apiVersion: v1
 kind: Service
 metadata:
-  name: {{ include "external-secrets.fullname" . }}-metrics
+  name: {{ include "external-secrets.componentName" (list . "-metrics") }}
   namespace: {{ template "external-secrets.namespace" . }}
   labels:
     {{- include "external-secrets.labels" . | nindent 4 }}

+ 3 - 3
deploy/charts/external-secrets/templates/validatingwebhook.yaml

@@ -26,7 +26,7 @@ webhooks:
   clientConfig:
     service:
       namespace: {{ template "external-secrets.namespace" . }}
-      name: {{ include "external-secrets.fullname" . }}-webhook
+      name: {{ include "external-secrets.componentName" (list . "-webhook") }}
       path: /validate-external-secrets-io-v1-secretstore
   admissionReviewVersions: ["v1", "v1beta1"]
   sideEffects: None
@@ -43,7 +43,7 @@ webhooks:
   clientConfig:
     service:
       namespace: {{ template "external-secrets.namespace" . }}
-      name: {{ include "external-secrets.fullname" . }}-webhook
+      name: {{ include "external-secrets.componentName" (list . "-webhook") }}
       path: /validate-external-secrets-io-v1-clustersecretstore
   admissionReviewVersions: ["v1", "v1beta1"]
   sideEffects: None
@@ -77,7 +77,7 @@ webhooks:
   clientConfig:
     service:
       namespace: {{ template "external-secrets.namespace" . }}
-      name: {{ include "external-secrets.fullname" . }}-webhook
+      name: {{ include "external-secrets.componentName" (list . "-webhook") }}
       path: /validate-external-secrets-io-v1-externalsecret
   admissionReviewVersions: ["v1", "v1beta1"]
   sideEffects: None

+ 4 - 4
deploy/charts/external-secrets/templates/webhook-certificate.yaml

@@ -13,11 +13,11 @@ metadata:
     {{- toYaml . | nindent 4 }}
   {{- end }}
 spec:
-  commonName: {{ include "external-secrets.fullname" . }}-webhook
+  commonName: {{ include "external-secrets.componentName" (list . "-webhook") }}
   dnsNames:
-    - {{ include "external-secrets.fullname" . }}-webhook
-    - {{ include "external-secrets.fullname" . }}-webhook.{{ template "external-secrets.namespace" . }}
-    - {{ include "external-secrets.fullname" . }}-webhook.{{ template "external-secrets.namespace" . }}.svc
+    - {{ include "external-secrets.componentName" (list . "-webhook") }}
+    - {{ include "external-secrets.componentName" (list . "-webhook") }}.{{ template "external-secrets.namespace" . }}
+    - {{ include "external-secrets.componentName" (list . "-webhook") }}.{{ template "external-secrets.namespace" . }}.svc
   issuerRef:
     {{- toYaml .Values.webhook.certManager.cert.issuerRef | nindent 4 }}
   {{- with .Values.webhook.certManager.cert.duration }}

+ 1 - 1
deploy/charts/external-secrets/templates/webhook-deployment.yaml

@@ -71,7 +71,7 @@ spec:
           args:
           - webhook
           - --port={{ .Values.webhook.port }}
-          - --dns-name={{ include "external-secrets.fullname" . }}-webhook.{{ template "external-secrets.namespace" . }}.svc
+          - --dns-name={{ include "external-secrets.componentName" (list . "-webhook") }}.{{ template "external-secrets.namespace" . }}.svc
           - --cert-dir={{ .Values.webhook.certDir }}
           - --check-interval={{ .Values.webhook.certCheckInterval }}
           - --metrics-addr=:{{ .Values.webhook.metrics.listen.port }}

+ 1 - 1
deploy/charts/external-secrets/templates/webhook-service.yaml

@@ -3,7 +3,7 @@
 apiVersion: v1
 kind: Service
 metadata:
-  name: {{ include "external-secrets.fullname" . }}-webhook
+  name: {{ include "external-secrets.componentName" (list . "-webhook") }}
   namespace: {{ template "external-secrets.namespace" . }}
   labels:
     {{- include "external-secrets-webhook.labels" . | nindent 4 }}

+ 164 - 0
deploy/charts/external-secrets/tests/name_truncation_test.yaml

@@ -0,0 +1,164 @@
+suite: test componentName DNS-label name truncation
+templates:
+  - webhook-service.yaml
+  - cert-controller-service.yaml
+  - cert-controller-deployment.yaml
+  - service.yaml
+  - validatingwebhook.yaml
+  - webhook-certificate.yaml
+tests:
+  - it: should not alter names that are well under the 63-char DNS label limit
+    set:
+      fullnameOverride: my-release
+      webhook.create: true
+      webhook.service.enabled: true
+    template: webhook-service.yaml
+    asserts:
+      - equal:
+          path: metadata.name
+          value: my-release-webhook
+      - matchRegex:
+          path: metadata.name
+          pattern: ^.{1,63}$
+
+  - it: should truncate from the right and preserve the name prefix for a short suffix (-webhook 8 chars)
+    # fullnameOverride: 60 chars ("external-secrets-production-cluster-for-long-release-testing").
+    # componentName maxLen = 63 - 8 = 55; trunc(55) keeps the first 55 chars of the
+    # base name so appending the 8-char suffix produces exactly 63 chars total.
+    set:
+      fullnameOverride: "external-secrets-production-cluster-for-long-release-testing"
+      webhook.create: true
+      webhook.service.enabled: true
+    template: webhook-service.yaml
+    asserts:
+      - equal:
+          path: metadata.name
+          value: "external-secrets-production-cluster-for-long-release-te-webhook"
+      - matchRegex:
+          path: metadata.name
+          pattern: ^.{1,63}$
+
+  - it: should truncate from the right for the longest suffix in the chart (-cert-controller-metrics 24 chars)
+    # Same 60-char fullnameOverride.
+    # componentName maxLen = 63 - 24 = 39; trunc(39) keeps the first 39 chars,
+    # so the total is exactly 63.  This is the primary failure case from issue #1997.
+    set:
+      fullnameOverride: "external-secrets-production-cluster-for-long-release-testing"
+      certController.create: true
+      certController.metrics.service.enabled: true
+    template: cert-controller-service.yaml
+    asserts:
+      - equal:
+          path: metadata.name
+          value: "external-secrets-production-cluster-for-cert-controller-metrics"
+      - matchRegex:
+          path: metadata.name
+          pattern: ^.{1,63}$
+
+  - it: should strip a trailing dash when the truncation point falls on a hyphen
+    # fullnameOverride: 60 chars with a hyphen at position 55
+    # ("external-secrets-for-production-release-cluster-naming-tests").
+    # trunc(55) yields the first 54 chars plus a trailing "-"; trimSuffix "-"
+    # removes it, leaving 54 chars, so the result is 54 + 8 = 62 chars with no
+    # double-dash at the suffix boundary.
+    set:
+      fullnameOverride: "external-secrets-for-production-release-cluster-naming-tests"
+      webhook.create: true
+      webhook.service.enabled: true
+    template: webhook-service.yaml
+    asserts:
+      - equal:
+          path: metadata.name
+          value: "external-secrets-for-production-release-cluster-naming-webhook"
+      - matchRegex:
+          path: metadata.name
+          pattern: ^.{1,63}$
+
+  - it: service.yaml - should truncate -metrics suffix (8 chars) when fullname is long
+    # service.yaml uses componentName with suffix "-metrics" (8 chars), same
+    # maxLen=55 as -webhook. Confirms the main controller metrics Service is
+    # covered by the helper, not just webhook/cert-controller Services.
+    set:
+      fullnameOverride: "external-secrets-production-cluster-for-long-release-testing"
+      metrics.service.enabled: true
+    template: service.yaml
+    asserts:
+      - equal:
+          path: metadata.name
+          value: "external-secrets-production-cluster-for-long-release-te-metrics"
+      - matchRegex:
+          path: metadata.name
+          pattern: ^.{1,63}$
+
+  - it: validatingwebhook.yaml - clientConfig.service.name must match the truncated webhook Service name
+    # The ValidatingWebhook routes admission requests to the webhook Service.
+    # If service.name here diverges from the Service metadata.name, TLS
+    # routing breaks. Both must use the same componentName helper output.
+    set:
+      fullnameOverride: "external-secrets-production-cluster-for-long-release-testing"
+      webhook.create: true
+    template: validatingwebhook.yaml
+    documentIndex: 0
+    asserts:
+      - equal:
+          path: webhooks[0].clientConfig.service.name
+          value: "external-secrets-production-cluster-for-long-release-te-webhook"
+      - matchRegex:
+          path: webhooks[0].clientConfig.service.name
+          pattern: ^.{1,63}$
+
+  - it: webhook-certificate.yaml - dnsNames must match the truncated webhook Service hostname
+    # cert-manager issues TLS certs for the webhook Service. The SAN must
+    # match the Service name exactly or the TLS handshake fails. Both
+    # commonName and the first dnsName entry use componentName so they stay
+    # in sync with the Service even when the release name is long.
+    set:
+      fullnameOverride: "external-secrets-production-cluster-for-long-release-testing"
+      webhook.create: true
+      webhook.certManager.enabled: true
+      webhook.certManager.cert.create: true
+      webhook.certManager.cert.issuerRef:
+        name: selfsigned
+        kind: ClusterIssuer
+    template: webhook-certificate.yaml
+    asserts:
+      - equal:
+          path: spec.commonName
+          value: "external-secrets-production-cluster-for-long-release-te-webhook"
+      - equal:
+          path: spec.dnsNames[0]
+          value: "external-secrets-production-cluster-for-long-release-te-webhook"
+      - matchRegex:
+          path: spec.commonName
+          pattern: ^.{1,63}$
+
+  - it: "cert-controller --service-name must match webhook Service name when truncation fires"
+    # cert-controller-deployment uses componentName with -webhook suffix for
+    # --service-name. The value must equal the webhook Service metadata.name or
+    # the cert-controller can't locate the Service and TLS breaks.
+    set:
+      fullnameOverride: "external-secrets-production-cluster-for-long-release-testing"
+      certController.create: true
+      webhook.create: true
+    template: cert-controller-deployment.yaml
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].args
+          content: "--service-name=external-secrets-production-cluster-for-long-release-te-webhook"
+
+  - it: "webhook Service metadata.name must match cert-controller --service-name when truncation fires"
+    # Mirror of the above: the webhook Service must be named identically to what
+    # cert-controller passes as --service-name. Both use componentName(-webhook)
+    # so they stay in sync; this test surfaces any future divergence.
+    set:
+      fullnameOverride: "external-secrets-production-cluster-for-long-release-testing"
+      webhook.create: true
+      webhook.service.enabled: true
+    template: webhook-service.yaml
+    asserts:
+      - equal:
+          path: metadata.name
+          value: "external-secrets-production-cluster-for-long-release-te-webhook"
+      - matchRegex:
+          path: metadata.name
+          pattern: ^.{1,63}$