Skip to content

Commit 2f9c8d0

Browse files
committed
fix(helm): anchor metrics port regex so host-qualified addresses parse
`regexFind "[0-9]+"` returns the first digit run, so a perfectly valid bindAddress like `127.0.0.1:8443` would extract `127` and the rendered Service would point at the wrong port. Anchor the match to the end of the string with `[0-9]+$` so every Go-style address form yields the trailing port: bare `:port`, host-qualified `host:port`, and bracketed IPv6 `[::1]:port` all parse correctly. The shared logic lives behind a `kagent.controller.metricsPort` helper in `_helpers.tpl` so the deployment containerPort and the Service targetPort cannot fall out of sync if the parsing rule evolves. Disabling metrics is exclusively the job of `controller.metrics.enabled=false`; the chart does not engage with the controller binary's `bindAddress=0` disable sentinel, since wiring two ways to express the same intent only invites contradictory configuration. Addresses review feedback on kagent-dev#1803. Signed-off-by: Daniel Orbach <ddorbach@gmail.com>
1 parent 5032b36 commit 2f9c8d0

5 files changed

Lines changed: 62 additions & 2 deletions

File tree

helm/kagent/templates/_helpers.tpl

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,20 @@ Check if leader election should be enabled (more than 1 replica)
132132
{{- gt (.Values.controller.replicas | int) 1 -}}
133133
{{- end -}}
134134

135+
{{/*
136+
Extract the TCP port from controller.metrics.bindAddress.
137+
138+
Anchors the digit run to the end of the string so every Go-style
139+
address form the controller binary accepts is handled correctly: bare
140+
":port", host-qualified "host:port", and bracketed IPv6 "[::1]:port"
141+
all yield the trailing port. Disabling metrics is exclusively the job
142+
of controller.metrics.enabled=false; bindAddress is expected to carry
143+
a real, non-zero port whenever the metrics resources render.
144+
*/}}
145+
{{- define "kagent.controller.metricsPort" -}}
146+
{{- regexFind "[0-9]+$" (.Values.controller.metrics.bindAddress | toString) -}}
147+
{{- end -}}
148+
135149
{{/*
136150
PostgreSQL service name for the bundled postgres instance
137151
*/}}

helm/kagent/templates/controller-deployment.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ spec:
105105
protocol: TCP
106106
{{- if .Values.controller.metrics.enabled }}
107107
- name: metrics
108-
containerPort: {{ .Values.controller.metrics.bindAddress | regexFind "[0-9]+" | int }}
108+
containerPort: {{ include "kagent.controller.metricsPort" . | int }}
109109
protocol: TCP
110110
{{- end }}
111111
resources:

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ spec:
1111
ports:
1212
- name: {{ ternary "https" "http-metrics" .Values.controller.metrics.secureServing }}
1313
port: {{ .Values.controller.metrics.service.port }}
14-
targetPort: {{ .Values.controller.metrics.bindAddress | regexFind "[0-9]+" | int }}
14+
targetPort: {{ include "kagent.controller.metricsPort" . | int }}
1515
protocol: TCP
1616
selector:
1717
{{- include "kagent.controller.selectorLabels" . | nindent 4 }}

helm/kagent/tests/controller-deployment_test.yaml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,32 @@ tests:
580580
containerPort: 9443
581581
protocol: TCP
582582

583+
- it: should derive metrics container port from a host-qualified bindAddress
584+
template: controller-deployment.yaml
585+
set:
586+
controller.metrics.enabled: true
587+
controller.metrics.bindAddress: "127.0.0.1:9443"
588+
asserts:
589+
- contains:
590+
path: spec.template.spec.containers[0].ports
591+
content:
592+
name: metrics
593+
containerPort: 9443
594+
protocol: TCP
595+
596+
- it: should derive metrics container port from a bracketed IPv6 bindAddress
597+
template: controller-deployment.yaml
598+
set:
599+
controller.metrics.enabled: true
600+
controller.metrics.bindAddress: "[::1]:9443"
601+
asserts:
602+
- contains:
603+
path: spec.template.spec.containers[0].ports
604+
content:
605+
name: metrics
606+
containerPort: 9443
607+
protocol: TCP
608+
583609
- it: should let controller.env override the chart-supplied metrics env
584610
template: controller-deployment.yaml
585611
set:

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,26 @@ tests:
5252
path: spec.ports[0].targetPort
5353
value: 9443
5454

55+
- it: should derive targetPort from a host-qualified bindAddress
56+
set:
57+
controller.metrics.enabled: true
58+
controller.metrics.bindAddress: "127.0.0.1:9443"
59+
controller.metrics.service.port: 9443
60+
asserts:
61+
- equal:
62+
path: spec.ports[0].targetPort
63+
value: 9443
64+
65+
- it: should derive targetPort from a bracketed IPv6 bindAddress
66+
set:
67+
controller.metrics.enabled: true
68+
controller.metrics.bindAddress: "[::1]:9443"
69+
controller.metrics.service.port: 9443
70+
asserts:
71+
- equal:
72+
path: spec.ports[0].targetPort
73+
value: 9443
74+
5575
- it: should rename port when secure serving disabled
5676
set:
5777
controller.metrics.enabled: true

0 commit comments

Comments
 (0)