From 2b17103264497789af9ea0859212b5ba2267699e Mon Sep 17 00:00:00 2001 From: Brian Fox <878612+onematchfox@users.noreply.github.com> Date: Sat, 6 May 2023 10:33:59 -0700 Subject: [PATCH] Fix locking on resources during Terraform actions (#263) --- argocd/resource_argocd_project.go | 99 ++++++++++++-------- argocd/resource_argocd_project_token.go | 8 +- argocd/resource_argocd_project_token_test.go | 12 ++- argocd/resource_argocd_repository.go | 4 +- 4 files changed, 76 insertions(+), 47 deletions(-) diff --git a/argocd/resource_argocd_project.go b/argocd/resource_argocd_project.go index ebbee182..bf0fe4ea 100644 --- a/argocd/resource_argocd_project.go +++ b/argocd/resource_argocd_project.go @@ -68,37 +68,6 @@ func resourceArgoCDProjectCreate(ctx context.Context, d *schema.ResourceData, me projectName := objectMeta.Name - if _, ok := tokenMutexProjectMap[projectName]; !ok { - tokenMutexProjectMap[projectName] = &sync.RWMutex{} - } - - tokenMutexProjectMap[projectName].RLock() - p, err := si.ProjectClient.Get(ctx, &projectClient.ProjectQuery{ - Name: projectName, - }) - tokenMutexProjectMap[projectName].RUnlock() - - if err != nil { - if !strings.Contains(err.Error(), "NotFound") { - return []diag.Diagnostic{ - { - Severity: diag.Error, - Summary: fmt.Sprintf("Project %s could not be created", projectName), - Detail: err.Error(), - }, - } - } - } - - if p != nil { - switch p.DeletionTimestamp { - case nil: - default: - // Pre-existing project is still in Kubernetes soft deletion queue - time.Sleep(time.Duration(*p.DeletionGracePeriodSeconds)) - } - } - featureProjectSourceNamespacesSupported, err := si.isFeatureSupported(featureProjectSourceNamespaces) if err != nil { return []diag.Diagnostic{ @@ -122,7 +91,34 @@ func resourceArgoCDProjectCreate(ctx context.Context, d *schema.ResourceData, me } } + if _, ok := tokenMutexProjectMap[projectName]; !ok { + tokenMutexProjectMap[projectName] = &sync.RWMutex{} + } + tokenMutexProjectMap[projectName].Lock() + + p, err := si.ProjectClient.Get(ctx, &projectClient.ProjectQuery{ + Name: projectName, + }) + if err != nil && !strings.Contains(err.Error(), "NotFound") { + tokenMutexProjectMap[projectName].Unlock() + + return []diag.Diagnostic{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("failed to get project %s", projectName), + Detail: err.Error(), + }, + } + } else if p != nil { + switch p.DeletionTimestamp { + case nil: + default: + // Pre-existing project is still in Kubernetes soft deletion queue + time.Sleep(time.Duration(*p.DeletionGracePeriodSeconds)) + } + } + p, err = si.ProjectClient.Create(ctx, &projectClient.ProjectCreateRequest{ Project: &application.AppProject{ ObjectMeta: objectMeta, @@ -132,6 +128,7 @@ func resourceArgoCDProjectCreate(ctx context.Context, d *schema.ResourceData, me // TODO: make that a resource flag with proper acceptance tests Upsert: false, }) + tokenMutexProjectMap[projectName].Unlock() if err != nil { @@ -238,6 +235,29 @@ func resourceArgoCDProjectUpdate(ctx context.Context, d *schema.ResourceData, me projectName := objectMeta.Name + featureProjectSourceNamespacesSupported, err := si.isFeatureSupported(featureProjectSourceNamespaces) + if err != nil { + return []diag.Diagnostic{ + { + Severity: diag.Error, + Summary: "feature not supported", + Detail: err.Error(), + }, + } + } else if !featureProjectSourceNamespacesSupported { + _, sourceNamespacesOk := d.GetOk("spec.0.source_namespaces") + if sourceNamespacesOk { + return []diag.Diagnostic{ + { + Severity: diag.Error, + Summary: fmt.Sprintf( + "project source_namespaces is only supported from ArgoCD %s onwards", + featureVersionConstraintsMap[featureProjectSourceNamespaces].String()), + }, + } + } + } + if _, ok := tokenMutexProjectMap[projectName]; !ok { tokenMutexProjectMap[projectName] = &sync.RWMutex{} } @@ -249,23 +269,22 @@ func resourceArgoCDProjectUpdate(ctx context.Context, d *schema.ResourceData, me }, } - tokenMutexProjectMap[projectName].RLock() + tokenMutexProjectMap[projectName].Lock() + p, err := si.ProjectClient.Get(ctx, &projectClient.ProjectQuery{ Name: d.Id(), }) - tokenMutexProjectMap[projectName].RUnlock() - if err != nil { + tokenMutexProjectMap[projectName].Unlock() + return []diag.Diagnostic{ { Severity: diag.Error, - Summary: "failed to get project", + Summary: fmt.Sprintf("failed to get existing project %s", projectName), Detail: err.Error(), }, } - } - - if p != nil { + } else if p != nil { // Kubernetes API requires providing the up-to-date correct ResourceVersion for updates projectRequest.Project.ResourceVersion = p.ResourceVersion @@ -282,6 +301,8 @@ func resourceArgoCDProjectUpdate(ctx context.Context, d *schema.ResourceData, me // i == -1 means the role does not exist // and was recently added within Terraform tf files if i != -1 { + tokenMutexProjectMap[projectName].Unlock() + return []diag.Diagnostic{ { Severity: diag.Error, @@ -296,8 +317,8 @@ func resourceArgoCDProjectUpdate(ctx context.Context, d *schema.ResourceData, me } } - tokenMutexProjectMap[projectName].Lock() _, err = si.ProjectClient.Update(ctx, projectRequest) + tokenMutexProjectMap[projectName].Unlock() if err != nil { diff --git a/argocd/resource_argocd_project_token.go b/argocd/resource_argocd_project_token.go index 9286ae67..b19c0f02 100644 --- a/argocd/resource_argocd_project_token.go +++ b/argocd/resource_argocd_project_token.go @@ -564,7 +564,12 @@ func resourceArgoCDProjectTokenDelete(ctx context.Context, d *schema.ResourceDat } tokenMutexProjectMap[projectName].Lock() - if _, err := si.ProjectClient.DeleteToken(ctx, opts); err != nil { + + _, err = si.ProjectClient.DeleteToken(ctx, opts) + + tokenMutexProjectMap[projectName].Unlock() + + if err != nil { return []diag.Diagnostic{ { Severity: diag.Error, @@ -573,7 +578,6 @@ func resourceArgoCDProjectTokenDelete(ctx context.Context, d *schema.ResourceDat }, } } - tokenMutexProjectMap[projectName].Unlock() d.SetId("") diff --git a/argocd/resource_argocd_project_token_test.go b/argocd/resource_argocd_project_token_test.go index 840a4097..0304c3f4 100644 --- a/argocd/resource_argocd_project_token_test.go +++ b/argocd/resource_argocd_project_token_test.go @@ -110,7 +110,7 @@ func TestAccArgoCDProjectToken_RenewBefore(t *testing.T) { func TestAccArgoCDProjectToken_RenewAfter(t *testing.T) { resourceName := "argocd_project_token.renew_after" - renewAfterSeconds := 1 + renewAfterSeconds := 2 resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -120,17 +120,21 @@ func TestAccArgoCDProjectToken_RenewAfter(t *testing.T) { Config: testAccArgoCDProjectTokenRenewAfter(renewAfterSeconds), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "renew_after", fmt.Sprintf("%ds", renewAfterSeconds)), - testDelay(renewAfterSeconds+1), ), - ExpectNonEmptyPlan: true, }, { Config: testAccArgoCDProjectTokenRenewAfter(renewAfterSeconds), Check: resource.ComposeTestCheckFunc( - testDelay(renewAfterSeconds), + testDelay(renewAfterSeconds + 1), ), ExpectNonEmptyPlan: true, // token should be recreated when refreshed at end of step due to delay above }, + { + Config: testAccArgoCDProjectTokenRenewAfter(renewAfterSeconds), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "renew_after", fmt.Sprintf("%ds", renewAfterSeconds)), + ), + }, }, }) } diff --git a/argocd/resource_argocd_repository.go b/argocd/resource_argocd_repository.go index bfc6ae47..4ed76c7e 100644 --- a/argocd/resource_argocd_repository.go +++ b/argocd/resource_argocd_repository.go @@ -91,7 +91,7 @@ func resourceArgoCDRepositoryCreate(ctx context.Context, d *schema.ResourceData, return resource.RetryableError(fmt.Errorf("handshake failed for repository %s, retrying in case a repository certificate has been set recently", repo.Repo)) } - return resource.NonRetryableError(fmt.Errorf("repository %s not found: %s", repo.Repo, err)) + return resource.NonRetryableError(fmt.Errorf("failed to create repository %s : %w", repo.Repo, err)) } else if r == nil { return resource.NonRetryableError(fmt.Errorf("ArgoCD did not return an error or a repository result: %s", err)) } else if r.ConnectionState.Status == application.ConnectionStatusFailed { @@ -139,7 +139,7 @@ func resourceArgoCDRepositoryRead(ctx context.Context, d *schema.ResourceData, m } } - r := &application.Repository{} + var r *application.Repository if featureRepositoryGetSupported { tokenMutexConfiguration.RLock()