Fail with the values path when metadata is not a mapping

The validation helpers ranged over labels/annotations without checking they were maps, and silently skipped non-map resourceMeta entries. A mis-typed value therefore escaped validation and failed later with an opaque error such as 'range can't iterate over oops' or 'can't evaluate field labels in type interface {}', which is the class of error this validation exists to replace. Guard every metadata map with an explicit type check that names the values path, and run the validation from every template that renders metadata so the reported error does not depend on which template Helm renders first. Also add coverage for scalar coercion driven by a values file. SetValues only ever produces strings, so the existing tests could not exercise the bool/number values that stringMap is there to handle. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-10-08 01:42:51 +02:00 · 2026-09-09 09:41:19 +02:00
parent e4a93fe4ae
commit 824e3b619b
20 changed files with 260 additions and 19 deletions
@@ -11,6 +11,19 @@ rendered as YAML booleans or numbers.
 {{- toYaml $out -}}
 {{- end }}

+{{/*
+Fail unless a value is absent or a mapping. Used to turn mis-typed metadata values into an
+error that names the values path, instead of an opaque "range can't iterate over" further
+down the render.
+Expects a dict with "value" and "path".
+*/}}
+{{- define "assert-map" -}}
+{{- $value := .value -}}
+{{- if and (not (kindIs "invalid" $value)) (not (kindIs "map" $value)) -}}
+{{- fail (printf "%s: must be a mapping, got %s" .path (kindOf $value)) -}}
+{{- end -}}
+{{- end }}
+
 {{/*
 Validate a label or annotation key against the Kubernetes qualified name rules.
 Expects a dict with "key", "kind" (label|annotation) and "path" (the values path used in the error message).
@@ -49,9 +62,11 @@ Expects a dict with "metadata" and "path".
 */}}
 {{- define "validate-metadata" -}}
 {{- $path := .path -}}
+{{- include "assert-map" (dict "value" .metadata "path" $path) -}}
 {{- $metadata := .metadata | default dict -}}
-{{- if kindIs "map" $metadata -}}
-{{- range $key, $value := ((index $metadata "labels") | default dict) -}}
+{{- $labels := index $metadata "labels" -}}
+{{- include "assert-map" (dict "value" $labels "path" (printf "%s.labels" $path)) -}}
+{{- range $key, $value := ($labels | default dict) -}}
 {{- include "validate-metadata-key" (dict "key" $key "kind" "label" "path" (printf "%s.labels" $path)) -}}
 {{- $rendered := printf "%v" $value -}}
 {{- if gt (len $rendered) 63 -}}
@@ -61,25 +76,26 @@ Expects a dict with "metadata" and "path".
 {{- fail (printf "%s.labels: invalid value %q for label %q: a valid label value must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character" $path $rendered $key) -}}
 {{- end -}}
 {{- end -}}
-{{- range $key, $value := ((index $metadata "annotations") | default dict) -}}
+{{- $annotations := index $metadata "annotations" -}}
+{{- include "assert-map" (dict "value" $annotations "path" (printf "%s.annotations" $path)) -}}
+{{- range $key, $value := ($annotations | default dict) -}}
 {{- include "validate-metadata-key" (dict "key" $key "kind" "annotation" "path" (printf "%s.annotations" $path)) -}}
 {{- end -}}
-{{- end -}}
 {{- end }}

 {{/*
 Validate every label and annotation map the chart can render onto resources it manages.
 */}}
 {{- define "validate-all-metadata" -}}
+{{- include "assert-map" (dict "value" .Values.resource "path" ".Values.resource") -}}
 {{- range $resource, $config := (.Values.resource | default dict) }}
-{{- if kindIs "map" $config }}
-{{- include "validate-metadata" (dict "metadata" (index $config "metadata") "path" (printf ".Values.resource.%s.metadata" $resource)) -}}
-{{- end }}
-{{- end }}
-{{- $runnerPod := (index (.Values.runner | default dict) "pod") | default dict }}
-{{- if kindIs "map" $runnerPod }}
-{{- include "validate-metadata" (dict "metadata" (index $runnerPod "metadata") "path" ".Values.runner.pod.metadata") -}}
+{{- include "assert-map" (dict "value" $config "path" (printf ".Values.resource.%s" $resource)) -}}
+{{- include "validate-metadata" (dict "metadata" (index ($config | default dict) "metadata") "path" (printf ".Values.resource.%s.metadata" $resource)) -}}
 {{- end }}
