fix: label values sanitization and normalization

pkg/formatting/k8labels.go

@@ -1,25 +1,105 @@
 package formatting
 
 import (
+	"slices"
 	"strings"
+
+	"k8s.io/apimachinery/pkg/util/validation"
 )
 
-// CleanValueKubernetes conform a string to kubernetes naming convention
+var (
+	managedSpecialCharsMap = map[rune]string{
+		':': "-",
+		'/': "-",
+		' ': "_",
+		'[': "__",
+		']': "__",
+	}
+	allowedSpecialCharsLabelValue      = ".-_"
+	allowedSpecialCharsLabelValueRunes = []rune(allowedSpecialCharsLabelValue)
+
+	// Build the replacer starting from the managedSpecialCharsMap.
+	managedSpecialCharsInLabelValueReplacer = strings.NewReplacer(pairs(managedSpecialCharsMap)...)
+)
+
+// CleanValueKubernetes conforms a string to kubernetes naming convention
 // see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names
 // rules are:
 // • contain at most 63 characters
-// • contain only lowercase alphanumeric characters or '-'
+// • contain only alphanumeric characters or '-', '.', and '_'
 // • start with an alphanumeric character
 // • end with an alphanumeric character.
 func CleanValueKubernetes(s string) string {
-	if len(s) >= 63 {
-		// keep the last 62 characters
-		s = s[len(s)-62:]
+	// cut short if the string is already a valid label
+	if len(validation.IsValidLabelValue(s)) == 0 {
+		return s
+	}
+
+	// sanitize the input
+	sanitized := sanitizeLabelValue(s)
+
+	// replace the managed special symbols
+	replaced := managedSpecialCharsInLabelValueReplacer.Replace(sanitized)
+
+	// trim unwanted chars
+	trimmed := strings.Trim(replaced, allowedSpecialCharsLabelValue)
+
+	// cut to max length
+	cut := cutToLabelValueMaxLength(trimmed)
+
+	// trim left again to ensure no invalid values
+	// are at the edge after the cut
+	return strings.TrimLeft(cut, allowedSpecialCharsLabelValue)
+}
+
+// cutToLabelValueMaxLength cuts the provided string to the maximum length allowed
+// for a kubernetes Label (63 chars).
+func cutToLabelValueMaxLength(s string) string {
+	if len(s) > validation.LabelValueMaxLength {
+		return s[len(s)-(validation.LabelValueMaxLength):]
 	}
+	return s
+}
+
+// sanitizeLabelValue removes all unmanaged characters from the input string.
+func sanitizeLabelValue(s string) string {
+	b := strings.Builder{}
+	for _, r := range s {
+		if isSafe(r) {
+			b.WriteRune(r)
+		}
+	}
+	return b.String()
+}
+
+// pairs extracts all the key-value pairs from a rune-string map.
+func pairs(m map[rune]string) []string {
+	ss := make([]string, 0, len(m)*2)
+	for k, v := range m {
+		ss = append(ss, string(k), v)
+	}
+	return ss
+}
+
+// isSafe a helper to identify if a rune is safe to process or should be dropped.
+func isSafe(r rune) bool {
+	return isAllowedSpecialCharLabelValue(r) || isAlphanumeric(r) || isManagedSpecialChar(r)
+}
+
+// isAlphanumeric returns true if the rune is an alphanumeric value.
+func isAlphanumeric(r rune) bool {
+	return (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9')
+}
+
+// isAllowedSpecialCharLabelValue returns true if the rune is an allowed char as per
+// Kubernetes Label Values.
+func isAllowedSpecialCharLabelValue(r rune) bool {
+	return slices.Contains(allowedSpecialCharsLabelValueRunes, r)
+}
 
-	replasoeur := strings.NewReplacer(":", "-", "/", "-", " ", "_", "[", "__", "]", "__")
-	s = strings.TrimRight(s, " -_[]")
-	s = strings.TrimLeft(s, " -_[]")
-	replaced := replasoeur.Replace(s)
-	return strings.TrimSpace(replaced)
+// isManagedSpecialChar returns true if the rune is a managed special char.
+// The normalization of the value will replace it with its counterpart.
+func isManagedSpecialChar(r rune) bool {
+	_, ok := managedSpecialCharsMap[r]
+	return ok
 }

pkg/formatting/k8labels_test.go

@@ -3,6 +3,8 @@ package formatting
 import (
 	"strings"
 	"testing"
+
+	"k8s.io/apimachinery/pkg/util/validation"
 )
 
 func TestK8LabelsCleanup(t *testing.T) {
@@ -13,7 +15,7 @@ func TestK8LabelsCleanup(t *testing.T) {
 	}{
 		{
 			name: "clean characters for k8 labels",
-			str:  "foo/bar hello",
+			str:  "foo/bar h#@?!ello",
 			want: "foo-bar_hello",
 		},
 		{
@@ -36,11 +38,15 @@ func TestK8LabelsCleanup(t *testing.T) {
 			str:  "foo\n",
 			want: "foo",
 		},
-
+		{
+			name: "remove new line from the middle",
+			str:  "foo\nbar",
+			want: "foobar",
+		},
 		{
 			name: "secret name longer than 63 characters",
 			str:  strings.Repeat("a", 64),
-			want: strings.Repeat("a", 62),
+			want: strings.Repeat("a", 63),
 		},
 		{
 			name: "secret name ends with non-alphanumeric character",
@@ -57,12 +63,100 @@ func TestK8LabelsCleanup(t *testing.T) {
 			str:  "secret:name/with_underscores",
 			want: "secret-name-with_underscores",
 		},
+		{
+			name: "has an invalid start (.)",
+			str:  ".i-start-with-an-invalid-char",
+			want: "i-start-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid end (.)",
+			str:  "i-end-with-an-invalid-char.",
+			want: "i-end-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid start (:)",
+			str:  ":i-start-with-an-invalid-char",
+			want: "i-start-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid end (:)",
+			str:  "i-end-with-an-invalid-char:",
+			want: "i-end-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid start (/)",
+			str:  "/i-start-with-an-invalid-char",
+			want: "i-start-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid end (/)",
+			str:  "i-end-with-an-invalid-char/",
+			want: "i-end-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid start (-)",
+			str:  "-i-start-with-an-invalid-char",
+			want: "i-start-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid end (-)",
+			str:  "i-end-with-an-invalid-char-",
+			want: "i-end-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid start ( )",
+			str:  " i-start-with-an-invalid-char",
+			want: "i-start-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid end ( )",
+			str:  "i-end-with-an-invalid-char ",
+			want: "i-end-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid start ([)",
+			str:  "[i-start-with-an-invalid-char",
+			want: "i-start-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid end ([)",
+			str:  "i-end-with-an-invalid-char[",
+			want: "i-end-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid start (])",
+			str:  "]i-start-with-an-invalid-char",
+			want: "i-start-with-an-invalid-char",
+		},
+		{
+			name: "has an invalid end (])",
+			str:  "i-end-with-an-invalid-char]",
+			want: "i-end-with-an-invalid-char",
+		},
+		{
+			name: "long value once cut starts with a .",
+			str:  "dropped." + strings.Repeat("a", 62),
+			want: strings.Repeat("a", 62),
+		},
+		{
+			name: "ones with special chars won't get longer",
+			str:  strings.Repeat("[exp-63]", 10),
+			want: "63____exp-63____exp-63____exp-63____exp-63____exp-63____exp-63",
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := CleanValueKubernetes(tt.str); got != tt.want {
-				t.Errorf("K8LabelsCleanup() = %v, want %v", got, tt.want)
+			got := CleanValueKubernetes(tt.str)
+			validationErrs := validation.IsValidLabelValue(got)
+
+			switch {
+			case len(got) > validation.LabelValueMaxLength:
+				t.Errorf("K8LabelsCleanup() = %v, produced string is too long (%v/%v)", got, len(got), validation.LabelValueMaxLength)
+			case len(validationErrs) > 0:
+				t.Errorf("K8LabelsCleanup() = %v, is not a valid label: %v", got, validationErrs)
+			case got != tt.want:
+				t.Errorf("K8LabelsCleanup() = given %v, got %v, want %v", tt.str, got, tt.want)
 			}
 		})
 	}