feat(helm): global values for registry, pull config, and namespace scope

[jjamroga]

What this gives operators

This PR adds a global: block. Each install-wide setting becomes one value. Helm passes global.* into subcharts automatically, so these values also reach the vendored subcharts as those adopt them.

• global.imageRegistry — for air-gapped and private-registry installs. Mirror the images, then set this one value. Every image this chart composes pulls from the mirror: controller, ui, the bundled postgres, grafana-mcp, and the agent runtime the controller launches (IMAGE_REGISTRY). Repository paths and tags stay per-image, so a mirror copies each image under its existing path.
• global.imagePullSecrets — the pull secret that mirror needs. The chart merges it (union) into each pod's own list. A local secret is never removed.
• global.imagePullPolicy — one pull policy for the install. An explicitly set component policy or top-level imagePullPolicy still wins.
• global.watchNamespaces — a namespace-scoped install in one value. A non-empty list renders Roles instead of ClusterRoles for the getter and writer RBAC. The controller's WATCH_NAMESPACES derives from the same list. RBAC scope and watch scope cannot disagree.

Image references resolve through one helper, kagent.images.image. It implements the precedence in one place: a set global overrides the per-image registry, repository and tag stay per-image, and a digest pins the image in place of the tag. The name is chart-scoped because Helm define names are global across a release. A generic name could collide with another chart's helper, and the last-loaded copy would win silently.

Not a breaking change

Every new key is opt-in. A default render is byte-identical to main. Two behavior notes:

• One configuration that used to render now fails, on purpose. Before this PR, rbac.namespaces: [a] with controller.watchNamespaces: [a, b] rendered successfully. The controller then watched namespace b with no Role in it. Every reconcile there returned Forbidden at runtime, with only a log line to show for it. That mix now fails the render, and the error names both keys and the fix. The only configurations that newly fail are ones that never worked.
• Multi-namespace RoleBindings now render in sorted order. writer-rolebinding.yaml was the one rbac template without sortAlpha; the other three sorted. Object content is unchanged. Tooling that diffs rendered output may see a reorder once.

Other compatibility notes

• An explicit rbac.namespaces: [] opts out of the global. An empty list has always meant "create ClusterRoles". The global is only a fallback, so it must not override that choice. But Helm's coalesce cannot see the choice: it skips empty values, so [] and "not set" look the same. The chart therefore checks key presence instead. Example: a parent sets global.watchNamespaces: [team-a, team-b] to scope its other subcharts. A kagent values file contains rbac.namespaces: []. That file keeps its ClusterRoles. Without the presence check, the same upgrade would silently replace them with Roles in team-a and team-b, and the controller would lose access everywhere else. One consequence: values.yaml no longer declares namespaces: [] as a default. A declared empty default would make every install look explicitly cluster-scoped, and the global fallback would never fire. The key now ships commented out, with this explanation next to it.
• A global that omits kagent's namespace does not break the install. The chart needs a Role where the controller runs. A parent may set global.watchNamespaces for its other subcharts and forget kagent's namespace. Failing the render for that would brick the whole umbrella install over a list that was never about kagent. So on the global path, the chart auto-appends its install namespace to the resolved scope. It also folds controller.watchNamespaces into that scope, so a wider watch gains matching Roles instead of failing. The hard errors remain for an explicit rbac.namespaces: a list without the install namespace, or a watch outside the list, stops the render and names the fix.
• Pull policies no longer declare IfNotPresent in values. The top-level imagePullPolicy and the bundled postgres policy move the default into the template's fallback chain. The rendered output is the same. The reason: a declared default is always "set", so global.imagePullPolicy could never fire. With the default in the template, the global fallback is reachable.
• grafana-mcp (in this repo) adopts the globals too. Its pod previously had no imagePullSecrets support at all; it gains the merge. Its image.registry value holds a docker.io org (mcp), not a host, so the mirror override prepends: the mirror serves mcp/grafana under its existing path.
• Coverage boundary. The vendored subcharts (kagent-tools, kmcp, substrate) adopt the globals in their own repositories, and this chart picks them up as their pinned versions bump. The values comments state this boundary. Companion PRs: feat(helm): global values for registry, pull config, and namespace scope tools#81, feat(helm): global values, and the registry/repository image split kmcp#143, feat(helm): global image values, and the registry/repository split substrate#38.

Verification

A default render is byte-identical to main for images, pull secrets, and RBAC kinds. The chart's test suite passes (288 tests), including a new global-values suite that pins: the registry override, per-image registry retention when the global is unset, the pull-secret merge, namespaced RBAC from the global alone, the install-namespace auto-append, the watch fold-in, the explicit-empty opt-out, local-wins precedence, and the watch-outside-scope failure.

🤖 Generated with Claude Code

[dhaifley]

If I'm reading this right, the watch helper reads the raw global while RBAC uses the resolved list. With release namespace kagent and only global.watchNamespaces=[team-a], the chart creates Roles in both namespaces but emits WATCH_NAMESPACES=team-a, so the install namespace is missing from both controller caches and Harnesses, AgentTemplates, ModelConfigs and MCP resources there are not watched (go/core/pkg/app/app.go:203-224 configures both caches from this value, and go/core/internal/controller/collections.go:60-70 applies it to the collections). The auto-append test only checks the Role.

There's a second effect of the separate fallback. With rbac.namespaces=[] and that same global, RBAC correctly goes cluster-wide but the watch stays restricted to team-a, which changes an existing explicitly cluster-wide configuration. Would it make sense to keep the controller.watchNamespaces override, then derive the default watch from kagent.rbacNamespaces with its key-presence and install-namespace rules, and test WATCH_NAMESPACES in both cases?

