diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index a33f40ebd..003186a36 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -70,7 +70,7 @@ func (e *docker) toConfig(step *types.Step, options BackendOptions) *container.C } func toContainerName(step *types.Step, workflowName string) string { - return "wp_" + step.UUID[:5] + "-" + workflowName + "-" + step.Name + return "wp_" + workflowName + "-" + step.Name + "-" + step.UUID[:5] } // returns a container host configuration. diff --git a/pipeline/backend/docker/convert_test.go b/pipeline/backend/docker/convert_test.go index 7a549e506..180aa1aa8 100644 --- a/pipeline/backend/docker/convert_test.go +++ b/pipeline/backend/docker/convert_test.go @@ -125,8 +125,8 @@ var ( ) func TestToContainerName(t *testing.T) { - assert.EqualValues(t, "wp_f5182-workflowNameTest-hello", toContainerName(testCmdStep, "workflowNameTest")) - assert.EqualValues(t, "wp_d841e-workflowNameTest-lint", toContainerName(testPluginStep, "workflowNameTest")) + assert.EqualValues(t, "wp_workflowNameTest-hello-f5182", toContainerName(testCmdStep, "workflowNameTest")) + assert.EqualValues(t, "wp_workflowNameTest-lint-d841e", toContainerName(testPluginStep, "workflowNameTest")) } func TestStepToConfig(t *testing.T) { diff --git a/pipeline/backend/kubernetes/pod.go b/pipeline/backend/kubernetes/pod.go index 11de5efad..506dec18a 100644 --- a/pipeline/backend/kubernetes/pod.go +++ b/pipeline/backend/kubernetes/pod.go @@ -77,7 +77,7 @@ func stepToPodName(step *types.Step, workflowName string) (name string, err erro } func podName(step *types.Step, workflowName string) (string, error) { - return dnsName(podPrefix + step.UUID[:5] + "-" + workflowName + "-" + step.Name) + return dnsName(podPrefix + workflowName + "-" + step.Name + "-" + step.UUID[:5]) } func podMeta(step *types.Step, config *config, options BackendOptions, podName, workflowName string) (meta_v1.ObjectMeta, error) { diff --git a/pipeline/backend/kubernetes/pod_test.go b/pipeline/backend/kubernetes/pod_test.go index 0f468b4d3..60961cf2c 100644 --- a/pipeline/backend/kubernetes/pod_test.go +++ b/pipeline/backend/kubernetes/pod_test.go @@ -28,31 +28,31 @@ import ( func TestPodName(t *testing.T) { name, err := podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0", Name: "go-test"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-01he8-workflownametest-go-test", name) + assert.Equal(t, "wp-workflownametest-go-test-01he8", name) - _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me\\0a"}, "workflowNameTest") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + _, err = podName(&types.Step{UUID: "01\\he8bebctabr3kgk0qj36d2me\\0a"}, "workflowNameTest") + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") - _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0-services-0..woodpecker-runtime.svc.cluster.local"}, "workflowNameTest") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + _, err = podName(&types.Step{UUID: "01..he8bebctabr3kgk0qj36d2me-0-services-0..woodpecker-runtime.svc.cluster.local"}, "workflowNameTest") + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } func TestStepToPodName(t *testing.T) { name, err := stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeClone}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-workflownametest-clone", name) + assert.EqualValues(t, "wp-workflownametest-clone-01he8", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "cache", Type: types.StepTypeCache}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-workflownametest-cache", name) + assert.EqualValues(t, "wp-workflownametest-cache-01he8", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "release", Type: types.StepTypePlugin}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-workflownametest-release", name) + assert.EqualValues(t, "wp-workflownametest-release-01he8", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "prepare-env", Type: types.StepTypeCommands}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-workflownametest-prepare-env", name) + assert.EqualValues(t, "wp-workflownametest-prepare-env-01he8", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "postgres", Type: types.StepTypeService}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-svc-01he8-workflownametest-postgres", name) + assert.EqualValues(t, "wp-svc-workflownametest-postgres-01he8", name) } func TestStepLabel(t *testing.T) { @@ -61,14 +61,14 @@ func TestStepLabel(t *testing.T) { assert.EqualValues(t, "build-image", name) _, err = stepLabel(&types.Step{Name: ".build.image"}) - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } func TestTinyPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -86,7 +86,7 @@ func TestTinyPod(t *testing.T) { ], "containers": [ { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "image": "gradle:8.4.0-jdk21", "command": [ "/bin/sh", @@ -133,7 +133,7 @@ func TestTinyPod(t *testing.T) { Environment: map[string]string{"CI": "woodpecker"}, }, &config{ Namespace: "woodpecker", - }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{}, "workflowNameTest") + }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{}, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) @@ -147,7 +147,7 @@ func TestFullPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -171,7 +171,7 @@ func TestFullPod(t *testing.T) { ], "containers": [ { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "image": "meltwater/drone-cache", "command": [ "/bin/sh", @@ -256,7 +256,7 @@ func TestFullPod(t *testing.T) { "name": "another-pull-secret" }, { - "name": "wp-01he8-workflownametest-go-test" + "name": "wp-workflownametest-go-test-01he8" } ], "tolerations": [ @@ -335,7 +335,7 @@ func TestFullPod(t *testing.T) { PodAnnotationsAllowFromStep: true, PodNodeSelector: map[string]string{"topology.kubernetes.io/region": "eu-central-1"}, SecurityContext: SecurityContextConfig{RunAsNonRoot: false}, - }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{ + }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{ Labels: map[string]string{"part-of": "woodpecker-ci"}, Annotations: map[string]string{"kubernetes.io/limit-ranger": "LimitRanger plugin set: cpu, memory request and limit for container"}, NodeSelector: map[string]string{"storage": "ssd"}, @@ -366,7 +366,7 @@ func TestPodPrivilege(t *testing.T) { }, &config{ Namespace: "woodpecker", SecurityContext: SecurityContextConfig{RunAsNonRoot: globalRunAsRoot}, - }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{ + }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{ SecurityContext: &secCtx, }, "workflowNameTest") } @@ -443,7 +443,7 @@ func TestScratchPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -453,7 +453,7 @@ func TestScratchPod(t *testing.T) { "spec": { "containers": [ { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "image": "quay.io/curl/curl", "command": [ "/usr/bin/curl", @@ -474,7 +474,7 @@ func TestScratchPod(t *testing.T) { Entrypoint: []string{"/usr/bin/curl", "-v", "google.com"}, }, &config{ Namespace: "woodpecker", - }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{}, "workflowNameTest") + }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{}, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) diff --git a/pipeline/backend/kubernetes/secrets_test.go b/pipeline/backend/kubernetes/secrets_test.go index 0bb8df75d..4300b4d91 100644 --- a/pipeline/backend/kubernetes/secrets_test.go +++ b/pipeline/backend/kubernetes/secrets_test.go @@ -208,7 +208,7 @@ func TestUsernameAndPasswordNeedsSecret(t *testing.T) { func TestRegistrySecret(t *testing.T) { const expected = `{ "metadata": { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "namespace": "woodpecker", "creationTimestamp": null, "labels": { diff --git a/pipeline/backend/kubernetes/service.go b/pipeline/backend/kubernetes/service.go index c0e907b4d..45c49472a 100644 --- a/pipeline/backend/kubernetes/service.go +++ b/pipeline/backend/kubernetes/service.go @@ -63,7 +63,7 @@ func mkService(step *types.Step, config *config, workflowName string) (*v1.Servi } func serviceName(step *types.Step, workflowName string) (string, error) { - return dnsName(servicePrefix + step.UUID[:5] + "-" + workflowName + "-" + step.Name) + return dnsName(servicePrefix + workflowName + "-" + step.Name + "-" + step.UUID[:5]) } func servicePort(port types.Port) v1.ServicePort { diff --git a/pipeline/backend/kubernetes/service_test.go b/pipeline/backend/kubernetes/service_test.go index b8fee573c..338d35cfb 100644 --- a/pipeline/backend/kubernetes/service_test.go +++ b/pipeline/backend/kubernetes/service_test.go @@ -26,22 +26,22 @@ import ( func TestServiceName(t *testing.T) { name, err := serviceName(&types.Step{Name: "database", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-workflownametest-database", name) + assert.Equal(t, "wp-svc-workflownametest-database-01he8", name) - name, err = serviceName(&types.Step{Name: "wp-01he8-workflownametest-clone-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") + name, err = serviceName(&types.Step{Name: "wp-workflownametest-clone-01he8-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-workflownametest-wp-01he8-workflownametest-clone-0-services-0.woodpecker-runtime.svc.cluster.local", name) + assert.Equal(t, "wp-svc-workflownametest-wp-workflownametest-clone-01he8-0-services-0.woodpecker-runtime.svc.cluster.local-01he8", name) name, err = serviceName(&types.Step{Name: "awesome_service", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-workflownametest-awesome-service", name) + assert.Equal(t, "wp-svc-workflownametest-awesome-service-01he8", name) } func TestService(t *testing.T) { expected := ` { "metadata": { - "name": "wp-svc-01he8-workflownametest-bar", + "name": "wp-svc-workflownametest-bar-01he8", "namespace": "foo", "creationTimestamp": null }, @@ -66,7 +66,7 @@ func TestService(t *testing.T) { } ], "selector": { - "service": "wp-svc-01he8-workflownametest-bar" + "service": "wp-svc-workflownametest-bar-01he8" }, "type": "ClusterIP" }, diff --git a/pipeline/backend/kubernetes/utils.go b/pipeline/backend/kubernetes/utils.go index 46e57970b..b68bc4731 100644 --- a/pipeline/backend/kubernetes/utils.go +++ b/pipeline/backend/kubernetes/utils.go @@ -15,7 +15,6 @@ package kubernetes import ( - "errors" "fmt" "os" "regexp" @@ -28,20 +27,22 @@ import ( ) var ( - dnsPattern = regexp.MustCompile(`^[a-z0-9]` + // must start with - `([-a-z0-9]*[a-z0-9])?` + // inside can als contain - - `(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`, // allow the same pattern as before with dots in between but only one dot - ) - dnsDisallowedCharacters = regexp.MustCompile(`[^-^.a-z0-9]+`) - ErrDNSPatternInvalid = errors.New("name is not a valid kubernetes DNS name") + dnsPattern = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) + dnsDisallowedCharacters = regexp.MustCompile(`[^-a-z0-9.]+`) ) func dnsName(i string) (string, error) { res := strings.ToLower(strings.ReplaceAll(i, "_", "-")) - found := dnsPattern.FindStringIndex(res) - if found == nil { - return "", fmt.Errorf("%w: found invalid characters '%v'", ErrDNSPatternInvalid, found) + // Check for invalid characters (dnsDisallowedCharacters) + invalidChars := dnsDisallowedCharacters.FindAllString(res, -1) + if len(invalidChars) > 0 { + return "", fmt.Errorf("name is not a valid kubernetes DNS name: found invalid characters '%v'", strings.Join(invalidChars, "")) + } + + // Check if the entire string matches the dnsPattern + if !dnsPattern.MatchString(res) { + return "", fmt.Errorf("name is not a valid kubernetes DNS name") } return res, nil } diff --git a/pipeline/backend/kubernetes/utils_test.go b/pipeline/backend/kubernetes/utils_test.go index 43c5b8a1a..7836ebb1f 100644 --- a/pipeline/backend/kubernetes/utils_test.go +++ b/pipeline/backend/kubernetes/utils_test.go @@ -34,25 +34,25 @@ func TestDNSName(t *testing.T) { assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local", name) _, err = dnsName(".0-a") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("ABC..DEF") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("0.-a") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("test-") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("-test") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("0-a.") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("abc\\def") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } func TestToDnsName(t *testing.T) { @@ -69,5 +69,5 @@ func TestToDnsName(t *testing.T) { assert.Equal(t, "build--deploy", name) _, err = toDNSName("-build-and-deploy") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } diff --git a/pipeline/backend/kubernetes/volume_test.go b/pipeline/backend/kubernetes/volume_test.go index 07ec7c30e..fa0315b5a 100644 --- a/pipeline/backend/kubernetes/volume_test.go +++ b/pipeline/backend/kubernetes/volume_test.go @@ -27,10 +27,10 @@ func TestPvcName(t *testing.T) { assert.Equal(t, "woodpecker-cache", name) _, err = volumeName("woodpecker\\cache") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = volumeName("-woodpecker.cache:/woodpecker/src/cache") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } func TestPvcMount(t *testing.T) {