fix(vm): yet another fix for disk reordering (#1297)

Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com>

fwprovider/tests/resource_vm_test.go

@@ -580,13 +580,10 @@ func TestAccResourceVMDisks(t *testing.T) {
 					}
 				}`),
 				Check: testResourceAttributes("proxmox_virtual_environment_vm.test_disk4", map[string]string{
-					// I'd love to test this, but the order of the list items is not guaranteed, apparently
-					//     resource_vm_test.go:669: Step 1/2 error: Check failed: proxmox_virtual_environment_vm.test_disk4: Attribute "disk.1.interface" value: expected 'virtio0' to match 'scsi0'
-					//    --- FAIL: TestAccResourceVMDisks/multiple_disks (5.24s)
-					// "disk.1.interface":         "scsi0",
-					// "disk.1.path_in_datastore": `vm-\d+-disk-0`,
-					// "disk.0.interface":         "virtio0",
-					// "disk.0.path_in_datastore": `vm-\d+-disk-1`,
+					"disk.0.interface":         "virtio0",
+					"disk.0.path_in_datastore": `vm-\d+-disk-1`,
+					"disk.1.interface":         "scsi0",
+					"disk.1.path_in_datastore": `vm-\d+-disk-0`,
 				}),
 			},
 			{

proxmoxtf/resource/vm/disk/disk.go

@@ -11,7 +11,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-	"golang.org/x/exp/maps"
 
 	"github.com/bpg/terraform-provider-proxmox/proxmox"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/vms"
@@ -73,7 +72,7 @@ func GetInfo(resp *vms.GetResponseData, d *schema.ResourceData) vms.CustomStorag
 
 	currentDisk := d.Get(MkDisk)
 
-	diskMap := utils.MapResourceList(currentDisk.([]interface{}), mkDiskInterface)
+	diskMap := utils.MapResourcesByAttribute(currentDisk.([]interface{}), mkDiskInterface)
 
 	for k, v := range storageDevices {
 		if v != nil && diskMap[k] != nil {
@@ -624,8 +623,7 @@ func Read(
 		var diskList []interface{}
 
 		if len(currentDiskList) > 0 {
-			resMap := utils.MapResourceList(currentDiskList, mkDiskInterface)
-			interfaces := maps.Keys[map[string]interface{}](resMap)
+			interfaces := utils.ListResourcesAttributeValue(currentDiskList, mkDiskInterface)
 			diskList = utils.OrderedListFromMapByKeyValues(diskMap, interfaces)
 		} else {
 			diskList = utils.OrderedListFromMap(diskMap)

proxmoxtf/resource/vm/vm.go

@@ -17,8 +17,6 @@ import (
 	"strings"
 	"time"
 
-	"golang.org/x/exp/maps"
-
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/uuid"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
@@ -3664,8 +3662,7 @@ func vmReadCustom(
 		var numaList []interface{}
 
 		if len(currentNUMAList) > 0 {
-			resMap := utils.MapResourceList(currentNUMAList, mkNUMADevice)
-			devices := maps.Keys[map[string]interface{}](resMap)
+			devices := utils.ListResourcesAttributeValue(currentNUMAList, mkNUMADevice)
 			numaList = utils.OrderedListFromMapByKeyValues(numaMap, devices)
 		} else {
 			numaList = utils.OrderedListFromMap(numaMap)

proxmoxtf/structure/schema.go

@@ -168,8 +168,8 @@ func SuppressIfListsOfMapsAreEqualIgnoringOrderByKey(
 			return false
 		}
 
-		oldMap := utils.MapResourceList(oldArray, keyAttr)
-		newMap := utils.MapResourceList(newArray, keyAttr)
+		oldMap := utils.MapResourcesByAttribute(oldArray, keyAttr)
+		newMap := utils.MapResourcesByAttribute(newArray, keyAttr)
 
 		for k, v := range oldMap {
 			if _, ok := newMap[k]; !ok {

utils/maps.go

@@ -18,12 +18,29 @@ func OrderedListFromMap(inputMap map[string]interface{}) []interface{} {
 	return OrderedListFromMapByKeyValues(inputMap, keyList)
 }
 
-// MapResourceList generates a list of strings from a Terraform resource list (list of maps).
-// The list is generated from the value of the specified attribute.
-//
-// "Map" in this context is a functional programming term, not a Go map.
-// "Resource" in this context is a Terraform resource, i.e. a map of attributes.
-func MapResourceList(resourceList []interface{}, attrName string) map[string]interface{} {
+// ListResourcesAttributeValue generates a list of strings from a Terraform resource list (which is list of maps).
+// The list is generated by extracting a specific key attribute from each resource. If the attribute is not found in a
+// resource, it is skipped.
+func ListResourcesAttributeValue(resourceList []interface{}, keyAttr string) []string {
+	var l []string
+
+	for _, resource := range resourceList {
+		if resource == nil {
+			continue
+		}
+
+		r := resource.(map[string]interface{})
+		if value, ok := r[keyAttr].(string); ok {
+			l = append(l, value)
+		}
+	}
+
+	return l
+}
+
+// MapResourcesByAttribute generates a map of resources from a resource list, using a specified attribute as the key
+// and the resource as the value. If the attribute is not found in a resource, it is skipped.
+func MapResourcesByAttribute(resourceList []interface{}, keyAttr string) map[string]interface{} {
 	m := make(map[string]interface{}, len(resourceList))
 
 	for _, resource := range resourceList {
@@ -32,7 +49,7 @@ func MapResourceList(resourceList []interface{}, attrName string) map[string]int
 		}
 
 		r := resource.(map[string]interface{})
-		if key, ok := r[attrName].(string); ok {
+		if key, ok := r[keyAttr].(string); ok {
 			m[key] = r
 		}
 	}

utils/maps_test.go

@@ -23,7 +23,7 @@ func TestOrderedListFromMap(t *testing.T) {
 	result := OrderedListFromMap(inputMap)
 
 	if !reflect.DeepEqual(result, expected) {
-		t.Errorf("MapResourceList() = %v, want %v", result, expected)
+		t.Errorf("ListResourcesAttributeValue() = %v, want %v", result, expected)
 	}
 }
 
@@ -35,18 +35,20 @@ func TestMapResourceList(t *testing.T) {
 		map[string]interface{}{"name": "resource2", "attr": "value2"},
 		nil,
 		map[string]interface{}{"name": "resource3", "attr": "value3"},
+		map[string]interface{}{"name": "resource4", "attr": "value4"},
 	}
 
-	expected := map[string]interface{}{
-		"value1": map[string]interface{}{"name": "resource1", "attr": "value1"},
-		"value2": map[string]interface{}{"name": "resource2", "attr": "value2"},
-		"value3": map[string]interface{}{"name": "resource3", "attr": "value3"},
+	expected := []string{
+		"value1",
+		"value2",
+		"value3",
+		"value4",
 	}
 
-	result := MapResourceList(resourceList, "attr")
+	result := ListResourcesAttributeValue(resourceList, "attr")
 
 	if !reflect.DeepEqual(result, expected) {
-		t.Errorf("MapResourceList() = %v, want %v", result, expected)
+		t.Errorf("ListResourcesAttributeValue() = %v, want %v", result, expected)
 	}
 }