azurerm_storage_account - fix hanging destroy caused by multipl… (#5565)

Fix for issue #5434 (and related issues: #5530 , #3156 )

* **Problem:** If you have a storage account with more than one network rule set - the destroy action hangs forever and the real destroy/deletion of the resource never happens. If you have a storage account with zero or one network rule attached then this problem is irrelevant - the destroy action completes successfully.

* **Reason:** In the `resourceArmStorageAccountDelete` method located in _azurerm/internal/services/storage/resource_arm_storage_account.go_ there is a string array which is passed to the mutex lock method `locks.MultipleByName`. The reason for the problem is that the creation of this array does not cover all possible cases. The logic of the method assumes that you have different virtual networks and inserts them into the array used for locking but does not work correctly in the corner case where you have one virtual network with few sub-networks and you are assigning these sub-networks as a network rules to your storage account. Hence the array passed to the lock method contains n-times strings with the same name (same strings). When this array goes to the mutex the first string is used to lock, the second string is used to lock etc. but at the end all the strings are the same and when the first unlock comes - everything is OK but when a second, third or what so ever unlock hits... dead lock there is nothing to unlock because the first unlock already used the string and it was removed and all other unlocks are trying to use something that simply does not exists.

* **Resolution:** Check if there are subnets and attach them to the network name hence all the strings used to populate the array are unique and the lock-unlock problem is resolved.

azurerm/helpers/common/arrays.go

@@ -0,0 +1,16 @@
+package common
+
+// Remove duplicates from the input array and return unify array (without duplicated elements)
+func RemoveDuplicatesFromStringArray(elements []string) []string {
+	visited := map[string]bool{}
+	result := []string{}
+
+	for v := range elements {
+		if !visited[elements[v]] {
+			visited[elements[v]] = true          // Mark the element as visited.
+			result = append(result, elements[v]) // Add it to the result.
+		}
+	}
+
+	return result
+}

azurerm/helpers/common/arrays_test.go

@@ -0,0 +1,38 @@
+package common
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestRemoveDuplicatesInStringArray(t *testing.T) {
+	cases := []struct {
+		Name   string
+		Input  []string
+		Result []string
+	}{
+		{
+			Name:   "contain duplicates",
+			Input:  []string{"string1", "string2", "string1", "string3", ""},
+			Result: []string{"string1", "string2", "string3", ""},
+		},
+		{
+			Name:   "does not contain duplicates",
+			Input:  []string{"string1", "string2", "string3", ""},
+			Result: []string{"string1", "string2", "string3", ""},
+		},
+		{
+			Name:   "empty array",
+			Input:  []string{},
+			Result: []string{},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			if !reflect.DeepEqual(RemoveDuplicatesFromStringArray(tc.Input), tc.Result) {
+				t.Fatalf("Expected TestRemoveDuplicatesInStringArray to return %v", tc.Result)
+			}
+		})
+	}
+}

azurerm/internal/locks/lock.go

@@ -1,6 +1,9 @@
 package locks
 
-import "github.com/hashicorp/terraform-plugin-sdk/helper/mutexkv"
+import (
+	"github.com/hashicorp/terraform-plugin-sdk/helper/mutexkv"
+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/common"
+)
 
 // armMutexKV is the instance of MutexKV for ARM resources
 var armMutexKV = mutexkv.NewMutexKV()
@@ -16,7 +19,9 @@ func ByName(name string, resourceType string) {
 }
 
 func MultipleByName(names *[]string, resourceType string) {
-	for _, name := range *names {
+	newSlice := common.RemoveDuplicatesFromStringArray(*names)
+
+	for _, name := range newSlice {
 		ByName(name, resourceType)
 	}
 }
@@ -31,7 +36,9 @@ func UnlockByName(name string, resourceType string) {
 }
 
 func UnlockMultipleByName(names *[]string, resourceType string) {
-	for _, name := range *names {
+	newSlice := common.RemoveDuplicatesFromStringArray(*names)
+
+	for _, name := range newSlice {
 		UnlockByName(name, resourceType)
 	}
 }