From 94934819c4f3b4624be9319dd5730a6f0ad46e64 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Fri, 9 Jun 2023 20:57:20 +0200 Subject: [PATCH] Trim repo/org/enterprise to 63 characters in label values (#2657) --- .../actions.github.com/resourcebuilder.go | 47 +++++++------ .../resourcebuilder_test.go | 69 +++++++++++++++++++ 2 files changed, 96 insertions(+), 20 deletions(-) diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 4a5bccc4..5be1163b 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -63,26 +63,24 @@ func (b *resourceBuilder) newAutoScalingListener(autoscalingRunnerSet *v1alpha1. effectiveMinRunners = *autoscalingRunnerSet.Spec.MinRunners } - githubConfig, err := actions.ParseGitHubConfigFromURL(autoscalingRunnerSet.Spec.GitHubConfigUrl) - if err != nil { - return nil, fmt.Errorf("failed to parse github config from url: %v", err) + labels := map[string]string{ + LabelKeyGitHubScaleSetNamespace: autoscalingRunnerSet.Namespace, + LabelKeyGitHubScaleSetName: autoscalingRunnerSet.Name, + LabelKeyKubernetesPartOf: labelValueKubernetesPartOf, + LabelKeyKubernetesComponent: "runner-scale-set-listener", + LabelKeyKubernetesVersion: autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], + labelKeyRunnerSpecHash: autoscalingRunnerSet.ListenerSpecHash(), + } + + if err := applyGitHubURLLabels(autoscalingRunnerSet.Spec.GitHubConfigUrl, labels); err != nil { + return nil, fmt.Errorf("failed to apply GitHub URL labels: %v", err) } autoscalingListener := &v1alpha1.AutoscalingListener{ ObjectMeta: metav1.ObjectMeta{ Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: namespace, - Labels: map[string]string{ - LabelKeyGitHubScaleSetNamespace: autoscalingRunnerSet.Namespace, - LabelKeyGitHubScaleSetName: autoscalingRunnerSet.Name, - LabelKeyKubernetesPartOf: labelValueKubernetesPartOf, - LabelKeyKubernetesComponent: "runner-scale-set-listener", - LabelKeyKubernetesVersion: autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], - LabelKeyGitHubEnterprise: githubConfig.Enterprise, - LabelKeyGitHubOrganization: githubConfig.Organization, - LabelKeyGitHubRepository: githubConfig.Repository, - labelKeyRunnerSpecHash: autoscalingRunnerSet.ListenerSpecHash(), - }, + Labels: labels, }, Spec: v1alpha1.AutoscalingListenerSpec{ GitHubConfigUrl: autoscalingRunnerSet.Spec.GitHubConfigUrl, @@ -323,7 +321,7 @@ func (b *resourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A } runnerSpecHash := autoscalingRunnerSet.RunnerSetSpecHash() - newLabels := map[string]string{ + labels := map[string]string{ labelKeyRunnerSpecHash: runnerSpecHash, LabelKeyKubernetesPartOf: labelValueKubernetesPartOf, LabelKeyKubernetesComponent: "runner-set", @@ -332,7 +330,7 @@ func (b *resourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A LabelKeyGitHubScaleSetNamespace: autoscalingRunnerSet.Namespace, } - if err := applyGitHubURLLabels(autoscalingRunnerSet.Spec.GitHubConfigUrl, newLabels); err != nil { + if err := applyGitHubURLLabels(autoscalingRunnerSet.Spec.GitHubConfigUrl, labels); err != nil { return nil, fmt.Errorf("failed to apply GitHub URL labels: %v", err) } @@ -345,7 +343,7 @@ func (b *resourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A ObjectMeta: metav1.ObjectMeta{ GenerateName: autoscalingRunnerSet.ObjectMeta.Name + "-", Namespace: autoscalingRunnerSet.ObjectMeta.Namespace, - Labels: newLabels, + Labels: labels, Annotations: newAnnotations, }, Spec: v1alpha1.EphemeralRunnerSetSpec{ @@ -545,14 +543,23 @@ func applyGitHubURLLabels(url string, labels map[string]string) error { } if len(githubConfig.Enterprise) > 0 { - labels[LabelKeyGitHubEnterprise] = githubConfig.Enterprise + labels[LabelKeyGitHubEnterprise] = trimLabelValue(githubConfig.Enterprise) } if len(githubConfig.Organization) > 0 { - labels[LabelKeyGitHubOrganization] = githubConfig.Organization + labels[LabelKeyGitHubOrganization] = trimLabelValue(githubConfig.Organization) } if len(githubConfig.Repository) > 0 { - labels[LabelKeyGitHubRepository] = githubConfig.Repository + labels[LabelKeyGitHubRepository] = trimLabelValue(githubConfig.Repository) } return nil } + +const trimLabelVauleSuffix = "-trim" + +func trimLabelValue(val string) string { + if len(val) > 63 { + return val[:63-len(trimLabelVauleSuffix)] + trimLabelVauleSuffix + } + return val +} diff --git a/controllers/actions.github.com/resourcebuilder_test.go b/controllers/actions.github.com/resourcebuilder_test.go index 925ba5cf..1a1ae44e 100644 --- a/controllers/actions.github.com/resourcebuilder_test.go +++ b/controllers/actions.github.com/resourcebuilder_test.go @@ -2,6 +2,8 @@ package actionsgithubcom import ( "context" + "fmt" + "strings" "testing" "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" @@ -91,3 +93,70 @@ func TestLabelPropagation(t *testing.T) { assert.Equal(t, ephemeralRunner.Labels[key], pod.Labels[key]) } } + +func TestGitHubURLTrimLabelValues(t *testing.T) { + enterprise := strings.Repeat("a", 64) + organization := strings.Repeat("b", 64) + repository := strings.Repeat("c", 64) + + autoscalingRunnerSet := v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-scale-set", + Namespace: "test-ns", + Labels: map[string]string{ + LabelKeyKubernetesPartOf: labelValueKubernetesPartOf, + LabelKeyKubernetesVersion: "0.2.0", + }, + Annotations: map[string]string{ + runnerScaleSetIdAnnotationKey: "1", + AnnotationKeyGitHubRunnerGroupName: "test-group", + }, + }, + } + + t.Run("org/repo", func(t *testing.T) { + autoscalingRunnerSet := autoscalingRunnerSet.DeepCopy() + autoscalingRunnerSet.Spec = v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: fmt.Sprintf("https://github.com/%s/%s", organization, repository), + } + + var b resourceBuilder + ephemeralRunnerSet, err := b.newEphemeralRunnerSet(autoscalingRunnerSet) + require.NoError(t, err) + assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise], 0) + assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubOrganization], 63) + assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubRepository], 63) + assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubOrganization], trimLabelVauleSuffix)) + assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubRepository], trimLabelVauleSuffix)) + + listener, err := b.newAutoScalingListener(autoscalingRunnerSet, ephemeralRunnerSet, autoscalingRunnerSet.Namespace, "test:latest", nil) + require.NoError(t, err) + assert.Len(t, listener.Labels[LabelKeyGitHubEnterprise], 0) + assert.Len(t, listener.Labels[LabelKeyGitHubOrganization], 63) + assert.Len(t, listener.Labels[LabelKeyGitHubRepository], 63) + assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubOrganization], trimLabelVauleSuffix)) + assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubRepository], trimLabelVauleSuffix)) + }) + + t.Run("enterprise", func(t *testing.T) { + autoscalingRunnerSet := autoscalingRunnerSet.DeepCopy() + autoscalingRunnerSet.Spec = v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: fmt.Sprintf("https://github.com/enterprises/%s", enterprise), + } + + var b resourceBuilder + ephemeralRunnerSet, err := b.newEphemeralRunnerSet(autoscalingRunnerSet) + require.NoError(t, err) + assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise], 63) + assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise], trimLabelVauleSuffix)) + assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubOrganization], 0) + assert.Len(t, ephemeralRunnerSet.Labels[LabelKeyGitHubRepository], 0) + + listener, err := b.newAutoScalingListener(autoscalingRunnerSet, ephemeralRunnerSet, autoscalingRunnerSet.Namespace, "test:latest", nil) + require.NoError(t, err) + assert.Len(t, listener.Labels[LabelKeyGitHubEnterprise], 63) + assert.True(t, strings.HasSuffix(ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise], trimLabelVauleSuffix)) + assert.Len(t, listener.Labels[LabelKeyGitHubOrganization], 0) + assert.Len(t, listener.Labels[LabelKeyGitHubRepository], 0) + }) +}