From 6d31ac88e0e5105df9dcae136cc51ddb6f0a0469 Mon Sep 17 00:00:00 2001
From: Jonathan Jamroga <jjamroga@gmail.com>
Date: Wed, 9 Sep 2026 16:18:14 -0400
Subject: [PATCH 1/7] feat(helm): global values for registry, pull config, and
 namespace scope

global.imageRegistry overrides every per-image registry at once -- the
air-gap mirror knob, resolved by a shared kagent.images.image helper so
repository and tag stay per-image and a mirror serves each image under
its existing path. global.imagePullSecrets merges (union) into each
pod's own list via kagent.imagePullSecrets. global.imagePullPolicy
fills the gap where neither a component nor the top-level
imagePullPolicy sets one, via kagent.imagePullPolicy; declared
IfNotPresent defaults moved into the template chains so the fallback is
reachable. global.watchNamespaces is the namespace-scope fallback:
rbac.namespaces overrides it when present -- including an explicit
empty list, which is why the declared [] default ships commented out --
and on the global path the install namespace and any
controller.watchNamespaces entries are folded into the resolved scope
rather than failing the render. For an explicit rbac.namespaces, a
watch outside the list fails the render and names both keys: a watched
namespace without a Role is a permanent Forbidden loop at runtime.
writer-rolebinding gains the sortAlpha the other three rbac templates
already had. grafana-mcp adopts the globals in the same change, and
gains the pull-secret surface its pod spec lacked entirely.

Verified: helm unittest passes (288 tests, including a global-values
suite pinning override, merge, fallback, fold-in, auto-append,
explicit-empty opt-out, and the watch-outside-scope failure); a default
render is byte-identical to main for images, pull secrets, and RBAC
kinds.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
---
 helm/kagent/templates/_helpers.tpl            |  94 ++++++++++-
 helm/kagent/templates/_images.tpl             |  32 ++++
 .../templates/controller-configmap.yaml       |   2 +-
 .../templates/controller-deployment.yaml      |   9 +-
 helm/kagent/templates/postgresql.yaml         |   4 +-
 helm/kagent/templates/rbac/getter-role.yaml   |   5 +-
 .../templates/rbac/getter-rolebinding.yaml    |   5 +-
 helm/kagent/templates/rbac/writer-role.yaml   |   6 +-
 .../templates/rbac/writer-rolebinding.yaml    |   5 +-
 helm/kagent/templates/ui-deployment.yaml      |   9 +-
 helm/kagent/tests/global-values_test.yaml     | 146 ++++++++++++++++++
 helm/kagent/values.yaml                       |  55 +++++--
 helm/tools/grafana-mcp/templates/_helpers.tpl |  25 ++-
 .../grafana-mcp/templates/deployment.yaml     |   3 +-
 helm/tools/grafana-mcp/values.yaml            |   3 +
 15 files changed, 363 insertions(+), 40 deletions(-)
 create mode 100644 helm/kagent/templates/_images.tpl
 create mode 100644 helm/kagent/tests/global-values_test.yaml

diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl
index 791614f36d..7992ea3779 100644
--- a/helm/kagent/templates/_helpers.tpl
+++ b/helm/kagent/templates/_helpers.tpl
@@ -59,9 +59,42 @@ Precedence: controller.watchNamespaces (explicit override) > rbac.namespaces > e
   {{- .Values.controller.watchNamespaces | uniq | join "," -}}
 {{- else if and .Values.rbac .Values.rbac.namespaces -}}
   {{- .Values.rbac.namespaces | uniq | join "," -}}
+{{- else if ((.Values.global).watchNamespaces) -}}
+  {{- (.Values.global).watchNamespaces | uniq | join "," -}}
 {{- end -}}
 {{- end -}}
 
+{{/*
+The resolved RBAC scope, as a JSON list so callers can range over it.
+Precedence: rbac.namespaces > global.watchNamespaces > empty (cluster-scoped).
+The global is a fallback, not an override: a values file that sets rbac.namespaces
+renders exactly what it rendered before the global existed.
+
+hasKey, not coalesce: an explicit `rbac.namespaces: []` means "cluster-scoped",
+and coalesce would skip it as empty -- silently namespacing an install that
+asked not to be. A present key always wins, even empty.
+
+controller.watchNamespaces joins the scope: the controller needs a Role in
+every namespace it watches, so a watch entry outside the RBAC list would be a
+permanent Forbidden loop. kagent.rbac.validate rejects that mix for an explicit
+rbac.namespaces; under the global the watch entries are folded in instead.
+
+The install namespace is appended only on the global path. The global is a
+shared signal an umbrella may aim at other charts entirely; failing this
+chart's render because that list omits its namespace would brick an install
+the value was never about. An explicit rbac.namespaces keeps the hard fail --
+there the operator is talking about this chart.
+*/}}
+{{- define "kagent.rbacNamespaces" -}}
+{{- $scope := list -}}
+{{- if and .Values.rbac (hasKey .Values.rbac "namespaces") -}}
+{{- $scope = .Values.rbac.namespaces | default list -}}
+{{- else if ((.Values.global).watchNamespaces) -}}
+{{- $scope = concat (.Values.global).watchNamespaces (.Values.controller.watchNamespaces | default list) (list (include "kagent.namespace" .)) -}}
+{{- end -}}
+{{- $scope | uniq | sortAlpha | toJson -}}
+{{- end -}}
+
 {{/*
 Guards on the rbac block
 */}}
@@ -69,11 +102,23 @@ Guards on the rbac block
 {{- if and .Values.rbac (hasKey .Values.rbac "clusterScoped") -}}
 {{- fail "rbac.clusterScoped has been removed. Leave rbac.namespaces empty for cluster-scoped RBAC, or set rbac.namespaces=[<ns>, ...] for namespaced RBAC." -}}
 {{- end -}}
+{{- $resolved := include "kagent.rbacNamespaces" . | fromJsonArray -}}
 {{- if and .Values.rbac .Values.rbac.namespaces -}}
 {{- $installNs := include "kagent.namespace" . -}}
 {{- if not (has $installNs .Values.rbac.namespaces) -}}
 {{- fail (printf "rbac.namespaces is set but does not include the install namespace %q" $installNs) -}}
 {{- end -}}
+{{/*
+A watch wider than the RBAC scope is never valid: the controller lists and
+watches namespaces its Roles do not cover, and every reconcile there returns
+Forbidden at runtime with only a log line to show for it. Narrower is fine --
+an operator may grant Roles broadly and watch a subset to keep the cache small.
+*/}}
+{{- range $ns := (.Values.controller.watchNamespaces | default list) -}}
+{{- if not (has $ns $.Values.rbac.namespaces) -}}
+{{- fail (printf "controller.watchNamespaces includes %q but rbac.namespaces does not. The controller would watch a namespace it has no Role in, and every list/watch there returns Forbidden at runtime. Add %q to rbac.namespaces, or remove it from controller.watchNamespaces. Prefer setting only global.watchNamespaces, which scopes RBAC and the watch together." $ns $ns) -}}
+{{- end -}}
+{{- end -}}
 {{- end -}}
 {{- end -}}
 
@@ -223,7 +268,8 @@ Bundled PostgreSQL image - constructs the full image reference from registry/rep
 */}}
 {{- define "kagent.postgresql.image" -}}
 {{- $pg := .Values.database.postgres.bundled -}}
-{{- $parts := compact (list $pg.image.registry $pg.image.repository $pg.image.name) -}}
+{{- $registry := default $pg.image.registry ((.Values.global).imageRegistry) -}}
+{{- $parts := compact (list $registry $pg.image.repository $pg.image.name) -}}
 {{- printf "%s:%s" (join "/" $parts) $pg.image.tag -}}
 {{- end -}}
 
@@ -254,11 +300,32 @@ Controller Service host:port for nginx upstream (no scheme).
 imagePullSecrets from global values (for subchart usage).
 Reads .Values.global.imagePullSecrets set by the parent chart.
 */}}
+{{/*
+imagePullSecrets for a pod spec: a component-local list (or the chart-level
+one) merged (union) with global.imagePullSecrets. One definition, called from
+every pod spec -- the merge written twice drifts, and the pod that misses a
+semantics change fails ImagePullBackOff only in the air-gap case the global
+exists for.
+
+Usage: {{ include "kagent.imagePullSecrets" (dict "root" $ "local" .Values.controller.imagePullSecrets) }}
+*/}}
+{{/*
+imagePullPolicy for a container: the component's own value, then the chart-level
+imagePullPolicy, then global.imagePullPolicy, then IfNotPresent. One definition so
+the fallback chain cannot drift between pods.
+
+Usage: {{ include "kagent.imagePullPolicy" (dict "root" $ "local" .Values.controller.image.pullPolicy) }}
+*/}}
+{{- define "kagent.imagePullPolicy" -}}
+{{- .local | default .root.Values.imagePullPolicy | default ((.root.Values.global).imagePullPolicy) | default "IfNotPresent" -}}
+{{- end -}}
+
 {{- define "kagent.imagePullSecrets" -}}