[jjamroga]

You read it correctly, and thank you for the trace into the cache setup. Fixed in 1f8d37e. The watch helper no longer reads the raw global. It now derives from the same resolved list that the RBAC templates use. The two cannot disagree now. A scope from global.watchNamespaces includes the install namespace. An explicit rbac.namespaces: [] restores the cluster-wide watch. controller.watchNamespaces stays an explicit override. Two new tests pin the global-only case and the explicit-empty case.

[dhaifley]

I'm not sure that changing IMAGE_REGISTRY redirects agent runtime images in the current controller. I didn't see a Go reader of IMAGE_REGISTRY, IMAGE_REPOSITORY or IMAGE_TAG at this head, and the harness compilers copy harness.Spec.Workload.Image into the revision (go/core/internal/translator/kagent/compiler.go:90). The optional substrate-workerpool.yaml also emits substrateWorkerPool.workerImage unchanged (line 18).

When I test rendered a WorkerPool under global.imageRegistry=mirror.example, its ghcr.io/.../ateom:v1 stayed on ghcr.io. The new ConfigMap test passes without showing that an agent can run from the mirror. Could the global be wired to the actual runtime image configuration, with a test that inspects the resulting runtime or WorkerPool image?

[jjamroga]

Confirmed. No code reads IMAGE_REGISTRY from the environment, and the harness compilers copy the workload image unchanged. The override changed a dead value. Fixed in 901e4c0: the configmap line is back to its previous expression, and the values comment no longer claims that coverage. The real gap was substrateWorkerPool.workerImage, which the chart emitted unchanged. It now goes through a rewrite helper that applies the container runtime's registry rule: a host segment is replaced, a bare name gets a prefix, and an unset global changes nothing. 6c1f5fa adds a negative test: the environment value must stay on its per-image registry when the global is set.

[dhaifley]

I think this fallback is unreachable for a normal install, since image.pullPolicy still defaults to Always in this subchart's values (values.yaml:9-13). With only global.imagePullPolicy=Never, the controller, UI, tools and kmcp all use Never, while grafana-mcp, which is enabled by default, still uses Always and contacts the registry. Would it work to move the default out of values and into the helper's final fallback, keeping Always as this chart's default when neither a local nor a global policy is set? An explicit local policy would still win, and the existing latest image behavior wouldn't change.

[jjamroga]

Yes, that works, and it is what we did in 1852efd. The values default is now empty. The helper's final fallback is Always. The precedence is: local value, then global value, then Always. After the merge with main, the image tag is pinned, so the old mutable-tag reason for Always is gone. We keep Always as the terminal fallback so that the default render does not change in this PR. A move to IfNotPresent can be a separate change. Three tests pin the default, the global fallback, and a local override.

[dhaifley]

The prepend works for the default Docker Hub shorthand (mcp/grafana), but image.registry is operator-configurable and can already hold a host. image.registry=docker.io, image.repository=mcp, image.name=grafana renders a valid docker.io/mcp/grafana:latest today, and with global.imageRegistry=mirror.example this helper emits mirror.example/docker.io/mcp/grafana:latest, which adds a directory rather than replacing the registry and disagrees with every other image override in the stack. Could the default shorthand be normalized to a registry plus repository, or the helper tell a real host apart from the shorthand before applying the global?

[jjamroga]

Agreed. The image.registry key held a Docker Hub organization, not a host, so the global could only be prepended. An operator who put a real host in the key got mirror.example/docker.io/... -- a directory, not a registry. Fixed in 6c35a1e. image.registry now takes only a host, with the default docker.io. The organization moves into image.repository. The global replaces the host, the same as every other image in the stack. A values file with the old shape fails the render, and the message names the new shape. Tests pin the default, the substitution, and the two refusals.

[EItanya]

Should we remove these if they're in global, could be very confusing, Do we need a 3 level override?

[jjamroga]

We keep the three levels, per our discussion: component value, then top-level value, then global value. The top level stays because removing it breaks every existing values file that sets it, and because one chart in an umbrella install sometimes must differ from the global. The values comments state the precedence at each key.

[jjamroga]

We did an internal review of this branch. We found more issues. Some are fixed in new commits. Some are notes for the release.

Fixed in code:

1. A trailing slash in global.imageRegistry made bad image references. A value such as mirror.example/ rendered mirror.example//.... The pods could not pull these images. The chart now removes the trailing slash in one shared helper (6c1f5fa).
2. The registry test in the single-string rewrite was not complete. The container runtime also treats a first segment with an uppercase letter as a host. The helper now applies this rule (6c1f5fa).
3. The removal of the IMAGE_REGISTRY override was not pinned. A new negative test makes sure that the global does not touch that environment value again. A new test also pins the ui image under the global (6c1f5fa).

Upgrade notes:

4. WATCH_NAMESPACES changes on upgrade for some installs. The chart now sorts the namespace list. Also, the global path now adds the install namespace. The controller reads the list as a set, so the meaning does not change. But the configmap checksum changes, so the controller restarts one time on upgrade.
5. substrateWorkerPool.workerImage now follows global.imageRegistry. An operator who sets both values gets a rewritten reference. If the mirror does not have the image, the worker pods cannot pull it. This agrees with the documented contract of the global value.
6. grafana-mcp now obeys global.imagePullPolicy. Before, its declared Always made the global unreachable. The default behavior does not change: when no local and no global value is set, the policy is still Always.
7. grafana-mcp image.registry changes shape. The key now takes a registry host only, with the default docker.io. The organization moves into image.repository. A values file that sets the old shape does not render; the failure message names the new shape. The default image reference gains the docker.io/ prefix, which resolves to the same image.