chore(appset): simplify cluster list code (#21820)

Signed-off-by: Michael Crenshaw <[email protected]>

applicationset/controllers/applicationset_controller.go

@@ -768,7 +768,7 @@ func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, logCtx *
 }
 
 // removeFinalizerOnInvalidDestination removes the Argo CD resources finalizer if the application contains an invalid target (eg missing cluster)
-func (r *ApplicationSetReconciler) removeFinalizerOnInvalidDestination(ctx context.Context, applicationSet argov1alpha1.ApplicationSet, app *argov1alpha1.Application, clusterList *argov1alpha1.ClusterList, appLog *log.Entry) error {
+func (r *ApplicationSetReconciler) removeFinalizerOnInvalidDestination(ctx context.Context, applicationSet argov1alpha1.ApplicationSet, app *argov1alpha1.Application, clusterList []utils.ClusterSpecifier, appLog *log.Entry) error {
 	// Only check if the finalizers need to be removed IF there are finalizers to remove
 	if len(app.Finalizers) == 0 {
 		return nil
@@ -783,7 +783,7 @@ func (r *ApplicationSetReconciler) removeFinalizerOnInvalidDestination(ctx conte
 	} else {
 		// Detect if the destination's server field does not match an existing cluster
 		matchingCluster := false
-		for _, cluster := range clusterList.Items {
+		for _, cluster := range clusterList {
 			if destCluster.Server != cluster.Server {
 				continue
 			}

applicationset/generators/cluster.go

@@ -70,7 +70,7 @@ func (g *ClusterGenerator) GenerateParams(appSetGenerator *argoappsetv1alpha1.Ap
 	// - Since local clusters do not have secrets, they do not have labels to match against
 	ignoreLocalClusters := len(appSetGenerator.Clusters.Selector.MatchExpressions) > 0 || len(appSetGenerator.Clusters.Selector.MatchLabels) > 0
 
-	// ListCluster from Argo CD's util/db package will include the local cluster in the list of clusters
+	// ListCluster will include the local cluster in the list of clusters
 	clustersFromArgoCD, err := utils.ListClusters(g.ctx, g.clientset, g.namespace)
 	if err != nil {
 		return nil, fmt.Errorf("error listing clusters: %w", err)
@@ -93,7 +93,7 @@ func (g *ClusterGenerator) GenerateParams(appSetGenerator *argoappsetv1alpha1.Ap
 	logCtx.Debugf("Using flat mode = %t for cluster generator", isFlatMode)
 	clustersParams := make([]map[string]any, 0)
 
-	for _, cluster := range clustersFromArgoCD.Items {
+	for _, cluster := range clustersFromArgoCD {
 		// If there is a secret for this cluster, then it's a non-local cluster, so it will be
 		// handled by the next step.
 		if secretForCluster, exists := clusterSecrets[cluster.Name]; exists {

applicationset/generators/duck_type.go

@@ -165,9 +165,6 @@ func (g *DuckTypeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.A
 	}
 	log.Infof("Number of decisions found: %v", len(clusterDecisions))
 
-	// Read this outside the loop to improve performance
-	argoClusters := clustersFromArgoCD.Items
-
 	if len(clusterDecisions) == 0 {
 		log.Warningf("clusterDecisionResource status.%s missing", statusListKey)
 		return nil, nil
@@ -188,7 +185,7 @@ func (g *DuckTypeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.A
 
 		found := false
 
-		for _, argoCluster := range argoClusters {
+		for _, argoCluster := range clustersFromArgoCD {
 			if argoCluster.Name == strMatchValue {
 				log.WithField(matchKey, argoCluster.Name).Info("matched cluster in ArgoCD")
 				params["name"] = argoCluster.Name

applicationset/utils/clusterUtils.go

@@ -3,7 +3,6 @@ package utils
 import (
 	"context"
 	"fmt"
-	"sync"
 
 	"github.com/argoproj/argo-cd/v3/common"
 	appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
@@ -13,16 +12,14 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
-var (
-	localCluster = appv1.Cluster{
-		Name:            "in-cluster",
-		Server:          appv1.KubernetesInternalAPIServerAddr,
-		ConnectionState: appv1.ConnectionState{Status: appv1.ConnectionStatusSuccessful},
-	}
-	initLocalCluster sync.Once
-)
+// ClusterSpecifier contains only the name and server URL of a cluster. We use this struct to avoid partially-populating
+// the full Cluster struct, which would be misleading.
+type ClusterSpecifier struct {
+	Name   string
+	Server string
+}
 
-func ListClusters(ctx context.Context, clientset kubernetes.Interface, namespace string) (*appv1.ClusterList, error) {
+func ListClusters(ctx context.Context, clientset kubernetes.Interface, namespace string) ([]ClusterSpecifier, error) {
 	clusterSecretsList, err := clientset.CoreV1().Secrets(namespace).List(ctx,
 		metav1.ListOptions{LabelSelector: common.LabelKeySecretType + "=" + common.LabelValueSecretTypeCluster})
 	if err != nil {
@@ -35,54 +32,29 @@ func ListClusters(ctx context.Context, clientset kubernetes.Interface, namespace
 
 	clusterSecrets := clusterSecretsList.Items
 
-	clusterList := appv1.ClusterList{
-		Items: make([]appv1.Cluster, len(clusterSecrets)),
-	}
+	clusterList := make([]ClusterSpecifier, len(clusterSecrets))
+
 	hasInClusterCredentials := false
 	for i, clusterSecret := range clusterSecrets {
-		// This line has changed from the original Argo CD code: now receives an error, and handles it
 		cluster, err := db.SecretToCluster(&clusterSecret)
 		if err != nil || cluster == nil {
 			return nil, fmt.Errorf("unable to convert cluster secret to cluster object '%s': %w", clusterSecret.Name, err)
 		}
-
-		// db.SecretToCluster populates these, but they're not meant to be available to the caller.
-		cluster.Labels = nil
-		cluster.Annotations = nil
-
-		clusterList.Items[i] = *cluster
+		clusterList[i] = ClusterSpecifier{
+			Name:   cluster.Name,
+			Server: cluster.Server,
+		}
 		if cluster.Server == appv1.KubernetesInternalAPIServerAddr {
 			hasInClusterCredentials = true
 		}
 	}
 	if !hasInClusterCredentials {
-		localCluster := getLocalCluster(clientset)
-		if localCluster != nil {
-			clusterList.Items = append(clusterList.Items, *localCluster)
-		}
+		// There was no secret for the in-cluster config, so we add it here. We don't fully-populate the Cluster struct,
+		// since only the name and server fields are used by the generator.
+		clusterList = append(clusterList, ClusterSpecifier{
+			Name:   "in-cluster",
+			Server: appv1.KubernetesInternalAPIServerAddr,
+		})
 	}
-	return &clusterList, nil
-}
-
-func getLocalCluster(clientset kubernetes.Interface) *appv1.Cluster {
-	initLocalCluster.Do(func() {
-		info, err := clientset.Discovery().ServerVersion()
-		if err == nil {
-			//nolint:staticcheck
-			localCluster.ServerVersion = fmt.Sprintf("%s.%s", info.Major, info.Minor)
-			//nolint:staticcheck
-			localCluster.ConnectionState = appv1.ConnectionState{Status: appv1.ConnectionStatusSuccessful}
-		} else {
-			//nolint:staticcheck
-			localCluster.ConnectionState = appv1.ConnectionState{
-				Status:  appv1.ConnectionStatusFailed,
-				Message: err.Error(),
-			}
-		}
-	})
-	cluster := localCluster.DeepCopy()
-	now := metav1.Now()
-	//nolint:staticcheck
-	cluster.ConnectionState.ModifiedAt = &now
-	return cluster
+	return clusterList, nil
 }