-{{- $global := ((.Values.global).imagePullSecrets) | default list -}}
-{{- if $global -}}
+{{- $local := .local | default .root.Values.imagePullSecrets | default list -}}
+{{- $merged := concat $local (((.root.Values.global).imagePullSecrets) | default list) | uniq -}}
+{{- if $merged -}}
 imagePullSecrets:
-{{- toYaml $global | nindent 2 }}
+{{- toYaml $merged | nindent 2 }}
 {{- end -}}
 {{- end -}}
 
@@ -319,3 +386,22 @@ forwarded to kagent's branded /login page.
 <body>Redirecting to login...</body>
 </html>
 {{- end -}}
+
+{{/*
+The controller container image. Builds the image root from controller.image and
+resolves it through kagent.images.image, so the deployment carries one short
+call. The top-level tag wins over the component tag, as it always has.
+*/}}
+{{- define "kagent.controllerImage" -}}
+{{- $root := dict "registry" (.Values.controller.image.registry | default .Values.registry) "repository" .Values.controller.image.repository "tag" (coalesce .Values.tag .Values.controller.image.tag .Chart.Version) -}}
+{{- include "kagent.images.image" (dict "imageRoot" $root "global" .Values.global) -}}
+{{- end -}}
+
+{{/*
+The ui container image. Same tag precedence as the controller: the top-level
+tag wins over the component tag.
+*/}}
+{{- define "kagent.uiImage" -}}
+{{- $root := dict "registry" (.Values.ui.image.registry | default .Values.registry) "repository" .Values.ui.image.repository "tag" (coalesce .Values.tag .Values.ui.image.tag .Chart.Version) -}}
+{{- include "kagent.images.image" (dict "imageRoot" $root "global" .Values.global) -}}
+{{- end -}}
diff --git a/helm/kagent/templates/_images.tpl b/helm/kagent/templates/_images.tpl
new file mode 100644
index 0000000000..1c47d4cd8f
--- /dev/null
+++ b/helm/kagent/templates/_images.tpl
@@ -0,0 +1,32 @@
+{{/*
+Copyright Broadcom, Inc. All Rights Reserved.
+SPDX-License-Identifier: APACHE-2.0
+*/}}
+
+{{/* vim: set filetype=mustache: */}}
+{{/*
+Return the proper image name.
+If image tag and digest are not defined, termination fallbacks to chart appVersion.
+{{ include "kagent.images.image" ( dict "imageRoot" .Values.path.to.the.image "global" .Values.global "chart" .Chart ) }}
+*/}}
+{{- define "kagent.images.image" -}}
+{{- $registryName := default .imageRoot.registry ((.global).imageRegistry) -}}
+{{- $repositoryName := .imageRoot.repository -}}
+{{- $separator := ":" -}}
+{{- $termination := .imageRoot.tag | toString -}}
+
+{{- if not .imageRoot.tag }}
+  {{- if .chart }}
+    {{- $termination = .chart.AppVersion | toString -}}
+  {{- end -}}
+{{- end -}}
+{{- if .imageRoot.digest }}
+    {{- $separator = "@" -}}
+    {{- $termination = .imageRoot.digest | toString -}}
+{{- end -}}
+{{- if $registryName }}
+    {{- printf "%s/%s%s%s" $registryName $repositoryName $separator $termination -}}
+{{- else -}}
+    {{- printf "%s%s%s"  $repositoryName $separator $termination -}}
+{{- end -}}
+{{- end -}}
diff --git a/helm/kagent/templates/controller-configmap.yaml b/helm/kagent/templates/controller-configmap.yaml
index 84caeaa5a3..7b6eaa6f7d 100644
--- a/helm/kagent/templates/controller-configmap.yaml
+++ b/helm/kagent/templates/controller-configmap.yaml
@@ -12,7 +12,7 @@ data:
   {{- if .Values.ui.externalUrl }}
   KAGENT_UI_URL: {{ .Values.ui.externalUrl | quote }}
   {{- end }}
-  IMAGE_REGISTRY: {{ .Values.controller.agentImage.registry | default .Values.registry | quote }}
+  IMAGE_REGISTRY: {{ ((.Values.global).imageRegistry) | default (.Values.controller.agentImage.registry | default .Values.registry) | quote }}
   IMAGE_REPOSITORY: {{ .Values.controller.agentImage.repository | quote }}
   IMAGE_TAG: {{ coalesce .Values.controller.agentImage.tag .Values.tag .Chart.Version | quote }}
   LEADER_ELECT: {{ include "kagent.leaderElectionEnabled" . | quote }}
diff --git a/helm/kagent/templates/controller-deployment.yaml b/helm/kagent/templates/controller-deployment.yaml
index 8859e4725f..e4bc49909c 100644
--- a/helm/kagent/templates/controller-deployment.yaml
+++ b/helm/kagent/templates/controller-deployment.yaml
@@ -29,10 +29,7 @@ spec:
         {{- $podLabels := mergeOverwrite (dict) (.Values.podLabels | default dict) (.Values.controller.podLabels | default dict) (include "kagent.controller.selectorLabels" . | fromYaml) }}
         {{- toYaml $podLabels | nindent 8 }}
     spec:
-      {{- with .Values.controller.imagePullSecrets | default .Values.imagePullSecrets }}
-      imagePullSecrets:
-        {{- toYaml . | nindent 8 }}
-      {{- end }}
+      {{- with include "kagent.imagePullSecrets" (dict "root" . "local" .Values.controller.imagePullSecrets) }}{{- . | nindent 6 }}{{- end }}
       {{- with (.Values.controller.podSecurityContext | default .Values.podSecurityContext) }}
       securityContext:
         {{- toYaml . | nindent 8 }}
@@ -80,8 +77,8 @@ spec:
       {{- end }}
       containers:
         - name: controller
-          image: "{{ .Values.controller.image.registry | default .Values.registry  }}/{{ .Values.controller.image.repository }}:{{ coalesce .Values.tag .Values.controller.image.tag .Chart.Version }}"
-          imagePullPolicy: {{ .Values.controller.image.pullPolicy | default .Values.imagePullPolicy }}
+          image: {{ include "kagent.controllerImage" . | quote }}
+          imagePullPolicy: {{ include "kagent.imagePullPolicy" (dict "root" . "local" .Values.controller.image.pullPolicy) }}
           env:
             - name: KAGENT_NAMESPACE
               valueFrom:
diff --git a/helm/kagent/templates/postgresql.yaml b/helm/kagent/templates/postgresql.yaml
index 9516725372..c930fbb954 100644
--- a/helm/kagent/templates/postgresql.yaml
+++ b/helm/kagent/templates/postgresql.yaml
@@ -51,7 +51,7 @@ spec:
         {{- include "kagent.selectorLabels" . | nindent 8 }}
         app.kubernetes.io/component: database
     spec:
-      {{- include "kagent.imagePullSecrets" $ | nindent 6 }}
+      {{- with include "kagent.imagePullSecrets" (dict "root" $) }}{{- . | nindent 6 }}{{- end }}
       serviceAccountName: {{ $fullname }}
       {{- with $pg.podSecurityContext }}
       securityContext:
@@ -72,7 +72,7 @@ spec:
       containers:
         - name: postgresql
           image: {{ include "kagent.postgresql.image" . }}
-          imagePullPolicy: {{ $pg.image.pullPolicy }}
+          imagePullPolicy: {{ include "kagent.imagePullPolicy" (dict "root" $ "local" $pg.image.pullPolicy) }}
           {{- with $pg.securityContext }}
           securityContext:
             {{- toYaml . | nindent 12 }}
diff --git a/helm/kagent/templates/rbac/getter-role.yaml b/helm/kagent/templates/rbac/getter-role.yaml
index dbf39146d1..32c3ad6f24 100644
--- a/helm/kagent/templates/rbac/getter-role.yaml
+++ b/helm/kagent/templates/rbac/getter-role.yaml
@@ -98,8 +98,9 @@
 {{- end -}}
 
 {{- include "kagent.rbac.validate" . -}}
