From 85ab0fa5abaf9db759d4a54f427e4f8da8c37176 Mon Sep 17 00:00:00 2001 From: sabsari Date: Mon, 31 Aug 2026 00:44:56 +0900 Subject: [PATCH] Fix spire-server PodMonitor controller-manager port names (#932) The PodMonitor targeted the port name prom-cm, but controller-manager containers expose pm-cm (and auto-suffixed/overridable names for external managers), so their metrics were never scraped. Resolve port names through a shared helper and enumerate every controller-manager in the PodMonitor. Signed-off-by: sabsari Co-authored-by: Claude Opus 4.8 --- .../_controller-manager-container.tpl | 97 +++++++++++++----- .../spire-server/templates/podmonitor.yaml | 6 +- tests/unit/spire_test.go | 98 +++++++++++++++++++ 3 files changed, 174 insertions(+), 27 deletions(-) diff --git a/charts/spire/charts/spire-server/templates/_controller-manager-container.tpl b/charts/spire/charts/spire-server/templates/_controller-manager-container.tpl index 14cc911..64d28fb 100644 --- a/charts/spire/charts/spire-server/templates/_controller-manager-container.tpl +++ b/charts/spire/charts/spire-server/templates/_controller-manager-container.tpl @@ -1,3 +1,72 @@ +{{/* 15-char-safe port-name suffix for a controller-manager cluster name ("" -> ""). Shared by container ports and the PodMonitor so they cannot drift. */}} +{{- define "spire-controller-manager.portSuffix" -}} +{{- $name := . -}} +{{- $portSuffix := "" -}} +{{- if ne $name "" -}} +{{- $portSuffix = printf "-%s" $name -}} +{{- if gt (len $name) 9 -}} +{{- $numberMatch := regexFind "[-]?[0-9]{1,2}$" $name -}} +{{- if $numberMatch -}} +{{- $numLen := len $numberMatch -}} +{{- $baseLen := sub (len $name) $numLen | int -}} +{{- $baseName := substr 0 $baseLen $name -}} +{{- if not (hasPrefix "-" $numberMatch) -}} +{{- $numberMatch = printf "-%s" $numberMatch -}} +{{- end -}} +{{- $maxBase := sub 9 (len $numberMatch) | int -}} +{{- $baseName = $baseName | trunc $maxBase | trimSuffix "-" -}} +{{- $portSuffix = printf "-%s%s" $baseName $numberMatch -}} +{{- else -}} +{{- $hash := sha256sum $name | trunc 3 -}} +{{- $portSuffix = printf "-%s-%s" ($name | trunc 5 | trimSuffix "-") $hash -}} +{{- end -}} +{{- end -}} +{{- end -}} +{{- $portSuffix -}} +{{- end -}} + +{{/* Resolve a port name: override wins, else prefix+suffix. dict: prefix, portSuffix, override */}} +{{- define "spire-controller-manager.portName" -}} +{{- if and (hasKey . "override") (ne (.override | toString) "") -}} +{{- .override -}} +{{- else -}} +{{- printf "%s%s" .prefix .portSuffix -}} +{{- end -}} +{{- end -}} + +{{/* Prometheus port name for one controller-manager. dict: name (""=main), settings (may hold prometheusPortName) */}} +{{- define "spire-controller-manager.promPortName" -}} +{{- $override := "" -}} +{{- if hasKey .settings "prometheusPortName" -}} +{{- $override = .settings.prometheusPortName -}} +{{- end -}} +{{- include "spire-controller-manager.portName" (dict "prefix" "pm-cm" "portSuffix" (include "spire-controller-manager.portSuffix" .name) "override" $override) -}} +{{- end -}} + +{{/* List of prometheus port names for every controller-manager that renders one. Consumed by the PodMonitor. */}} +{{- define "spire-controller-manager.prometheusPortNames" -}} +{{- $root := . -}} +{{- $names := list -}} +{{- if eq (.Values.controllerManager.enabled | toString) "true" -}} +{{- $names = append $names (include "spire-controller-manager.promPortName" (dict "name" "" "settings" .Values.controllerManager)) -}} +{{- end -}} +{{- if .Values.externalControllerManagers.enabled -}} +{{- $clusters := default .Values.kubeConfigs .Values.externalControllerManagers.clusters -}} +{{- range $name, $_ := $clusters -}} +{{- $clusterSettings := dict -}} +{{- if hasKey $root.Values.externalControllerManagers.clusters $name -}} +{{- $clusterSettings = index $root.Values.externalControllerManagers.clusters $name -}} +{{- end -}} +{{- $pmName := include "spire-controller-manager.promPortName" (dict "name" $name "settings" $clusterSettings) -}} +{{- if has $pmName $names -}} +{{- fail (printf "controller-manager prometheus port name %q collides for cluster %q; set a distinct prometheusPortName override" $pmName $name) -}} +{{- end -}} +{{- $names = append $names $pmName -}} +{{- end -}} +{{- end -}} +{{- $names | toYaml -}} +{{- end -}} + {{- define "spire-controller-manager.containers" }} {{- $root := . }} {{- $settings := dict }} @@ -41,23 +110,7 @@ Auto-generation preserves trailing numbers from cluster names or uses hash for u {{- $prometheusPortName = $clusterSettings.prometheusPortName }} {{- end }} {{- if or (eq $healthPortName "") (eq $prometheusPortName "") }} -{{- if gt (len $name) 9 }} -{{- $numberMatch := regexFind "[-]?[0-9]{1,2}$" $name }} -{{- if $numberMatch }} -{{- $numLen := len $numberMatch }} -{{- $baseLen := sub (len $name) $numLen | int }} -{{- $baseName := substr 0 $baseLen $name }} -{{- if not (hasPrefix "-" $numberMatch) }} -{{- $numberMatch = printf "-%s" $numberMatch }} -{{- end }} -{{- $maxBase := sub 9 (len $numberMatch) | int }} -{{- $baseName = $baseName | trunc $maxBase | trimSuffix "-" }} -{{- $portSuffix = printf "-%s%s" $baseName $numberMatch }} -{{- else }} -{{- $hash := sha256sum $name | trunc 3 }} -{{- $portSuffix = printf "-%s-%s" ($name | trunc 5 | trimSuffix "-") $hash }} -{{- end }} -{{- end }} +{{- $portSuffix = include "spire-controller-manager.portSuffix" $name }} {{- end }} {{- $startPort = add $startPort 2 }} @@ -127,17 +180,11 @@ Auto-generation preserves trailing numbers from cluster names or uses hash for u containerPort: 9443 protocol: TCP {{- end }} - {{- $hpName := .healthPortName }} - {{- if eq $hpName "" }} - {{- $hpName = printf "hp-cm%s" .portSuffix }} - {{- end }} + {{- $hpName := include "spire-controller-manager.portName" (dict "prefix" "hp-cm" "portSuffix" .portSuffix "override" .healthPortName) }} - containerPort: {{ $healthPort }} name: {{ $hpName }} {{- if or (dig "telemetry" "prometheus" "enabled" .Values.telemetry.prometheus.enabled .Values.global) (and (dig "spire" "recommendations" "enabled" false .Values.global) (dig "spire" "recommendations" "prometheus" true .Values.global)) }} - {{- $pmName := .prometheusPortName }} - {{- if eq $pmName "" }} - {{- $pmName = printf "pm-cm%s" .portSuffix }} - {{- end }} + {{- $pmName := include "spire-controller-manager.portName" (dict "prefix" "pm-cm" "portSuffix" .portSuffix "override" .prometheusPortName) }} - containerPort: {{ $promPort }} name: {{ $pmName }} {{- end }} diff --git a/charts/spire/charts/spire-server/templates/podmonitor.yaml b/charts/spire/charts/spire-server/templates/podmonitor.yaml index d73034c..f17e180 100644 --- a/charts/spire/charts/spire-server/templates/podmonitor.yaml +++ b/charts/spire/charts/spire-server/templates/podmonitor.yaml @@ -21,10 +21,12 @@ spec: {{- include "spire-server.selectorLabels" . | nindent 6 }} podMetricsEndpoints: - port: prom - - port: prom-cm + {{- range (include "spire-controller-manager.prometheusPortNames" . | fromYamlArray) }} + - port: {{ . }} + {{- end }} {{- if ne $namespace $podNamespace }} namespaceSelector: kubernetes.io/metadata.name: {{ $podNamespace }} - {{- end }} + {{- end }} {{- end }} {{- end }} diff --git a/tests/unit/spire_test.go b/tests/unit/spire_test.go index 4f11873..c2819bb 100644 --- a/tests/unit/spire_test.go +++ b/tests/unit/spire_test.go @@ -810,6 +810,104 @@ spire-server: } }) }) + Describe("spire-server.telemetry.podMonitor controller-manager ports", func() { + podMonitorTmpl := "spire/charts/spire-server/templates/podmonitor.yaml" + serverTmpl := "spire/charts/spire-server/templates/server-resource.yaml" + + It("targets the real main controller-manager port name, not the legacy prom-cm", func() { + objs, err := ValueStringRender(chart, ` +spire-server: + controllerManager: + enabled: true + telemetry: + prometheus: + enabled: true + podMonitor: + enabled: true +`) + Expect(err).Should(Succeed()) + podMonitor := objs[podMonitorTmpl] + Expect(podMonitor).Should(ContainSubstring("- port: prom")) + Expect(podMonitor).Should(ContainSubstring("- port: pm-cm")) + // Regression: the PodMonitor used to hardcode a port name the container never renders. + Expect(podMonitor).ShouldNot(ContainSubstring("prom-cm")) + // The endpoint must match the actual container port. + Expect(objs[serverTmpl]).Should(ContainSubstring("name: pm-cm")) + }) + + It("enumerates external controller-managers, honouring auto-suffix and prometheusPortName override", func() { + objs, err := ValueStringRender(chart, ` +spire-server: + controllerManager: + enabled: true + telemetry: + prometheus: + enabled: true + podMonitor: + enabled: true + kubeConfigs: + child01: + kubeConfig: | + apiVersion: v1 + kind: Config + verylongclustername: + kubeConfig: | + apiVersion: v1 + kind: Config + externalControllerManagers: + enabled: true + clusters: + child01: + kubeConfigName: child01 + verylongclustername: + kubeConfigName: verylongclustername + prometheusPortName: prom-ext2 +`) + Expect(err).Should(Succeed()) + podMonitor := objs[podMonitorTmpl] + server := objs[serverTmpl] + // Auto-suffixed external CM and the overridden one must both be scraped, + // and each endpoint must match a real container port name. + for _, port := range []string{"pm-cm", "pm-cm-child01", "prom-ext2"} { + Expect(podMonitor).Should(ContainSubstring("- port: " + port)) + Expect(server).Should(ContainSubstring("name: " + port)) + } + }) + + It("derives external controller-managers from kubeConfigs when clusters is the default {}", func() { + objs, err := ValueStringRender(chart, ` +spire-server: + controllerManager: + enabled: true + telemetry: + prometheus: + enabled: true + podMonitor: + enabled: true + kubeConfigs: + child01: + kubeConfig: | + apiVersion: v1 + kind: Config + child02: + kubeConfig: | + apiVersion: v1 + kind: Config + externalControllerManagers: + enabled: true + clusters: {} +`) + Expect(err).Should(Succeed()) + podMonitor := objs[podMonitorTmpl] + server := objs[serverTmpl] + // clusters={} (the chart default) falls back to kubeConfigs, so each + // kubeConfig-derived controller-manager must be scraped and match its port. + for _, port := range []string{"pm-cm", "pm-cm-child01", "pm-cm-child02"} { + Expect(podMonitor).Should(ContainSubstring("- port: " + port)) + Expect(server).Should(ContainSubstring("name: " + port)) + } + }) + }) Describe("spire-server.dataStore.sql.postgres passwordless", func() { It("omits password and the -dbpw Secret for cert auth with an empty password", func() { objs, err := ValueStringRender(chart, `