From ae1d078b7da761ed41cf19691f28a2a25a231d05 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Wed, 15 Nov 2023 13:55:02 -0500 Subject: [PATCH] ensure label keys/values are legal --- pkg/embeddedcluster/node_join.go | 6 +- pkg/embeddedcluster/roles.go | 34 +++++++-- pkg/embeddedcluster/roles_test.go | 111 ++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 9 deletions(-) diff --git a/pkg/embeddedcluster/node_join.go b/pkg/embeddedcluster/node_join.go index 1591bef602..62ef585b40 100644 --- a/pkg/embeddedcluster/node_join.go +++ b/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 diff --git a/pkg/embeddedcluster/roles.go b/pkg/embeddedcluster/roles.go index 9e9152e671..4dd6f1c46c 100644 --- a/pkg/embeddedcluster/roles.go +++ b/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,27 +90,45 @@ 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))) } } } @@ -114,5 +136,5 @@ func getRoleNodeLabels(ctx context.Context, roles []string) ([]string, error) { sort.Strings(toReturn) - return toReturn, nil + return toReturn } diff --git a/pkg/embeddedcluster/roles_test.go b/pkg/embeddedcluster/roles_test.go index 094fdf6230..f471695cce 100644 --- a/pkg/embeddedcluster/roles_test.go +++ b/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) + }) + } +}