-{{- if .Values.rbac.namespaces }}
-{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }}
+{{- $rbacNamespaces := include "kagent.rbacNamespaces" . | fromJsonArray }}
+{{- if $rbacNamespaces }}
+{{- range $namespace := $rbacNamespaces }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
diff --git a/helm/kagent/templates/rbac/getter-rolebinding.yaml b/helm/kagent/templates/rbac/getter-rolebinding.yaml
index 9d143f72d0..e8e463d144 100644
--- a/helm/kagent/templates/rbac/getter-rolebinding.yaml
+++ b/helm/kagent/templates/rbac/getter-rolebinding.yaml
@@ -1,6 +1,7 @@
 {{- include "kagent.rbac.validate" . -}}
-{{- if .Values.rbac.namespaces }}
-{{- range $namespace := .Values.rbac.namespaces | uniq | sortAlpha }}
+{{- $rbacNamespaces := include "kagent.rbacNamespaces" . | fromJsonArray }}
+{{- if $rbacNamespaces }}
+{{- range $namespace := $rbacNamespaces }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
diff --git a/helm/kagent/templates/rbac/writer-role.yaml b/helm/kagent/templates/rbac/writer-role.yaml
index 5866de2725..6f5b5e3e66 100644
--- a/helm/kagent/templates/rbac/writer-role.yaml
+++ b/helm/kagent/templates/rbac/writer-role.yaml
@@ -74,9 +74,9 @@
 {{- end -}}
 
 {{- include "kagent.rbac.validate" . -}}
-{{- if .Values.rbac.namespaces }}
-{{- $namespaces := .Values.rbac.namespaces | uniq | sortAlpha }}
-{{- range $namespace := $namespaces }}
+{{- $rbacNamespaces := include "kagent.rbacNamespaces" . | fromJsonArray }}
+{{- if $rbacNamespaces }}
+{{- range $namespace := $rbacNamespaces }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
diff --git a/helm/kagent/templates/rbac/writer-rolebinding.yaml b/helm/kagent/templates/rbac/writer-rolebinding.yaml
index d2120b35ee..3045e80869 100644
--- a/helm/kagent/templates/rbac/writer-rolebinding.yaml
+++ b/helm/kagent/templates/rbac/writer-rolebinding.yaml
@@ -1,6 +1,7 @@
 {{- include "kagent.rbac.validate" . -}}
-{{- if .Values.rbac.namespaces }}
-{{- range $namespace := (.Values.rbac.namespaces | uniq) }}
+{{- $rbacNamespaces := include "kagent.rbacNamespaces" . | fromJsonArray }}
+{{- if $rbacNamespaces }}
+{{- range $namespace := $rbacNamespaces }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
diff --git a/helm/kagent/templates/ui-deployment.yaml b/helm/kagent/templates/ui-deployment.yaml
index ef04ecd348..bc5f609e57 100644
--- a/helm/kagent/templates/ui-deployment.yaml
+++ b/helm/kagent/templates/ui-deployment.yaml
@@ -28,10 +28,7 @@ spec:
         {{- $podLabels := mergeOverwrite (dict) (.Values.podLabels | default dict) (.Values.ui.podLabels | default dict) (include "kagent.ui.selectorLabels" . | fromYaml) }}
         {{- toYaml $podLabels | nindent 8 }}
     spec:
-      {{- with .Values.imagePullSecrets }}
-      imagePullSecrets:
-        {{- toYaml . | nindent 8 }}
-      {{- end }}
+      {{- with include "kagent.imagePullSecrets" (dict "root" .) }}{{- . | nindent 6 }}{{- end }}
       {{- with (.Values.ui.podSecurityContext | default .Values.podSecurityContext) }}
       securityContext:
         {{- toYaml . | nindent 8 }}
@@ -66,8 +63,8 @@ spec:
           securityContext:
             {{- toYaml . | nindent 12 }}
           {{- end }}
-          image: "{{ .Values.ui.image.registry | default .Values.registry  }}/{{ .Values.ui.image.repository }}:{{ coalesce .Values.tag .Values.ui.image.tag .Chart.Version }}"
-          imagePullPolicy: {{ .Values.ui.image.pullPolicy | default .Values.imagePullPolicy }}
+          image: {{ include "kagent.uiImage" . | quote }}
+          imagePullPolicy: {{ include "kagent.imagePullPolicy" (dict "root" . "local" .Values.ui.image.pullPolicy) }}
           {{- /* The UI is a static bundle; these are read by init.sh, which
                  renders them into the config.json the browser fetches at
                  startup. Nothing here is consumed by a server process. */}}
diff --git a/helm/kagent/tests/global-values_test.yaml b/helm/kagent/tests/global-values_test.yaml
new file mode 100644
index 0000000000..b7dfee928d
--- /dev/null
+++ b/helm/kagent/tests/global-values_test.yaml
@@ -0,0 +1,146 @@
+suite: test global values
+templates:
+  - controller-deployment.yaml
+  - postgresql-secret.yaml
+  - controller-configmap.yaml
+  - rbac/getter-role.yaml
+tests:
+  - it: should override every image registry from global.imageRegistry
+    set:
+      global.imageRegistry: mirror.example
+    template: controller-deployment.yaml
+    asserts:
+      - matchRegex:
+          path: spec.template.spec.containers[0].image
+          pattern: "^mirror\\.example/kagent-dev/kagent/controller:"
+  - it: should point the agent runtime at global.imageRegistry
+    set:
+      global.imageRegistry: mirror.example
+    template: controller-configmap.yaml
+    asserts:
+      - equal:
+          path: data.IMAGE_REGISTRY
+          value: mirror.example
+  - it: should keep per-image registry when the global is unset
+    template: controller-deployment.yaml
+    asserts:
+      - matchRegex:
+          path: spec.template.spec.containers[0].image
+          pattern: "^ghcr\\.io/kagent-dev/kagent/controller:"
+  - it: should merge global.imagePullSecrets with the local list
+    set:
+      imagePullSecrets:
+        - name: local
+      global.imagePullSecrets:
+        - name: shared
+    template: controller-deployment.yaml
+    asserts:
+      - contains:
+          path: spec.template.spec.imagePullSecrets
+          content:
+            name: local
+      - contains:
+          path: spec.template.spec.imagePullSecrets
+          content:
+            name: shared
+  - it: should render namespaced RBAC from global.watchNamespaces alone
+    release:
+      namespace: NAMESPACE
+    set:
+      global.watchNamespaces:
+        - NAMESPACE
+        - other
+    template: rbac/getter-role.yaml
+    asserts:
+      - containsDocument:
+          apiVersion: rbac.authorization.k8s.io/v1
+          kind: Role
+          namespace: other
+          any: true
+  - it: should let rbac.namespaces win over global.watchNamespaces
+    release:
+      namespace: NAMESPACE
+    set:
+      rbac.namespaces:
+        - NAMESPACE
+      global.watchNamespaces:
+        - NAMESPACE
+        - other
+    template: rbac/getter-role.yaml
+    asserts:
+      - containsDocument:
+          apiVersion: rbac.authorization.k8s.io/v1
+          kind: Role
+          namespace: NAMESPACE
+          any: true
+      - not: true
+        containsDocument:
+          apiVersion: rbac.authorization.k8s.io/v1
+          kind: Role
+          namespace: other
+          any: true
+
+  - it: should let an explicit empty rbac.namespaces restore cluster-scoped RBAC under the global
+    release:
+      namespace: NAMESPACE
+    set:
+      rbac.namespaces: []
+      global.watchNamespaces:
+        - NAMESPACE
+        - other
+    template: rbac/getter-role.yaml
+    asserts:
+      - containsDocument:
+          apiVersion: rbac.authorization.k8s.io/v1
+          kind: ClusterRole
+          any: true
+
+  - it: should auto-append the install namespace when scope comes only from the global
+    release:
+      namespace: NAMESPACE
+    set:
+      global.watchNamespaces:
+        - other
+    template: rbac/getter-role.yaml
+    asserts:
+      - containsDocument:
+          apiVersion: rbac.authorization.k8s.io/v1
+          kind: Role
+          namespace: NAMESPACE
+          any: true
+      - containsDocument:
+          apiVersion: rbac.authorization.k8s.io/v1
+          kind: Role
+          namespace: other
+          any: true
+
+  - it: should fold controller.watchNamespaces into the RBAC scope under the global
+    release:
+      namespace: NAMESPACE
+    set:
+      global.watchNamespaces:
+        - NAMESPACE
+      controller.watchNamespaces:
+        - NAMESPACE
+        - extra
+    template: rbac/getter-role.yaml
+    asserts:
+      - containsDocument:
+          apiVersion: rbac.authorization.k8s.io/v1
+          kind: Role
+          namespace: extra
+          any: true
+
+  - it: should fail when the watch exceeds an explicit rbac.namespaces
+    release:
+      namespace: NAMESPACE
+    set:
+      rbac.namespaces:
+        - NAMESPACE
+      controller.watchNamespaces:
+        - NAMESPACE
+        - outside
+    template: rbac/getter-role.yaml
+    asserts:
+      - failedTemplate:
+          errorPattern: 'controller\.watchNamespaces includes "outside" but rbac\.namespaces does not'
diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml
index 261f402297..3b5002938e 100644
--- a/helm/kagent/values.yaml
+++ b/helm/kagent/values.yaml
@@ -14,11 +14,40 @@
 ipv6:
   enabled: false
 
+# Values under `global` are visible to this chart and to every subchart. A parent
+# chart or an operator sets one value here instead of one value per chart.
+global:
+  # -- Registry that overrides every per-image registry when set. This is the
+  # air-gap mirror knob: one value redirects all images. Repository and tag stay
+  # per-image. For per-image control, leave this unset and set the per-image
+  # registry keys. Covers this chart's own images: controller, ui, bundled
+  # postgres, grafana-mcp, and the agent runtime. The vendored subcharts
+  # (kagent-tools, kmcp, substrate) adopt it when their pinned versions bump.
+  imageRegistry: ""
+  # -- Pull secrets for every pod. The chart merges this list (union) into each
+  # pod's own imagePullSecrets, so a local secret is never removed. Coverage
+  # matches imageRegistry above.
+  imagePullSecrets: []
+  # -- Fallback imagePullPolicy where neither a component nor the top-level
+  # imagePullPolicy sets one.
+  imagePullPolicy: ""
+  # -- Namespace scope for the whole install. A non-empty list replaces
+  # ClusterRoles with Roles and scopes the controller's watch. Both derive from
+  # this one value, so they cannot disagree. Prefer this value over the
+  # per-surface keys. rbac.namespaces overrides it when the key is present. An
+  # explicit empty rbac.namespaces restores cluster-scoped RBAC. Mixing in
+  # controller.watchNamespaces is validated: the watch must stay inside the RBAC
+  # scope. A watched namespace without a Role is a permanent Forbidden loop at
+  # runtime.
+  watchNamespaces: []
+
 tag: ""
 registry: "ghcr.io"
 
 imagePullSecrets: []
-imagePullPolicy: IfNotPresent
+# -- Pull policy for all containers. Empty falls back to global.imagePullPolicy,
+# then IfNotPresent.
+imagePullPolicy: ""
 
 nameOverride: ""
 fullnameOverride: ""
@@ -109,8 +138,10 @@ database:
         name: postgres
         # -- Bundled PostgreSQL image tag
         tag: "18.6-alpine3.23"
-        # -- Bundled PostgreSQL image pull policy
-        pullPolicy: IfNotPresent
+        # -- Bundled PostgreSQL image pull policy. Empty falls back to the
+        # top-level imagePullPolicy, then global.imagePullPolicy, then
+        # IfNotPresent. A declared default here would shadow both fallbacks.
+        pullPolicy: ""
       # -- PersistentVolumeClaim size for demo PostgreSQL data
       storage: 500Mi
       # -- StorageClass for the PostgreSQL PVC. Defaults to the cluster default when empty.
@@ -155,12 +186,18 @@ database:
 
 rbac:
   # -- Namespaces in which to create Role and RoleBinding resources.
-  # If empty (default), the chart creates cluster-scoped ClusterRole and ClusterRoleBinding
-  # resources and the controller watches all namespaces.
-  # If set, the chart creates a Role + RoleBinding per listed namespace and the controller's
-  # WATCH_NAMESPACES is derived from this list (unless controller.watchNamespaces is set
-  # explicitly, which always takes precedence).
-  namespaces: []
+  # Left unset (the default), the scope falls back to global.watchNamespaces.
+  # If that is also unset, the chart creates cluster-scoped ClusterRole and
+  # ClusterRoleBinding resources, and the controller watches all namespaces.
+  # If set, the chart creates a Role and a RoleBinding per listed namespace.
+  # The controller's WATCH_NAMESPACES derives from the same list, unless
+  # controller.watchNamespaces overrides it.
+  # An explicit empty list forces cluster-scoped RBAC, even under a parent that
+  # sets global.watchNamespaces. This is why the key ships commented out: a
+  # declared default [] would make every install look explicitly cluster-scoped,
+  # and the global fallback would be unreachable.
+  # namespaces: []
+  {}
 
 # ==============================================================================
 # CONTROLLER CONFIGURATION
diff --git a/helm/tools/grafana-mcp/templates/_helpers.tpl b/helm/tools/grafana-mcp/templates/_helpers.tpl
index 1f2d8dcbb8..f53f7c0e34 100644
--- a/helm/tools/grafana-mcp/templates/_helpers.tpl
+++ b/helm/tools/grafana-mcp/templates/_helpers.tpl
@@ -106,6 +106,27 @@ Join registry/repository/name/tag for grafana-mcp image, skipping empty segments
 */}}
 {{- define "grafana-mcp.image" -}}
 {{- $img := .Values.image -}}
-{{- $parts := compact (list $img.registry $img.repository $img.name) -}}
+{{/* image.registry holds the docker.io org here ("mcp"), not a host, so the
+     air-gap override is prepended rather than substituted: the mirror serves
+     the image under its existing mcp/grafana path. */}}
+{{- $parts := compact (list ((.Values.global).imageRegistry) $img.registry $img.repository $img.name) -}}
 {{- printf "%s:%s" (join "/" $parts) $img.tag -}}
-{{- end -}}
\ No newline at end of file
+{{- end -}}
+{{/*
+Pull secrets for the pod: the chart's own list merged (union) with
+global.imagePullSecrets. Renders nothing when both are empty.
+*/}}
+{{- define "grafana-mcp.imagePullSecrets" -}}
+{{- $merged := concat (.Values.imagePullSecrets | default list) (((.Values.global).imagePullSecrets) | default list) | uniq -}}
+{{- if $merged -}}
+imagePullSecrets:
+{{- toYaml $merged | nindent 2 }}
+{{- end -}}
+{{- end -}}
+
+{{/*
+imagePullPolicy: image.pullPolicy, then global.imagePullPolicy, then IfNotPresent.
+*/}}
+{{- define "grafana-mcp.imagePullPolicy" -}}
+{{- .Values.image.pullPolicy | default ((.Values.global).imagePullPolicy) | default "IfNotPresent" -}}
+{{- end -}}
diff --git a/helm/tools/grafana-mcp/templates/deployment.yaml b/helm/tools/grafana-mcp/templates/deployment.yaml
index 299610920a..922e0c1b24 100644
--- a/helm/tools/grafana-mcp/templates/deployment.yaml
+++ b/helm/tools/grafana-mcp/templates/deployment.yaml
@@ -19,6 +19,7 @@ spec:
         {{- include "grafana-mcp.selectorLabels" . | nindent 8 }}
     spec:
       serviceAccountName: {{ include "grafana-mcp.serviceAccountName" . }}
+      {{- with include "grafana-mcp.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }}
       {{- with .Values.podSecurityContext }}
       securityContext:
         {{- toYaml . | nindent 8 }}
@@ -36,7 +37,7 @@ spec:
           securityContext:
             {{- toYaml .Values.securityContext | nindent 12 }}
           image: {{ include "grafana-mcp.image" . }}
-          imagePullPolicy: {{ .Values.image.pullPolicy | default "IfNotPresent" }}
+          imagePullPolicy: {{ include "grafana-mcp.imagePullPolicy" . }}
           args:
             - --transport
             - streamable-http
diff --git a/helm/tools/grafana-mcp/values.yaml b/helm/tools/grafana-mcp/values.yaml
index 65c01526ec..e5160ff8ee 100644
--- a/helm/tools/grafana-mcp/values.yaml
+++ b/helm/tools/grafana-mcp/values.yaml
@@ -12,6 +12,9 @@ image:
   pullPolicy: Always
   tag: "latest" # Only latest is available via docker hub at present. See https://github.com/grafana/mcp-grafana/issues/180
 
+# -- Pull secrets for the pod, merged (union) with global.imagePullSecrets.
+imagePullSecrets: []
+
 nameOverride: ""
 fullnameOverride: ""
 

From 1f8d37ea7f15b6631a1d962ae42c48af5c6ba1fd Mon Sep 17 00:00:00 2001
From: Jonathan Jamroga <jjamroga@gmail.com>
Date: Fri, 18 Sep 2026 08:43:33 -0400
Subject: [PATCH 2/7] fix(helm): derive the watch scope from the resolved RBAC
 scope

WATCH_NAMESPACES read only rbac.namespaces and controller.watchNamespaces,
so a scope set through global.watchNamespaces produced namespaced Roles while
the controller still watched every namespace -- a cluster-wide cache backed by
Roles that do not cover it. The watch now derives from the same resolved scope
the RBAC templates use, so the two cannot disagree: global-only scope watches
the listed namespaces plus the install namespace, and an explicit
rbac.namespaces: [] restores cluster-wide watch as well as cluster-scoped RBAC.

controller.watchNamespaces stays an explicit override, unchanged.

Verified with helm unittest (290 passing, two new pins for the global-only and
explicit-empty cases) and by rendering all four scope combinations.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
---
 helm/kagent/templates/_helpers.tpl            | 13 +++++-----
 .../tests/controller-deployment_test.yaml     | 24 +++++++++++++++++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl
index 7992ea3779..bc9f1ae3a0 100644
--- a/helm/kagent/templates/_helpers.tpl
+++ b/helm/kagent/templates/_helpers.tpl
@@ -51,16 +51,17 @@ Allows overriding it for multi-namespace deployments in combined charts.
 {{- end }}
 
 {{/*
-Watch namespaces - transforms list of namespaces cached by the controller into comma-separated string.
-Precedence: controller.watchNamespaces (explicit override) > rbac.namespaces > empty (watch all).
+Watch namespaces - transforms the list of namespaces cached by the controller into a comma-separated string.
+controller.watchNamespaces is an explicit override; otherwise the watch scope is the resolved RBAC scope
+(kagent.rbacNamespaces), so the controller never watches a namespace its Roles do not cover and never
+holds a cluster-wide cache when RBAC is namespaced. An explicit rbac.namespaces: [] therefore also
+clears the watch scope back to cluster-wide.
 */}}
 {{- define "kagent.watchNamespaces" -}}
 {{- if .Values.controller.watchNamespaces -}}
   {{- .Values.controller.watchNamespaces | uniq | join "," -}}
-{{- else if and .Values.rbac .Values.rbac.namespaces -}}
-  {{- .Values.rbac.namespaces | uniq | join "," -}}
-{{- else if ((.Values.global).watchNamespaces) -}}
-  {{- (.Values.global).watchNamespaces | uniq | join "," -}}
+{{- else -}}
+  {{- include "kagent.rbacNamespaces" . | fromJsonArray | join "," -}}
 {{- end -}}
 {{- end -}}
 
diff --git a/helm/kagent/tests/controller-deployment_test.yaml b/helm/kagent/tests/controller-deployment_test.yaml
index be2890f8be..490c30792f 100644
--- a/helm/kagent/tests/controller-deployment_test.yaml
+++ b/helm/kagent/tests/controller-deployment_test.yaml
@@ -262,6 +262,30 @@ tests:
           path: data.WATCH_NAMESPACES
           value: "explicit-ns"
 
+  - it: should derive watch namespaces from global.watchNamespaces, install namespace included
+    template: controller-configmap.yaml
+    set:
+      global:
+        watchNamespaces:
+          - team-a
+    asserts:
+      - equal:
+          path: data.WATCH_NAMESPACES
+          value: "NAMESPACE,team-a"
+
+  - it: should watch all namespaces when an explicit empty rbac.namespaces overrides the global
+    template: controller-configmap.yaml
+    set:
+      rbac:
+        namespaces: []
+      global:
+        watchNamespaces:
+          - team-a
+    asserts:
+      - equal:
+          path: data.WATCH_NAMESPACES
+          value: ""
+
   - it: should set podAnnotations
     template: controller-deployment.yaml
     set:

From 901e4c0208adefbe715b8e56455b75f0faf7f14c Mon Sep 17 00:00:00 2001
From: Jonathan Jamroga <jjamroga@gmail.com>
Date: Fri, 18 Sep 2026 08:46:07 -0400
Subject: [PATCH 3/7] fix(helm): mirror the worker pool image, not the
 agent-runtime env nothing reads

The global registry override was wired into the configmap's IMAGE_REGISTRY, but
no code reads that key from the environment -- agent workload images are copied
verbatim from each harness spec, so the override changed a dead value and the
values comment claimed coverage the chart did not have. The configmap line goes
back to its previous expression.

The real single-string image boundary is substrateWorkerPool.workerImage, which
was emitted verbatim and escaped the mirror. It now routes through
kagent.mirroredImage, which applies the container runtime's first-segment rule:
a host-carrying reference has its registry replaced, a bare name is prefixed,
and an unset global passes the reference through untouched.

Verified with helm unittest (291 passing; the agent-runtime pin is repointed at
the WorkerPool plus a pass-through pin) and by rendering a bare-name
workerImage under the global.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
---
 helm/kagent/templates/_helpers.tpl            | 26 +++++++++++++++++++
 .../templates/controller-configmap.yaml       |  2 +-
 .../templates/substrate-workerpool.yaml       |  2 +-
 helm/kagent/tests/global-values_test.yaml     | 22 +++++++++++++---
 helm/kagent/values.yaml                       |  5 ++--
 5 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl
index bc9f1ae3a0..5643c6b170 100644
--- a/helm/kagent/templates/_helpers.tpl
+++ b/helm/kagent/templates/_helpers.tpl
@@ -398,6 +398,32 @@ call. The top-level tag wins over the component tag, as it always has.
 {{- include "kagent.images.image" (dict "imageRoot" $root "global" .Values.global) -}}
 {{- end -}}
 
+{{/*
+Rewrite a full image reference onto global.imageRegistry, for values that carry
+a whole reference in one string rather than registry/repository/tag keys.
+Follows the container runtime's rule for deciding whether the first path
+segment is a registry: it is one only when it contains a dot or a colon or is
+exactly "localhost". A host-carrying reference has that segment replaced so the
+mirror sees a stable path; a bare Docker Hub-style name is prefixed instead.
+When global.imageRegistry is unset the reference passes through unchanged.
+Call with (dict "root" $ "image" <reference>).
+*/}}
+{{- define "kagent.mirroredImage" -}}
+{{- $ref := .image -}}
+{{- $mirror := ((.root.Values.global).imageRegistry) -}}
+{{- if and $mirror $ref -}}
+  {{- $parts := splitList "/" $ref -}}
+  {{- $first := first $parts -}}
+  {{- if and (gt (len $parts) 1) (or (contains "." $first) (contains ":" $first) (eq $first "localhost")) -}}
+    {{- printf "%s/%s" $mirror (join "/" (rest $parts)) -}}
+  {{- else -}}
+    {{- printf "%s/%s" $mirror $ref -}}
+  {{- end -}}
+{{- else -}}
+  {{- $ref -}}
+{{- end -}}
+{{- end -}}
+
 {{/*
 The ui container image. Same tag precedence as the controller: the top-level
 tag wins over the component tag.
diff --git a/helm/kagent/templates/controller-configmap.yaml b/helm/kagent/templates/controller-configmap.yaml
index 7b6eaa6f7d..84caeaa5a3 100644
--- a/helm/kagent/templates/controller-configmap.yaml
+++ b/helm/kagent/templates/controller-configmap.yaml
@@ -12,7 +12,7 @@ data:
   {{- if .Values.ui.externalUrl }}
   KAGENT_UI_URL: {{ .Values.ui.externalUrl | quote }}
   {{- end }}
-  IMAGE_REGISTRY: {{ ((.Values.global).imageRegistry) | default (.Values.controller.agentImage.registry | default .Values.registry) | quote }}
+  IMAGE_REGISTRY: {{ .Values.controller.agentImage.registry | default .Values.registry | quote }}
   IMAGE_REPOSITORY: {{ .Values.controller.agentImage.repository | quote }}
   IMAGE_TAG: {{ coalesce .Values.controller.agentImage.tag .Values.tag .Chart.Version | quote }}
   LEADER_ELECT: {{ include "kagent.leaderElectionEnabled" . | quote }}
diff --git a/helm/kagent/templates/substrate-workerpool.yaml b/helm/kagent/templates/substrate-workerpool.yaml
index ac46a7a252..fc217e46d1 100644
--- a/helm/kagent/templates/substrate-workerpool.yaml
+++ b/helm/kagent/templates/substrate-workerpool.yaml
@@ -15,7 +15,7 @@ metadata:
     {{- end }}
 spec:
   replicas: {{ .Values.substrateWorkerPool.replicas }}
-  workerImage: {{ .Values.substrateWorkerPool.workerImage | quote }}
+  workerImage: {{ include "kagent.mirroredImage" (dict "root" . "image" .Values.substrateWorkerPool.workerImage) | quote }}
   sandboxClass: {{ .Values.substrateWorkerPool.sandboxClass | default "gvisor" | quote }}
   {{- with .Values.substrateWorkerPool.template }}
   template:
diff --git a/helm/kagent/tests/global-values_test.yaml b/helm/kagent/tests/global-values_test.yaml
index b7dfee928d..c6df142dd1 100644
--- a/helm/kagent/tests/global-values_test.yaml
+++ b/helm/kagent/tests/global-values_test.yaml
@@ -4,6 +4,7 @@ templates:
   - postgresql-secret.yaml
   - controller-configmap.yaml
   - rbac/getter-role.yaml
+  - substrate-workerpool.yaml
 tests:
   - it: should override every image registry from global.imageRegistry
     set:
@@ -13,14 +14,27 @@ tests:
       - matchRegex:
           path: spec.template.spec.containers[0].image
           pattern: "^mirror\\.example/kagent-dev/kagent/controller:"
-  - it: should point the agent runtime at global.imageRegistry
+  - it: should rewrite the worker pool image onto global.imageRegistry
     set:
       global.imageRegistry: mirror.example
-    template: controller-configmap.yaml
+      controller.substrate.enabled: true
+      substrateWorkerPool.create: true
+      substrateWorkerPool.workerImage: ghcr.io/kagent-dev/substrate/ateom:v1
+    template: substrate-workerpool.yaml
     asserts:
       - equal:
-          path: data.IMAGE_REGISTRY
-          value: mirror.example
+          path: spec.workerImage
+          value: mirror.example/kagent-dev/substrate/ateom:v1
+  - it: should leave the worker pool image alone when the global is unset
+    set:
+      controller.substrate.enabled: true
+      substrateWorkerPool.create: true
+      substrateWorkerPool.workerImage: ghcr.io/kagent-dev/substrate/ateom:v1
+    template: substrate-workerpool.yaml
+    asserts:
+      - equal:
+          path: spec.workerImage
+          value: ghcr.io/kagent-dev/substrate/ateom:v1
   - it: should keep per-image registry when the global is unset
     template: controller-deployment.yaml
     asserts:
diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml
index 3b5002938e..7245195ce5 100644
--- a/helm/kagent/values.yaml
+++ b/helm/kagent/values.yaml
@@ -21,8 +21,9 @@ global:
   # air-gap mirror knob: one value redirects all images. Repository and tag stay
   # per-image. For per-image control, leave this unset and set the per-image
   # registry keys. Covers this chart's own images: controller, ui, bundled
-  # postgres, grafana-mcp, and the agent runtime. The vendored subcharts
-  # (kagent-tools, kmcp, substrate) adopt it when their pinned versions bump.
+  # postgres, grafana-mcp, and substrateWorkerPool.workerImage. The vendored
+  # subcharts (kagent-tools, kmcp, substrate) adopt it when their pinned
+  # versions bump.
   imageRegistry: ""
   # -- Pull secrets for every pod. The chart merges this list (union) into each
   # pod's own imagePullSecrets, so a local secret is never removed. Coverage

From 1852efd614b0b3fbded98d72c8bfa45ac6b22ae5 Mon Sep 17 00:00:00 2001
From: Jonathan Jamroga <jjamroga@gmail.com>
Date: Fri, 18 Sep 2026 08:48:03 -0400
Subject: [PATCH 4/7] fix(helm): let global.imagePullPolicy reach grafana-mcp

The chart declared pullPolicy: Always in its values, so the key was always
present and the global fallback could never fire -- the one chart in the family
whose global pull-policy knob did nothing. The declared value moves into the
helper as the terminal default and the values default becomes empty, the same
shape every other chart here uses.

The terminal default stays Always rather than IfNotPresent because the default
tag is mutable (latest is the only published tag): with a cached image,
IfNotPresent never picks up a new push of the same tag.

Verified with helm unittest: three new pins cover the Always default, the
global fallback, and a local override winning over the global.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
---
 helm/tools/grafana-mcp/templates/_helpers.tpl |  7 +++--
 .../grafana-mcp/tests/deployment_test.yaml    | 28 +++++++++++++++++++
 helm/tools/grafana-mcp/values.yaml            |  6 +++-
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/helm/tools/grafana-mcp/templates/_helpers.tpl b/helm/tools/grafana-mcp/templates/_helpers.tpl
index f53f7c0e34..a40bab4680 100644
--- a/helm/tools/grafana-mcp/templates/_helpers.tpl
+++ b/helm/tools/grafana-mcp/templates/_helpers.tpl
@@ -125,8 +125,11 @@ imagePullSecrets:
 {{- end -}}
 
 {{/*
-imagePullPolicy: image.pullPolicy, then global.imagePullPolicy, then IfNotPresent.
+imagePullPolicy: image.pullPolicy, then global.imagePullPolicy, then Always.
+The terminal default is Always rather than IfNotPresent because the default tag
+is mutable ("latest"): with a cached image, IfNotPresent never picks up a new
+push of the same tag.
 */}}
 {{- define "grafana-mcp.imagePullPolicy" -}}
-{{- .Values.image.pullPolicy | default ((.Values.global).imagePullPolicy) | default "IfNotPresent" -}}
+{{- .Values.image.pullPolicy | default ((.Values.global).imagePullPolicy) | default "Always" -}}
 {{- end -}}
diff --git a/helm/tools/grafana-mcp/tests/deployment_test.yaml b/helm/tools/grafana-mcp/tests/deployment_test.yaml
index 0f2ae3aa99..9378cd6aad 100644
--- a/helm/tools/grafana-mcp/tests/deployment_test.yaml
+++ b/helm/tools/grafana-mcp/tests/deployment_test.yaml
@@ -157,3 +157,31 @@ tests:
       - equal:
           path: spec.template.spec.containers[0].args[3]
           value: "*"
+
+  # The default tag is mutable, so the terminal pull policy must stay Always;
+  # the values default is empty so global.imagePullPolicy can reach this chart.
+  - it: should default the pull policy to Always
+    template: deployment.yaml
+    asserts:
+      - equal:
+          path: spec.template.spec.containers[0].imagePullPolicy
+          value: Always
+
+  - it: should fall back to global.imagePullPolicy when image.pullPolicy is empty
+    template: deployment.yaml
+    set:
+      global.imagePullPolicy: IfNotPresent
+    asserts:
+      - equal:
+          path: spec.template.spec.containers[0].imagePullPolicy
+          value: IfNotPresent
+
+  - it: should let image.pullPolicy win over the global
+    template: deployment.yaml
+    set:
+      image.pullPolicy: Never
+      global.imagePullPolicy: IfNotPresent
+    asserts:
+      - equal:
+          path: spec.template.spec.containers[0].imagePullPolicy
+          value: Never
diff --git a/helm/tools/grafana-mcp/values.yaml b/helm/tools/grafana-mcp/values.yaml
index e5160ff8ee..75e6119a71 100644
--- a/helm/tools/grafana-mcp/values.yaml
+++ b/helm/tools/grafana-mcp/values.yaml
@@ -9,7 +9,11 @@ grafana:
 image:
   registry: mcp
   repository: grafana
-  pullPolicy: Always
+  # -- Pull policy for this image. Empty falls back to global.imagePullPolicy,
+  # then to Always -- the tag below is mutable, so a cached image goes stale
+  # under any other terminal default. A value declared here would shadow the
+  # global, which is why the default is empty rather than Always.
+  pullPolicy: ""
   tag: "latest" # Only latest is available via docker hub at present. See https://github.com/grafana/mcp-grafana/issues/180
 
 # -- Pull secrets for the pod, merged (union) with global.imagePullSecrets.

From 6c1f5fae4cc3318188ce0601a5d21562ce08bb7c Mon Sep 17 00:00:00 2001
From: Jonathan Jamroga <jjamroga@gmail.com>
Date: Fri, 18 Sep 2026 09:20:02 -0400
Subject: [PATCH 5/7] fix(helm): tolerate a trailing slash on
 global.imageRegistry, and pin what the mirror must not touch

A trailing slash is an easy value to ship (mirror.example/), and every
consumer joins the registry onto a path with its own slash, so the raw value
rendered image references with a double slash that fail at pull time. The
global now resolves through one helper that trims it, used by the controller,
ui and postgres images and by the worker-pool rewrite; grafana-mcp trims
inline because it is its own chart.

The single-string rewrite also gains the container runtime's fourth registry
test: a first segment with an uppercase letter is a host, because repository
paths are lowercase-only. Registry/foo:v1 now substitutes the host instead of
rendering an invalid prefixed reference.

Two absences are now pinned as well: the configmap's IMAGE_REGISTRY env must
stay on its per-image registry under a set global (nothing reads it from the
environment, so mirroring it would claim coverage the runtime does not have),
and the ui image must follow the global. The suite's controller-configmap
entry runs an assertion again instead of sitting dead in the templates list.

Verified with helm unittest (310 passing) and by rendering with a
trailing-slash global across every image and with an uppercase-host worker
image; the default render is byte-identical to the branch tip.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
---
 helm/kagent/templates/_helpers.tpl            | 33 ++++++++++++-----
 helm/kagent/tests/global-values_test.yaml     | 35 +++++++++++++++++++
 helm/tools/grafana-mcp/templates/_helpers.tpl |  7 ++--
 3 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl
index 5643c6b170..d92be4a169 100644
--- a/helm/kagent/templates/_helpers.tpl
+++ b/helm/kagent/templates/_helpers.tpl
@@ -269,7 +269,7 @@ Bundled PostgreSQL image - constructs the full image reference from registry/rep
 */}}
 {{- define "kagent.postgresql.image" -}}
 {{- $pg := .Values.database.postgres.bundled -}}
-{{- $registry := default $pg.image.registry ((.Values.global).imageRegistry) -}}
+{{- $registry := default $pg.image.registry (include "kagent.globalImageRegistry" .) -}}
 {{- $parts := compact (list $registry $pg.image.repository $pg.image.name) -}}
 {{- printf "%s:%s" (join "/" $parts) $pg.image.tag -}}
 {{- end -}}
@@ -395,26 +395,40 @@ call. The top-level tag wins over the component tag, as it always has.
 */}}
 {{- define "kagent.controllerImage" -}}
 {{- $root := dict "registry" (.Values.controller.image.registry | default .Values.registry) "repository" .Values.controller.image.repository "tag" (coalesce .Values.tag .Values.controller.image.tag .Chart.Version) -}}
