refactor: rework k8s utils to handle resource names and labels (#6855)

This commit is contained in:
Robert Kaussow
2026-07-16 15:21:01 +02:00
committed by GitHub
parent fce6fb820c
commit f1f8b2be00
9 changed files with 416 additions and 89 deletions
+2 -2
View File
@@ -89,7 +89,7 @@ func stepToPodName(step *types.Step) (name string, err error) {
}
func podName(step *types.Step) (string, error) {
return dnsName(podPrefix + step.UUID)
return toDNSName(podPrefix + step.UUID)
}
func podMeta(step *types.Step, config *config, options BackendOptions, podName, taskUUID string) (kube_meta_v1.ObjectMeta, error) {
@@ -116,7 +116,7 @@ func podLabels(step *types.Step, config *config, options BackendOptions, taskUUI
// Only copy user labels if allowed by agent config.
// Internal labels are filtered on the server-side.
if config.PodLabelsAllowFromStep || strings.HasPrefix(k, pipeline.InternalLabelPrefix) {
labels[k], err = toDNSName(v)
labels[k], err = toLabelValue(v)
if err != nil {
return labels, err
}
+9 -6
View File
@@ -33,11 +33,13 @@ func TestPodName(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0", name)
_, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me\\0a"})
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
name, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me\\0a"})
assert.NoError(t, err)
assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0a", name)
_, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0-services-0..woodpecker-runtime.svc.cluster.local"})
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
name, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0-services-0..woodpecker-runtime.svc.cluster.local"})
assert.NoError(t, err)
assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local", name)
}
func TestStepToPodName(t *testing.T) {
@@ -118,8 +120,9 @@ func TestStepLabel(t *testing.T) {
assert.NoError(t, err)
assert.EqualValues(t, "build-image", name)
_, err = stepLabel(&types.Step{Name: ".build.image"})
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
name, err = stepLabel(&types.Step{Name: ".build.image"})
assert.NoError(t, err)
assert.EqualValues(t, "build.image", name)
}
func TestPodHostnameSanitized(t *testing.T) {
+1 -1
View File
@@ -260,7 +260,7 @@ func registrySecretLabels(step *types.Step, config *config) (map[string]string,
// Only copy user labels if allowed by agent config.
// Internal labels are filtered on the server-side.
if config.PodLabelsAllowFromStep || strings.HasPrefix(k, pipeline.InternalLabelPrefix) {
labels[k], err = toDNSName(v)
labels[k], err = toLabelValue(v)
if err != nil {
return labels, err
}
+2 -2
View File
@@ -56,7 +56,7 @@ func mkHeadlessService(namespace, taskUUID string) (*kube_core_v1.Service, error
}
func serviceName(step *types.Step) (string, error) {
return dnsName(ServicePrefix + step.UUID + "-" + step.Name)
return toDNSName(ServicePrefix + step.UUID + "-" + step.Name)
}
func isService(step *types.Step) bool {
@@ -64,7 +64,7 @@ func isService(step *types.Step) bool {
}
func subdomain(taskUUID string) (string, error) {
return dnsName(HeadlessServicePrefix + taskUUID)
return toDNSName(HeadlessServicePrefix + taskUUID)
}
func startHeadlessService(ctx context.Context, engine *kube, namespace, taskUUID string) (*kube_core_v1.Service, error) {
+9 -7
View File
@@ -67,8 +67,9 @@ func TestHeadlessService(t *testing.T) {
}
func TestInvalidHeadlessService(t *testing.T) {
_, err := mkHeadlessService("foo", "invalid_task_uuid!")
assert.Error(t, err, "expected error due to invalid task UUID")
svc, err := mkHeadlessService("foo", "invalid_task_uuid!")
assert.NoError(t, err, "invalid characters are sanitized")
assert.Equal(t, "wp-hsvc-invalid-task-uuid", svc.Name)
}
func TestStartHeadlessService(t *testing.T) {
@@ -93,14 +94,15 @@ func TestStartHeadlessService(t *testing.T) {
assert.Equal(t, svc.Name, createdSvc.Name, "expected created service name to match")
})
t.Run("error on invalid task UUID resulting in invalid domain-name", func(t *testing.T) {
t.Run("sanitizes invalid task UUID", func(t *testing.T) {
engine := &kube{
client: fake.NewClientset(),
config: &config{Namespace: "test-namespace"},
}
_, err := startHeadlessService(t.Context(), engine, "test-namespace", "invalid_task_uuid!")
assert.Error(t, err, "expected error due to invalid task UUID")
svc, err := startHeadlessService(t.Context(), engine, "test-namespace", "invalid_task_uuid!")
assert.NoError(t, err, "invalid characters are sanitized")
assert.Equal(t, "wp-hsvc-invalid-task-uuid", svc.Name)
})
}
@@ -138,13 +140,13 @@ func TestStopHeadlessService(t *testing.T) {
assert.NoError(t, err, "expected no error when deleting a non-existent service")
})
t.Run("error on invalid task UUID resulting in invalid domain-name", func(t *testing.T) {
t.Run("sanitizes invalid task UUID", func(t *testing.T) {
engine := &kube{
client: fake.NewClientset(),
config: &config{Namespace: "test-namespace"},
}
err := engine.stopHeadlessService(t.Context(), engine, "test-namespace", "invalid_task_uuid!")
assert.Error(t, err, "expected error due to invalid task UUID")
assert.NoError(t, err, "invalid characters are sanitized and missing service is tolerated")
})
}
+47 -17
View File
@@ -15,12 +15,15 @@
package kubernetes
import (
"crypto/sha256"
"encoding/hex"
"errors"
"os"
"regexp"
"strings"
kube_core_v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
kube_client_cmd "k8s.io/client-go/tools/clientcmd"
@@ -29,14 +32,12 @@ import (
const maxDNSLabelLen = 63
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
)
dnsLabelPattern = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`)
dnsDisallowedCharacters = regexp.MustCompile(`[^-^.a-z0-9]+`)
dnsDisallowedCharacters = regexp.MustCompile(`[^-.a-z0-9]+`)
dotsAndDashes = regexp.MustCompile(`[.-]{2,}`)
labelDisallowedChars = regexp.MustCompile(`[^-_.a-z0-9]+`)
labelSeparators = regexp.MustCompile(`[-_.]{2,}`)
ErrDNSPatternInvalid = errors.New("name is not a valid kubernetes DNS name")
ErrLabelInvalid = errors.New("value is not a valid kubernetes label value")
)
func getHostnameOrEmpty(name string) string {
@@ -52,28 +53,57 @@ func getHostnameOrEmpty(name string) string {
clean = strings.Trim(clean, "-")
if dnsLabelPattern.MatchString(clean) {
if len(validation.IsDNS1123Label(clean)) == 0 {
return clean
}
return ""
}
func dnsName(i string) (string, error) {
res := strings.ToLower(strings.ReplaceAll(i, "_", "-"))
func toDNSName(in string) (string, error) {
res := strings.ToLower(in)
res = dnsDisallowedCharacters.ReplaceAllString(res, "-")
res = dotsAndDashes.ReplaceAllStringFunc(res, func(s string) string {
if strings.ContainsRune(s, '.') {
return "."
}
return "-"
})
res = strings.Trim(res, "-.")
if found := dnsPattern.FindStringIndex(res); found == nil {
if len(res) > validation.DNS1123SubdomainMaxLength {
res = truncateWithHash(res, in, validation.DNS1123SubdomainMaxLength, "-.")
}
if res == "" || len(validation.IsDNS1123Subdomain(res)) > 0 {
return "", ErrDNSPatternInvalid
}
return res, nil
}
func toDNSName(in string) (string, error) {
lower := strings.ToLower(in)
withoutUnderscores := strings.ReplaceAll(lower, "_", "-")
withoutSpaces := strings.ReplaceAll(withoutUnderscores, " ", "-")
almostDNS := dnsDisallowedCharacters.ReplaceAllString(withoutSpaces, "")
return dnsName(almostDNS)
func truncateWithHash(s, original string, maxLen int, trimChars string) string {
hash := sha256.Sum256([]byte(original))
hashStr := hex.EncodeToString(hash[:])[:16]
maxBaseLength := maxLen - 1 - len(hashStr)
truncated := strings.TrimRight(s[:maxBaseLength], trimChars)
return truncated + "-" + hashStr
}
func toLabelValue(in string) (string, error) {
res := strings.ToLower(in)
res = labelDisallowedChars.ReplaceAllString(res, "-")
res = labelSeparators.ReplaceAllString(res, "-")
res = strings.Trim(res, "-_.")
if len(res) > validation.LabelValueMaxLength {
res = truncateWithHash(res, in, validation.LabelValueMaxLength, "-_.")
}
if len(validation.IsValidLabelValue(res)) > 0 {
return "", ErrLabelInvalid
}
return res, nil
}
func isImagePullBackOffState(pod *kube_core_v1.Pod) bool {
+336 -47
View File
@@ -15,75 +15,364 @@
package kubernetes
import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/util/validation"
)
func TestDNSName(t *testing.T) {
name, err := dnsName("wp_01he8bebctabr3kgk0qj36d2me_0_services_0")
assert.NoError(t, err)
assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0-services-0", name)
func TestToDnsName(t *testing.T) {
tests := []struct {
name string
in string
want string
wantErr bool
}{
{
name: "underscores to dashes",
in: "wp_01he8bebctabr3kgk0qj36d2me_0_services_0",
want: "wp-01he8bebctabr3kgk0qj36d2me-0-services-0",
},
{
name: "mixed case with dots and dashes",
in: "a.0-AA",
want: "a.0-aa",
},
{
name: "long valid fqdn unchanged",
in: "wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local",
want: "wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local",
},
{
name: "uppercase with underscores",
in: "BUILD_AND_DEPLOY_0",
want: "build-and-deploy-0",
},
{
name: "spaces to dashes",
in: "build and deploy",
want: "build-and-deploy",
},
{
name: "special char ampersand",
in: "build & deploy",
want: "build-deploy",
},
{
name: "backslash to dash",
in: "abc\\def",
want: "abc-def",
},
{
name: "leading dash trimmed",
in: "-build-and-deploy",
want: "build-and-deploy",
},
{
name: "trailing dash trimmed",
in: "test-",
want: "test",
},
{
name: "leading dot trimmed",
in: ".0-a",
want: "0-a",
},
{
name: "consecutive dots collapsed",
in: "ABC..DEF",
want: "abc.def",
},
{
name: "dot-dash collapsed",
in: "0.-a",
want: "0.a",
},
{
name: "trailing dot trimmed",
in: "0-a.",
want: "0-a",
},
{
name: "consecutive underscores single dash",
in: "a__b",
want: "a-b",
},
{
name: "mixed dots and dashes",
in: "a.-.-b",
want: "a.b",
},
{
name: "unicode emoji replaced",
in: "hello🚀world",
want: "hello-world",
},
{
name: "single valid char",
in: "a",
want: "a",
},
{
name: "numbers only",
in: "123",
want: "123",
},
{
name: "mixed leading trailing",
in: "-.-test-.-",
want: "test",
},
{
name: "all special chars empty",
in: "!!!",
wantErr: true,
},
{
name: "all dots empty",
in: "...",
wantErr: true,
},
{
name: "all dashes empty",
in: "---",
wantErr: true,
},
{
name: "empty string",
in: "",
wantErr: true,
},
{
name: "truncation needed",
in: strings.Repeat("a", 300),
},
{
name: "truncation boundary exact 253",
in: strings.Repeat("a", 253),
want: strings.Repeat("a", 253),
},
{
name: "truncation with trailing dash at cut",
in: strings.Repeat("a", 240) + "----" + strings.Repeat("b", 60),
},
{
name: "truncation at dot boundary",
in: strings.Repeat("a.", 130),
},
{
name: "truncation with long label segment",
in: strings.Repeat("a", 70) + "." + strings.Repeat("b", 70) + "." + strings.Repeat("c", 70),
},
}
name, err = dnsName("a.0-AA")
assert.NoError(t, err)
assert.Equal(t, "a.0-aa", name)
name, err = dnsName("wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local")
assert.NoError(t, err)
assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local", name)
_, err = dnsName(".0-a")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
_, err = dnsName("ABC..DEF")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
_, err = dnsName("0.-a")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
_, err = dnsName("test-")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
_, err = dnsName("-test")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
_, err = dnsName("0-a.")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
_, err = dnsName("abc\\def")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := toDNSName(tt.in)
if tt.wantErr {
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
return
}
assert.NoError(t, err)
if tt.name == "truncation needed" || tt.name == "truncation with trailing dash at cut" || tt.name == "truncation at dot boundary" || tt.name == "truncation with long label segment" {
assert.LessOrEqual(t, len(got), validation.DNS1123SubdomainMaxLength)
assert.Len(t, validation.IsDNS1123Subdomain(got), 0)
return
}
assert.Equal(t, tt.want, got)
})
}
}
func TestToDnsName(t *testing.T) {
name, err := toDNSName("BUILD_AND_DEPLOY_0")
assert.NoError(t, err)
assert.Equal(t, "build-and-deploy-0", name)
func TestToDnsNameTruncationUniqueness(t *testing.T) {
long1 := strings.Repeat("a", 300)
long2 := strings.Repeat("a", 299) + "b"
name, err = toDNSName("build and deploy")
got1, err := toDNSName(long1)
assert.NoError(t, err)
assert.Equal(t, "build-and-deploy", name)
name, err = toDNSName("build & deploy")
got2, err := toDNSName(long2)
assert.NoError(t, err)
assert.Equal(t, "build--deploy", name)
_, err = toDNSName("-build-and-deploy")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
assert.NotEqual(t, got1, got2, "different long inputs should produce different truncated names")
}
func TestToLabelValue(t *testing.T) {
tests := []struct {
name string
in string
want string
wantErr bool
}{
{
name: "underscores preserved",
in: "wp_01he8bebctabr3kgk0qj36d2me_0_services_0",
want: "wp_01he8bebctabr3kgk0qj36d2me_0_services_0",
},
{
name: "dots preserved",
in: "a.0-AA",
want: "a.0-aa",
},
{
name: "uppercase with underscores",
in: "BUILD_AND_DEPLOY_0",
want: "build_and_deploy_0",
},
{
name: "spaces to dashes",
in: "build and deploy",
want: "build-and-deploy",
},
{
name: "special char ampersand",
in: "build & deploy",
want: "build-deploy",
},
{
name: "backslash to dash",
in: "abc\\def",
want: "abc-def",
},
{
name: "leading dash trimmed",
in: "-build-and-deploy",
want: "build-and-deploy",
},
{
name: "trailing dash trimmed",
in: "test-",
want: "test",
},
{
name: "leading dot trimmed",
in: ".0-a",
want: "0-a",
},
{
name: "trailing dot trimmed",
in: "0-a.",
want: "0-a",
},
{
name: "consecutive underscores collapsed",
in: "a__b",
want: "a-b",
},
{
name: "consecutive dots collapsed",
in: "ABC..DEF",
want: "abc-def",
},
{
name: "mixed separators collapsed",
in: "a._-b",
want: "a-b",
},
{
name: "unicode emoji replaced",
in: "hello🚀world",
want: "hello-world",
},
{
name: "single valid char",
in: "a",
want: "a",
},
{
name: "numbers only",
in: "123",
want: "123",
},
{
name: "mixed leading trailing",
in: "-.-test-.-",
want: "test",
},
{
name: "all special chars become empty",
in: "!!!",
want: "",
},
{
name: "empty string is valid",
in: "",
want: "",
},
{
name: "truncation needed",
in: strings.Repeat("a", 100),
},
{
name: "truncation boundary exact 63",
in: strings.Repeat("a", 63),
want: strings.Repeat("a", 63),
},
{
name: "truncation with trailing dash at cut",
in: strings.Repeat("a", 50) + "----" + strings.Repeat("b", 30),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := toLabelValue(tt.in)
if tt.wantErr {
assert.ErrorIs(t, err, ErrLabelInvalid)
return
}
assert.NoError(t, err)
if tt.name == "truncation needed" || tt.name == "truncation with trailing dash at cut" {
assert.LessOrEqual(t, len(got), validation.LabelValueMaxLength)
assert.Len(t, validation.IsValidLabelValue(got), 0)
return
}
assert.Equal(t, tt.want, got)
})
}
}
func TestToLabelValueTruncationUniqueness(t *testing.T) {
long1 := strings.Repeat("a", 100)
long2 := strings.Repeat("a", 99) + "b"
got1, err := toLabelValue(long1)
assert.NoError(t, err)
got2, err := toLabelValue(long2)
assert.NoError(t, err)
assert.NotEqual(t, got1, got2, "different long inputs should produce different truncated labels")
}
func TestGetHostnameOrEmpty(t *testing.T) {
tests := []struct {
name string
in string
want string
}{
{"Update repos", "update-repos"},
{"MY_STEP", "my-step"},
{"Build 🚀", ""},
{
name: "spaces to dashes",
in: "Update repos",
want: "update-repos",
},
{
name: "underscores to dashes",
in: "MY_STEP",
want: "my-step",
},
{
name: "emoji removed",
in: "Build 🚀",
want: "build",
},
}
for _, tt := range tests {
got := getHostnameOrEmpty(tt.in)
assert.Equal(t, tt.want, got, "input: %q", tt.in)
t.Run(tt.name, func(t *testing.T) {
got := getHostnameOrEmpty(tt.in)
assert.Equal(t, tt.want, got)
})
}
}
+1 -1
View File
@@ -64,7 +64,7 @@ func mkPersistentVolumeClaim(config *config, name, namespace string) (*kube_core
}
func volumeName(name string) (string, error) {
return dnsName(strings.Split(name, ":")[0])
return toDNSName(strings.Split(name, ":")[0])
}
func volumeMountPath(name string) string {
+9 -6
View File
@@ -26,11 +26,13 @@ func TestPvcName(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "woodpecker-cache", name)
_, err = volumeName("woodpecker\\cache")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
name, err = volumeName("woodpecker\\cache")
assert.NoError(t, err)
assert.Equal(t, "woodpecker-cache", name)
_, err = volumeName("-woodpecker.cache:/woodpecker/src/cache")
assert.ErrorIs(t, err, ErrDNSPatternInvalid)
name, err = volumeName("-woodpecker.cache:/woodpecker/src/cache")
assert.NoError(t, err)
assert.Equal(t, "woodpecker.cache", name)
}
func TestPvcMount(t *testing.T) {
@@ -107,11 +109,12 @@ func TestPersistentVolumeClaim(t *testing.T) {
assert.NoError(t, err)
assert.JSONEq(t, expectedRwo, string(j))
_, err = mkPersistentVolumeClaim(&config{
pvc, err = mkPersistentVolumeClaim(&config{
Namespace: namespace,
StorageClass: "local-storage",
VolumeSize: "1Gi",
StorageRwx: false,
}, "some0..INVALID3name", namespace)
assert.Error(t, err)
assert.NoError(t, err)
assert.Equal(t, "some0.invalid3name", pvc.Name)
}