ensure label keys/values are legal

pkg/embeddedcluster/node_join.go

@@ -310,13 +310,13 @@ func getControllerNodeIP(ctx context.Context, client kubernetes.Interface) (stri
 }
 
 func getRolesNodeLabels(ctx context.Context, roles []string) (string, error) {
-	roleLabels := getRoleListLabels(roles)
+	roleListLabels := getRoleListLabels(roles)
 
 	labels, err := getRoleNodeLabels(ctx, roles)
 	if err != nil {
 		return "", fmt.Errorf("failed to get node labels for roles %v: %w", roles, err)
 	}
-	roleLabels = append(roleLabels, labels...)
+	roleLabels := append(roleListLabels, labels...)
 
 	return strings.Join(roleLabels, ","), nil
 }
@@ -328,7 +328,7 @@ func getRoleListLabels(roles []string) []string {
 	toReturn = append(toReturn, fmt.Sprintf("%s=total-%d", types.EMBEDDED_CLUSTER_ROLE_LABEL, len(roles)))
 
 	for idx, role := range roles {
-		toReturn = append(toReturn, fmt.Sprintf("%s-%d=%s", types.EMBEDDED_CLUSTER_ROLE_LABEL, idx, role))
+		toReturn = append(toReturn, fmt.Sprintf("%s-%d=%s", types.EMBEDDED_CLUSTER_ROLE_LABEL, idx, labelify(role)))
 	}
 
 	return toReturn

pkg/embeddedcluster/roles.go

@@ -3,13 +3,17 @@ package embeddedcluster
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"sort"
+	"strings"
 
 	embeddedclusterv1beta1 "github.com/replicatedhq/embedded-cluster-operator/api/v1beta1"
 )
 
 const DEFAULT_CONTROLLER_ROLE_NAME = "controller"
 
+var labelValueRegex = regexp.MustCompile(`[^a-zA-Z0-9-_.]+`)
+
 // GetRoles will get a list of role names
 func GetRoles(ctx context.Context) ([]string, error) {
 	config, err := ClusterConfig(ctx)
@@ -86,33 +90,51 @@ func SortRoles(controllerRole string, inputRoles []string) []string {
 
 // getRoleNodeLabels looks up roles in the cluster config and determines the additional labels to be applied from that
 func getRoleNodeLabels(ctx context.Context, roles []string) ([]string, error) {
-	toReturn := []string{}
-
 	config, err := ClusterConfig(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get cluster config: %w", err)
 	}
 
+	return getRoleLabelsImpl(config, roles), nil
+}
+
+func labelify(s string) string {
+	// remove illegal characters
+	removechars := labelValueRegex.ReplaceAllString(s, "-")
+	// remove leading dashes
+	trimmed := strings.TrimPrefix(removechars, "-")
+	// restrict it to 63 characters
+	if len(trimmed) > 63 {
+		trimmed = trimmed[:63]
+	}
+	// remove trailing dashes
+	trimmed = strings.TrimSuffix(trimmed, "-")
+	return trimmed
+}
+
+func getRoleLabelsImpl(config *embeddedclusterv1beta1.ConfigSpec, roles []string) []string {
+	toReturn := []string{}
+
 	if config == nil {
-		return toReturn, nil
+		return toReturn
 	}
 
 	for _, role := range roles {
 		if role == config.Controller.Name {
 			for k, v := range config.Controller.Labels {
-				toReturn = append(toReturn, fmt.Sprintf("%s=%s", k, v))
+				toReturn = append(toReturn, fmt.Sprintf("%s=%s", labelify(k), labelify(v)))
 			}
 		}
 		for _, customRole := range config.Custom {
 			if role == customRole.Name {
 				for k, v := range customRole.Labels {
-					toReturn = append(toReturn, fmt.Sprintf("%s=%s", k, v))
+					toReturn = append(toReturn, fmt.Sprintf("%s=%s", labelify(k), labelify(v)))
 				}
 			}
 		}
 	}
 
 	sort.Strings(toReturn)
 
-	return toReturn, nil
+	return toReturn
 }

pkg/embeddedcluster/roles_test.go

@@ -3,6 +3,7 @@ package embeddedcluster
 import (
 	"testing"
 
+	embeddedclusterv1beta1 "github.com/replicatedhq/embedded-cluster-operator/api/v1beta1"
 	"github.com/stretchr/testify/require"
 )
 
@@ -70,3 +71,113 @@ func TestSortRoles(t *testing.T) {
 		})
 	}
 }
+
+func Test_getRoleLabelsImpl(t *testing.T) {
+	tests := []struct {
+		name   string
+		config *embeddedclusterv1beta1.ConfigSpec
+		roles  []string
+		want   []string
+	}{
+		{
+			name:   "no config",
+			config: nil,
+			roles:  []string{"a", "b", "c"},
+			want:   []string{},
+		},
+		{
+			name: "no roles",
+			config: &embeddedclusterv1beta1.ConfigSpec{
+				Controller: embeddedclusterv1beta1.NodeRole{
+					Name: "a",
+				},
+			},
+			roles: []string{"a", "b", "c"},
+			want:  []string{},
+		},
+		{
+			name: "roles with labels",
+			config: &embeddedclusterv1beta1.ConfigSpec{
+				Controller: embeddedclusterv1beta1.NodeRole{
+					Name: "a",
+					Labels: map[string]string{
+						"a-role": "a-role",
+					},
+				},
+				Custom: []embeddedclusterv1beta1.NodeRole{
+					{
+						Name: "b",
+						Labels: map[string]string{
+							"b-role":  "b-role",
+							"b-role2": "b-role2",
+						},
+					},
+					{
+						Name: "c", // no labels for c
+					},
+					{
+						Name: "d", // d is not in the list of roles to make labels for
+						Labels: map[string]string{
+							"d-role":  "d-role",
+							"d-role2": "d-role2",
+						},
+					},
+				},
+			},
+			roles: []string{"a", "b", "c"},
+			want:  []string{"a-role=a-role", "b-role2=b-role2", "b-role=b-role"},
+		},
+		{
+			name: "roles with labels with bad characters",
+			config: &embeddedclusterv1beta1.ConfigSpec{
+				Controller: embeddedclusterv1beta1.NodeRole{
+					Name: "a",
+					Labels: map[string]string{
+						"a-role": "this is the a role",
+					},
+				},
+				Custom: []embeddedclusterv1beta1.NodeRole{
+					{
+						Name: "b",
+						Labels: map[string]string{
+							"b-role":  " this is the b role ",
+							"b-role2": "This! Is! The! Second! B! Role!",
+						},
+					},
+					{
+						Name: "c", // no labels for c
+					},
+					{
+						Name: "d", // d is not in the list of roles to make labels for
+						Labels: map[string]string{
+							"d-role":  "d-role",
+							"d-role2": "d-role2",
+						},
+					},
+				},
+			},
+			roles: []string{"a", "b", "c"},
+			want:  []string{"a-role=this-is-the-a-role", "b-role2=This-Is-The-Second-B-Role", "b-role=this-is-the-b-role"},
+		},
+		{
+			name: "roles more than 63 character labels",
+			config: &embeddedclusterv1beta1.ConfigSpec{
+				Controller: embeddedclusterv1beta1.NodeRole{
+					Name: "a",
+					Labels: map[string]string{
+						"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{"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"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := require.New(t)
+			got := getRoleLabelsImpl(tt.config, tt.roles)
+			req.Equal(tt.want, got)
+		})
+	}
+}