-{{- include "kagent.images.image" (dict "imageRoot" $root "global" .Values.global) -}}
+{{- $global := dict "imageRegistry" (include "kagent.globalImageRegistry" .) -}}
+{{- include "kagent.images.image" (dict "imageRoot" $root "global" $global) -}}
+{{- end -}}
+
+{{/*
+global.imageRegistry, normalized. A trailing slash is an easy value to ship
+("mirror.example/") and every consumer joins the registry onto a path with its
+own "/", so the raw value would render an image reference with a double slash
+that fails at pull time. Every template that reads the global goes through
+this helper so the tolerance is uniform across the chart.
+*/}}
+{{- define "kagent.globalImageRegistry" -}}
+{{- ((.Values.global).imageRegistry) | default "" | trimSuffix "/" -}}
 {{- end -}}
 
 {{/*
 Rewrite a full image reference onto global.imageRegistry, for values that carry
 a whole reference in one string rather than registry/repository/tag keys.
 Follows the container runtime's rule for deciding whether the first path
-segment is a registry: it is one only when it contains a dot or a colon or is
-exactly "localhost". A host-carrying reference has that segment replaced so the
-mirror sees a stable path; a bare Docker Hub-style name is prefixed instead.
-When global.imageRegistry is unset the reference passes through unchanged.
+segment is a registry: it is one only when it contains a dot or a colon, is
+exactly "localhost", or contains an uppercase letter (a repository path is
+lowercase-only, so an uppercase segment can only be a host). A host-carrying
+reference has that segment replaced so the mirror sees a stable path; a bare
+Docker Hub-style name is prefixed instead. When global.imageRegistry is unset
+the reference passes through unchanged.
 Call with (dict "root" $ "image" <reference>).
 */}}
 {{- define "kagent.mirroredImage" -}}
 {{- $ref := .image -}}
