From 487c7021a422c16f94116ecfbfccd73fb441080d Mon Sep 17 00:00:00 2001 From: Hugo Hars <54102154+w4rgrum@users.noreply.github.com> Date: Wed, 23 Oct 2024 17:35:33 +0200 Subject: [PATCH] fix(argocd_cluster): use cluster list api to avoid 403 issues with cluster get api at cluster read time (#399) * fix(argocd_cluster): use cluster list api to avoid 403 issues with cluster get api at cluster read time Signed-off-by: Hugo HARS <54102154+w4rgrum@users.noreply.github.com> * chore: Align with recent PR 474 Signed-off-by: Marco Maurer (-Kilchhofer) * fix: formatting Signed-off-by: Nathanael Liechti --------- Signed-off-by: Hugo HARS <54102154+w4rgrum@users.noreply.github.com> Signed-off-by: Marco Maurer (-Kilchhofer) Signed-off-by: Nathanael Liechti Co-authored-by: Marco Maurer (-Kilchhofer) Co-authored-by: Nathanael Liechti --- argocd/resource_argocd_cluster.go | 56 +++++++++++++- argocd/resource_argocd_cluster_test.go | 100 +++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 1 deletion(-) diff --git a/argocd/resource_argocd_cluster.go b/argocd/resource_argocd_cluster.go index 33bdd9e1..69e6524f 100644 --- 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 index 541fac92..982e5aed 100644 --- a/argocd/resource_argocd_cluster_test.go +++ b/argocd/resource_argocd_cluster_test.go @@ -1,11 +1,18 @@ package argocd import ( + "context" "fmt" + "os" "regexp" "runtime" + "strconv" "testing" + "time" + "github.com/argoproj-labs/terraform-provider-argocd/internal/provider" + "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" "k8s.io/client-go/rest" @@ -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 +}