feat(helm): honour bindAddress=0 as a metrics disable signal

The `controller.metrics.bindAddress` value mirrors the controller's
`--metrics-bind-address` flag, and that flag's documented contract
treats `0` as "disable the metrics server" (see
go/core/pkg/app/app.go:145-146). Letting the chart silently render an
invalid `containerPort: 0` when the operator follows that contract is
surprising; treat the binary's disable sentinel as another way of
saying `controller.metrics.enabled=false`.

Wrap the existing `kagent.controller.metricsPort` helper with a new
`kagent.controller.metricsEnabled` helper that returns truthy only
when `enabled` is true *and* the resolved port is a real, non-zero
value. Switch the Service, both ClusterRoles, the ClusterRoleBinding,
and the Deployment env/port wiring over to the new helper so the two
disable signals are equivalent in their effect on rendered manifests.

The escape-hatch behaviour is unchanged: setting
`METRICS_BIND_ADDRESS` via `controller.env` still shifts only the
runtime listener and cannot move the chart-rendered Service.

Addresses review feedback on #1803.

Signed-off-by: Daniel Orbach <ddorbach@gmail.com>

Author: danielorbach

helm/kagent/templates/_helpers.tpl

@@ -138,14 +138,27 @@ Extract the TCP port from controller.metrics.bindAddress.
 Anchors the digit run to the end of the string so every Go-style
 address form the controller binary accepts is handled correctly: bare
 ":port", host-qualified "host:port", and bracketed IPv6 "[::1]:port"
-all yield the trailing port. Disabling metrics is exclusively the job
-of controller.metrics.enabled=false; bindAddress is expected to carry
-a real, non-zero port whenever the metrics resources render.
+all yield the trailing port. Returns "0" or "" when the binary's
+disable sentinel is in use; callers must consult
+`kagent.controller.metricsEnabled` before rendering manifests.
 */}}
 {{- define "kagent.controller.metricsPort" -}}
 {{- regexFind "[0-9]+$" (.Values.controller.metrics.bindAddress | toString) -}}
 {{- end -}}
 
+{{/*
+Returns "1" when the controller metrics resources (Service, RBAC,
+container port, env vars) should render, empty otherwise. Honours both
+disable signals: `controller.metrics.enabled=false` and the binary's
+own `--metrics-bind-address=0` sentinel reached through `bindAddress`.
+The two are equivalent so the field name keeps faith with the binary's
+documented contract (see go/core/pkg/app/app.go).
+*/}}
+{{- define "kagent.controller.metricsEnabled" -}}
+{{- $port := include "kagent.controller.metricsPort" . -}}
+{{- if and .Values.controller.metrics.enabled $port (ne $port "0") -}}1{{- end -}}
+{{- end -}}
+
 {{/*
 PostgreSQL service name for the bundled postgres instance
 */}}

helm/kagent/templates/controller-deployment.yaml

@@ -84,7 +84,7 @@ spec:
             {{- else }}
             {{ fail "No database connection configured. Set database.postgres.url, database.postgres.urlFile, or enable database.postgres.bundled." }}
             {{- end }}
-            {{- if .Values.controller.metrics.enabled }}
+            {{- if include "kagent.controller.metricsEnabled" . }}
             - name: METRICS_BIND_ADDRESS
               value: {{ .Values.controller.metrics.bindAddress | quote }}
             - name: METRICS_SECURE

helm/kagent/templates/controller-metrics-service.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.controller.metrics.enabled }}
+{{- if include "kagent.controller.metricsEnabled" . }}
 apiVersion: v1
 kind: Service
 metadata:

helm/kagent/templates/rbac/metrics-auth-clusterrole.yaml

@@ -1,4 +1,4 @@
-{{- if and .Values.controller.metrics.enabled .Values.controller.metrics.secureServing }}
+{{- if and (include "kagent.controller.metricsEnabled" .) .Values.controller.metrics.secureServing }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:

helm/kagent/templates/rbac/metrics-auth-clusterrolebinding.yaml

@@ -1,4 +1,4 @@
-{{- if and .Values.controller.metrics.enabled .Values.controller.metrics.secureServing }}
+{{- if and (include "kagent.controller.metricsEnabled" .) .Values.controller.metrics.secureServing }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
 metadata:

helm/kagent/templates/rbac/metrics-reader-clusterrole.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.controller.metrics.enabled }}
+{{- if include "kagent.controller.metricsEnabled" . }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:

helm/kagent/tests/controller-deployment_test.yaml

@@ -606,6 +606,27 @@ tests:
             containerPort: 9443
             protocol: TCP
 
+  - it: should not gain metrics wiring when bindAddress disables metrics
+    template: controller-deployment.yaml
+    set:
+      controller.metrics.enabled: true
+      controller.metrics.bindAddress: "0"
+    asserts:
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: METRICS_BIND_ADDRESS
+          any: true
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: METRICS_SECURE
+          any: true
+      - notContains:
+          path: spec.template.spec.containers[0].ports
+          content:
+            name: metrics
+
   - it: should let controller.env override the chart-supplied metrics env
     template: controller-deployment.yaml
     set:

helm/kagent/tests/controller-metrics-rbac_test.yaml

@@ -111,3 +111,18 @@ tests:
       - hasDocuments:
           count: 0
         template: rbac/metrics-auth-clusterrolebinding.yaml
+
+  - it: should skip all metrics rbac when bindAddress disables metrics
+    set:
+      controller.metrics.enabled: true
+      controller.metrics.bindAddress: "0"
+    asserts:
+      - hasDocuments:
+          count: 0
+        template: rbac/metrics-reader-clusterrole.yaml
+      - hasDocuments:
+          count: 0
+        template: rbac/metrics-auth-clusterrole.yaml
+      - hasDocuments:
+          count: 0
+        template: rbac/metrics-auth-clusterrolebinding.yaml

helm/kagent/tests/controller-metrics-service_test.yaml

@@ -72,6 +72,22 @@ tests:
           path: spec.ports[0].targetPort
           value: 9443
 
+  - it: should not render when bindAddress disables metrics
+    set:
+      controller.metrics.enabled: true
+      controller.metrics.bindAddress: "0"
+    asserts:
+      - hasDocuments:
+          count: 0
+
+  - it: should not render when bindAddress is empty
+    set:
+      controller.metrics.enabled: true
+      controller.metrics.bindAddress: ""
+    asserts:
+      - hasDocuments:
+          count: 0
+
   - it: should rename port when secure serving disabled
     set:
       controller.metrics.enabled: true

helm/kagent/values.yaml

@@ -230,7 +230,10 @@ controller:
   # `targetPort` and the pod `containerPort` are derived from it at
   # template time, so overriding `METRICS_BIND_ADDRESS` via
   # `controller.env` shifts only the runtime listener and leaves the
-  # rendered Service pointing at the chart-time port.
+  # rendered Service pointing at the chart-time port. Setting
+  # `bindAddress: "0"` (or empty) is treated as a disable signal —
+  # equivalent to `enabled: false` — to keep faith with the controller
+  # binary's documented contract for `--metrics-bind-address`.
   # @default -- disabled
   metrics:
     enabled: false