-{{- $mirror := ((.root.Values.global).imageRegistry) -}}
+{{- $mirror := include "kagent.globalImageRegistry" .root -}}
 {{- if and $mirror $ref -}}
   {{- $parts := splitList "/" $ref -}}
   {{- $first := first $parts -}}
-  {{- if and (gt (len $parts) 1) (or (contains "." $first) (contains ":" $first) (eq $first "localhost")) -}}
+  {{- if and (gt (len $parts) 1) (or (contains "." $first) (contains ":" $first) (eq $first "localhost") (ne $first ($first | lower))) -}}
     {{- printf "%s/%s" $mirror (join "/" (rest $parts)) -}}
   {{- else -}}
     {{- printf "%s/%s" $mirror $ref -}}
@@ -430,5 +444,6 @@ tag wins over the component tag.
 */}}
 {{- define "kagent.uiImage" -}}
 {{- $root := dict "registry" (.Values.ui.image.registry | default .Values.registry) "repository" .Values.ui.image.repository "tag" (coalesce .Values.tag .Values.ui.image.tag .Chart.Version) -}}
-{{- include "kagent.images.image" (dict "imageRoot" $root "global" .Values.global) -}}
+{{- $global := dict "imageRegistry" (include "kagent.globalImageRegistry" .) -}}
+{{- include "kagent.images.image" (dict "imageRoot" $root "global" $global) -}}
 {{- end -}}