+{{- include "assert-map" (dict "value" .Values.runner "path" ".Values.runner") -}}
+{{- $runnerPod := index (.Values.runner | default dict) "pod" -}}
+{{- include "assert-map" (dict "value" $runnerPod "path" ".Values.runner.pod") -}}
+{{- include "validate-metadata" (dict "metadata" (index ($runnerPod | default dict) "metadata") "path" ".Values.runner.pod.metadata") -}}
 {{- end }}

 {{/*
@@ -1,3 +1,4 @@
+{{- include "validate-all-metadata" . }}
 {{- $usesKubernetesSecrets := or (not .Values.secretResolution) (eq .Values.secretResolution.type "kubernetes") -}}

 {{- if and (not $usesKubernetesSecrets) (empty .Values.auth.secretName) -}}
@@ -1,3 +1,4 @@
+{{- include "validate-all-metadata" . }}
 {{- $runner := (.Values.runner | default dict) -}}
 {{- $runnerMode := (index $runner "mode" | default "") -}}
 {{- $kubeMode := (index $runner "kubernetesMode" | default dict) -}}
@@ -1,3 +1,4 @@
+{{- include "validate-all-metadata" . }}
 {{- $runner := (.Values.runner | default dict) -}}
 {{- $runnerMode := (index $runner "mode" | default "") -}}
 {{- $kubeMode := (index $runner "kubernetesMode" | default dict) -}}
@@ -1,3 +1,4 @@
+{{- include "validate-all-metadata" . }}
 {{- $runner := (.Values.runner | default dict) -}}
 {{- $runnerMode := (index $runner "mode" | default "") -}}
 {{- $kubeMode := (index $runner "kubernetesMode" | default dict) -}}
@@ -1,3 +1,4 @@
+{{- include "validate-all-metadata" . }}
 {{- $runner := (.Values.runner | default dict) -}}
 {{- $runnerMode := (index $runner "mode" | default "") -}}
 {{- $kubeMode := (index $runner "kubernetesMode" | default dict) -}}
@@ -1,3 +1,4 @@
+{{- include "validate-all-metadata" . }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
@@ -1,3 +1,4 @@
+{{- include "validate-all-metadata" . }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
 metadata:
@@ -1,3 +1,4 @@
+{{- include "validate-all-metadata" . }}
 {{- $runnerMode := (.Values.runner.mode | default "") -}}
 {{- if ne $runnerMode "kubernetes" -}}
 apiVersion: v1
@@ -110,3 +110,54 @@ tests:
      - equal:
          path: spec.ephemeralRunnerMetadata.annotations["enabled"]
          value: "true"
+
+  - it: should fail when a metadata block is not a mapping
+    set:
+      scaleset.name: "test"
+      auth.url: "https://github.com/org"
+      auth.githubToken: "gh_token12345"
+      controllerServiceAccount.name: "arc"
+      controllerServiceAccount.namespace: "arc-system"
+      runner:
+        pod:
+          metadata: "oops"
+    release:
+      name: "test-name"
+      namespace: "test-namespace"
+    asserts:
+      - failedTemplate:
+          errorMessage: '.Values.runner.pod.metadata: must be a mapping, got string'
+
+  - it: should fail when a labels block is not a mapping
+    set:
+      scaleset.name: "test"
+      auth.url: "https://github.com/org"
+      auth.githubToken: "gh_token12345"
+      controllerServiceAccount.name: "arc"
+      controllerServiceAccount.namespace: "arc-system"
+      resource:
+        all:
+          metadata:
+            labels: "oops"
+    release:
+      name: "test-name"
+      namespace: "test-namespace"
+    asserts:
+      - failedTemplate:
+          errorMessage: '.Values.resource.all.metadata.labels: must be a mapping, got string'
+
+  - it: should fail when a resource entry is not a mapping
+    set:
+      scaleset.name: "test"
+      auth.url: "https://github.com/org"
+      auth.githubToken: "gh_token12345"
+      controllerServiceAccount.name: "arc"
+      controllerServiceAccount.namespace: "arc-system"
+      resource:
+        ephemeralRunner: "oops"
+    release:
+      name: "test-name"
+      namespace: "test-namespace"
+    asserts:
+      - failedTemplate:
+          errorMessage: '.Values.resource.ephemeralRunner: must be a mapping, got string'
@@ -67,6 +67,19 @@ rendered as YAML booleans or numbers.
 {{- toYaml $out -}}
 {{- end }}

+{{/*
+Fail unless a value is absent or a mapping. Used to turn mis-typed metadata values into an
+error that names the values path, instead of an opaque "range can't iterate over" further
+down the render.
+Expects a dict with "value" and "path".
+*/}}
+{{- define "gha-runner-scale-set.assertMap" -}}
+{{- $value := .value -}}
+{{- if and (not (kindIs "invalid" $value)) (not (kindIs "map" $value)) -}}
+{{- fail (printf "%s: must be a mapping, got %s" .path (kindOf $value)) -}}
+{{- end -}}
+{{- end }}
+
 {{/*
 Validate a label or annotation key against the Kubernetes qualified name rules.
 Expects a dict with "key", "kind" (label|annotation) and "path" (the values path used in the error message).
@@ -104,6 +117,7 @@ Expects a dict with "labels" and "path".
 */}}
 {{- define "gha-runner-scale-set.validateLabels" -}}
 {{- $path := .path -}}
