From 55f1ad560b767c1bcbf4dab727dcbeaeaf63bb2b Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Wed, 9 Sep 2026 09:56:58 +0200 Subject: [PATCH] Validate and coerce listener metadata, and match Kubernetes key rules Follow-up review of the metadata validation surfaced four gaps: Listener pod metadata took the same unvalidated, uncoerced path that the runner pod metadata used to: an invalid label only failed once the controller created the listener pod, which is the silent failure mode this change set exists to remove. Both charts now validate it at render time and route its values through the string coercion. Values loaded from a values file arrive as float64, so "%v" rendered a large integer such as 12345678901234 in scientific notation, silently corrupting the annotation. Integral floats are now formatted without an exponent, and the remaining sites that quoted metadata values directly go through the same helper. A map or list value was flattened into Go's own formatting, producing a meaningless "map[a:b]" label. Such values are now rejected with the values path that produced them. The label key prefix check rejected dot-separated segments longer than 63 characters. Kubernetes only bounds the prefix at 253 characters in total, so the check was stricter than the API server and would have rejected keys that already work. While covering the listener path, the experimental chart turned out to render invalid YAML whenever listener.podTemplate carried both metadata and spec: the last metadata value and the following "spec:" key ended up on the same line. That is fixed here too, with a regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../templates/_helpers.tpl | 44 ++++++-- .../templates/_listener_template.tpl | 11 +- ...g_runner_set_metadata_validation_test.yaml | 95 ++++++++++++++++- .../templates/_helpers.tpl | 47 ++++++-- .../templates/autoscalingrunnerset.yaml | 24 ++++- .../templates/githubsecret.yaml | 2 +- .../templates/kube_mode_role.yaml | 2 +- .../templates/kube_mode_role_binding.yaml | 2 +- .../templates/kube_mode_serviceaccount.yaml | 2 +- .../templates/manager_role.yaml | 2 +- .../templates/manager_role_binding.yaml | 2 +- .../no_permission_serviceaccount.yaml | 2 +- .../tests/template_test.go | 100 ++++++++++++++++++ .../tests/values_scalar_metadata.yaml | 13 +++ 14 files changed, 319 insertions(+), 29 deletions(-) diff --git a/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl b/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl index 7ddf2220..27a4d353 100644 --- a/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl @@ -3,10 +3,18 @@ Render a labels or annotations map with all values coerced to strings. Kubernetes only accepts string values, so scalars such as `true` or `1` must not be rendered as YAML booleans or numbers. */}} +{{- define "metadata-value" -}} +{{- if and (kindIs "float64" .) (eq . (floor .)) -}} +{{- printf "%.0f" . -}} +{{- else -}} +{{- printf "%v" . -}} +{{- end -}} +{{- end }} + {{- define "string-map" -}} {{- $out := dict -}} {{- range $k, $v := . -}} -{{- $_ := set $out $k (printf "%v" $v) -}} +{{- $_ := set $out $k (include "metadata-value" $v) -}} {{- end -}} {{- toYaml $out -}} {{- end }} @@ -24,6 +32,19 @@ Expects a dict with "value" and "path". {{- end -}} {{- end }} +{{/* +Fail unless a metadata value is a scalar. A map or list would otherwise be flattened by the +string coercion into Go's own formatting (`map[a:b]`), which is a syntactically valid but +meaningless label or annotation, and a null would become "". +Expects a dict with "value", "key", "kind" and "path". +*/}} +{{- define "assert-scalar" -}} +{{- $value := .value -}} +{{- if or (kindIs "map" $value) (kindIs "slice" $value) (kindIs "invalid" $value) -}} +{{- fail (printf "%s: invalid value for %s %q: must be a scalar, got %s. Quote the value if it is meant to be a string" .path .kind .key (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). @@ -40,13 +61,11 @@ Expects a dict with "key", "kind" (label|annotation) and "path" (the values path {{- if eq (len $parts) 2 -}} {{- $prefix := index $parts 0 -}} {{- $name = index $parts 1 -}} -{{- if or (eq $prefix "") (gt (len $prefix) 253) -}} +{{- if gt (len $prefix) 253 -}} {{- fail (printf "%s: invalid %s key %q: the prefix %q must be a DNS subdomain of no more than 253 characters" $path $kind $key $prefix) -}} {{- end -}} -{{- range $_, $seg := splitList "." $prefix -}} -{{- if or (eq $seg "") (gt (len $seg) 63) (not (regexMatch "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$" $seg)) -}} -{{- fail (printf "%s: invalid %s key %q: the prefix %q must be a DNS subdomain, so each dot-separated segment must be no more than 63 characters, consist of lowercase alphanumeric characters or '-', and start and end with an alphanumeric character" $path $kind $key $prefix) -}} -{{- end -}} +{{- if not (regexMatch "^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$" $prefix) -}} +{{- fail (printf "%s: invalid %s key %q: the prefix %q must be a DNS subdomain, so it must consist of dot-separated segments of lowercase alphanumeric characters or '-', each starting and ending with an alphanumeric character" $path $kind $key $prefix) -}} {{- end -}} {{- end -}} {{- if or (eq $name "") (gt (len $name) 63) (not (regexMatch "^[A-Za-z0-9]([-A-Za-z0-9_.]*[A-Za-z0-9])?$" $name)) -}} @@ -68,7 +87,8 @@ Expects a dict with "metadata" and "path". {{- 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 -}} +{{- include "assert-scalar" (dict "value" $value "key" $key "kind" "label" "path" (printf "%s.labels" $path)) -}} +{{- $rendered := include "metadata-value" $value -}} {{- if gt (len $rendered) 63 -}} {{- fail (printf "%s.labels: invalid value %q for label %q: a label value must be no more than 63 characters" $path $rendered $key) -}} {{- end -}} @@ -80,6 +100,7 @@ Expects a dict with "metadata" and "path". {{- 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)) -}} +{{- include "assert-scalar" (dict "value" $value "key" $key "kind" "annotation" "path" (printf "%s.annotations" $path)) -}} {{- end -}} {{- end }} @@ -96,6 +117,13 @@ Validate every label and annotation map the chart can render onto resources it m {{- $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") -}} +{{- $listener := .Values.listener | default dict -}} +{{- if kindIs "map" $listener -}} +{{- $listenerPod := index $listener "podTemplate" -}} +{{- if kindIs "map" ($listenerPod | default dict) -}} +{{- include "validate-metadata" (dict "metadata" (index ($listenerPod | default dict) "metadata") "path" ".Values.listener.podTemplate.metadata") -}} +{{- end -}} +{{- end -}} {{- end }} {{/* @@ -162,7 +190,7 @@ non-string YAML values, which Kubernetes rejects for labels and annotations. {{- $processed := dict -}} {{- range $key, $value := $userLabels -}} {{- if not (hasPrefix "actions.github.com/" $key) -}} - {{- $_ := set $processed $key (printf "%v" $value) -}} + {{- $_ := set $processed $key (include "metadata-value" $value) -}} {{- end -}} {{- end -}} {{- if not (empty $processed) -}} diff --git a/charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl b/charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl index 5236a2ce..e7eac22d 100644 --- a/charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl +++ b/charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl @@ -5,9 +5,16 @@ {{- fail ".Values.listener.podTemplate must have at least metadata or spec defined" -}} {{- end -}} {{- with $metadata -}} +{{- $out := omit . "labels" "annotations" -}} +{{- with .labels -}} +{{- $_ := set $out "labels" (fromYaml (include "string-map" .)) -}} +{{- end -}} +{{- with .annotations -}} +{{- $_ := set $out "annotations" (fromYaml (include "string-map" .)) -}} +{{- end -}} metadata: - {{- toYaml . | nindent 2 }} -{{- end }} + {{- toYaml $out | nindent 2 }} +{{ end }} {{- with $spec -}} spec: {{- $containers := (index . "containers" | default (list)) -}} diff --git a/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_metadata_validation_test.yaml b/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_metadata_validation_test.yaml index 97cfc980..ca97ac53 100644 --- a/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_metadata_validation_test.yaml +++ b/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_metadata_validation_test.yaml @@ -38,7 +38,7 @@ tests: namespace: "test-namespace" asserts: - failedTemplate: - errorMessage: '.Values.runner.pod.metadata.labels: invalid label key "Invalid.Prefix/purpose": the prefix "Invalid.Prefix" must be a DNS subdomain, so each dot-separated segment must be no more than 63 characters, consist of lowercase alphanumeric characters or ''-'', and start and end with an alphanumeric character' + errorMessage: '.Values.runner.pod.metadata.labels: invalid label key "Invalid.Prefix/purpose": the prefix "Invalid.Prefix" must be a DNS subdomain, so it must consist of dot-separated segments of lowercase alphanumeric characters or ''-'', each starting and ending with an alphanumeric character' - it: should fail when a resource label value is not a valid Kubernetes label value set: @@ -161,3 +161,96 @@ tests: asserts: - failedTemplate: errorMessage: '.Values.resource.ephemeralRunner: must be a mapping, got string' + + - it: should fail when a listener pod label value is invalid + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + listener: + podTemplate: + metadata: + labels: + purpose: "“true”" + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - failedTemplate: + errorMessage: '.Values.listener.podTemplate.metadata.labels: invalid value "“true”" for label "purpose": a valid label value must be an empty string or consist of alphanumeric characters, ''-'', ''_'' or ''.'', and must start and end with an alphanumeric character' + + - it: should fail when a metadata value is a mapping instead of a scalar + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + runner: + pod: + metadata: + annotations: + nested: + inner: "value" + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - failedTemplate: + errorMessage: '.Values.runner.pod.metadata.annotations: invalid value for annotation "nested": must be a scalar, got map. Quote the value if it is meant to be a string' + + - it: should accept a prefix segment longer than 63 characters, matching Kubernetes + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + runner: + pod: + metadata: + labels: + ? "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/purpose" + : "yes" + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: spec.template.metadata.labels["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/purpose"] + value: "yes" + + # A listener pod template carrying both metadata and spec used to render "true" and the + # following "spec:" key onto the same line, producing invalid YAML. + - it: should render listener metadata and spec together, coercing scalar values to strings + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + listener: + podTemplate: + metadata: + labels: + listener-bool: true + annotations: + listener-int: 11 + spec: + containers: + - name: listener + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: spec.listenerTemplate.metadata.labels.listener-bool + value: "true" + - equal: + path: spec.listenerTemplate.metadata.annotations.listener-int + value: "11" + - equal: + path: spec.listenerTemplate.spec.containers[0].name + value: "listener" diff --git a/charts/gha-runner-scale-set/templates/_helpers.tpl b/charts/gha-runner-scale-set/templates/_helpers.tpl index 3c1c395c..f176927c 100644 --- a/charts/gha-runner-scale-set/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set/templates/_helpers.tpl @@ -54,6 +54,20 @@ app.kubernetes.io/name: {{ include "gha-runner-scale-set.scale-set-name" . }} app.kubernetes.io/instance: {{ include "gha-runner-scale-set.scale-set-name" . }} {{- end }} +{{/* +Render a single label or annotation value as a string. +Values from a values file arrive as float64, so "%v" would turn large integers into +scientific notation (12345678901234 -> 1.2345678901234e+13) and silently write a value the +user never asked for. Integral floats are therefore formatted without an exponent. +*/}} +{{- define "gha-runner-scale-set.metadataValue" -}} +{{- if and (kindIs "float64" .) (eq . (floor .)) -}} +{{- printf "%.0f" . -}} +{{- else -}} +{{- printf "%v" . -}} +{{- end -}} +{{- end }} + {{/* Render a labels or annotations map with all values coerced to strings. Kubernetes only accepts string values, so scalars such as `true` or `1` must not be @@ -62,7 +76,7 @@ rendered as YAML booleans or numbers. {{- define "gha-runner-scale-set.stringMap" -}} {{- $out := dict -}} {{- range $k, $v := . -}} -{{- $_ := set $out $k (printf "%v" $v) -}} +{{- $_ := set $out $k (include "gha-runner-scale-set.metadataValue" $v) -}} {{- end -}} {{- toYaml $out -}} {{- end }} @@ -96,13 +110,11 @@ Expects a dict with "key", "kind" (label|annotation) and "path" (the values path {{- if eq (len $parts) 2 -}} {{- $prefix := index $parts 0 -}} {{- $name = index $parts 1 -}} -{{- if or (eq $prefix "") (gt (len $prefix) 253) -}} +{{- if gt (len $prefix) 253 -}} {{- fail (printf "%s: invalid %s key %q: the prefix %q must be a DNS subdomain of no more than 253 characters" $path $kind $key $prefix) -}} {{- end -}} -{{- range $_, $seg := splitList "." $prefix -}} -{{- if or (eq $seg "") (gt (len $seg) 63) (not (regexMatch "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$" $seg)) -}} -{{- fail (printf "%s: invalid %s key %q: the prefix %q must be a DNS subdomain, so each dot-separated segment must be no more than 63 characters, consist of lowercase alphanumeric characters or '-', and start and end with an alphanumeric character" $path $kind $key $prefix) -}} -{{- end -}} +{{- if not (regexMatch "^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$" $prefix) -}} +{{- fail (printf "%s: invalid %s key %q: the prefix %q must be a DNS subdomain, so it must consist of dot-separated segments of lowercase alphanumeric characters or '-', each starting and ending with an alphanumeric character" $path $kind $key $prefix) -}} {{- end -}} {{- end -}} {{- if or (eq $name "") (gt (len $name) 63) (not (regexMatch "^[A-Za-z0-9]([-A-Za-z0-9_.]*[A-Za-z0-9])?$" $name)) -}} @@ -110,6 +122,19 @@ Expects a dict with "key", "kind" (label|annotation) and "path" (the values path {{- end -}} {{- end }} +{{/* +Fail unless a metadata value is a scalar. A map or list would otherwise be flattened by the +string coercion into Go's own formatting (`map[a:b]`), which is a syntactically valid but +meaningless label or annotation, and a null would become "". +Expects a dict with "value", "key", "kind" and "path". +*/}} +{{- define "gha-runner-scale-set.assertScalar" -}} +{{- $value := .value -}} +{{- if or (kindIs "map" $value) (kindIs "slice" $value) (kindIs "invalid" $value) -}} +{{- fail (printf "%s: invalid value for %s %q: must be a scalar, got %s. Quote the value if it is meant to be a string" .path .kind .key (kindOf $value)) -}} +{{- end -}} +{{- end }} + {{/* Validate a map of labels. Invalid labels are only rejected once the controller creates the runner pod, which leaves the scale set without runners, so fail at render time instead. @@ -120,7 +145,8 @@ Expects a dict with "labels" and "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 -}} +{{- include "gha-runner-scale-set.assertScalar" (dict "value" $value "key" $key "kind" "label" "path" $path) -}} +{{- $rendered := include "gha-runner-scale-set.metadataValue" $value -}} {{- if gt (len $rendered) 63 -}} {{- fail (printf "%s: invalid value %q for label %q: a label value must be no more than 63 characters" $path $rendered $key) -}} {{- end -}} @@ -139,6 +165,7 @@ Expects a dict with "annotations" and "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) -}} +{{- include "gha-runner-scale-set.assertScalar" (dict "value" $value "key" $key "kind" "annotation" "path" $path) -}} {{- end -}} {{- end }} @@ -154,6 +181,12 @@ Validate every label and annotation map the chart can render onto resources it m {{- $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.listenerTemplate "path" ".Values.listenerTemplate") -}} +{{- $listenerMetadata := index (.Values.listenerTemplate | default dict) "metadata" -}} +{{- include "gha-runner-scale-set.assertMap" (dict "value" $listenerMetadata "path" ".Values.listenerTemplate.metadata") -}} +{{- $listenerMetadata = $listenerMetadata | default dict -}} +{{- include "gha-runner-scale-set.validateLabels" (dict "labels" (index $listenerMetadata "labels") "path" ".Values.listenerTemplate.metadata.labels") -}} +{{- include "gha-runner-scale-set.validateAnnotations" (dict "annotations" (index $listenerMetadata "annotations") "path" ".Values.listenerTemplate.metadata.annotations") -}} {{- include "gha-runner-scale-set.assertMap" (dict "value" .Values.resourceMeta "path" ".Values.resourceMeta") -}} {{- range $resource, $meta := (.Values.resourceMeta | default dict) }} {{- include "gha-runner-scale-set.assertMap" (dict "value" $meta "path" (printf ".Values.resourceMeta.%s" $resource)) -}} diff --git a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml index b5f5f34c..41babfc0 100644 --- a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml +++ b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml @@ -20,7 +20,7 @@ metadata: {{- with .Values.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} @@ -28,7 +28,7 @@ metadata: {{- with .Values.resourceMeta.autoscalingRunnerSet.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} @@ -39,7 +39,7 @@ metadata: {{- with .Values.annotations }} {{- range $k, $v := . }} {{- if not (or (hasPrefix "actions.github.com/cleanup-" $k) (eq $k "actions.github.com/values-hash")) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} @@ -47,7 +47,7 @@ metadata: {{- with .Values.resourceMeta.autoscalingRunnerSet.annotations }} {{- range $k, $v := . }} {{- if not (or (hasPrefix "actions.github.com/cleanup-" $k) (eq $k "actions.github.com/values-hash")) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} @@ -161,7 +161,23 @@ spec: {{- with .Values.listenerTemplate }} listenerTemplate: + {{- with .metadata }} + metadata: + {{- with .labels }} + labels: + {{- include "gha-runner-scale-set.stringMap" . | nindent 8 }} + {{- end }} + {{- with .annotations }} + annotations: + {{- include "gha-runner-scale-set.stringMap" . | nindent 8 }} + {{- end }} + {{- with omit . "labels" "annotations" }} + {{- toYaml . | nindent 6 }} + {{- end }} + {{- end }} + {{- with omit . "metadata" }} {{- toYaml . | nindent 4}} + {{- end }} {{- end }} {{- with .Values.listenerMetrics }} diff --git a/charts/gha-runner-scale-set/templates/githubsecret.yaml b/charts/gha-runner-scale-set/templates/githubsecret.yaml index 5df122b0..509f5aa8 100644 --- a/charts/gha-runner-scale-set/templates/githubsecret.yaml +++ b/charts/gha-runner-scale-set/templates/githubsecret.yaml @@ -13,7 +13,7 @@ metadata: {{- with .Values.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} diff --git a/charts/gha-runner-scale-set/templates/kube_mode_role.yaml b/charts/gha-runner-scale-set/templates/kube_mode_role.yaml index 5a6cb7c4..ef9743cf 100644 --- a/charts/gha-runner-scale-set/templates/kube_mode_role.yaml +++ b/charts/gha-runner-scale-set/templates/kube_mode_role.yaml @@ -15,7 +15,7 @@ metadata: {{- with .Values.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} diff --git a/charts/gha-runner-scale-set/templates/kube_mode_role_binding.yaml b/charts/gha-runner-scale-set/templates/kube_mode_role_binding.yaml index bf1c6cf0..9b7cd1cf 100644 --- a/charts/gha-runner-scale-set/templates/kube_mode_role_binding.yaml +++ b/charts/gha-runner-scale-set/templates/kube_mode_role_binding.yaml @@ -14,7 +14,7 @@ metadata: {{- with .Values.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} diff --git a/charts/gha-runner-scale-set/templates/kube_mode_serviceaccount.yaml b/charts/gha-runner-scale-set/templates/kube_mode_serviceaccount.yaml index d6255d3d..094897d3 100644 --- a/charts/gha-runner-scale-set/templates/kube_mode_serviceaccount.yaml +++ b/charts/gha-runner-scale-set/templates/kube_mode_serviceaccount.yaml @@ -25,7 +25,7 @@ metadata: {{- with .Values.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} diff --git a/charts/gha-runner-scale-set/templates/manager_role.yaml b/charts/gha-runner-scale-set/templates/manager_role.yaml index fdb41eea..59873cc2 100644 --- a/charts/gha-runner-scale-set/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set/templates/manager_role.yaml @@ -12,7 +12,7 @@ metadata: {{- with .Values.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} diff --git a/charts/gha-runner-scale-set/templates/manager_role_binding.yaml b/charts/gha-runner-scale-set/templates/manager_role_binding.yaml index c4296b32..45379ac9 100644 --- a/charts/gha-runner-scale-set/templates/manager_role_binding.yaml +++ b/charts/gha-runner-scale-set/templates/manager_role_binding.yaml @@ -12,7 +12,7 @@ metadata: {{- with .Values.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} diff --git a/charts/gha-runner-scale-set/templates/no_permission_serviceaccount.yaml b/charts/gha-runner-scale-set/templates/no_permission_serviceaccount.yaml index a14f3531..e44c4c60 100644 --- a/charts/gha-runner-scale-set/templates/no_permission_serviceaccount.yaml +++ b/charts/gha-runner-scale-set/templates/no_permission_serviceaccount.yaml @@ -14,7 +14,7 @@ metadata: {{- with .Values.labels }} {{- range $k, $v := . }} {{- if not (or (hasKey $reserved $k) (hasPrefix "actions.github.com/" $k)) }} - {{ $k }}: {{ $v | quote }} + {{ $k }}: {{ include "gha-runner-scale-set.metadataValue" $v | quote }} {{- end }} {{- end }} {{- end }} diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index 46f7470a..fff892d9 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -3289,12 +3289,19 @@ func TestTemplateRenderedAutoScalingRunnerSet_ScalarMetadataValuesAreRenderedAsS 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, "12345678901234", autoscalingRunnerSet.Annotations["chart-big-int-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.ListenerTemplate) + assert.Equal(t, "true", autoscalingRunnerSet.Spec.ListenerTemplate.Labels["listener-bool"]) + assert.Equal(t, "11", autoscalingRunnerSet.Spec.ListenerTemplate.Annotations["listener-int-annotation"]) + require.Len(t, autoscalingRunnerSet.Spec.ListenerTemplate.Spec.Containers, 1) + assert.Equal(t, "listener", autoscalingRunnerSet.Spec.ListenerTemplate.Spec.Containers[0].Name) + 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"]) @@ -3336,6 +3343,11 @@ func TestTemplateRenderedAutoScalingRunnerSet_NonMapMetadataValidationError(t *t value: "oops", expectedError: ".Values.resourceMeta.githubConfigSecret: must be a mapping, got string", }, + "listener metadata is not a map": { + key: "listenerTemplate.metadata", + value: "oops", + expectedError: ".Values.listenerTemplate.metadata: must be a mapping, got string", + }, } for name, tc := range tt { @@ -3363,3 +3375,91 @@ func TestTemplateRenderedAutoScalingRunnerSet_NonMapMetadataValidationError(t *t }) } } + +func TestTemplateRenderedAutoScalingRunnerSet_ListenerMetadataIsValidated(t *testing.T) { + t.Parallel() + + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + 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", + "listenerTemplate.metadata.labels.purpose": "“true”", + "listenerTemplate.spec.containers[0].name": "listener", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + _, err = helm.RenderTemplateE(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) + require.Error(t, err) + assert.Contains(t, err.Error(), `.Values.listenerTemplate.metadata.labels: invalid value "“true”" for label "purpose"`) +} + +func TestTemplateRenderedAutoScalingRunnerSet_NonScalarMetadataValueValidationError(t *testing.T) { + t.Parallel() + + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + 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", + }, + SetJsonValues: map[string]string{ + "template.metadata.annotations": `{"nested":{"inner":"value"}}`, + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + _, err = helm.RenderTemplateE(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) + require.Error(t, err) + assert.Contains(t, err.Error(), `.Values.template.metadata.annotations: invalid value for annotation "nested": must be a scalar, got map`) +} + +// Kubernetes only bounds a label key prefix at 253 characters in total, so the chart must +// not impose the stricter per-segment 63 character limit that applies to DNS labels. +func TestTemplateRenderedAutoScalingRunnerSet_LongPrefixSegmentIsAccepted(t *testing.T) { + t.Parallel() + + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + releaseName := "test-runners" + namespaceName := "test-" + strings.ToLower(random.UniqueID()) + + key := strings.Repeat("a", 64) + ".example.com/purpose" + + 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", + "template.metadata.labels." + strings.ReplaceAll(key, ".", `\.`): "yes", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + 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, "yes", autoscalingRunnerSet.Spec.Template.Labels[key]) +} diff --git a/charts/gha-runner-scale-set/tests/values_scalar_metadata.yaml b/charts/gha-runner-scale-set/tests/values_scalar_metadata.yaml index c1898e78..9a5f497c 100644 --- a/charts/gha-runner-scale-set/tests/values_scalar_metadata.yaml +++ b/charts/gha-runner-scale-set/tests/values_scalar_metadata.yaml @@ -14,6 +14,9 @@ labels: annotations: chart-bool-annotation: false chart-float-annotation: 1.5 + # Large integers must not be rendered in scientific notation. Values loaded from a file + # arrive as float64, so a naive "%v" would produce "1.2345678901234e+13". + chart-big-int-annotation: 12345678901234 template: metadata: @@ -34,3 +37,13 @@ resourceMeta: githubConfigSecret: labels: secret-int: 5 + +listenerTemplate: + metadata: + labels: + listener-bool: true + annotations: + listener-int-annotation: 11 + spec: + containers: + - name: listener