diff --git a/helm/kagent/tests/global-values_test.yaml b/helm/kagent/tests/global-values_test.yaml
index c6df142dd1..da360e223d 100644
--- a/helm/kagent/tests/global-values_test.yaml
+++ b/helm/kagent/tests/global-values_test.yaml
@@ -5,6 +5,10 @@ templates:
   - controller-configmap.yaml
   - rbac/getter-role.yaml
   - substrate-workerpool.yaml
+  # ui-deployment.yaml includes ui-nginx-configmap.yaml, and helm-unittest can
+  # only resolve cross-template includes when both are listed here.
+  - ui-deployment.yaml
+  - ui-nginx-configmap.yaml
 tests:
   - it: should override every image registry from global.imageRegistry
     set:
@@ -35,6 +39,37 @@ tests:
       - equal:
           path: spec.workerImage
           value: ghcr.io/kagent-dev/substrate/ateom:v1
+  - it: should tolerate a trailing slash on global.imageRegistry
+    set:
+      global.imageRegistry: "mirror.example/"
+      controller.substrate.enabled: true
+      substrateWorkerPool.create: true
+      substrateWorkerPool.workerImage: ghcr.io/kagent-dev/substrate/ateom:v1
+    template: substrate-workerpool.yaml
+    asserts:
+      - equal:
+          path: spec.workerImage
+          value: mirror.example/kagent-dev/substrate/ateom:v1
+  # Nothing reads IMAGE_REGISTRY from the pod environment: agent workload images
+  # come from each harness spec, not from this env. The global must therefore
+  # not touch it -- rewiring it would claim mirror coverage the runtime does
+  # not have. This pins the removal itself.
+  - it: should keep the agent-runtime env on its per-image registry under the global
+    set:
+      global.imageRegistry: mirror.example
+    template: controller-configmap.yaml
+    asserts:
+      - equal:
+          path: data.IMAGE_REGISTRY
+          value: ghcr.io
+  - it: should point the ui image at global.imageRegistry
+    set:
+      global.imageRegistry: mirror.example
+    template: ui-deployment.yaml
+    asserts:
+      - matchRegex:
+          path: spec.template.spec.containers[0].image
+          pattern: "^mirror\\.example/"
   - it: should keep per-image registry when the global is unset
     template: controller-deployment.yaml
     asserts:
