fix(argocd_cluster): use cluster list api to avoid 403 issues with cl…

…uster get api at cluster read time Signed-off-by: Hugo HARS <54102154+w4rgrum@users.noreply.github.com>
argoproj-labs · Jun 13, 2024 · ca58239 · ca58239
1 parent 76d2d70
commit ca58239
Show file tree

Hide file tree

Showing 2 changed files with 155 additions and 1 deletion.
diff --git a/argocd/resource_argocd_cluster.go b/argocd/resource_argocd_cluster.go
@@ -43,6 +43,9 @@ func resourceArgoCDClusterCreate(ctx context.Context, d *schema.ResourceData, me
 
 	// Cluster are unique by "server address" so we should check there is no existing cluster with this address before
 	existingClusters, err := si.ClusterClient.List(ctx, &clusterClient.ClusterQuery{
+		// Starting argo-cd server v2.8.0 filtering on list api endpoint is fixed, else it is ignored, see:
+		// - https://github.com/oboukili/terraform-provider-argocd/issues/266#issuecomment-1739122022
+		// - https://github.com/argoproj/argo-cd/pull/13363
 		Id: &clusterClient.ClusterID{
 			Type:  "server",
 			Value: rtrimmedServer,
@@ -53,6 +56,7 @@ func resourceArgoCDClusterCreate(ctx context.Context, d *schema.ResourceData, me
 		return errorToDiagnostics(fmt.Sprintf("failed to list existing clusters when creating cluster %s", cluster.Server), err)
 	}
 
+	// Here we will filter ourselves on the list so that we are backward compatible for argo-cd server with version < v2.8.0 (see coment above)
 	if len(existingClusters.Items) > 0 {
 		for _, existingCluster := range existingClusters.Items {
 			if rtrimmedServer == strings.TrimRight(existingCluster.Server, "/") {
@@ -103,7 +107,57 @@ func resourceArgoCDClusterRead(ctx context.Context, d *schema.ResourceData, meta
 			return nil
 		}
 
-		return argoCDAPIError("read", "cluster", d.Id(), err)
+		// Fix for https://github.com/oboukili/terraform-provider-argocd/issues/266
+		// This fix is added here as a workaround to ensure backward compatibility, as
+		//   it is triggered only on the specific usecase where the issue happens.
+		// Additional remarks about this code:
+		// * it is a copy/paste of the code used by resourceArgoCDClusterCreate to check if
+		//     the cluster already exists (with some obvious changes to return value and mutex type)
+		// * it should at term replace the `si.ClusterClient.Get` code for this method
+		if strings.Contains(err.Error(), "PermissionDenied") {
+			cluster, err := expandCluster(d)
+			if err != nil {
+				return errorToDiagnostics("failed to expand cluster", err)
+			}
+
+			tokenMutexClusters.RLock()
+
+			rtrimmedServer := strings.TrimRight(cluster.Server, "/")
+
+			// Cluster are unique by "server address" so we should check there is no existing cluster with this address before
+			existingClusters, err := si.ClusterClient.List(ctx, &clusterClient.ClusterQuery{
+				// Starting argo-cd server v2.8.0 filtering on list api endpoint is fixed, else it is ignored, see:
+				// - https://github.com/oboukili/terraform-provider-argocd/issues/266#issuecomment-1739122022
+				// - https://github.com/argoproj/argo-cd/pull/13363
+				Id: &clusterClient.ClusterID{
+					Type:  "server",
+					Value: rtrimmedServer,
+				},
+			})
+
+			tokenMutexClusters.RUnlock()
+
+			if err != nil {
+				return errorToDiagnostics(fmt.Sprintf("failed to list existing clusters when reading cluster %s", cluster.Server), err)
+			}
+
+			// Here we will filter ourselves on the list so that we are backward compatible for argo-cd server with version < v2.8.0 (see coment above)
+			if len(existingClusters.Items) > 0 {
+				for _, existingCluster := range existingClusters.Items {
+					if rtrimmedServer == strings.TrimRight(existingCluster.Server, "/") {
+						// Cluster was found, return
+						return nil
+					}
+				}
+			}
+
+			// Cluster was not found, return with empty Id
+			d.SetId("")
+
+			return nil
+		} else {
+			return argoCDAPIError("read", "cluster", d.Id(), err)
+		}
 	}
 
 	if err = flattenCluster(c, d); err != nil {

diff --git a/argocd/resource_argocd_cluster_test.go b/argocd/resource_argocd_cluster_test.go
@@ -1,13 +1,20 @@
 package argocd
 
 import (
+	"context"
 	"fmt"
+	"os"
 	"regexp"
 	"runtime"
+	"strconv"
 	"testing"
+	"time"
 
+	"github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster"
+	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/oboukili/terraform-provider-argocd/internal/provider"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/util/homedir"
@@ -292,6 +299,74 @@ func TestAccArgoCDCluster_invalidSameServer(t *testing.T) {
 	})
 }
 
+func TestAccArgoCDCluster_outsideDeletion(t *testing.T) {
+	clusterName := acctest.RandString(10)
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccArgoCDClusterMetadata(clusterName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(
+						"argocd_cluster.cluster_metadata",
+						"info.0.connection_state.0.status",
+						"Successful",
+					),
+					resource.TestCheckResourceAttr(
+						"argocd_cluster.cluster_metadata",
+						"config.0.tls_client_config.0.insecure",
+						"true",
+					),
+					resource.TestCheckResourceAttr(
+						"argocd_cluster.cluster_metadata",
+						"name",
+						clusterName,
+					),
+				),
+			},
+			{
+				PreConfig: func() {
+					// delete cluster and validate refresh generates a plan
+					// (non-regression test for https://github.com/oboukili/terraform-provider-argocd/issues/266)
+					si, err := getServerInterface()
+					if err != nil {
+						t.Error(fmt.Errorf("failed to get server interface: %s", err.Error()))
+					}
+					ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+					defer cancel()
+					_, err = si.ClusterClient.Delete(ctx, &cluster.ClusterQuery{Name: clusterName})
+					if err != nil {
+						t.Error(fmt.Errorf("failed to delete cluster '%s': %s", clusterName, err.Error()))
+					}
+				},
+				RefreshState:       true,
+				ExpectNonEmptyPlan: true,
+			},
+			{
+				Config: testAccArgoCDClusterMetadata(clusterName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(
+						"argocd_cluster.cluster_metadata",
+						"info.0.connection_state.0.status",
+						"Successful",
+					),
+					resource.TestCheckResourceAttr(
+						"argocd_cluster.cluster_metadata",
+						"config.0.tls_client_config.0.insecure",
+						"true",
+					),
+					resource.TestCheckResourceAttr(
+						"argocd_cluster.cluster_metadata",
+						"name",
+						clusterName,
+					),
+				),
+			},
+		},
+	})
+}
+
 func TestAccArgoCDCluster_namespacesErrorWhenEmpty(t *testing.T) {
 	name := acctest.RandString(10)
 
@@ -619,3 +694,28 @@ func getInternalRestConfig() (*rest.Config, error) {
 
 	return nil, fmt.Errorf("could not find a kind-argocd cluster from the current ~/.kube/config file")
 }
+
+// build & init ArgoCD server interface
+func getServerInterface() (*provider.ServerInterface, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+
+	insecure, err := strconv.ParseBool(os.Getenv("ARGOCD_INSECURE"))
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse 'ARGOCD_INSECURE' env var to bool: %s", err.Error())
+	}
+
+	si := provider.NewServerInterface(provider.ArgoCDProviderConfig{
+		ServerAddr: types.StringValue(os.Getenv("ARGOCD_SERVER")),
+		Insecure:   types.BoolValue(insecure),
+		Username:   types.StringValue(os.Getenv("ARGOCD_AUTH_USERNAME")),
+		Password:   types.StringValue(os.Getenv("ARGOCD_AUTH_PASSWORD")),
+	})
+
+	diag := si.InitClients(ctx)
+	if diag.HasError() {
+		return nil, fmt.Errorf("failed to init clients: %v", diag.Errors())
+	}
+
+	return si, nil
+}