Skip to content

fix: merge serviceMonitor additionalLabels with chart defaults#2365

Open
Shion1305 wants to merge 1 commit into
goharbor:mainfrom
Shion1305:fix/servicemonitor-additionallabels-merge
Open

fix: merge serviceMonitor additionalLabels with chart defaults#2365
Shion1305 wants to merge 1 commit into
goharbor:mainfrom
Shion1305:fix/servicemonitor-additionallabels-merge

Conversation

@Shion1305

Copy link
Copy Markdown

Closes #2364.

Problem

templates/metrics/metrics-svcmon.yaml rendered metadata.labels by emitting the harbor.labels helper output and appending metrics.serviceMonitor.additionalLabels verbatim. When a user supplies a label that the helper already emits — most commonly release: kube-prometheus-stack so kube-prometheus-stack's release: kube-prometheus-stack selector picks the ServiceMonitor up — the rendered manifest contains two release: keys in the same map.

This is invalid YAML by strict-schema rules and is rejected by client-side validators (kubeconform --strict, kubectl-validate, GitOps render-validation pipelines). The Kubernetes API server silently keeps one entry, unpredictably.

Reproduce (before this PR)

helm pull harbor --repo https://helm.goharbor.io --version 1.19.0 --untar --untardir /tmp/h
helm template harbor /tmp/h/harbor \
  --set metrics.enabled=true \
  --set metrics.serviceMonitor.enabled=true \
  --set metrics.serviceMonitor.additionalLabels.release=kube-prometheus-stack \
  | yq 'select(.kind == "ServiceMonitor") | .metadata.labels'
# release appears twice (release: harbor and release: kube-prometheus-stack)

Fix

Merge additionalLabels over the helper-provided defaults before rendering, so user-supplied keys win and the rendered metadata.labels map has a single value per key. This matches the convention used by most charts that combine helper labels with additionalLabels/labels overrides.

  labels:
{{- $defaults := (include "harbor.labels" . | fromYaml) }}
{{- $merged := merge (default dict .Values.metrics.serviceMonitor.additionalLabels) $defaults }}
{{ toYaml $merged | indent 4 }}

merge keeps keys already present in the destination (the user's additionalLabels) and only fills in missing keys from $defaults — so user overrides win, and the no-override case (empty dict) is unchanged.

Verification

  • helm lint . → pass (only the pre-existing engine field warning).
  • helm unittest -f 'test/unittest/*/*.yaml' . → 115/115 pass (3 new).
  • Default render — exactly one release: harbor (chart default preserved):
    helm template harbor . --set metrics.enabled=true --set metrics.serviceMonitor.enabled=true \
      | yq 'select(.kind=="ServiceMonitor") | .metadata.labels.release'
    # harbor
  • Override render — exactly one release: kube-prometheus-stack (override wins):
    helm template harbor . --set metrics.enabled=true --set metrics.serviceMonitor.enabled=true \
        --set metrics.serviceMonitor.additionalLabels.release=kube-prometheus-stack \
      | yq 'select(.kind=="ServiceMonitor") | .metadata.labels.release'
    # kube-prometheus-stack
  • helm template ... --set ...additionalLabels.release=kube-prometheus-stack | kubeconform -strict -kubernetes-version 1.31.0 -ignore-missing-schemas - → no errors.

Tests added

test/unittest/metrics/metrics_svcmon_test.yaml — 3 cases covering:

  1. Default labels render once with no additionalLabels set.
  2. New additionalLabels keys are merged alongside chart defaults.
  3. additionalLabels.release overrides the chart default release value (single key, override wins).

Background

Found while building a kubeconform --strict render-validation pipeline for a GitOps repo (every PR re-renders all ArgoCD apps and validates the output). The duplicate-key warning surfaced as soon as release: kube-prometheus-stack was set — the only way to make Prometheus Operator pick this ServiceMonitor up alongside everything else managed by kube-prometheus-stack.

The ServiceMonitor template appended `metrics.serviceMonitor.additionalLabels`
verbatim under `metadata.labels`, producing duplicate keys when the user
overrode a label that the `harbor.labels` helper already emits (most commonly
`release`, set to `kube-prometheus-stack` for Prometheus Operator discovery).
Strict YAML validators (kubeconform --strict, kubectl-validate) reject the
output; the API server silently keeps one entry, unpredictably.

Merge `additionalLabels` over the helper-provided defaults so user-supplied
keys win and the rendered `metadata.labels` map has a single value per key.

Closes goharbor#2364

Signed-off-by: Shion Ichikawa <shion1305@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: ServiceMonitor additionalLabels appended instead of merged, producing duplicate release key

2 participants