From f4fcb9bde63d7fa38b1962ab0a0f5e952b9b4246 Mon Sep 17 00:00:00 2001
From: Matt Moyer <moyerm@vmware.com>
Date: Wed, 3 Mar 2021 14:03:27 -0600
Subject: [PATCH] Sort CredentialIssuer strategies in preferred order.

This updates our issuerconfig.UpdateStrategy to sort strategies according to a weighted preference.
The TokenCredentialRequest API strategy is preffered, followed by impersonation proxy, followed by any other unknown types.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
---
 .../impersonator_config_test.go               |  2 +-
 .../issuerconfig/update_strategy.go           | 20 +++++++++---
 .../issuerconfig/update_strategy_test.go      | 31 +++++++++++++++++++
 3 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go
index 0c0f2147..73fb91c2 100644
--- a/internal/controller/impersonatorconfig/impersonator_config_test.go
+++ b/internal/controller/impersonatorconfig/impersonator_config_test.go
@@ -2116,7 +2116,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 				requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1])
 				requireCASecretWasCreated(kubeAPIClient.Actions()[2])
 				credentialIssuer := getCredentialIssuer()
-				r.Equal([]v1alpha1.CredentialIssuerStrategy{newPendingStrategy(), preExistingStrategy}, credentialIssuer.Status.Strategies)
+				r.Equal([]v1alpha1.CredentialIssuerStrategy{preExistingStrategy, newPendingStrategy()}, credentialIssuer.Status.Strategies)
 			})
 		})
 	}, spec.Parallel(), spec.Report(report.Terminal{}))
diff --git a/internal/controller/issuerconfig/update_strategy.go b/internal/controller/issuerconfig/update_strategy.go
index 669516ff..f357aed9 100644
--- a/internal/controller/issuerconfig/update_strategy.go
+++ b/internal/controller/issuerconfig/update_strategy.go
@@ -52,9 +52,21 @@ func mergeStrategy(configToUpdate *v1alpha1.CredentialIssuerStatus, strategy v1a
 	}
 }
 
-// TODO: sort strategies by server preference rather than alphanumerically by type.
+// weights are a set of priorities for each strategy type.
+//nolint: gochecknoglobals
+var weights = map[v1alpha1.StrategyType]int{
+	v1alpha1.KubeClusterSigningCertificateStrategyType: 2, // most preferred strategy
+	v1alpha1.ImpersonationProxyStrategyType:            1,
+	// unknown strategy types will have weight 0 by default
+}
+
 type sortableStrategies []v1alpha1.CredentialIssuerStrategy
 
-func (s sortableStrategies) Len() int           { return len(s) }
-func (s sortableStrategies) Less(i, j int) bool { return s[i].Type < s[j].Type }
-func (s sortableStrategies) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+func (s sortableStrategies) Len() int { return len(s) }
+func (s sortableStrategies) Less(i, j int) bool {
+	if wi, wj := weights[s[i].Type], weights[s[j].Type]; wi != wj {
+		return wi > wj
+	}
+	return s[i].Type < s[j].Type
+}
+func (s sortableStrategies) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
diff --git a/internal/controller/issuerconfig/update_strategy_test.go b/internal/controller/issuerconfig/update_strategy_test.go
index b1b90429..16302499 100644
--- a/internal/controller/issuerconfig/update_strategy_test.go
+++ b/internal/controller/issuerconfig/update_strategy_test.go
@@ -4,9 +4,13 @@
 package issuerconfig
 
 import (
+	"math/rand"
+	"sort"
 	"testing"
+	"testing/quick"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -185,3 +189,30 @@ func TestMergeStrategy(t *testing.T) {
 		})
 	}
 }
+
+func TestStrategySorting(t *testing.T) {
+	expected := []v1alpha1.CredentialIssuerStrategy{
+		{Type: v1alpha1.KubeClusterSigningCertificateStrategyType},
+		{Type: v1alpha1.ImpersonationProxyStrategyType},
+		{Type: "Type1"},
+		{Type: "Type2"},
+		{Type: "Type3"},
+	}
+	require.NoError(t, quick.Check(func(seed int64) bool {
+		// Create a randomly shuffled copy of the expected output.
+		//nolint:gosec // this is not meant to be a secure random, just a seeded RNG for shuffling deterministically
+		rng := rand.New(rand.NewSource(seed))
+		output := make([]v1alpha1.CredentialIssuerStrategy, len(expected))
+		copy(output, expected)
+		rng.Shuffle(
+			len(output),
+			func(i, j int) { output[i], output[j] = output[j], output[i] },
+		)
+
+		// Sort it using the code under test.
+		sort.Stable(sortableStrategies(output))
+
+		// Assert that it's sorted back to the expected output order.
+		return assert.Equal(t, expected, output)
+	}, nil))
+}