diff --git a/helm/tools/grafana-mcp/templates/_helpers.tpl b/helm/tools/grafana-mcp/templates/_helpers.tpl
index a40bab4680..e0d72208f3 100644
--- a/helm/tools/grafana-mcp/templates/_helpers.tpl
+++ b/helm/tools/grafana-mcp/templates/_helpers.tpl
@@ -108,8 +108,11 @@ Join registry/repository/name/tag for grafana-mcp image, skipping empty segments
 {{- $img := .Values.image -}}
 {{/* image.registry holds the docker.io org here ("mcp"), not a host, so the
      air-gap override is prepended rather than substituted: the mirror serves
-     the image under its existing mcp/grafana path. */}}
-{{- $parts := compact (list ((.Values.global).imageRegistry) $img.registry $img.repository $img.name) -}}
+     the image under its existing mcp/grafana path. The global is trimmed of a
+     trailing slash because the join below adds its own, and a double slash is
+     an invalid reference that fails at pull time. */}}
+{{- $mirror := ((.Values.global).imageRegistry) | default "" | trimSuffix "/" -}}
+{{- $parts := compact (list $mirror $img.registry $img.repository $img.name) -}}
 {{- printf "%s:%s" (join "/" $parts) $img.tag -}}
 {{- end -}}
 {{/*

From 62ac55c35e1c0a058b67c56ad467ec494e852059 Mon Sep 17 00:00:00 2001
From: Jonathan Jamroga <jjamroga@gmail.com>
Date: Fri, 18 Sep 2026 09:26:05 -0400
Subject: [PATCH 6/7] docs(helm): the mutable-tag rationale is gone, the
 historical default remains

Upstream pinned grafana-mcp to a release tag, so the pull-policy helper's
stated reason for Always (a mutable latest tag) is no longer true. The
terminal default stays Always so the default render does not change; only the
comment now says why.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
---
 helm/tools/grafana-mcp/templates/_helpers.tpl | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/helm/tools/grafana-mcp/templates/_helpers.tpl b/helm/tools/grafana-mcp/templates/_helpers.tpl
index e0d72208f3..8494e8583c 100644
--- a/helm/tools/grafana-mcp/templates/_helpers.tpl
+++ b/helm/tools/grafana-mcp/templates/_helpers.tpl
@@ -129,9 +129,8 @@ imagePullSecrets:
 
 {{/*
 imagePullPolicy: image.pullPolicy, then global.imagePullPolicy, then Always.
-The terminal default is Always rather than IfNotPresent because the default tag
-is mutable ("latest"): with a cached image, IfNotPresent never picks up a new
-push of the same tag.
+The terminal default stays Always, this chart's previous declared default, so
+an install that sets neither value keeps the pull behavior it already has.
 */}}
 {{- define "grafana-mcp.imagePullPolicy" -}}
 {{- .Values.image.pullPolicy | default ((.Values.global).imagePullPolicy) | default "Always" -}}

