allow embedded cluster node labels with dns prefix (#4514)

* allow embedded cluster node labels with dns prefix

* allow dots in dns1123IllegalCharsRegex

pkg/embeddedcluster/roles.go

@@ -98,6 +98,20 @@ func getRoleNodeLabels(ctx context.Context, roles []string) ([]string, error) {
 	return getRoleLabelsImpl(config, roles), nil
 }
 
+// labelKey will clean up a string to be a valid label key.
+// ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
+func labelKey(s string) string {
+	parts := strings.SplitN(s, "/", 2)
+	if len(parts) == 1 {
+		return labelify(parts[0])
+	}
+	prefix := ConvertToRFC1123(parts[0])
+	name := labelify(parts[1])
+	return fmt.Sprintf("%s/%s", prefix, name)
+}
+
+// labelify will clean up a string to be a valid label value.
+// ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
 func labelify(s string) string {
 	// remove illegal characters
 	removechars := labelValueRegex.ReplaceAllString(s, "-")
@@ -112,6 +126,60 @@ func labelify(s string) string {
 	return trimmed
 }
 
+// borrowed from https://github.com/kubernetes/apimachinery/blob/9254095ca5cab3666d500ec67cd00f9ab0d113d7/pkg/util/validation/validation.go#L206-L208
+
+const dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
+const dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*"
+
+// DNS1123SubdomainMaxLength is a subdomain's max length in DNS (RFC 1123)
+const dns1123SubdomainMaxLength int = 253
+
+var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$")
+
+// IsValidRFC1123 tests for a string that conforms to the definition of a
+// subdomain in DNS (RFC 1123).
+func IsValidRFC1123(value string) bool {
+	if len(value) > dns1123SubdomainMaxLength {
+		return false
+	}
+	if !dns1123SubdomainRegexp.MatchString(value) {
+		return false
+	}
+	return true
+}
+
+var dns1123IllegalStartRegex = regexp.MustCompile(`^[^0-9a-z]+`)
+var dns1123IllegalEndRegex = regexp.MustCompile(`[^0-9a-z]+$`)
+var dns1123IllegalCharsRegex = regexp.MustCompile(`[^0-9a-z-.]`)
+
+func ConvertToRFC1123(value string, args ...int) string {
+	value = strings.ToLower(value)
+
+	if len(value) == 0 || IsValidRFC1123(value) {
+		return value
+	}
+
+	// failsafe
+	depth := 1
+	if len(args) > 0 {
+		depth = args[0]
+	}
+	if depth == 50 {
+		panic(fmt.Sprintf("failed to convert %q to valid dns 1123", value))
+	}
+	depth++
+
+	value = dns1123IllegalStartRegex.ReplaceAllString(value, "")
+	value = dns1123IllegalEndRegex.ReplaceAllString(value, "")
+	value = dns1123IllegalCharsRegex.ReplaceAllString(value, "")
+
+	if len(value) > dns1123SubdomainMaxLength {
+		value = value[0:dns1123SubdomainMaxLength]
+	}
+
+	return ConvertToRFC1123(value, depth)
+}
+
 func getRoleLabelsImpl(config *embeddedclusterv1beta1.ConfigSpec, roles []string) []string {
 	toReturn := []string{}
 
@@ -122,13 +190,13 @@ func getRoleLabelsImpl(config *embeddedclusterv1beta1.ConfigSpec, roles []string
 	for _, role := range roles {
 		if role == config.Roles.Controller.Name {
 			for k, v := range config.Roles.Controller.Labels {
-				toReturn = append(toReturn, fmt.Sprintf("%s=%s", labelify(k), labelify(v)))
+				toReturn = append(toReturn, fmt.Sprintf("%s=%s", labelKey(k), labelify(v)))
 			}
 		}
 		for _, customRole := range config.Roles.Custom {
 			if role == customRole.Name {
 				for k, v := range customRole.Labels {
-					toReturn = append(toReturn, fmt.Sprintf("%s=%s", labelify(k), labelify(v)))
+					toReturn = append(toReturn, fmt.Sprintf("%s=%s", labelKey(k), labelify(v)))
 				}
 			}
 		}

pkg/embeddedcluster/roles_test.go

@@ -180,6 +180,53 @@ func Test_getRoleLabelsImpl(t *testing.T) {
 			roles: []string{"a"},
 			want:  []string{"this-is-a-more-than-63-character-label-with-a-lot-of-filler-to=this-is-a-more-than-63-character-value-with-a-lot-of-filler-to"},
 		},
+		{
+			name: "roles with a dns prefix",
+			config: &embeddedclusterv1beta1.ConfigSpec{
+				Roles: embeddedclusterv1beta1.Roles{
+					Controller: embeddedclusterv1beta1.NodeRole{
+						Name: "a",
+						Labels: map[string]string{
+							"test-label.example.com/a-role": "a-role",
+						},
+					},
+				},
+			},
+			roles: []string{"a"},
+			want:  []string{"test-label.example.com/a-role=a-role"},
+		},
+		{
+			name: "roles with a dns prefix and name that is more than 63 characters",
+			config: &embeddedclusterv1beta1.ConfigSpec{
+				Roles: embeddedclusterv1beta1.Roles{
+					Controller: embeddedclusterv1beta1.NodeRole{
+						Name: "a",
+						Labels: map[string]string{
+							"test-label.example.com/this-is-a-more-than-63-character-label-with-a-lot-of-filler-to-ensure-that": "this is a more than 63 character value with a lot of filler to ensure that",
+						},
+					},
+				},
+			},
+			roles: []string{"a"},
+			want:  []string{"test-label.example.com/this-is-a-more-than-63-character-label-with-a-lot-of-filler-to=this-is-a-more-than-63-character-value-with-a-lot-of-filler-to"},
+		},
+		{
+			name: "roles with an invalid dns prefix",
+			config: &embeddedclusterv1beta1.ConfigSpec{
+				Roles: embeddedclusterv1beta1.Roles{
+					Controller: embeddedclusterv1beta1.NodeRole{
+						Name: "a",
+						Labels: map[string]string{
+							"this.is.not.valid-/test1":  "value1",
+							".this.is.not.valid./test2": "value2",
+							"!this.is.not.valid/test3":  "value3",
+						},
+					},
+				},
+			},
+			roles: []string{"a"},
+			want:  []string{"this.is.not.valid/test1=value1", "this.is.not.valid/test2=value2", "this.is.not.valid/test3=value3"},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {