fix: prevent deadlock (#712)

* fix: prevent deadlock

Fixes #710. While migrating from an older version using sdkv2 to
terraform-plugin-framework may need to recalculate the computed
attributes, it should definitely not deadlock in the meantime.

Added a test where we create a repository using `7.8.0`, and then
update it using the current version of this provider. Whereas before this
deadlocked, it now updates as intended, and once this initial migration
update has been done there is no state diff afterwards.

Signed-off-by: Blake Pettersson <[email protected]>

* chore: please linter

Signed-off-by: Blake Pettersson <[email protected]>

---------

Signed-off-by: Blake Pettersson <[email protected]>

Author: blakepettersson

internal/provider/resource_repository.go

@@ -194,14 +194,18 @@ func (r *repositoryResource) Update(ctx context.Context, req resource.UpdateRequ
 		return
 	}
 
-	// Update repository
-	sync.RepositoryMutex.Lock()
-	defer sync.RepositoryMutex.Unlock()
+	var updatedRepo *v1alpha1.Repository
 
-	updatedRepo, err := r.si.RepositoryClient.UpdateRepository(
-		ctx,
-		&repository.RepoUpdateRequest{Repo: repo},
-	)
+	func() {
+		// Keep mutex enclosed in a function to keep the lock scoped to it and to prevent deadlocking
+		sync.RepositoryMutex.Lock()
+		defer sync.RepositoryMutex.Unlock()
+
+		updatedRepo, err = r.si.RepositoryClient.UpdateRepository(
+			ctx,
+			&repository.RepoUpdateRequest{Repo: repo},
+		)
+	}()
 
 	if err != nil {
 		resp.Diagnostics.Append(diagnostics.ArgoCDAPIError("update", "repository", repo.Repo, err)...)
@@ -225,7 +229,7 @@ func (r *repositoryResource) Update(ctx context.Context, req resource.UpdateRequ
 	tflog.Trace(ctx, fmt.Sprintf("updated repository %s", updatedRepo.Repo))
 
 	// Save updated data into Terraform state
-	resp.Diagnostics.Append(resp.State.Set(ctx, data)...)
+	resp.Diagnostics.Append(resp.State.Set(ctx, data.updateFromAPI(updatedRepo))...)
 
 	// Perform a read to get the latest state
 	if !resp.Diagnostics.HasError() {

internal/provider/resource_repository_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -481,6 +482,43 @@ resource "argocd_repository" "boolean_fields" {
 	})
 }
 
+func TestAccArgoCDRepository_ProviderUpgradeStateMigration(t *testing.T) {
+	config := `
+resource "argocd_repository" "private" {
+  count = 1
+  repo  = "https://github.com/kubernetes-sigs/kustomize"
+  name  = "gitlab-private"
+  type  = "git"
+}
+`
+	resource.Test(t, resource.TestCase{
+		Steps: []resource.TestStep{
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"argocd": {
+						VersionConstraint: "7.8.0",
+						Source:            "argoproj-labs/argocd",
+					},
+				},
+				Config: config,
+			},
+			{
+				ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+				Config:                   config,
+			},
+			{
+				ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+				Config:                   config,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+		},
+	})
+}
+
 // TestAccArgoCDRepository_EmptyStringFieldsConsistency tests handling of empty string fields
 func TestAccArgoCDRepository_EmptyStringFieldsConsistency(t *testing.T) {
 	config := `