Add util functions to convert Kubernetes DNS-1123 incompatible names …

…to compatible names (flyteorg#211)

* Add util functions to convert DNS-1123 incompatible name to compatible name
* Move encoder from flytepropeller
* Add DNS-1123 conversion to pod name in task
Signed-off-by: Sean Lin <sean@union.ai>

go/tasks/pluginmachinery/encoding/encoder.go

@@ -0,0 +1,49 @@
+package encoding
+
+import (
+	"encoding/base32"
+	"fmt"
+	"hash/fnv"
+	"strings"
+)
+
+const specialEncoderKey = "abcdefghijklmnopqrstuvwxyz123456"
+
+var Base32Encoder = base32.NewEncoding(specialEncoderKey).WithPadding(base32.NoPadding)
+
+// Creates a new UniqueID that is based on the inputID and of a specified length, if the given id is longer than the
+// maxLength.
+func FixedLengthUniqueID(inputID string, maxLength int) (string, error) {
+	if len(inputID) <= maxLength {
+		return inputID, nil
+	}
+
+	hasher := fnv.New32a()
+	// Using 32a an error can never happen, so this will always remain not covered by a unit test
+	_, _ = hasher.Write([]byte(inputID)) // #nosec
+	b := hasher.Sum(nil)
+	// expected length after this step is 8 chars (1 + 7 chars from Base32Encoder.EncodeToString(b))
+	finalStr := "f" + Base32Encoder.EncodeToString(b)
+	if len(finalStr) > maxLength {
+		return finalStr, fmt.Errorf("max Length is too small, cannot create an encoded string that is so small")
+	}
+	return finalStr, nil
+}
+
+// Creates a new uniqueID using the parts concatenated using `-` and ensures that the uniqueID is not longer than the
+// maxLength. In case a simple concatenation yields a longer string, a new hashed ID is created which is always
+// around 8 characters in length
+func FixedLengthUniqueIDForParts(maxLength int, parts ...string) (string, error) {
+	b := strings.Builder{}
+	for i, p := range parts {
+		if i > 0 && b.Len() > 0 {
+			// Ignoring the error as it always returns a nil error
+			_, _ = b.WriteRune('-') // #nosec
+		}
+
+		// Ignoring the error as this is always nil
+		_, _ = b.WriteString(p) // #nosec
+	}
+
+	return FixedLengthUniqueID(b.String(), maxLength)
+}

go/tasks/pluginmachinery/encoding/encoder_test.go

@@ -0,0 +1,61 @@
+package encoding
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFixedLengthUniqueID(t *testing.T) {
+	tests := []struct {
+		name        string
+		input       string
+		maxLength   int
+		output      string
+		expectError bool
+	}{
+		{"smallerThanMax", "x", 5, "x", false},
+		{"veryLowLimit", "xx", 1, "flfryc2i", true},
+		{"highLimit", "xxxxxx", 5, "fufiti6i", true},
+		{"higherLimit", "xxxxx", 10, "xxxxx", false},
+		{"largeID", "xxxxxxxxxxx", 10, "fggddjly", false},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			i, err := FixedLengthUniqueID(test.input, test.maxLength)
+			if test.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, i, test.output)
+		})
+	}
+}
+
+func TestFixedLengthUniqueIDForParts(t *testing.T) {
+	tests := []struct {
+		name        string
+		parts       []string
+		maxLength   int
+		output      string
+		expectError bool
+	}{
+		{"smallerThanMax", []string{"x", "y", "z"}, 10, "x-y-z", false},
+		{"veryLowLimit", []string{"x", "y"}, 1, "fz2jizji", true},
+		{"fittingID", []string{"x"}, 2, "x", false},
+		{"highLimit", []string{"x", "y", "z"}, 4, "fxzsoqrq", true},
+		{"largeID", []string{"x", "y", "z", "m", "n"}, 8, "fsigbmty", false},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			i, err := FixedLengthUniqueIDForParts(test.maxLength, test.parts...)
+			if test.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, i, test.output)
+		})
+	}
+}

go/tasks/pluginmachinery/utils/dns.go

@@ -0,0 +1,39 @@
+package utils
+
+import (
+	"regexp"
+	"strings"
+
+	"github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/encoding"
+	"k8s.io/apimachinery/pkg/util/validation"
+)
+
+var dns1123InvalidRegex = regexp.MustCompile("[^-.a-z0-9]")
+var camelCaseRegex = regexp.MustCompile("([a-z0-9])([A-Z])")
+
+const maxUniqueIDLength = 20
+
+// ConvertToDNS1123SubdomainCompatibleString converts a string that doesn't conform to the definition of a subdomain in DNS (RFC 1123) to a string that conforms. It doesn't do well on labels (separated by dots) starting or ending with hyphens.
+func ConvertToDNS1123SubdomainCompatibleString(name string) string {
+	if errs := validation.IsDNS1123Subdomain(name); len(errs) == 0 {
+		return name
+	}
+	name = ConvertCamelCaseToKebabCase(name) // best effort to preserve readability for Java class name
+	name = strings.ToLower(name)
+	name = dns1123InvalidRegex.ReplaceAllString(name, "")
+	name = strings.Trim(name, ".-")
+	if len(name) > validation.DNS1123SubdomainMaxLength {
+		fixedLengthID, err := encoding.FixedLengthUniqueID(name, maxUniqueIDLength)
+		if err == nil {
+			name = name[:validation.DNS1123SubdomainMaxLength-maxUniqueIDLength-1] + "-" + fixedLengthID
+		} else {
+			name = name[:validation.DNS1123SubdomainMaxLength]
+		}
+	}
+	return name
+}
+
+// ConvertCamelCaseToKebabCase rewrites a string written in camel case (e.g. PenPineappleApplePen) in kebab case (pen-pineapple-apple-pen)
+func ConvertCamelCaseToKebabCase(name string) string {
+	return strings.ToLower(camelCaseRegex.ReplaceAllString(name, "${1}-${2}"))
+}