+{{- include "gha-runner-scale-set.assertMap" (dict "value" .labels "path" $path) -}}
 {{- range $key, $value := (.labels | default dict) -}}
 {{- include "gha-runner-scale-set.validateMetadataKey" (dict "key" $key "kind" "label" "path" $path) -}}
 {{- $rendered := printf "%v" $value -}}
@@ -122,6 +136,7 @@ Expects a dict with "annotations" and "path".
 */}}
 {{- define "gha-runner-scale-set.validateAnnotations" -}}
 {{- $path := .path -}}
+{{- include "gha-runner-scale-set.assertMap" (dict "value" .annotations "path" $path) -}}
 {{- range $key, $value := (.annotations | default dict) -}}
 {{- include "gha-runner-scale-set.validateMetadataKey" (dict "key" $key "kind" "annotation" "path" $path) -}}
 {{- end -}}
@@ -133,19 +148,20 @@ Validate every label and annotation map the chart can render onto resources it m
 {{- define "gha-runner-scale-set.validateMetadata" -}}
 {{- include "gha-runner-scale-set.validateLabels" (dict "labels" .Values.labels "path" ".Values.labels") -}}
 {{- include "gha-runner-scale-set.validateAnnotations" (dict "annotations" .Values.annotations "path" ".Values.annotations") -}}
-{{- with .Values.template }}
-{{- with .metadata }}
-{{- include "gha-runner-scale-set.validateLabels" (dict "labels" .labels "path" ".Values.template.metadata.labels") -}}
-{{- include "gha-runner-scale-set.validateAnnotations" (dict "annotations" .annotations "path" ".Values.template.metadata.annotations") -}}
-{{- end }}
-{{- end }}
+{{- include "gha-runner-scale-set.assertMap" (dict "value" .Values.template "path" ".Values.template") -}}
+{{- $templateMetadata := index (.Values.template | default dict) "metadata" -}}
+{{- include "gha-runner-scale-set.assertMap" (dict "value" $templateMetadata "path" ".Values.template.metadata") -}}
+{{- $templateMetadata = $templateMetadata | default dict -}}
+{{- include "gha-runner-scale-set.validateLabels" (dict "labels" (index $templateMetadata "labels") "path" ".Values.template.metadata.labels") -}}
+{{- include "gha-runner-scale-set.validateAnnotations" (dict "annotations" (index $templateMetadata "annotations") "path" ".Values.template.metadata.annotations") -}}
+{{- include "gha-runner-scale-set.assertMap" (dict "value" .Values.resourceMeta "path" ".Values.resourceMeta") -}}
 {{- range $resource, $meta := (.Values.resourceMeta | default dict) }}
-{{- if kindIs "map" $meta }}
+{{- include "gha-runner-scale-set.assertMap" (dict "value" $meta "path" (printf ".Values.resourceMeta.%s" $resource)) -}}
+{{- $meta = $meta | default dict -}}
 {{- include "gha-runner-scale-set.validateLabels" (dict "labels" (index $meta "labels") "path" (printf ".Values.resourceMeta.%s.labels" $resource)) -}}
 {{- include "gha-runner-scale-set.validateAnnotations" (dict "annotations" (index $meta "annotations") "path" (printf ".Values.resourceMeta.%s.annotations" $resource)) -}}
 {{- end }}
 {{- end }}
