Fix locking on resources during Terraform actions (#263)

argoproj-labs · May 6, 2023 · 2b17103 · 2b17103
1 parent 33775f3
commit 2b17103
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 47 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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()