diff --git a/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl b/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl index 2274431b..102d102e 100644 --- a/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set-experimental/templates/_helpers.tpl @@ -125,6 +125,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 }} {{/* 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 9d02b695..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 @@ -162,6 +162,25 @@ tests: - 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" @@ -202,3 +221,36 @@ tests: - 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 b37031be..4780019b 100644 --- a/charts/gha-runner-scale-set/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set/templates/_helpers.tpl @@ -183,6 +183,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 1de996d6..41babfc0 100644 --- a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml +++ b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml @@ -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/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index 2032e8f1..579ffcb1 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -3153,57 +3153,6 @@ func TestAutoscalingRunnerSetCustomAnnotationsAndLabelsApplied(t *testing.T) { assert.NotEqual(t, "not-propagated", autoscalingRunnerSet.Labels["app.kubernetes.io/component"]) } -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, "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.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_InvalidMetadataValidationError(t *testing.T) { t.Parallel() @@ -3308,6 +3257,66 @@ func TestTemplateRenderedAutoScalingRunnerSet_ValidMetadataIsAccepted(t *testing 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, "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"]) + 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() @@ -3334,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 { @@ -3362,6 +3376,33 @@ 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.RenderTemplateContextE(t, t.Context(), 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() 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 06cb5477..9a5f497c 100644 --- a/charts/gha-runner-scale-set/tests/values_scalar_metadata.yaml +++ b/charts/gha-runner-scale-set/tests/values_scalar_metadata.yaml @@ -37,3 +37,13 @@ resourceMeta: githubConfigSecret: labels: secret-int: 5 + +listenerTemplate: + metadata: + labels: + listener-bool: true + annotations: + listener-int-annotation: 11 + spec: + containers: + - name: listener