From aadbb2e8c70e20e72a8ee4aee599bc156c974b44 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 9 Sep 2026 16:21:20 -0400 Subject: [PATCH] feat(helm): global values for registry, pull config, and namespace scope Matches the convention landing across the kagent-family charts. global.imageRegistry overrides the image registry (air-gap mirror knob) via a shared kagent-tools.images.image helper. global.imagePullSecrets merges into the pod's own list via kagent-tools.imagePullSecrets. global.imagePullPolicy fills the gap when tools.image.pullPolicy is unset, via kagent-tools.imagePullPolicy; the declared IfNotPresent default moved into the template chain 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 is auto-appended. The resolved scope drives the Role/ClusterRole switch and drops the nonResourceURLs rule namespaced Roles cannot carry. Verified: helm unittest passes (23 tests, including pins for the global scope, explicit-empty opt-out, and local-beats-global precedence); a default render is byte-identical to main. Signed-off-by: Jonathan Jamroga --- helm/kagent-tools/templates/_helpers.tpl | 50 +++++++++++++++++++ helm/kagent-tools/templates/_images.tpl | 32 ++++++++++++ helm/kagent-tools/templates/clusterrole.yaml | 7 +-- .../templates/clusterrolebinding.yaml | 5 +- helm/kagent-tools/templates/deployment.yaml | 9 ++-- helm/kagent-tools/tests/rbac_test.yaml | 29 +++++++++++ helm/kagent-tools/values.yaml | 21 +++++++- 7 files changed, 140 insertions(+), 13 deletions(-) create mode 100644 helm/kagent-tools/templates/_images.tpl diff --git a/helm/kagent-tools/templates/_helpers.tpl b/helm/kagent-tools/templates/_helpers.tpl index f2d27f3..f98bd5a 100644 --- a/helm/kagent-tools/templates/_helpers.tpl +++ b/helm/kagent-tools/templates/_helpers.tpl @@ -88,6 +88,26 @@ Precedence: controller.watchNamespaces (explicit override) > rbac.namespaces > e {{/* Guards on the rbac block */}} +{{/* +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, and an explicit empty +list forces cluster-scoped RBAC (hasKey, not coalesce, so a present-but-empty key +wins). On the global path the install namespace is auto-appended: the global is a +shared signal a parent may aim at other charts, and failing this chart's render +over it would brick an install the value was never about. +*/}} +{{- define "kagent-tools.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 (list (include "kagent-tools.namespace" .)) -}} +{{- end -}} +{{- $scope | uniq | sortAlpha | toJson -}} +{{- end -}} + {{- define "kagent-tools.rbac.validate" -}} {{- 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=[, ...] for namespaced RBAC." -}} @@ -99,3 +119,33 @@ Guards on the rbac block {{- end -}} {{- end -}} {{- end -}} + +{{/* +Pull secrets for the pod: the chart's own list merged (union) with +global.imagePullSecrets. Renders nothing when both are empty. +*/}} +{{- define "kagent-tools.imagePullSecrets" -}} +{{- $merged := concat (.Values.imagePullSecrets | default list) (((.Values.global).imagePullSecrets) | default list) | uniq -}} +{{- if $merged -}} +imagePullSecrets: +{{- toYaml $merged | nindent 2 }} +{{- end -}} +{{- end -}} + +{{/* +imagePullPolicy for a container: the component's own value, then +global.imagePullPolicy, then IfNotPresent. One definition so the fallback chain +cannot drift between pods. +*/}} +{{- define "kagent-tools.imagePullPolicy" -}} +{{- .local | default (((.root.Values.global)).imagePullPolicy) | default "IfNotPresent" -}} +{{- end -}} + +{{/* +The tools container image. Builds the image root from tools.image and resolves +it through kagent-tools.images.image, so the deployment carries one short call. +*/}} +{{- define "kagent-tools.image" -}} +{{- $root := dict "registry" .Values.tools.image.registry "repository" .Values.tools.image.repository "tag" (coalesce .Values.global.tag .Values.tools.image.tag .Chart.Version) -}} +{{- include "kagent-tools.images.image" (dict "imageRoot" $root "global" .Values.global) -}} +{{- end -}} diff --git a/helm/kagent-tools/templates/_images.tpl b/helm/kagent-tools/templates/_images.tpl new file mode 100644 index 0000000..7d97bc2 --- /dev/null +++ b/helm/kagent-tools/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-tools.images.image" ( dict "imageRoot" .Values.path.to.the.image "global" .Values.global "chart" .Chart ) }} +*/}} +{{- define "kagent-tools.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-tools/templates/clusterrole.yaml b/helm/kagent-tools/templates/clusterrole.yaml index 2ddb85d..4fc0909 100644 --- a/helm/kagent-tools/templates/clusterrole.yaml +++ b/helm/kagent-tools/templates/clusterrole.yaml @@ -74,7 +74,7 @@ - apiGroups: ["*"] resources: ["*"] verbs: ["*"] -{{- if not .Values.rbac.namespaces }} +{{- if not (include "kagent-tools.rbacNamespaces" . | fromJsonArray) }} - nonResourceURLs: ["*"] verbs: ["*"] {{- end }} @@ -83,8 +83,9 @@ {{- if and (not .Values.useDefaultServiceAccount) .Values.rbac.create }} {{- include "kagent-tools.rbac.validate" . -}} -{{- if .Values.rbac.namespaces }} -{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }} +{{- $rbacNamespaces := include "kagent-tools.rbacNamespaces" . | fromJsonArray }} +{{- if $rbacNamespaces }} +{{- range $namespace := $rbacNamespaces }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role diff --git a/helm/kagent-tools/templates/clusterrolebinding.yaml b/helm/kagent-tools/templates/clusterrolebinding.yaml index 4324043..40e94e5 100644 --- a/helm/kagent-tools/templates/clusterrolebinding.yaml +++ b/helm/kagent-tools/templates/clusterrolebinding.yaml @@ -1,8 +1,9 @@ {{- if and (not .Values.useDefaultServiceAccount) .Values.rbac.create }} {{- include "kagent-tools.rbac.validate" . -}} -{{- if .Values.rbac.namespaces }} -{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }} +{{- $rbacNamespaces := include "kagent-tools.rbacNamespaces" . | fromJsonArray }} +{{- if $rbacNamespaces }} +{{- range $namespace := $rbacNamespaces }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding diff --git a/helm/kagent-tools/templates/deployment.yaml b/helm/kagent-tools/templates/deployment.yaml index c92c9c6..d2cca92 100644 --- a/helm/kagent-tools/templates/deployment.yaml +++ b/helm/kagent-tools/templates/deployment.yaml @@ -19,10 +19,7 @@ spec: labels: {{- include "kagent-tools.selectorLabels" . | nindent 8 }} spec: - {{- with .Values.imagePullSecrets }} - imagePullSecrets: - {{- toYaml . | nindent 8 }} - {{- end }} + {{- with include "kagent-tools.imagePullSecrets" . }}{{- . | nindent 6 }}{{- end }} {{- with .Values.nodeSelector }} nodeSelector: {{- toYaml . | nindent 8 }} @@ -69,8 +66,8 @@ spec: {{- end }} securityContext: {{- toYaml .Values.securityContext | nindent 12 }} - image: "{{ .Values.tools.image.registry }}/{{ .Values.tools.image.repository }}:{{ coalesce .Values.global.tag .Values.tools.image.tag .Chart.Version }}" - imagePullPolicy: {{ .Values.tools.image.pullPolicy }} + image: {{ include "kagent-tools.image" . | quote }} + imagePullPolicy: {{ include "kagent-tools.imagePullPolicy" (dict "root" . "local" .Values.tools.image.pullPolicy) }} resources: {{- toYaml .Values.tools.resources | nindent 12 }} env: diff --git a/helm/kagent-tools/tests/rbac_test.yaml b/helm/kagent-tools/tests/rbac_test.yaml index 41a991e..980f5ef 100644 --- a/helm/kagent-tools/tests/rbac_test.yaml +++ b/helm/kagent-tools/tests/rbac_test.yaml @@ -131,3 +131,32 @@ tests: path: metadata.namespace value: other-ns documentIndex: 1 + + - it: should render namespaced RBAC from global.watchNamespaces alone + set: + global.watchNamespaces: + - NAMESPACE + - other + template: clusterrole.yaml + asserts: + - containsDocument: + apiVersion: rbac.authorization.k8s.io/v1 + kind: Role + namespace: other + any: true + + - it: should let rbac.namespaces win over global.watchNamespaces + set: + rbac.namespaces: + - NAMESPACE + global.watchNamespaces: + - NAMESPACE + - other + template: clusterrole.yaml + asserts: + - not: true + containsDocument: + apiVersion: rbac.authorization.k8s.io/v1 + kind: Role + namespace: other + any: true diff --git a/helm/kagent-tools/values.yaml b/helm/kagent-tools/values.yaml index b398a00..45755f3 100644 --- a/helm/kagent-tools/values.yaml +++ b/helm/kagent-tools/values.yaml @@ -7,6 +7,19 @@ useDefaultServiceAccount: false global: tag: "" + # -- Registry that overrides the per-image registry when set. This is the + # air-gap mirror knob: one value redirects the image. Repository and tag stay + # per-image. For control without the override, leave this unset and set + # tools.image.registry. + imageRegistry: "" + # -- Pull secrets merged (union) into the pod's own imagePullSecrets list. + imagePullSecrets: [] + # -- Fallback imagePullPolicy when tools.image.pullPolicy is unset. + imagePullPolicy: "" + # -- Namespace scope fallback. rbac.namespaces overrides it when the key is + # present. A non-empty resolved list replaces the ClusterRole with a Role per + # namespace. + watchNamespaces: [] tools: metrics: @@ -32,7 +45,8 @@ tools: registry: ghcr.io repository: kagent-dev/kagent/tools tag: "" - pullPolicy: IfNotPresent + # -- Pull policy. Empty falls back to global.imagePullPolicy, then IfNotPresent. + pullPolicy: "" resources: requests: cpu: 100m @@ -107,7 +121,10 @@ rbac: # -- Namespaces in which to create Role and RoleBinding resources. # If empty (default), the chart creates cluster-scoped ClusterRole and ClusterRoleBinding resources. # If set, the chart creates a Role + RoleBinding per listed namespace (install namespace must be included). - namespaces: [] + # An explicit empty list forces cluster-scoped RBAC. The key ships commented + # out: a declared default [] would read as an explicit cluster-scoped choice, + # and the global.watchNamespaces fallback would be unreachable. + # namespaces: [] # When true, deploys a read-only ClusterRole (get, list, watch) instead of cluster-admin. # Pairs well with the --read-only CLI flag which disables write operations at the application layer. # Only applies when rbac.create is true.