From 238b46b92c8e74d493543539e5de51108e9b254d Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Thu, 10 Sep 2026 08:26:43 -0400 Subject: [PATCH] feat(helm): global image values, and the registry/repository split global.imageRegistry redirects every image at once for air-gapped mirrors. Component images now resolve through the same registry/repository split every kagent-family chart uses: image.registry was one string carrying its path (ghcr.io/kagent-dev/substrate) and is now the registry host only, joined onto image.repository, so one global value (which overrides image.registry) redirects the whole family. This is a breaking change for values files that put a full prefix in image.registry: rendered silently they would produce a doubled prefix failing only at pod start, so the render fails instead, naming the split. A default render is byte-identical to main. Single-string images.* references (postgres, rustfs, aws-cli, agentgateway) have their registry segment replaced by the containerd rule (first path segment with a dot or colon), preserving repository paths either way. global.imagePullSecrets merges (union) into every pod spec, which previously had no pull-secret surface at all. global.imagePullPolicy replaces the hardcoded IfNotPresent values as a fallback, via substrate.imagePullPolicy. Verified: a default render is byte-identical to main; the mirror knob redirects all 9 images with paths preserved; the old-shape registry fails loudly at template time; pull secrets land on all 9 pod specs; the pullPolicy fallback fires. Signed-off-by: Jonathan Jamroga --- .github/workflows/helm-e2e.yaml | 6 +- .github/workflows/helm-verify.yaml | 35 +++++++++++ charts/substrate/templates/_helpers.tpl | 63 ++++++++++++++++++- .../substrate/templates/ate-api-server.yaml | 1 + .../substrate/templates/ate-controller.yaml | 1 + charts/substrate/templates/atelet.yaml | 1 + charts/substrate/templates/atenet-egress.yaml | 3 +- charts/substrate/templates/atenet-router.yaml | 3 +- .../templates/pod-certificate-controller.yaml | 1 + charts/substrate/templates/postgres.yaml | 5 +- charts/substrate/templates/rustfs.yaml | 8 ++- charts/substrate/values.yaml | 27 +++++++- hack/render-manifests.sh | 3 +- 13 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 .github/workflows/helm-verify.yaml diff --git a/.github/workflows/helm-e2e.yaml b/.github/workflows/helm-e2e.yaml index 81b3a46103..df19eaa856 100644 --- a/.github/workflows/helm-e2e.yaml +++ b/.github/workflows/helm-e2e.yaml @@ -56,8 +56,12 @@ jobs: kubectl apply -f manifests/ate-install/kind/prometheus.yaml - name: Build chart images run: | + # Pushed under the image's real path (kagent-dev/substrate/): + # the chart composes {registry}/{repository}/{component}, so the local + # registry serves each image where the default repository expects it -- + # the same path-preserving rule a production mirror follows. for component in ateapi atecontroller atelet podcertcontroller atenet; do - KO_DOCKER_REPO="localhost:5001/${component}" \ + KO_DOCKER_REPO="localhost:5001/kagent-dev/substrate/${component}" \ ./hack/run-tool.sh ko build --bare --tags helm-e2e \ --platform linux/amd64 "./cmd/${component}" done diff --git a/.github/workflows/helm-verify.yaml b/.github/workflows/helm-verify.yaml new file mode 100644 index 0000000000..8eca1f5de3 --- /dev/null +++ b/.github/workflows/helm-verify.yaml @@ -0,0 +1,35 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Enforces the chart checks that were previously developer-run only: +# verify-helm-template's own comment says "Used in CI", but no workflow ran +# it -- so a chart change that broke hack/render-manifests.sh shipped a green +# PR and was caught in review by hand. +name: helm-verify +on: + pull_request: + push: + branches: [main] +permissions: + contents: read +jobs: + helm-verify: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: azure/setup-helm@v4 + - name: Lint the chart + run: helm lint charts/substrate charts/substrate-crds + - name: Verify committed manifests match the chart output + run: make verify-helm-template diff --git a/charts/substrate/templates/_helpers.tpl b/charts/substrate/templates/_helpers.tpl index 1179dda559..3b81dc9c0b 100644 --- a/charts/substrate/templates/_helpers.tpl +++ b/charts/substrate/templates/_helpers.tpl @@ -149,7 +149,20 @@ are emitted without a tag, letting `ko resolve` supply the digest at build time. {{- define "substrate.componentImage" -}} {{- $name := index . 0 -}} {{- $ctx := index . 1 -}} -{{- $registry := $ctx.Values.image.registry -}} +{{/* image.registry used to carry the full prefix (ghcr.io/kagent-dev/substrate). + It is now the registry host only, joined onto image.repository -- the same + registry/repository split every kagent-family chart uses, so one + global.imageRegistry value redirects them all. A values file still carrying + a path in registry would render a doubled prefix that fails only at pod + start, so it fails the render here instead and names the split. */}} +{{- /* A scheme'd registry (ko://...) is hack/render-manifests.sh passing an + importpath prefix for `ko resolve` to substitute, same as the "" tag + sentinel below -- unambiguously not the old host+path shape, so the guard + lets it through. */ -}} +{{- if and (contains "/" $ctx.Values.image.registry) (not (contains "://" $ctx.Values.image.registry)) -}} +{{- fail (printf "image.registry (%q) carries a path. It is now the registry host only: keep the path in image.repository, e.g. registry: ghcr.io, repository: kagent-dev/substrate." $ctx.Values.image.registry) -}} +{{- end -}} +{{- $registry := printf "%s/%s" (default $ctx.Values.image.registry (($ctx.Values.global).imageRegistry)) $ctx.Values.image.repository -}} {{- $tag := $ctx.Values.image.tag | default $ctx.Chart.AppVersion -}} {{- if ne $tag "" -}} {{- printf "%s/%s:%s" $registry $name $tag -}} @@ -157,3 +170,51 @@ are emitted without a tag, letting `ko resolve` supply the digest at build time. {{- printf "%s/%s" $registry $name -}} {{- end -}} {{- end -}} + +{{/* +Rewrite a full image reference ({registry}/{path}:{tag}) onto global.imageRegistry. + +The `images.*` values are single-string references, some digest-pinned, so the +mirror knob has to edit the string. The first path segment is a registry only when +it contains "." or ":" (the containerd rule); otherwise the reference is +docker.io-implied and the mirror is prefixed. The repository path is preserved +either way, so a mirror copies images under their existing paths. + +Usage: {{ include "substrate.thirdPartyImage" (list .Values.images.postgres .) }} +*/}} +{{- define "substrate.thirdPartyImage" -}} +{{- $ref := index . 0 -}} +{{- $ctx := index . 1 -}} +{{- $mirror := (($ctx.Values.global).imageRegistry) -}} +{{- if not $mirror -}} +{{- $ref -}} +{{- else -}} +{{- $parts := splitList "/" $ref -}} +{{- $first := first $parts -}} +{{- if and (gt (len $parts) 1) (or (contains "." $first) (contains ":" $first)) -}} +{{- printf "%s/%s" $mirror (join "/" (rest $parts)) -}} +{{- else -}} +{{- printf "%s/%s" $mirror $ref -}} +{{- end -}} +{{- end -}} +{{- end -}} + +{{/* +imagePullSecrets for a pod spec: the chart's own list merged (union) with +global.imagePullSecrets. Renders nothing when both are empty. +*/}} +{{- define "substrate.imagePullSecrets" -}} +{{- $merged := concat (.Values.imagePullSecrets | default list) (((.Values.global).imagePullSecrets) | default list) | uniq -}} +{{- if $merged -}} +imagePullSecrets: +{{- toYaml $merged | nindent 0 }} +{{- end -}} +{{- end -}} + +{{/* +imagePullPolicy: global.imagePullPolicy when set, IfNotPresent otherwise. One +definition so the fallback cannot drift between pods. +*/}} +{{- define "substrate.imagePullPolicy" -}} +{{- ((.Values.global).imagePullPolicy) | default "IfNotPresent" -}} +{{- end -}} diff --git a/charts/substrate/templates/ate-api-server.yaml b/charts/substrate/templates/ate-api-server.yaml index 7718ed3827..36c2af14d3 100644 --- a/charts/substrate/templates/ate-api-server.yaml +++ b/charts/substrate/templates/ate-api-server.yaml @@ -77,6 +77,7 @@ spec: spec: serviceAccountName: {{ include "substrate.fullname" (list "ate-api-server" .) }} terminationGracePeriodSeconds: 40 + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: ate-api-server image: {{ include "substrate.componentImage" (list "ateapi" .) }} diff --git a/charts/substrate/templates/ate-controller.yaml b/charts/substrate/templates/ate-controller.yaml index d324d05af5..9e8588ceb2 100644 --- a/charts/substrate/templates/ate-controller.yaml +++ b/charts/substrate/templates/ate-controller.yaml @@ -67,6 +67,7 @@ spec: app: ate-controller spec: serviceAccountName: {{ include "substrate.fullname" (list "ate-controller" .) }} + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: ate-controller image: {{ include "substrate.componentImage" (list "atecontroller" .) }} diff --git a/charts/substrate/templates/atelet.yaml b/charts/substrate/templates/atelet.yaml index 2de2def545..001974553d 100644 --- a/charts/substrate/templates/atelet.yaml +++ b/charts/substrate/templates/atelet.yaml @@ -101,6 +101,7 @@ spec: prometheus.io/port: "9090" spec: serviceAccountName: {{ include "substrate.fullname" (list "atelet" .) }} + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: atelet image: {{ include "substrate.componentImage" (list "atelet" .) }} diff --git a/charts/substrate/templates/atenet-egress.yaml b/charts/substrate/templates/atenet-egress.yaml index 6de04ca9ee..c921bb9ac4 100644 --- a/charts/substrate/templates/atenet-egress.yaml +++ b/charts/substrate/templates/atenet-egress.yaml @@ -96,9 +96,10 @@ spec: - name: net.ipv4.ip_unprivileged_port_start value: "0" terminationGracePeriodSeconds: 60 + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: agentgateway - image: {{ .Values.images.agentgateway }} + image: {{ include "substrate.thirdPartyImage" (list .Values.images.agentgateway .) }} args: - -f - /etc/agentgateway/config.yaml diff --git a/charts/substrate/templates/atenet-router.yaml b/charts/substrate/templates/atenet-router.yaml index bfa6c9e10c..63f14d4c38 100644 --- a/charts/substrate/templates/atenet-router.yaml +++ b/charts/substrate/templates/atenet-router.yaml @@ -211,6 +211,7 @@ spec: prometheus.io/port: "9090" spec: serviceAccountName: {{ include "substrate.fullname" (list "atenet-router" .) }} + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: atenet-router image: {{ include "substrate.componentImage" (list "atenet" .) }} @@ -258,7 +259,7 @@ spec: - { name: servicedns-ca, mountPath: /run/servicedns-ca, readOnly: true } - { name: podidentity, mountPath: /run/podidentity.podcert.ate.dev, readOnly: true } - name: agentgateway - image: {{ .Values.images.agentgateway }} + image: {{ include "substrate.thirdPartyImage" (list .Values.images.agentgateway .) }} args: - "-f" - "/etc/agentgateway/config.yaml" diff --git a/charts/substrate/templates/pod-certificate-controller.yaml b/charts/substrate/templates/pod-certificate-controller.yaml index 86fc23b4a9..b3fc65ee21 100644 --- a/charts/substrate/templates/pod-certificate-controller.yaml +++ b/charts/substrate/templates/pod-certificate-controller.yaml @@ -139,6 +139,7 @@ spec: labels: app: podcertificate-controller spec: + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: controller image: {{ include "substrate.componentImage" (list "podcertcontroller" .) }} diff --git a/charts/substrate/templates/postgres.yaml b/charts/substrate/templates/postgres.yaml index 26ddb0b1ec..77df191879 100644 --- a/charts/substrate/templates/postgres.yaml +++ b/charts/substrate/templates/postgres.yaml @@ -124,7 +124,7 @@ spec: initContainers: - name: tls-reloader restartPolicy: Always - image: {{ .Values.images.postgres }} + image: {{ include "substrate.thirdPartyImage" (list .Values.images.postgres .) }} securityContext: runAsUser: 70 command: @@ -145,9 +145,10 @@ spec: requests: cpu: 10m memory: 32Mi + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: postgres - image: {{ .Values.images.postgres }} + image: {{ include "substrate.thirdPartyImage" (list .Values.images.postgres .) }} lifecycle: postStart: exec: diff --git a/charts/substrate/templates/rustfs.yaml b/charts/substrate/templates/rustfs.yaml index edaad3cfa8..14854822af 100644 --- a/charts/substrate/templates/rustfs.yaml +++ b/charts/substrate/templates/rustfs.yaml @@ -63,10 +63,11 @@ spec: runAsUser: 10001 runAsGroup: 10001 fsGroup: 10001 + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: rustfs - image: {{ .Values.images.rustfs }} - imagePullPolicy: IfNotPresent + image: {{ include "substrate.thirdPartyImage" (list .Values.images.rustfs .) }} + imagePullPolicy: {{ include "substrate.imagePullPolicy" . }} ports: - containerPort: 9000 name: api @@ -103,9 +104,10 @@ spec: template: spec: restartPolicy: OnFailure + {{- with include "substrate.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} containers: - name: create-bucket - image: {{ .Values.images.awsCli }} + image: {{ include "substrate.thirdPartyImage" (list .Values.images.awsCli .) }} env: - name: AWS_ACCESS_KEY_ID value: {{ .Values.rustfs.accessKey | quote }} diff --git a/charts/substrate/values.yaml b/charts/substrate/values.yaml index 7f456a20af..f6edd00575 100644 --- a/charts/substrate/values.yaml +++ b/charts/substrate/values.yaml @@ -80,8 +80,33 @@ otel: enabled: true endpoint: "" +# 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: + # -- Mirror registry that overrides where every image is pulled from. This is + # the air-gap knob. It wins over image.registry for component images. Each + # `images.*` reference has its registry segment replaced. Repository paths are + # preserved, so a mirror only has to copy images under their existing paths. + # For control without the override, leave this unset and set image.registry. + imageRegistry: "" + # -- Pull secrets merged (union) into each pod's own imagePullSecrets list. + imagePullSecrets: [] + # -- Fallback imagePullPolicy where a container does not set one. + imagePullPolicy: "" + +# Pull secrets for every pod this chart renders. Merged with global.imagePullSecrets. +imagePullSecrets: [] + image: - registry: ghcr.io/kagent-dev/substrate + # Registry host for the component images, and nothing else. To change + # environments, change only this value or global.imageRegistry, which overrides + # it. A path inside `registry` fails the render, and the error names this + # split. + registry: ghcr.io + # Image path prefix under the registry, ahead of each component name. The path + # is identical on every registry that serves the images. A mirror copies the + # images under this same path. + repository: kagent-dev/substrate tag: "" images: diff --git a/hack/render-manifests.sh b/hack/render-manifests.sh index 1f6790bb73..2187044970 100755 --- a/hack/render-manifests.sh +++ b/hack/render-manifests.sh @@ -60,7 +60,8 @@ helm template substrate "${CHART_DIR}" \ --namespace ate-system \ --set auth.mode=mtls \ --set createNamespace=true \ - --set image.registry=ko://github.com/agent-substrate/substrate/cmd \ + --set image.registry=ko://github.com \ + --set image.repository=agent-substrate/substrate/cmd \ --set image.tag="" \ > "${TMP_DIR}/all.yaml"