Sanitize pod's step label (#3275)

Closes #3272

pipeline/backend/kubernetes/pod.go

@@ -36,7 +36,12 @@ const (
 )
 
 func mkPod(step *types.Step, config *config, podName, goos string) (*v1.Pod, error) {
-	meta := podMeta(step, config, podName)
+	var err error
+
+	meta, err := podMeta(step, config, podName)
+	if err != nil {
+		return nil, err
+	}
 
 	spec, err := podSpec(step, config)
 	if err != nil {
@@ -68,7 +73,8 @@ func podName(step *types.Step) (string, error) {
 	return dnsName(podPrefix + step.UUID)
 }
 
-func podMeta(step *types.Step, config *config, podName string) metav1.ObjectMeta {
+func podMeta(step *types.Step, config *config, podName string) (metav1.ObjectMeta, error) {
+	var err error
 	meta := metav1.ObjectMeta{
 		Name:      podName,
 		Namespace: config.Namespace,
@@ -78,7 +84,10 @@ func podMeta(step *types.Step, config *config, podName string) metav1.ObjectMeta
 	if meta.Labels == nil {
 		meta.Labels = make(map[string]string, 1)
 	}
-	meta.Labels[StepLabel] = step.Name
+	meta.Labels[StepLabel], err = stepLabel(step)
+	if err != nil {
+		return meta, err
+	}
 
 	if step.Type == types.StepTypeService {
 		meta.Labels[ServiceLabel] = step.Name
@@ -97,7 +106,11 @@ func podMeta(step *types.Step, config *config, podName string) metav1.ObjectMeta
 		}
 	}
 
-	return meta
+	return meta, nil
+}
+
+func stepLabel(step *types.Step) (string, error) {
+	return toDNSName(step.Name)
 }
 
 func podSpec(step *types.Step, config *config) (v1.PodSpec, error) {
@@ -224,7 +237,7 @@ func containerPort(port types.Port) v1.ContainerPort {
 
 // Here is the service IPs (placed in /etc/hosts in the Pod)
 func hostAliases(extraHosts []types.HostAlias) []v1.HostAlias {
-	hostAliases := []v1.HostAlias{}
+	var hostAliases []v1.HostAlias
 	for _, extraHost := range extraHosts {
 		hostAlias := hostAlias(extraHost)
 		hostAliases = append(hostAliases, hostAlias)

pipeline/backend/kubernetes/pod_test.go

@@ -40,18 +40,27 @@ func TestStepToPodName(t *testing.T) {
 	name, err := stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeClone})
 	assert.NoError(t, err)
 	assert.EqualValues(t, "wp-01he8bebctabr3kg", name)
-	name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeCache})
+	name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "cache", Type: types.StepTypeCache})
 	assert.NoError(t, err)
 	assert.EqualValues(t, "wp-01he8bebctabr3kg", name)
-	name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypePlugin})
+	name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "release", Type: types.StepTypePlugin})
 	assert.NoError(t, err)
 	assert.EqualValues(t, "wp-01he8bebctabr3kg", name)
-	name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeCommands})
+	name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "prepare-env", Type: types.StepTypeCommands})
 	assert.NoError(t, err)
 	assert.EqualValues(t, "wp-01he8bebctabr3kg", name)
-	name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeService})
+	name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "postgres", Type: types.StepTypeService})
 	assert.NoError(t, err)
-	assert.EqualValues(t, "clone", name)
+	assert.EqualValues(t, "postgres", name)
+}
+
+func TestStepLabel(t *testing.T) {
+	name, err := stepLabel(&types.Step{Name: "Build image"})
+	assert.NoError(t, err)
+	assert.EqualValues(t, "build-image", name)
+
+	_, err = stepLabel(&types.Step{Name: ".build.image"})
+	assert.ErrorIs(t, err, ErrDNSPatternInvalid)
 }
 
 func TestTinyPod(t *testing.T) {
@@ -133,11 +142,11 @@ func TestTinyPod(t *testing.T) {
 	}, "wp-01he8bebctabr3kgk0qj36d2me-0", "linux/amd64")
 	assert.NoError(t, err)
 
-	json, err := json.Marshal(pod)
+	podJSON, err := json.Marshal(pod)
 	assert.NoError(t, err)
 
 	ja := jsonassert.New(t)
-	ja.Assertf(string(json), expected)
+	ja.Assertf(string(podJSON), expected)
 }
 
 func TestFullPod(t *testing.T) {
@@ -336,9 +345,9 @@ func TestFullPod(t *testing.T) {
 	}, "wp-01he8bebctabr3kgk0qj36d2me-0", "linux/amd64")
 	assert.NoError(t, err)
 
-	json, err := json.Marshal(pod)
+	podJSON, err := json.Marshal(pod)
 	assert.NoError(t, err)
 
 	ja := jsonassert.New(t)
-	ja.Assertf(string(json), expected)
+	ja.Assertf(string(podJSON), expected)
 }

pipeline/backend/kubernetes/utils.go

@@ -31,7 +31,8 @@ var (
 		`([-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
 	)
-	ErrDNSPatternInvalid = errors.New("name is not a valid kubernetes DNS name")
+	dnsDisallowedCharacters = regexp.MustCompile(`[^-^.a-z0-9]+`)
+	ErrDNSPatternInvalid    = errors.New("name is not a valid kubernetes DNS name")
 )
 
 func dnsName(i string) (string, error) {
@@ -44,6 +45,14 @@ func dnsName(i string) (string, error) {
 	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 isImagePullBackOffState(pod *v1.Pod) bool {
 	for _, containerState := range pod.Status.ContainerStatuses {
 		if containerState.State.Waiting != nil {

pipeline/backend/kubernetes/utils_test.go

@@ -54,3 +54,20 @@ func TestDNSName(t *testing.T) {
 	_, err = dnsName("abc\\def")
 	assert.ErrorIs(t, err, ErrDNSPatternInvalid)
 }
+
+func TestToDnsName(t *testing.T) {
+	name, err := toDNSName("BUILD_AND_DEPLOY_0")
+	assert.NoError(t, err)
+	assert.Equal(t, "build-and-deploy-0", name)
+
+	name, err = toDNSName("build and deploy")
+	assert.NoError(t, err)
+	assert.Equal(t, "build-and-deploy", name)
+
+	name, err = toDNSName("build & deploy")
+	assert.NoError(t, err)
+	assert.Equal(t, "build--deploy", name)
+
+	_, err = toDNSName("-build-and-deploy")
+	assert.ErrorIs(t, err, ErrDNSPatternInvalid)
+}