go/tasks/pluginmachinery/utils/dns_test.go

@@ -0,0 +1,109 @@
+package utils
+
+import (
+	"testing"
+
+	"k8s.io/apimachinery/pkg/util/validation"
+)
+
+func TestConvertToDNS1123CompatibleString(t *testing.T) {
+	type args struct {
+		name string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "flytekit-java task execution",
+			args: args{"orgflyteexamplesHelloWorldTask-0"},
+			want: "orgflyteexamples-hello-world-task-0",
+		},
+		{
+			name: "good pod name",
+			args: args{"t7vyqhzju1-fib-5-0"},
+			want: "t7vyqhzju1-fib-5-0",
+		},
+		{
+			name: "good pod name with dots",
+			args: args{"t7v.yqh.zju1-fib-5-0"},
+			want: "t7v.yqh.zju1-fib-5-0",
+		},
+		{
+			name: "leading hyphen",
+			args: args{"-t7vyqhzju1-fib-5-0"},
+			want: "t7vyqhzju1-fib-5-0",
+		},
+		{
+			name: "leading dot",
+			args: args{".t7vyqhzju1-fib-5-0"},
+			want: "t7vyqhzju1-fib-5-0",
+		},
+		{
+			name: "trailing hyphen",
+			args: args{"t7vyqhzju1-fib-5-0-"},
+			want: "t7vyqhzju1-fib-5-0",
+		},
+		{
+			name: "trailing dot",
+			args: args{"t7vyqhzju1-fib-5-0."},
+			want: "t7vyqhzju1-fib-5-0",
+		},
+		{
+			name: "long name",
+			args: args{"0123456789012345678901234567890123456789012345678901234567890123456789"},
+			want: "0123456789012345678901234567890123456789012345678901234567890123456789",
+		},
+		{
+			name: "longer than max len (253)",
+			args: args{"0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"},
+			want: "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901-fbbrvh4i",
+		},
+		{
+			name: "very invalid name",
+			args: args{"---..t7vyqhzjJcI==u1-HelloWorldTask[].-.-."},
+			want: "t7vyqhzj-jc-iu1-hello-world-task",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ConvertToDNS1123SubdomainCompatibleString(tt.args.name)
+			if errs := validation.IsDNS1123Subdomain(got); len(errs) > 0 {
+				t.Errorf("ConvertToDNS1123SubdomainCompatibleString() = %v, which is not DNS-1123 subdomain compatible", got)
+			}
+			if got != tt.want {
+				t.Errorf("ConvertToDNS1123SubdomainCompatibleString() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestConvertCamelCaseToKebabCase(t *testing.T) {
+	type args struct {
+		name string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "flytekit-java task execution",
+			args: args{"orgflyteexamplesHelloWorldTask"},
+			want: "orgflyteexamples-hello-world-task",
+		},
+		{
+			name: "good pod name",
+			args: args{"t7vyqhzju1-fib-5-0"},
+			want: "t7vyqhzju1-fib-5-0",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ConvertCamelCaseToKebabCase(tt.args.name); got != tt.want {
+				t.Errorf("ConvertCamelCaseToKebabCase() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

go/tasks/plugins/array/k8s/launcher.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/utils"
+
 	"github.com/flyteorg/flyteplugins/go/tasks/plugins/array/errorcollector"
 
 	arrayCore "github.com/flyteorg/flyteplugins/go/tasks/plugins/array/core"
@@ -32,7 +34,7 @@ var arrayJobEnvVars = []corev1.EnvVar{
 }
 
 func formatSubTaskName(_ context.Context, parentName, suffix string) (subTaskName string) {
-	return fmt.Sprintf("%v-%v", parentName, suffix)
+	return utils.ConvertToDNS1123SubdomainCompatibleString(fmt.Sprintf("%v-%v", parentName, suffix))
 }
 
 func ApplyPodPolicies(_ context.Context, cfg *Config, pod *corev1.Pod) *corev1.Pod {

go/tasks/plugins/array/k8s/task_test.go

@@ -25,6 +25,7 @@ func TestFinalize(t *testing.T) {
 	podTemplate, _, _ := FlyteArrayJobToK8sPodTemplate(ctx, tCtx, "")
 	pod := addPodFinalizer(&podTemplate)
 	pod.Name = formatSubTaskName(ctx, tCtx.TaskExecutionMetadata().GetTaskExecutionID().GetGeneratedName(), "1")
+	assert.Equal(t, "notfound-1", pod.Name)
 	assert.NoError(t, kubeClient.GetClient().Create(ctx, pod))
 
 	resourceManager.OnReleaseResourceMatch(mock.Anything, mock.Anything, mock.Anything).Return(nil)