From 6c35a1eac8493eb4d82e8ae51dce601bebdfe3ce Mon Sep 17 00:00:00 2001
From: Jonathan Jamroga <jjamroga@gmail.com>
Date: Fri, 18 Sep 2026 09:34:05 -0400
Subject: [PATCH 7/7] fix(helm): give grafana-mcp a real registry key, and
 refuse the org shape

image.registry held a Docker Hub organization, not a host, so the global
override could only be prepended -- correct for the default shorthand, but an
operator who put a host in the key got mirror.example/docker.io/...: the old
host turned into a directory, and this one chart disagreed with every other
image override in the stack.

image.registry now takes only a host, default docker.io, and the organization
moves into image.repository. The global replaces the host outright, the same
substitution the rest of the family uses. A values file still carrying the old
shape fails the render with a message naming the new one, rather than pulling
from a path that does not exist. The unused image.name segment is refused the
same way.

The default reference gains an explicit docker.io prefix; it resolves to the
same image.

Verified with helm unittest (19 in the chart, 329 in the umbrella) and by
rendering the umbrella under a trailing-slash mirror and under an old-shape
override, which fails with the guard's message.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
---
 helm/tools/grafana-mcp/templates/_helpers.tpl | 23 ++++++++++-----
 .../grafana-mcp/tests/deployment_test.yaml    | 29 +++++++++++++++++--
 helm/tools/grafana-mcp/values.yaml            |  7 +++--
 3 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/helm/tools/grafana-mcp/templates/_helpers.tpl b/helm/tools/grafana-mcp/templates/_helpers.tpl
index 8494e8583c..a1bf0b5413 100644
--- a/helm/tools/grafana-mcp/templates/_helpers.tpl
+++ b/helm/tools/grafana-mcp/templates/_helpers.tpl
@@ -102,17 +102,26 @@ check off behind a proxy that rewrites Host.
 {{- end }}
 
 {{/*
-Join registry/repository/name/tag for grafana-mcp image, skipping empty segments, then append tag
+The container image. image.registry is a host only and image.repository is the
+path below it, so global.imageRegistry replaces the host outright -- the same
+substitution every other image in this chart family uses. The registry once
+held a Docker Hub organization; that shape is refused rather than rendered,
+because prepending a mirror to it would turn the old host into a directory and
+the operator would find out at pull time. The global is trimmed of a trailing
+slash because the join below adds its own, and a double slash is an invalid
+reference that fails at pull time.
 */}}
 {{- define "grafana-mcp.image" -}}
 {{- $img := .Values.image -}}
-{{/* image.registry holds the docker.io org here ("mcp"), not a host, so the
-     air-gap override is prepended rather than substituted: the mirror serves
-     the image under its existing mcp/grafana path. The global is trimmed of a
-     trailing slash because the join below adds its own, and a double slash is
-     an invalid reference that fails at pull time. */}}
+{{- if hasKey $img "name" -}}
+{{- fail "image.name was removed: append it to image.repository instead (e.g. repository: grafana/mcp-grafana)." -}}
+{{- end -}}
+{{- if and $img.registry (not (or (contains "." $img.registry) (contains ":" $img.registry) (eq $img.registry "localhost") (ne $img.registry ($img.registry | lower)))) -}}
+{{- fail (printf "image.registry (%q) is not a registry host. image.registry now takes only a host (default docker.io) and image.repository takes the full path below it (e.g. grafana/mcp-grafana). Move the organization into image.repository." $img.registry) -}}
+{{- end -}}
 {{- $mirror := ((.Values.global).imageRegistry) | default "" | trimSuffix "/" -}}
-{{- $parts := compact (list $mirror $img.registry $img.repository $img.name) -}}
+{{- $registry := $mirror | default $img.registry -}}
+{{- $parts := compact (list $registry $img.repository) -}}
 {{- printf "%s:%s" (join "/" $parts) $img.tag -}}
 {{- end -}}
 {{/*
diff --git a/helm/tools/grafana-mcp/tests/deployment_test.yaml b/helm/tools/grafana-mcp/tests/deployment_test.yaml
index e7395d7e95..21e3c8566f 100644
--- a/helm/tools/grafana-mcp/tests/deployment_test.yaml
+++ b/helm/tools/grafana-mcp/tests/deployment_test.yaml
@@ -106,7 +106,7 @@ tests:
     asserts:
       - equal:
           path: spec.template.spec.containers[0].image
-          value: grafana/mcp-grafana:1.4.1
+          value: docker.io/grafana/mcp-grafana:1.4.1
 
   - it: should omit empty registry segment in image
     template: deployment.yaml
@@ -116,11 +116,36 @@ tests:
     asserts:
       - equal:
           path: spec.template.spec.containers[0].image
-          value: mcp-grafana:1.4.1
+          value: grafana/mcp-grafana:1.4.1
       - notMatchRegex:
           path: spec.template.spec.containers[0].image
           pattern: "^/"          # no leading slash
 
+  - it: should substitute the registry host from global.imageRegistry
+    template: deployment.yaml
+    set:
+      global.imageRegistry: mirror.example
+    asserts:
+      - equal:
+          path: spec.template.spec.containers[0].image
+          value: mirror.example/grafana/mcp-grafana:1.4.1
+
+  - it: should refuse an organization in image.registry
+    template: deployment.yaml
+    set:
+      image.registry: grafana
+    asserts:
+      - failedTemplate:
+          errorPattern: 'image\.registry \("grafana"\) is not a registry host'
+
+  - it: should refuse a removed image.name
+    template: deployment.yaml
+    set:
+      image.name: grafana
+    asserts:
+      - failedTemplate:
+          errorPattern: 'image\.name was removed'
+
   # =============================================================================
   # Host allow-list
   #
diff --git a/helm/tools/grafana-mcp/values.yaml b/helm/tools/grafana-mcp/values.yaml
index 8aeb978b4b..699cc819ad 100644
--- a/helm/tools/grafana-mcp/values.yaml
+++ b/helm/tools/grafana-mcp/values.yaml
@@ -7,8 +7,11 @@ grafana:
   # secretRef: ""  # Name of Secret to reference (contains GRAFANA_SERVICE_ACCOUNT_TOKEN or GRAFANA_API_KEY)
 
 image:
-  registry: grafana
-  repository: mcp-grafana
+  # -- Registry host only (a mirror or docker.io), never a path: the global
+  # override replaces this whole value, so a path segment here would be lost
+  # under global.imageRegistry. The path lives in repository.
+  registry: docker.io
+  repository: grafana/mcp-grafana
   # -- Pull policy for this image. Empty falls back to global.imagePullPolicy,
   # then to Always, this chart's previous declared default. A value declared
   # here would shadow the global, which is why the default is empty.