-{{- end }}

 {{/*
 Render a ResourceMeta block for AutoscalingRunnerSet spec fields.
@@ -1,3 +1,4 @@
+{{- include "gha-runner-scale-set.validateMetadata" . }}
 {{- if not (kindIs "string" .Values.githubConfigSecret) }}
 {{- $hasCustomResourceMeta := (and .Values.resourceMeta .Values.resourceMeta.githubConfigSecret) }}
 apiVersion: v1
@@ -1,3 +1,4 @@
+{{- include "gha-runner-scale-set.validateMetadata" . }}
 {{- $containerMode := .Values.containerMode }}
 {{- $hasCustomResourceMeta := (and .Values.resourceMeta .Values.resourceMeta.kubernetesModeRole) }}
 {{- if and (or (eq $containerMode.type "kubernetes") (eq $containerMode.type "kubernetes-novolume")) (not .Values.template.spec.serviceAccountName) }}
@@ -1,3 +1,4 @@
+{{- include "gha-runner-scale-set.validateMetadata" . }}
 {{- $containerMode := .Values.containerMode }}
 {{- $hasCustomResourceMeta := (and .Values.resourceMeta .Values.resourceMeta.kubernetesModeRoleBinding) }}
 {{- if and (or (eq $containerMode.type "kubernetes") (eq $containerMode.type "kubernetes-novolume")) (not .Values.template.spec.serviceAccountName) }}
@@ -1,3 +1,4 @@
+{{- include "gha-runner-scale-set.validateMetadata" . }}
 {{- $containerMode := .Values.containerMode }}
 {{- $hasCustomResourceMeta := (and .Values.resourceMeta .Values.resourceMeta.kubernetesModeServiceAccount) }}
 {{- if and (or (eq $containerMode.type "kubernetes") (eq $containerMode.type "kubernetes-novolume")) (not .Values.template.spec.serviceAccountName) }}
@@ -1,3 +1,4 @@
+{{- include "gha-runner-scale-set.validateMetadata" . }}
 {{- $hasCustomResourceMeta := (and .Values.resourceMeta .Values.resourceMeta.managerRole) }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
@@ -1,3 +1,4 @@
+{{- include "gha-runner-scale-set.validateMetadata" . }}
 {{- $hasCustomResourceMeta := (and .Values.resourceMeta .Values.resourceMeta.managerRoleBinding) }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
@@ -1,3 +1,4 @@
+{{- include "gha-runner-scale-set.validateMetadata" . }}
 {{- $hasCustomResourceMeta := (and .Values.resourceMeta .Values.resourceMeta.noPermissionServiceAccount) }}
 {{- $containerMode := .Values.containerMode }}
 {{- if and (ne $containerMode.type "kubernetes") (ne $containerMode.type "kubernetes-novolume") (not .Values.template.spec.serviceAccountName) }}
@@ -3256,3 +3256,110 @@ func TestTemplateRenderedAutoScalingRunnerSet_ValidMetadataIsAccepted(t *testing
 	assert.Equal(t, "", autoscalingRunnerSet.Spec.Template.Labels["empty"])
 	assert.Equal(t, "any value is allowed: ✅", autoscalingRunnerSet.Spec.Template.Annotations["example.com/an"])
 }
+
+// Kubernetes only accepts string label and annotation values. Values supplied as unquoted
+// YAML scalars parse as bools/numbers, so the chart has to coerce them when rendering.
+// SetValues always yields strings, so this has to come from a values file to be meaningful.
+func TestTemplateRenderedAutoScalingRunnerSet_ScalarMetadataValuesAreRenderedAsStrings(t *testing.T) {
+	t.Parallel()
+
+	helmChartPath, err := filepath.Abs("../../gha-runner-scale-set")
+	require.NoError(t, err)
+
+	testValuesPath, err := filepath.Abs("../tests/values_scalar_metadata.yaml")
+	require.NoError(t, err)
+
+	releaseName := "test-runners"
+	namespaceName := "test-" + strings.ToLower(random.UniqueID())
+
+	options := &helm.Options{
+		Logger:         logger.Discard,
+		ValuesFiles:    []string{testValuesPath},
+		KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName),
+	}
+
+	// UnmarshalK8SYaml fails outright if a value decodes as a bool or number rather than a
+	// string, so a successful decode is itself part of the assertion.
+	output := helm.RenderTemplateContext(t, t.Context(), options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"})
+
+	var autoscalingRunnerSet v1alpha1.AutoscalingRunnerSet
+	helm.UnmarshalK8SYaml(t, output, &autoscalingRunnerSet)
+
+	assert.Equal(t, "true", autoscalingRunnerSet.Labels["chart-bool"])
+	assert.Equal(t, "1", autoscalingRunnerSet.Labels["chart-int"])
+	assert.Equal(t, "false", autoscalingRunnerSet.Annotations["chart-bool-annotation"])
+	assert.Equal(t, "1.5", autoscalingRunnerSet.Annotations["chart-float-annotation"])
+
+	assert.Equal(t, "true", autoscalingRunnerSet.Spec.Template.Labels["pod-bool"])
+	assert.Equal(t, "42", autoscalingRunnerSet.Spec.Template.Labels["pod-int"])
+	assert.Equal(t, "true", autoscalingRunnerSet.Spec.Template.Annotations["pod-bool-annotation"])
+	assert.Equal(t, "7", autoscalingRunnerSet.Spec.Template.Annotations["pod-int-annotation"])
+
+	require.NotNil(t, autoscalingRunnerSet.Spec.EphemeralRunnerMetadata)
+	assert.Equal(t, "false", autoscalingRunnerSet.Spec.EphemeralRunnerMetadata.Labels["runner-bool"])
+	assert.Equal(t, "3", autoscalingRunnerSet.Spec.EphemeralRunnerMetadata.Labels["runner-int"])
+	assert.Equal(t, "9", autoscalingRunnerSet.Spec.EphemeralRunnerMetadata.Annotations["runner-int-annotation"])
+
+	output = helm.RenderTemplateContext(t, t.Context(), options, helmChartPath, releaseName, []string{"templates/githubsecret.yaml"})
+
+	var githubSecret corev1.Secret
+	helm.UnmarshalK8SYaml(t, output, &githubSecret)
+
+	assert.Equal(t, "5", githubSecret.Labels["secret-int"])
+	assert.Equal(t, "true", githubSecret.Labels["chart-bool"])
+	assert.Equal(t, "1.5", githubSecret.Annotations["chart-float-annotation"])
+}
+
+func TestTemplateRenderedAutoScalingRunnerSet_NonMapMetadataValidationError(t *testing.T) {
+	t.Parallel()
+
+	helmChartPath, err := filepath.Abs("../../gha-runner-scale-set")
+	require.NoError(t, err)
+
+	tt := map[string]struct {
+		key           string
+		value         string
+		expectedError string
+	}{
+		"chart labels is not a map": {
+			key:           "labels",
+			value:         "oops",
+			expectedError: ".Values.labels: must be a mapping, got string",
+		},
+		"runner pod labels is not a map": {
+			key:           "template.metadata.labels",
+			value:         "oops",
+			expectedError: ".Values.template.metadata.labels: must be a mapping, got string",
+		},
+		"resourceMeta entry is not a map": {
+			key:           "resourceMeta.githubConfigSecret",
+			value:         "oops",
+			expectedError: ".Values.resourceMeta.githubConfigSecret: must be a mapping, got string",
+		},
+	}
+
+	for name, tc := range tt {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			releaseName := "test-runners"
+			namespaceName := "test-" + strings.ToLower(random.UniqueID())
+
+			options := &helm.Options{
+				Logger: logger.Discard,
+				SetValues: map[string]string{
+					"githubConfigUrl":                    "https://github.com/actions",
+					"githubConfigSecret.github_token":    "gh_token12345",
+					"controllerServiceAccount.name":      "arc",
+					"controllerServiceAccount.namespace": "arc-system",
+					tc.key:                               tc.value,
+				},
+				KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName),
+			}
+
+			_, err := helm.RenderTemplateContextE(t, t.Context(), options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"})
+			require.Error(t, err)
+			assert.ErrorContains(t, err, tc.expectedError)
+		})
+	}
+}
@@ -0,0 +1,36 @@
+githubConfigUrl: https://github.com/actions
+githubConfigSecret:
+  github_token: gh_token12345
+controllerServiceAccount:
+  name: arc
+  namespace: arc-system
+
+# Every value below is an unquoted YAML scalar, so it parses as a bool, int or float
+# rather than a string. Kubernetes only accepts string label and annotation values, so
+# the chart has to coerce these when rendering.
+labels:
+  chart-bool: true
+  chart-int: 1
+annotations:
+  chart-bool-annotation: false
+  chart-float-annotation: 1.5
+
+template:
+  metadata:
+    labels:
+      pod-bool: true
+      pod-int: 42
+    annotations:
+      pod-bool-annotation: true
+      pod-int-annotation: 7
+
+resourceMeta:
+  ephemeralRunner:
+    labels:
+      runner-bool: false
+      runner-int: 3
+    annotations:
+      runner-int-annotation: 9
+  githubConfigSecret:
+    labels:
+      secret-int: 5