Fix bug when deduplicating the discovered credentials.

cmd/clusters-service/pkg/credentials/credentials.go

@@ -11,6 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/discovery"
 
 	// load the gcp plugin (only required to authenticate against GKE clusters).
@@ -19,35 +20,34 @@ import (
 	kyaml "sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
-type IdentityParams struct {
+var identityParamsList = []struct {
 	Group        string
 	Versions     []string
 	Kind         string
 	ClusterKinds []string
-}
+}{
 
-var IdentityParamsList = []IdentityParams{
 	{
 		Group:        "infrastructure.cluster.x-k8s.io",
-		Versions:     []string{"v1alpha3", "v1alpha4"},
+		Versions:     []string{"v1alpha3", "v1alpha4", "v1beta1"},
 		Kind:         "AWSClusterStaticIdentity",
 		ClusterKinds: []string{"AWSCluster", "AWSManagedCluster", "AWSManagedControlPlane"},
 	},
 	{
 		Group:        "infrastructure.cluster.x-k8s.io",
-		Versions:     []string{"v1alpha3", "v1alpha4"},
+		Versions:     []string{"v1alpha3", "v1alpha4", "v1beta1"},
 		Kind:         "AWSClusterRoleIdentity",
 		ClusterKinds: []string{"AWSCluster", "AWSManagedCluster", "AWSManagedControlPlane"},
 	},
 	{
 		Group:        "infrastructure.cluster.x-k8s.io",
-		Versions:     []string{"v1alpha3", "v1alpha4"},
+		Versions:     []string{"v1alpha3", "v1alpha4", "v1beta1"},
 		Kind:         "AzureClusterIdentity",
 		ClusterKinds: []string{"AzureCluster", "AzureManagedCluster"},
 	},
 	{
 		Group:        "infrastructure.cluster.x-k8s.io",
-		Versions:     []string{"v1alpha3", "v1alpha4"},
+		Versions:     []string{"v1alpha3", "v1alpha4", "v1beta1"},
 		Kind:         "VSphereClusterIdentity",
 		ClusterKinds: []string{"VSphereCluster"},
 	},
@@ -62,10 +62,10 @@ func isEmptyCredentials(creds *capiv1_proto.Credential) bool {
 }
 
 // FindCredentials returns all the custom resources in the cluster that we think are CAPI identities. What we "think" are
-// capi identities are hardcoded in this file in `IdentityParamsList`
+// capi identities are hardcoded in this file in `identityParamsList`
 func FindCredentials(ctx context.Context, c client.Client, dc discovery.DiscoveryInterface) ([]unstructured.Unstructured, error) {
 	identities := []unstructured.Unstructured{}
-	for _, identityParams := range IdentityParamsList {
+	for _, identityParams := range identityParamsList {
 		for _, v := range identityParams.Versions {
 			gvk := schema.GroupVersionKind{
 				Group:   identityParams.Group,
@@ -99,9 +99,9 @@ func FindCredentials(ctx context.Context, c client.Client, dc discovery.Discover
 	// k8s doesn't internally differentiate between different apiVersions so we de-dup them
 	// https://github.com/kubernetes/kubernetes/issues/58131#issuecomment-403829566
 	//
-	identityIndex := map[schema.GroupKind]unstructured.Unstructured{}
+	identityIndex := map[types.UID]unstructured.Unstructured{}
 	for _, ident := range identities {
-		identityIndex[ident.GroupVersionKind().GroupKind()] = ident
+		identityIndex[ident.GetUID()] = ident
 	}
 	uniqueIdentities := []unstructured.Unstructured{}
 	for _, v := range identityIndex {
@@ -120,11 +120,13 @@ func checkCRDExists(dc discovery.DiscoveryInterface, gvk schema.GroupVersionKind
 	if err != nil {
 		return false, fmt.Errorf("failed to get resources for GV %v: %w", gv, err)
 	}
+
 	availableKindsIndex := map[string]bool{}
 	for _, api := range apiResources.APIResources {
 		availableKindsIndex[api.Kind] = true
 	}
 	_, ok := availableKindsIndex[gvk.Kind]
+
 	return ok, nil
 }
 
@@ -186,7 +188,7 @@ func InjectCredentials(tmplWithValues [][]byte, creds *capiv1_proto.Credential)
 	newBits := [][]byte{}
 	for _, bit := range tmplWithValues {
 		var err error
-		for _, identityParams := range IdentityParamsList {
+		for _, identityParams := range identityParamsList {
 			for _, v := range identityParams.Versions {
 				// see if we can find the capi type in the list here.
 				if creds.Group == identityParams.Group && creds.Kind == identityParams.Kind && creds.Version == v {

cmd/clusters-service/pkg/credentials/credentials_test.go

@@ -3,15 +3,21 @@ package credentials
 import (
 	"context"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	capiv1 "github.com/weaveworks/weave-gitops-enterprise/cmd/clusters-service/api/capi/v1alpha1"
 	capiv1_protos "github.com/weaveworks/weave-gitops-enterprise/cmd/clusters-service/pkg/protos"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+	fakediscovery "k8s.io/client-go/discovery/fake"
+	k8stesting "k8s.io/client-go/testing"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
@@ -71,7 +77,7 @@ func TestCheckCredentialsExist(t *testing.T) {
 		Version: "v1alpha4",
 	})
 
-	c := createFakeClient(t)
+	c := newFakeClient(t)
 	_ = c.Create(context.Background(), u)
 
 	creds := &capiv1_protos.Credential{
@@ -160,15 +166,91 @@ spec:
 	}
 }
 
-func convertToStringArray(in [][]byte) []string {
-	var result []string
-	for _, i := range in {
-		result = append(result, string(i))
+func TestFindCredentials(t *testing.T) {
+	apiResources := []*metav1.APIResourceList{
+		{
+			GroupVersion: "infrastructure.cluster.x-k8s.io/v1alpha4",
+			APIResources: []metav1.APIResource{
+				{Name: "awsclusterroleidentities", SingularName: "awsclusterroleidentity", Kind: "AWSClusterRoleIdentity", Namespaced: true},
+				{Name: "azureclusteridentities", SingularName: "azureclusteridentity", Kind: "AzureClusterIdentity", Namespaced: true},
+			},
+		},
+	}
+	fakeDiscovery := &fakediscovery.FakeDiscovery{Fake: &k8stesting.Fake{Resources: apiResources}}
+
+	findTests := []struct {
+		name        string
+		clusterObjs []runtime.Object
+		want        []unstructured.Unstructured
+	}{
+		{
+			"no credentials",
+			[]runtime.Object{},
+			[]unstructured.Unstructured{},
+		},
+		{
+			"single credential",
+			[]runtime.Object{newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds", "test-ns", "uid1")},
+			[]unstructured.Unstructured{
+				*newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds", "test-ns", "uid1")},
+		},
+		{
+			"multi credentials",
+			[]runtime.Object{
+				newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds1", "test-ns", "uid1"),
+				newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds2", "test-ns", "uid2"),
+			},
+			[]unstructured.Unstructured{
+				*newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds1", "test-ns", "uid1"),
+				*newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds2", "test-ns", "uid2"),
+			},
+		},
+		{
+			"multi credentials - returned for different versions",
+			[]runtime.Object{
+				newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha3", "AWSClusterRoleIdentity", "test-creds1", "test-ns", "uid1"),
+				newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds1", "test-ns", "uid1"),
+			},
+			[]unstructured.Unstructured{
+				*newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds1", "test-ns", "uid1"),
+			},
+		},
+		{
+			"multi different kind & identities",
+			[]runtime.Object{
+				newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AzureClusterIdentity", "test-creds1", "test-ns", "uid1"),
+				newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds2", "test-ns", "uid2"),
+			},
+			[]unstructured.Unstructured{
+				*newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AzureClusterIdentity", "test-creds1", "test-ns", "uid1"),
+				*newUnstructured("infrastructure.cluster.x-k8s.io/v1alpha4", "AWSClusterRoleIdentity", "test-creds2", "test-ns", "uid2"),
+			},
+		},
+	}
+
+	for _, tt := range findTests {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeClient := newFakeClient(t, tt.clusterObjs...)
+
+			found, err := FindCredentials(context.TODO(), fakeClient, fakeDiscovery)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			credSorter := func(a, b unstructured.Unstructured) bool {
+				return strings.Compare(string(a.GetUID()), string(b.GetUID())) < 0
+			}
+			resourceVersion := func(k string, _ interface{}) bool {
+				return k == "resourceVersion"
+			}
+			if diff := cmp.Diff(tt.want, found, cmpopts.SortSlices(credSorter), cmpopts.IgnoreMapEntries(resourceVersion)); diff != "" {
+				t.Fatalf("FindCredentials() failed:\n%s", diff)
+			}
+		})
 	}
-	return result
 }
 
-func createFakeClient(t *testing.T) client.Client {
+func newFakeClient(t *testing.T, objs ...runtime.Object) client.Client {
 	scheme := runtime.NewScheme()
 	schemeBuilder := runtime.SchemeBuilder{
 		corev1.AddToScheme,
@@ -181,5 +263,24 @@ func createFakeClient(t *testing.T) client.Client {
 
 	return fake.NewClientBuilder().
 		WithScheme(scheme).
+		WithRuntimeObjects(objs...).
 		Build()
 }
+
+func newUnstructured(apiVersion, kind, name, namespace, uid string) *unstructured.Unstructured {
+	u := &unstructured.Unstructured{}
+	u.SetName(name)
+	u.SetNamespace(namespace)
+	u.SetKind(kind)
+	u.SetAPIVersion(apiVersion)
+	u.SetUID(types.UID(uid))
+	return u
+}
+
+func convertToStringArray(in [][]byte) []string {
+	var result []string
+	for _, i := range in {
+		result = append(result, string(i))
+	}
+	return result
+}