commonLabels is concatenated, not merged, in the shared chart labels helper (6 charts)
Description
The commonLabels Helm value is concatenated onto the chart's own labels as raw YAML text rather than merged, so any user-supplied key that collides with a chart-owned key produces two mapping entries for the same key and breaks rendering:
Error: map[string]interface {}(nil): yaml: unmarshal errors:
line 35: mapping key "app" already defined at line 29The same helper body appears in at least six charts in the org:
| Chart | Helper |
|---|---|
| google-cas-issuer | deploy/charts/google-cas-issuer/templates/_helpers.tpl |
| trust-manager | deploy/charts/trust-manager/templates/_helpers.tpl |
| approver-policy | deploy/charts/approver-policy/templates/_helpers.tpl |
| istio-csr | deploy/charts/istio-csr/templates/_helpers.tpl |
| csi-driver | deploy/charts/csi-driver/templates/_helpers.tpl |
| csi-driver-spiffe | deploy/charts/csi-driver-spiffe/templates/_helpers.tpl |
All six end with the same two lines:
{{- if .Values.commonLabels}}
{{ toYaml .Values.commonLabels }}
{{- end }}so setting any of app.kubernetes.io/name, app.kubernetes.io/instance, app.kubernetes.io/version, app.kubernetes.io/managed-by or helm.sh/chart via commonLabels breaks the render in every one of them.
This was reported against google-cas-issuer as cert-manager/google-cas-issuer#162 and is being fixed there in cert-manager/google-cas-issuer#562. Opening this so the other five don't drift: the risk flagged in review is that each chart lands different semantics (chart-wins in one, user-wins in another, fail in a third), so commonLabels.app behaves differently depending on which chart you install.
Proposed
Settle the precedence once, then apply the same three-line helper everywhere, alongside a note like the one the .image helper already carries (see #6329).
The approach taken in cert-manager/google-cas-issuer#562, for reference:
{{- $labels := dict
"app.kubernetes.io/name" (include "<chart>.name" .)
"helm.sh/chart" (include "<chart>.chart" .)
"app.kubernetes.io/instance" .Release.Name
"app.kubernetes.io/managed-by" .Release.Service
-}}
{{- if .Chart.AppVersion }}
{{- $labels = set $labels "app.kubernetes.io/version" .Chart.AppVersion }}
{{- end }}
{{- toYaml (mergeOverwrite $labels (.Values.commonLabels | default dict)) }}Rendering with toYaml rather than emitting keys by hand also matters: a bare key such as on, no, yes or y is decoded as a YAML 1.1 boolean by the client Helm uses, so it must stay quoted.
Note that selector labels need care per chart — a label that a Deployment's spec.selector.matchLabels reads cannot follow commonLabels, because selectors are immutable.
I'm happy to open the follow-up PRs for the other five charts if that's useful — wanted to check the preferred precedence first, since that decision should be made once rather than per chart.
Source: cert-manager/cert-manager