From 10ab56229d2ad600db14030f8753d50e15718bf4 Mon Sep 17 00:00:00 2001
From: Andrew Lavery <laverya@umich.edu>
Date: Tue, 14 Nov 2023 22:16:37 -0500
Subject: [PATCH] sort node roles before creating the node join command

---
 pkg/embeddedcluster/roles.go                  | 46 ++++++++++++
 pkg/embeddedcluster/roles_test.go             | 72 +++++++++++++++++++
 pkg/embeddedcluster/util.go                   | 14 ----
 .../embedded_cluster_node_join_command.go     |  3 +
 4 files changed, 121 insertions(+), 14 deletions(-)
 create mode 100644 pkg/embeddedcluster/roles_test.go

diff --git a/pkg/embeddedcluster/roles.go b/pkg/embeddedcluster/roles.go
index b5d1d4887f..3111a528df 100644
--- a/pkg/embeddedcluster/roles.go
+++ b/pkg/embeddedcluster/roles.go
@@ -3,6 +3,7 @@ package embeddedcluster
 import (
 	"context"
 	"fmt"
+	"sort"
 
 	embeddedclusterv1beta1 "github.com/replicatedhq/embedded-cluster-operator/api/v1beta1"
 )
@@ -37,3 +38,48 @@ func GetRoles(ctx context.Context) ([]string, error) {
 
 	return roles, nil
 }
+
+// ControllerRoleName determines the name for the 'controller' role
+// this might be part of the config, or it might be the default
+func ControllerRoleName(ctx context.Context) (string, error) {
+	conf, err := ClusterConfig(ctx)
+	if err != nil {
+		return "", fmt.Errorf("failed to get cluster config: %w", err)
+	}
+
+	if conf != nil && conf.Controller.Name != "" {
+		return conf.Controller.Name, nil
+	}
+	return DEFAULT_CONTROLLER_ROLE_NAME, nil
+}
+
+// sort roles by name, but put controller first
+func SortRoles(controllerRole string, inputRoles []string) []string {
+	roles := inputRoles
+
+	// determine if the controller role is present
+	hasController := false
+	controllerIdx := 0
+	for idx, role := range roles {
+		if role == controllerRole {
+			hasController = true
+			controllerIdx = idx
+			break
+		}
+	}
+
+	// if the controller role is present, remove it
+	if hasController {
+		roles = append(roles[:controllerIdx], roles[controllerIdx+1:]...)
+	}
+
+	// sort the roles
+	sort.Strings(roles)
+
+	// if the controller role was present, add it back to the front
+	if hasController {
+		roles = append([]string{controllerRole}, roles...)
+	}
+
+	return roles
+}
diff --git a/pkg/embeddedcluster/roles_test.go b/pkg/embeddedcluster/roles_test.go
new file mode 100644
index 0000000000..094fdf6230
--- /dev/null
+++ b/pkg/embeddedcluster/roles_test.go
@@ -0,0 +1,72 @@
+package embeddedcluster
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSortRoles(t *testing.T) {
+	tests := []struct {
+		name           string
+		controllerRole string
+		inputRoles     []string
+		want           []string
+	}{
+		{
+			name:           "no controller",
+			controllerRole: "",
+			inputRoles:     []string{"c", "b", "a"},
+			want:           []string{"a", "b", "c"},
+		},
+		{
+			name:           "controller in front",
+			controllerRole: "controller",
+			inputRoles:     []string{"controller", "c", "b"},
+			want:           []string{"controller", "b", "c"},
+		},
+		{
+			name:           "controller in middle",
+			controllerRole: "controller",
+			inputRoles:     []string{"c", "controller", "b"},
+			want:           []string{"controller", "b", "c"},
+		},
+		{
+			name:           "controller at end",
+			controllerRole: "controller",
+			inputRoles:     []string{"c", "b", "controller"},
+			want:           []string{"controller", "b", "c"},
+		},
+		{
+			name:           "controller not present",
+			controllerRole: "controller",
+			inputRoles:     []string{"c", "b", "d"},
+			want:           []string{"b", "c", "d"},
+		},
+		{
+			name:           "other controller name",
+			controllerRole: "other-controller",
+			inputRoles:     []string{"c", "b", "a"},
+			want:           []string{"a", "b", "c"},
+		},
+		{
+			name:           "other controller name in list",
+			controllerRole: "other-controller",
+			inputRoles:     []string{"c", "b", "other-controller"},
+			want:           []string{"other-controller", "b", "c"},
+		},
+		{
+			name:           "more items",
+			controllerRole: "controller",
+			inputRoles:     []string{"a", "b", "c", "controller", "e", "f", "x", "y", "z", "g", "h", "i", "j", "k", "l", "m", "n"},
+			want:           []string{"controller", "a", "b", "c", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "x", "y", "z"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := require.New(t)
+			got := SortRoles(tt.controllerRole, tt.inputRoles)
+			req.Equal(tt.want, got)
+		})
+	}
+}
diff --git a/pkg/embeddedcluster/util.go b/pkg/embeddedcluster/util.go
index 0f21b26906..1ed8576092 100644
--- a/pkg/embeddedcluster/util.go
+++ b/pkg/embeddedcluster/util.go
@@ -89,17 +89,3 @@ func ClusterConfig(ctx context.Context) (*embeddedclusterv1beta1.ConfigSpec, err
 	latest := installationList.Items[0]
 	return latest.Spec.Config, nil
 }
-
-// ControllerRoleName determines the name for the 'controller' role
-// this might be part of the config, or it might be the default
-func ControllerRoleName(ctx context.Context) (string, error) {
-	conf, err := ClusterConfig(ctx)
-	if err != nil {
-		return "", fmt.Errorf("failed to get cluster config: %w", err)
-	}
-
-	if conf != nil && conf.Controller.Name != "" {
-		return conf.Controller.Name, nil
-	}
-	return DEFAULT_CONTROLLER_ROLE_NAME, nil
-}
diff --git a/pkg/handlers/embedded_cluster_node_join_command.go b/pkg/handlers/embedded_cluster_node_join_command.go
index e18789b9e2..76856cb464 100644
--- a/pkg/handlers/embedded_cluster_node_join_command.go
+++ b/pkg/handlers/embedded_cluster_node_join_command.go
@@ -92,6 +92,9 @@ func (h *Handler) GetEmbeddedClusterNodeJoinCommand(w http.ResponseWriter, r *ht
 		}
 	}
 
+	// sort roles by name, but put controller first
+	roles = embeddedcluster.SortRoles(controllerRoleName, roles)
+
 	k0sToken, err := embeddedcluster.GenerateAddNodeToken(r.Context(), client, k0sRole)
 	if err != nil {
 		logger.Error(fmt.Errorf("failed to generate add node token: %w", err))