From 00cb8e7e025ccc67e364c317f7758c2e704d1849 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Thu, 24 Sep 2026 12:32:24 +0200 Subject: [PATCH] Reject unsafe chart metadata integers (#4679) --- .../templates/_helpers.tpl | 15 +- ...g_runner_set_metadata_validation_test.yaml | 118 +++++++++++- .../tests/values_test.go | 55 ++++++ .../values.yaml | 68 +++---- .../templates/_helpers.tpl | 13 +- .../tests/template_test.go | 178 +++++++++++++++--- .../values_overlong_metadata_prefix.yaml | 9 + .../tests/values_scalar_metadata.yaml | 4 + .../tests/values_unsafe_metadata.yaml | 9 + 9 files changed, 399 insertions(+), 70 deletions(-) create mode 100644 charts/gha-runner-scale-set-experimental/tests/values_test.go create mode 100644 charts/gha-runner-scale-set/tests/values_overlong_metadata_prefix.yaml create mode 100644 charts/gha-runner-scale-set/tests/values_unsafe_metadata.yaml diff --git a/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl b/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl index 102d102e..fa2fbd34 100644 --- a/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl @@ -1,8 +1,8 @@ {{/* 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. +Values from a values file arrive as float64. Integral values in the IEEE 754 safe integer +range are formatted without an exponent. Unsafe integral values are rejected by assert-scalar +before reaching this helper, because their original value may already have been rounded. */}} {{- define "metadata-value" -}} {{- if eq . nil -}} @@ -50,6 +50,8 @@ Expects a dict with "value", "key", "kind" and "path". {{- $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)) -}} +{{- else if and (or (kindIs "int" $value) (kindIs "int64" $value) (and (kindIs "float64" $value) (eq $value (floor $value)))) (or (ge (float64 $value) 9007199254740992.0) (le (float64 $value) -9007199254740992.0)) -}} +{{- fail (printf "%s: invalid value for %s %q: unquoted integers outside the IEEE 754 safe range must be quoted to preserve their exact value" .path .kind .key) -}} {{- end -}} {{- end }} @@ -72,6 +74,11 @@ Expects a dict with "key", "kind" (label|annotation) and "path" (the values path {{- 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 $segment := splitList "." $prefix -}} +{{- if gt (len $segment) 63 -}} +{{- fail (printf "%s: invalid %s key %q: the prefix segment %q must be no more than 63 characters" $path $kind $key $segment) -}} +{{- end -}} +{{- 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 -}} @@ -318,5 +325,3 @@ Behavior: path: {{ $key | quote }} {{ end }} {{ end }} - - 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 ca97ac53..a5221451 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 @@ -92,6 +92,7 @@ tests: sync-wave: 1 annotations: enabled: true + quoted-unsafe-integer: "9007199254740993" runner: pod: metadata: @@ -110,6 +111,95 @@ tests: - equal: path: spec.ephemeralRunnerMetadata.annotations["enabled"] value: "true" + - equal: + path: spec.ephemeralRunnerMetadata.annotations["quoted-unsafe-integer"] + value: "9007199254740993" + + - it: should render the inclusive safe integer boundaries without an exponent + 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: + maximum-safe-integer: 9007199254740991 + annotations: + maximum-safe-integer: 9007199254740991 + minimum-safe-integer: -9007199254740991 + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: metadata.labels["maximum-safe-integer"] + value: "9007199254740991" + - equal: + path: metadata.annotations["maximum-safe-integer"] + value: "9007199254740991" + - equal: + path: metadata.annotations["minimum-safe-integer"] + value: "-9007199254740991" + + - it: should fail at the first unsafe positive integer + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + resource: + all: + metadata: + annotations: + unsafe-integer: 9007199254740992 + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - failedTemplate: + errorMessage: '.Values.resource.all.metadata.annotations: invalid value for annotation "unsafe-integer": unquoted integers outside the IEEE 754 safe range must be quoted to preserve their exact value' + + - it: should fail at the first unsafe negative integer + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + resource: + all: + metadata: + annotations: + unsafe-integer: -9007199254740992 + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - failedTemplate: + errorMessage: '.Values.resource.all.metadata.annotations: invalid value for annotation "unsafe-integer": unquoted integers outside the IEEE 754 safe range must be quoted to preserve their exact value' + + - it: should fail when an unquoted metadata integer is outside the IEEE 754 safe range + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + resource: + all: + metadata: + annotations: + unsafe-integer: 9007199254740993 + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - failedTemplate: + errorMessage: '.Values.resource.all.metadata.annotations: invalid value for annotation "unsafe-integer": unquoted integers outside the IEEE 754 safe range must be quoted to preserve their exact value' - it: should fail when a metadata block is not a mapping set: @@ -201,7 +291,28 @@ tests: - 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 + - it: should render a metadata prefix segment of exactly 63 characters + 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: + ? "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/purpose" + : "yes" + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: spec.template.metadata.labels["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/purpose"] + value: "yes" + + - it: should fail when a metadata prefix segment is longer than 63 characters set: scaleset.name: "test" auth.url: "https://github.com/org" @@ -218,9 +329,8 @@ tests: name: "test-name" namespace: "test-namespace" asserts: - - equal: - path: spec.template.metadata.labels["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/purpose"] - value: "yes" + - failedTemplate: + errorMessage: '.Values.runner.pod.metadata.labels: invalid label key "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/purpose": the prefix segment "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" must be no more than 63 characters' # 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. diff --git a/charts/gha-runner-scale-set-experimental/tests/values_test.go b/charts/gha-runner-scale-set-experimental/tests/values_test.go new file mode 100644 index 00000000..9a1fba3f --- /dev/null +++ b/charts/gha-runner-scale-set-experimental/tests/values_test.go @@ -0,0 +1,55 @@ +package tests + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestValuesExamplesAreTopLevel(t *testing.T) { + valuesPath, err := filepath.Abs("../values.yaml") + if err != nil { + t.Fatal(err) + } + + values, err := os.ReadFile(valuesPath) + if err != nil { + t.Fatal(err) + } + + for _, test := range []struct { + name string + marker string + example string + }{ + { + name: "proxy", + marker: "## Proxy can be used to define proxy settings", + example: "# proxy:", + }, + { + name: "github server TLS", + marker: "## A self-signed CA certificate for communication with the GitHub server", + example: "# githubServerTLS:", + }, + } { + t.Run(test.name, func(t *testing.T) { + markerIndex := strings.Index(string(values), test.marker) + if markerIndex == -1 { + t.Fatalf("could not find example marker %q", test.marker) + } + + exampleIndex := markerIndex + strings.Index(string(values[markerIndex:]), test.example) + if exampleIndex < markerIndex { + t.Fatalf("could not find example %q after marker %q", test.example, test.marker) + } + + lineStart := strings.LastIndex(string(values[:exampleIndex]), "\n") + 1 + lineEnd := exampleIndex + strings.Index(string(values[exampleIndex:]), "\n") + if got := string(values[lineStart:lineEnd]); got != test.example { + t.Errorf("example must be top-level: got %q, want %q", got, test.example) + } + }) + } +} diff --git a/charts/gha-runner-scale-set-experimental/values.yaml b/charts/gha-runner-scale-set-experimental/values.yaml index 80054bf6..a818a036 100644 --- a/charts/gha-runner-scale-set-experimental/values.yaml +++ b/charts/gha-runner-scale-set-experimental/values.yaml @@ -62,20 +62,41 @@ secretResolution: # tenant_id: "" # certificate_path: "" - ## Proxy can be used to define proxy settings that will be used by the - ## controller, the listener and the runner of this scale set. - # proxy: - # http: - # url: http://proxy.com:1234 - # credentialSecretRef: proxy-auth # a secret with `username` and `password` keys - # https: - # url: http://proxy.com:1234 - # credentialSecretRef: proxy-auth # a secret with `username` and `password` keys - # noProxy: - # - example.com - # - example.org +## Proxy can be used to define proxy settings that will be used by the +## controller, the listener and the runner of this scale set. +# proxy: +# http: +# url: http://proxy.com:1234 +# credentialSecretRef: proxy-auth # a secret with `username` and `password` keys +# https: +# url: http://proxy.com:1234 +# credentialSecretRef: proxy-auth # a secret with `username` and `password` keys +# noProxy: +# - example.com +# - example.org - ## Resource object allows modifying resources created by the chart itself +## A self-signed CA certificate for communication with the GitHub server can be +## provided using a config map key selector. If `runnerMountPath` is set, for +## each runner pod ARC will: +## - create a `github-server-tls-cert` volume containing the certificate +## specified in `certificateFrom` +## - mount that volume on path `runnerMountPath`/{certificate name} +## - set NODE_EXTRA_CA_CERTS environment variable to that same path +## - set RUNNER_UPDATE_CA_CERTS environment variable to "1" (as of version +## 2.303.0 this will instruct the runner to reload certificates on the host) +## +## If any of the above had already been set by the user in the runner pod +## template, ARC will observe those and not overwrite them. +## Example configuration: +# +# githubServerTLS: +# certificateFrom: +# configMapKeyRef: +# name: config-map-name +# key: ca.crt +# runnerMountPath: /usr/local/share/ca-certificates/ + +## Resource object allows modifying resources created by the chart itself resource: # Specifies metadata that will be applied to all resources managed by ARC all: @@ -269,27 +290,6 @@ runner: # spec: # containers: [] - ## A self-signed CA certificate for communication with the GitHub server can be - ## provided using a config map key selector. If `runnerMountPath` is set, for - ## each runner pod ARC will: - ## - create a `github-server-tls-cert` volume containing the certificate - ## specified in `certificateFrom` - ## - mount that volume on path `runnerMountPath`/{certificate name} - ## - set NODE_EXTRA_CA_CERTS environment variable to that same path - ## - set RUNNER_UPDATE_CA_CERTS environment variable to "1" (as of version - ## 2.303.0 this will instruct the runner to reload certificates on the host) - ## - ## If any of the above had already been set by the user in the runner pod - ## template, ARC will observe those and not overwrite them. - ## Example configuration: - # - # githubServerTLS: - # certificateFrom: - # configMapKeyRef: - # name: config-map-name - # key: ca.crt - # runnerMountPath: /usr/local/share/ca-certificates/ - ## controllerServiceAccount is the service account of the controller controllerServiceAccount: namespace: "" diff --git a/charts/gha-runner-scale-set/templates/_helpers.tpl b/charts/gha-runner-scale-set/templates/_helpers.tpl index 4780019b..b3a41ff9 100644 --- a/charts/gha-runner-scale-set/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set/templates/_helpers.tpl @@ -56,9 +56,9 @@ app.kubernetes.io/instance: {{ include "gha-runner-scale-set.scale-set-name" . } {{/* 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. +Values from a values file arrive as float64. Integral values in the IEEE 754 safe integer +range are formatted without an exponent. Unsafe integral values are rejected by assertScalar +before reaching this helper, because their original value may already have been rounded. */}} {{- define "gha-runner-scale-set.metadataValue" -}} {{- if eq . nil -}} @@ -115,6 +115,11 @@ Expects a dict with "key", "kind" (label|annotation) and "path" (the values path {{- 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 $segment := splitList "." $prefix -}} +{{- if gt (len $segment) 63 -}} +{{- fail (printf "%s: invalid %s key %q: the prefix segment %q must be no more than 63 characters" $path $kind $key $segment) -}} +{{- end -}} +{{- 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 -}} @@ -134,6 +139,8 @@ Expects a dict with "value", "key", "kind" and "path". {{- $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)) -}} +{{- else if and (or (kindIs "int" $value) (kindIs "int64" $value) (and (kindIs "float64" $value) (eq $value (floor $value)))) (or (ge (float64 $value) 9007199254740992.0) (le (float64 $value) -9007199254740992.0)) -}} +{{- fail (printf "%s: invalid value for %s %q: unquoted integers outside the IEEE 754 safe range must be quoted to preserve their exact value" .path .kind .key) -}} {{- 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 23f7a7e0..25ee902e 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -3259,7 +3259,7 @@ func TestTemplateRenderedAutoScalingRunnerSet_ValidMetadataIsAccepted(t *testing // 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. +// A values file exercises float64 integers rather than the int64 values produced by --set. func TestTemplateRenderedAutoScalingRunnerSet_ScalarMetadataValuesAreRenderedAsStrings(t *testing.T) { t.Parallel() @@ -3287,9 +3287,13 @@ func TestTemplateRenderedAutoScalingRunnerSet_ScalarMetadataValuesAreRenderedAsS assert.Equal(t, "true", autoscalingRunnerSet.Labels["chart-bool"]) assert.Equal(t, "1", autoscalingRunnerSet.Labels["chart-int"]) + assert.Equal(t, "9007199254740991", autoscalingRunnerSet.Labels["chart-max-safe-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, "9007199254740991", autoscalingRunnerSet.Annotations["chart-max-safe-int-annotation"]) + assert.Equal(t, "-9007199254740991", autoscalingRunnerSet.Annotations["chart-min-safe-int-annotation"]) + assert.Equal(t, "9007199254740993", autoscalingRunnerSet.Annotations["chart-quoted-unsafe-int-annotation"]) assert.Equal(t, "true", autoscalingRunnerSet.Spec.Template.Labels["pod-bool"]) assert.Equal(t, "42", autoscalingRunnerSet.Spec.Template.Labels["pod-int"]) @@ -3317,6 +3321,122 @@ func TestTemplateRenderedAutoScalingRunnerSet_ScalarMetadataValuesAreRenderedAsS assert.Equal(t, "1.5", githubSecret.Annotations["chart-float-annotation"]) } +func TestTemplateRenderedAutoScalingRunnerSet_UnsafeIntegerMetadataValueValidationError(t *testing.T) { + t.Parallel() + + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + testValuesPath, err := filepath.Abs("../tests/values_unsafe_metadata.yaml") + require.NoError(t, err) + + options := &helm.Options{ + Logger: logger.Discard, + ValuesFiles: []string{testValuesPath}, + KubectlOptions: k8s.NewKubectlOptions("", "", "test"), + } + + _, err = helm.RenderTemplateContextE(t, t.Context(), options, helmChartPath, "test-runners", []string{"templates/autoscalingrunnerset.yaml"}) + require.Error(t, err) + assert.ErrorContains(t, err, `.Values.annotations: invalid value for annotation "unsafe-integer": unquoted integers outside the IEEE 754 safe range must be quoted to preserve their exact value`) +} + +func TestTemplateRenderedAutoScalingRunnerSet_IntegerMetadataBoundariesFromSet(t *testing.T) { + t.Parallel() + + charts := map[string]struct { + urlKey string + tokenKey string + annotationPath string + templateFile string + }{ + "gha-runner-scale-set": { + urlKey: "githubConfigUrl", + tokenKey: "githubConfigSecret.github_token", + annotationPath: "annotations", + templateFile: "templates/autoscalingrunnerset.yaml", + }, + "gha-runner-scale-set-experimental": { + urlKey: "auth.url", + tokenKey: "auth.githubToken", + annotationPath: "resource.all.metadata.annotations", + templateFile: "templates/autoscalingrunnserset.yaml", + }, + } + + tt := map[string]struct { + value string + invalid bool + }{ + "maximum safe integer": {value: "9007199254740991"}, + "minimum safe integer": {value: "-9007199254740991"}, + "first unsafe positive integer": {value: "9007199254740992", invalid: true}, + "first unsafe negative integer": {value: "-9007199254740992", invalid: true}, + "rounded unsafe positive integer": {value: "9007199254740993", invalid: true}, + "rounded unsafe negative integer": {value: "-9007199254740993", invalid: true}, + } + + // helm-unittest's set values are float64, so exercise --set's int64 path in both charts. + for chart, config := range charts { + t.Run(chart, func(t *testing.T) { + t.Parallel() + + helmChartPath, err := filepath.Abs("../../" + chart) + require.NoError(t, err) + + for name, tc := range tt { + t.Run(name, func(t *testing.T) { + t.Parallel() + + options := &helm.Options{ + Logger: logger.Discard, + SetValues: map[string]string{ + config.urlKey: "https://github.com/actions", + config.tokenKey: "gh_token12345", + "controllerServiceAccount.name": "arc", + "controllerServiceAccount.namespace": "arc-system", + config.annotationPath + ".integer-boundary": tc.value, + }, + KubectlOptions: k8s.NewKubectlOptions("", "", "test"), + } + + output, err := helm.RenderTemplateContextE(t, t.Context(), options, helmChartPath, "test-runners", []string{config.templateFile}) + if tc.invalid { + require.Error(t, err) + assert.ErrorContains(t, err, ".Values."+config.annotationPath+`: invalid value for annotation "integer-boundary": unquoted integers outside the IEEE 754 safe range must be quoted to preserve their exact value`) + return + } + require.NoError(t, err) + + var autoscalingRunnerSet v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &autoscalingRunnerSet) + assert.Equal(t, tc.value, autoscalingRunnerSet.Annotations["integer-boundary"]) + }) + } + }) + } +} + +func TestTemplateRenderedAutoScalingRunnerSet_OverlongMetadataPrefixSegmentValidationError(t *testing.T) { + t.Parallel() + + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + testValuesPath, err := filepath.Abs("../tests/values_overlong_metadata_prefix.yaml") + require.NoError(t, err) + + options := &helm.Options{ + Logger: logger.Discard, + ValuesFiles: []string{testValuesPath}, + KubectlOptions: k8s.NewKubectlOptions("", "", "test"), + } + + _, err = helm.RenderTemplateContextE(t, t.Context(), options, helmChartPath, "test-runners", []string{"templates/autoscalingrunnerset.yaml"}) + require.Error(t, err) + assert.ErrorContains(t, err, `.Values.annotations: invalid annotation key "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/overlong-prefix": the prefix segment "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" must be no more than 63 characters`) +} + func TestTemplateRenderedAutoScalingRunnerSet_NonMapMetadataValidationError(t *testing.T) { t.Parallel() @@ -3431,35 +3551,45 @@ func TestTemplateRenderedAutoScalingRunnerSet_NonScalarMetadataValueValidationEr 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) { +func TestTemplateRenderedAutoScalingRunnerSet_MetadataPrefixSegmentLengthBoundary(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()) + for _, length := range []int{63, 64} { + t.Run(fmt.Sprintf("%d characters", length), func(t *testing.T) { + t.Parallel() - key := strings.Repeat("a", 64) + ".example.com/purpose" + 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", - "template.metadata.labels." + strings.ReplaceAll(key, ".", `\.`): "yes", - }, - KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + segment := strings.Repeat("a", length) + key := segment + ".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, err := helm.RenderTemplateContextE(t, t.Context(), options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) + if length == 64 { + require.Error(t, err) + assert.ErrorContains(t, err, `.Values.template.metadata.labels: invalid label key "`+key+`": the prefix segment "`+segment+`" must be no more than 63 characters`) + return + } + require.NoError(t, err) + + var autoscalingRunnerSet v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &autoscalingRunnerSet) + assert.Equal(t, "yes", autoscalingRunnerSet.Spec.Template.Labels[key]) + }) } - - 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_overlong_metadata_prefix.yaml b/charts/gha-runner-scale-set/tests/values_overlong_metadata_prefix.yaml new file mode 100644 index 00000000..a1cdcce6 --- /dev/null +++ b/charts/gha-runner-scale-set/tests/values_overlong_metadata_prefix.yaml @@ -0,0 +1,9 @@ +githubConfigUrl: https://github.com/actions +githubConfigSecret: + github_token: gh_token12345 +controllerServiceAccount: + name: arc + namespace: arc-system + +annotations: + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/overlong-prefix: value 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 9a5f497c..926d6c02 100644 --- a/charts/gha-runner-scale-set/tests/values_scalar_metadata.yaml +++ b/charts/gha-runner-scale-set/tests/values_scalar_metadata.yaml @@ -11,12 +11,16 @@ controllerServiceAccount: labels: chart-bool: true chart-int: 1 + chart-max-safe-int: 9007199254740991 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 + chart-max-safe-int-annotation: 9007199254740991 + chart-min-safe-int-annotation: -9007199254740991 + chart-quoted-unsafe-int-annotation: "9007199254740993" template: metadata: diff --git a/charts/gha-runner-scale-set/tests/values_unsafe_metadata.yaml b/charts/gha-runner-scale-set/tests/values_unsafe_metadata.yaml new file mode 100644 index 00000000..5de37f7a --- /dev/null +++ b/charts/gha-runner-scale-set/tests/values_unsafe_metadata.yaml @@ -0,0 +1,9 @@ +githubConfigUrl: https://github.com/actions +githubConfigSecret: + github_token: gh_token12345 +controllerServiceAccount: + name: arc + namespace: arc-system + +annotations: + unsafe-integer: 9007199254740993