acceptance tests!

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

.golangci.yml

@@ -73,6 +73,7 @@ linters:
     - ireturn
     - maintidx
     - nlreturn
+    - perfsprint
     - tagliatelle
     - testpackage
     - varnamelen

fwprovider/tests/resource_vm_test.go

@@ -9,93 +9,221 @@ package tests
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
-	"github.com/stretchr/testify/require"
-)
-
-const (
-	accTestVMName      = "proxmox_virtual_environment_vm.test_vm"
-	accTestVMCloneName = "proxmox_virtual_environment_vm.test_vm_clone"
 )
 
 func TestAccResourceVM(t *testing.T) {
 	t.Parallel()
 
+	tests := []struct {
+		name string
+		step resource.TestStep
+	}{
+		{"multiline description", resource.TestStep{
+			Config: `
+				resource "proxmox_virtual_environment_vm" "test_vm1" {
+					node_name = "pve"
+					started   = false
+					
+					description = <<-EOT
+						my
+						description
+						value
+					EOT
+				}`,
+			Check: resource.ComposeTestCheckFunc(
+				resource.TestCheckResourceAttr("proxmox_virtual_environment_vm.test_vm1", "description", "my\ndescription\nvalue"),
+			),
+		}},
+		{"single line description", resource.TestStep{
+			Config: `
+				resource "proxmox_virtual_environment_vm" "test_vm2" {
+					node_name = "pve"
+					started   = false
+					
+					description = "my description value"
+				}`,
+			Check: resource.ComposeTestCheckFunc(
+				resource.TestCheckResourceAttr("proxmox_virtual_environment_vm.test_vm2", "description", "my description value"),
+			),
+		}},
+		{"no description", resource.TestStep{
+			Config: `
+				resource "proxmox_virtual_environment_vm" "test_vm3" {
+					node_name = "pve"
+					started   = false
+					
+					description = ""
+				}`,
+			Check: resource.ComposeTestCheckFunc(
+				resource.TestCheckResourceAttr("proxmox_virtual_environment_vm.test_vm3", "description", ""),
+			),
+		}},
+	}
+
 	accProviders := testAccMuxProviders(context.Background(), t)
 
-	resource.Test(t, resource.TestCase{
+	for _, tt := range tests {
-		ProtoV6ProviderFactories: accProviders,
+		tt := tt
-		Steps: []resource.TestStep{
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			resource.Test(t, resource.TestCase{
+				ProtoV6ProviderFactories: accProviders,
+				Steps:                    []resource.TestStep{tt.step},
+			})
+		})
+	}
+}
+
+func TestAccResourceVMDisks(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		steps []resource.TestStep
+	}{
+		{"create disk with default parameters", []resource.TestStep{{
+			Config: `
+				resource "proxmox_virtual_environment_vm" "test_disk1" {
+					node_name = "pve"
+					started   = false
+					name 	  = "test-disk1"
+					
+					disk {
+						// note: default qcow2 is not supported by lvm (?)
+						file_format  = "raw"
+						datastore_id = "local-lvm"
+						interface    = "virtio0"
+						size         = 8
+					}
+				}`,
+			Check: resource.ComposeTestCheckFunc(
+				testResourceAttributes("proxmox_virtual_environment_vm.test_disk1", map[string]string{
+					"disk.0.cache":        "none",
+					"disk.0.datastore_id": "local-lvm",
+					"disk.0.discard":      "ignore",
+					"disk.0.file_format":  "raw",
+					// "disk.0.file_id":           "",  // is empty by default, but we can't check for that
+					"disk.0.interface":         "virtio0",
+					"disk.0.iothread":          "false",
+					"disk.0.path_in_datastore": `vm-\d+-disk-\d+`,
+					"disk.0.size":              "8",
+					"disk.0.ssd":               "false",
+				}),
+			),
+		}}},
+		{"create disk from an image", []resource.TestStep{{
+			Config: `
+				resource "proxmox_virtual_environment_download_file" "test_disk2_image" {
+					content_type = "iso"
+					datastore_id = "local"
+					node_name    = "pve"
+					url          = "https://cloud-images.ubuntu.com/jammy/current/jammy-server-cloudimg-amd64.img"
+				}
+				resource "proxmox_virtual_environment_vm" "test_disk2" {
+					node_name = "pve"
+					started   = false
+					name 	  = "test-disk2"	
+					disk {
+						datastore_id = "local-lvm"
+						file_id      = proxmox_virtual_environment_download_file.test_disk2_image.id
+						interface    = "virtio0"
+						iothread     = true
+						discard      = "on"
+						size         = 20
+					}
+				}`,
+			Check: resource.ComposeTestCheckFunc(
+				testResourceAttributes("proxmox_virtual_environment_vm.test_disk2", map[string]string{
+					"disk.0.cache":        "none",
+					"disk.0.datastore_id": "local-lvm",
+					"disk.0.discard":      "on",
+					"disk.0.file_format":  "raw",
+					// "disk.0.file_id":           "",  // is empty by default, but we can't check for that
+					"disk.0.interface":         "virtio0",
+					"disk.0.iothread":          "true",
+					"disk.0.path_in_datastore": `vm-\d+-disk-\d+`,
+					"disk.0.size":              "20",
+					"disk.0.ssd":               "false",
+				}),
+			),
+		}}},
+		{"clone default disk", []resource.TestStep{
 			{
-				Config: testAccResourceVMCreateConfig(false),
+				Config: `
-				Check:  testAccResourceVMCreateCheck(t),
+				resource "proxmox_virtual_environment_vm" "test_disk3_template" {
+					node_name = "pve"
+					started   = false
+					name 	  = "test-disk3-template"
+					template  = "true"
+					
+					disk {
+						file_format  = "raw"
+						datastore_id = "local-lvm"
+						interface    = "virtio0"
+						size         = 8
+					}
+				}
+				resource "proxmox_virtual_environment_vm" "test_disk3" {
+					node_name = "pve"
+					started   = false
+					name 	  = "test-disk3"
+
+					clone {
+						vm_id = proxmox_virtual_environment_vm.test_disk3_template.id
+					}
+				}
+				`,
+				Check: resource.ComposeTestCheckFunc(
+					// fully cloned disk, does not have any attributes in state
+					resource.TestCheckNoResourceAttr("proxmox_virtual_environment_vm.test_disk3", "disk.0"),
+				),
 			},
 			{
-				Config: testAccResourceVMCreateConfig(true) + testAccResourceVMCreateCloneConfig(),
+				RefreshState: true,
-				Check:  testAccResourceVMCreateCloneCheck(t),
 			},
-		},
+		}},
-	})
+		//{"default disk parameters", resource.TestStep{}},
+		//{"default disk parameters", resource.TestStep{}},
+	}
+
+	accProviders := testAccMuxProviders(context.Background(), t)
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			resource.Test(t, resource.TestCase{
+				ProtoV6ProviderFactories: accProviders,
+				Steps:                    tt.steps,
+			})
+		})
+	}
 }
 
-func testAccResourceVMCreateConfig(isTemplate bool) string {
+func testResourceAttributes(res string, attrs map[string]string) resource.TestCheckFunc {
-	return fmt.Sprintf(`
+	return func(s *terraform.State) error {
-resource "proxmox_virtual_environment_vm" "test_vm" {
+		for k, v := range attrs {
-  node_name = "%s"
+			if err := resource.TestCheckResourceAttrWith(res, k, func(got string) error {
-  vm_id     = 2100
+				match, err := regexp.Match(v, []byte(got)) //nolint:mirror
-  template  = %t
+				if err != nil {
-  started = false
+					return fmt.Errorf("error matching %s: %w", v, err)
-
+				}
-  disk {
+				if !match {
-	file_format= "raw"
+					return fmt.Errorf("expected %s to match %s", got, v)
-	datastore_id = "local-lvm"
+				}
-	interface    = "virtio0"
+				return nil
-    size         = 8
+			})(s); err != nil {
-  }
+				return err
+			}
+		}
 
-}
+		return nil
-`, accTestNodeName, isTemplate)
+	}
-}
-
-func testAccResourceVMCreateCheck(t *testing.T) resource.TestCheckFunc {
-	t.Helper()
-
-	return resource.ComposeTestCheckFunc(
-		func(*terraform.State) error {
-			err := getNodesClient().VM(2100).WaitForVMStatus(context.Background(), "stopped", 10, 1)
-			require.NoError(t, err, "vm did not start")
-			return nil
-		},
-	)
-}
-
-func testAccResourceVMCreateCloneConfig() string {
-	return fmt.Sprintf(`
-resource "proxmox_virtual_environment_vm" "test_vm_clone" {
-  depends_on = [proxmox_virtual_environment_vm.test_vm]
-
-  node_name = "%s"
-  vm_id     = 2101
-  started = false
-
-  clone {
-	vm_id = 2100
-  }
-}
-`, accTestNodeName)
-}
-
-func testAccResourceVMCreateCloneCheck(t *testing.T) resource.TestCheckFunc {
-	t.Helper()
-
-	return resource.ComposeTestCheckFunc(
-		func(*terraform.State) error {
-			err := getNodesClient().VM(2101).WaitForVMStatus(context.Background(), "stopped", 20, 1)
-			require.NoError(t, err, "vm did not start")
-			return nil
-		},
-	)
 }

proxmoxtf/resource/file.go

@@ -22,8 +22,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/ssh"
-
 	"github.com/google/uuid"
 	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
@@ -227,7 +225,7 @@ func File() *schema.Resource {
 		DeleteContext: fileDelete,
 		UpdateContext: fileUpdate,
 		Importer: &schema.ResourceImporter{
-			StateContext: func(ctx context.Context, d *schema.ResourceData, i interface{}) ([]*schema.ResourceData, error) {
+			StateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) ([]*schema.ResourceData, error) {
 				node, volID, err := fileParseImportID(d.Id())
 				if err != nil {
 					return nil, err

proxmoxtf/resource/vm/disk.go

@@ -3,21 +3,22 @@ package vm
 import (
 	"context"
 	"fmt"
-	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/ssh"
 	"regexp"
 	"strconv"
 	"strings"
 
-	"github.com/bpg/terraform-provider-proxmox/proxmox"
-	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/vms"
-	"github.com/bpg/terraform-provider-proxmox/proxmox/types"
-	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
-	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
-	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/structure"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
+
+	"github.com/bpg/terraform-provider-proxmox/proxmox"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/vms"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/ssh"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/types"
+	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
+	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
+	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/structure"
 )
 
 const (
@@ -199,7 +200,7 @@ func diskSchema() *schema.Schema {
 	}
 }
 
-// called from vmCreateClone
+// called from vmCreateClone.
 func createDisks(
 	ctx context.Context, vmConfig *vms.GetResponseData, d *schema.ResourceData, vmAPI *vms.Client,
 ) (vms.CustomStorageDevices, error) {
@@ -226,7 +227,6 @@ func createDisks(
 		}
 
 		// disk is present, i.e. when cloning a template, but we need to check if it needs to be moved or resized
-
 		timeoutSec := d.Get(mkTimeoutMoveDisk).(int)
 
 		err := resizeDiskIfRequired(ctx, currentDisk, planDisk, vmAPI, timeoutSec)
@@ -263,7 +263,7 @@ func resizeDiskIfRequired(
 
 		err := vmAPI.ResizeVMDisk(ctx, diskResizeBody, timeoutSec)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to resize disk: %w", err)
 		}
 	}
 
@@ -291,7 +291,7 @@ func moveDiskIfRequired(
 
 		err := vmAPI.MoveVMDisk(ctx, diskMoveBody, timeoutSec)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to move disk: %w", err)
 		}
 	}
 
@@ -322,7 +322,7 @@ func createDisk(ctx context.Context, disk *vms.CustomStorageDevice, vmAPI *vms.C
 
 	err := vmAPI.UpdateVM(ctx, diskUpdateBody)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to create disk: %w", err)
 	}
 
 	return nil
@@ -331,7 +331,7 @@ func createDisk(ctx context.Context, disk *vms.CustomStorageDevice, vmAPI *vms.C
 func vmImportCustomDisks(ctx context.Context, d *schema.ResourceData, m interface{}) error {
 	vmID, err := strconv.Atoi(d.Id())
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to convert vm id to int: %w", err)
 	}
 
 	planDisks, err := getStorageDevicesFromResource(d)
@@ -398,7 +398,7 @@ func vmImportCustomDisks(ctx context.Context, d *schema.ResourceData, m interfac
 
 		api, err := config.GetClient()
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to get client: %w", err)
 		}
 
 		nodeName := d.Get(mkNodeName).(string)
@@ -406,10 +406,10 @@ func vmImportCustomDisks(ctx context.Context, d *schema.ResourceData, m interfac
 		out, err := api.SSH().ExecuteNodeCommands(ctx, nodeName, commands)
 		if err != nil {
 			if matches, e := regexp.Match(`pvesm: .* not found`, out); e == nil && matches {
-				return ssh.NewErrSSHUserNoPermission(api.SSH().Username())
+				return ssh.NewErrUserHasNoPermission(api.SSH().Username()) //nolint:wrapcheck
 			}
 
-			return err
+			return fmt.Errorf("failed to import disks: %w", err)
 		}
 
 		tflog.Debug(ctx, "vmCreateCustomDisks", map[string]interface{}{
@@ -457,7 +457,7 @@ func getDiskDeviceObjects1(d *schema.ResourceData, disks []interface{}) (vms.Cus
 			false,
 		)
 		if err != nil {
-			return diskDeviceObjects, err
+			return diskDeviceObjects, fmt.Errorf("failed to read disk speed: %w", err)
 		}
 
 		if fileFormat == "" {
@@ -519,7 +519,7 @@ func getDiskDeviceObjects1(d *schema.ResourceData, disks []interface{}) (vms.Cus
 
 		if storageInterface != "virtio" && storageInterface != "scsi" && storageInterface != "sata" {
 			return diskDeviceObjects, fmt.Errorf(
-				"The disk interface '%s' is not supported, should be one of 'virtioN', 'sataN', or 'scsiN'",
+				"the disk interface '%s' is not supported, should be one of 'virtioN', 'sataN', or 'scsiN'",
 				diskInterface,
 			)
 		}
@@ -657,7 +657,7 @@ func readDisk1(ctx context.Context, d *schema.ResourceData,
 	return diags
 }
 
-func updateDisk(d *schema.ResourceData, vmConfig *vms.GetResponseData, updateBody *vms.UpdateRequestBody) error {
+func updateDisk(d *schema.ResourceData, updateBody *vms.UpdateRequestBody) error {
 	// Prepare the new disk device configuration.
 	if !d.HasChange(mkDisk) {
 		return nil

proxmoxtf/resource/vm/schema.go

@@ -6,11 +6,12 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
-	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/structure"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
+
+	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
+	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/structure"
 )
 
 const (
@@ -568,6 +569,15 @@ func VM() *schema.Resource {
 				Description: "The description",
 				Optional:    true,
 				Default:     dvDescription,
+				StateFunc: func(i interface{}) string {
+					// PVE always adds a newline to the description, so we have to do the same,
+					// also taking in account the CLRF case (Windows)
+					if i.(string) != "" {
+						return strings.ReplaceAll(strings.TrimSpace(i.(string)), "\r\n", "\n")
+					}
+
+					return ""
+				},
 			},
 			mkDisk: diskSchema(),
 			mkEFIDisk: {

proxmoxtf/resource/vm/vm.go

@@ -183,7 +183,6 @@ func vmCreateClone(ctx context.Context, d *schema.ResourceData, m interface{}) d
 
 		vmID = *vmIDNew
 		err = d.Set(mkVMID, vmID)
-
 		if err != nil {
 			return diag.FromErr(err)
 		}
@@ -1471,8 +1470,8 @@ func vmGetHostPCIDeviceObjects(d *schema.ResourceData) vms.CustomPCIDevices {
 		}
 
 		if ids != "" {
-			dIds := strings.Split(ids, ";")
+			dIDs := strings.Split(ids, ";")
-			device.DeviceIDs = &dIds
+			device.DeviceIDs = &dIDs
 		}
 
 		if mdev != "" {
@@ -1741,7 +1740,7 @@ func vmGetVGADeviceObject(d *schema.ResourceData) (*vms.CustomVGADevice, error)
 		true,
 	)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error reading VGA block: %w", err)
 	}
 
 	vgaEnabled := types.CustomBool(vgaBlock[mkVGAEnabled].(bool))
@@ -2458,7 +2457,8 @@ func vmReadCustom(
 
 	currentInitialization := d.Get(mkInitialization).([]interface{})
 
-	if len(clone) > 0 {
+	switch {
+	case len(clone) > 0:
 		if len(currentInitialization) > 0 {
 			if len(initialization) > 0 {
 				err := d.Set(
@@ -2471,10 +2471,10 @@ func vmReadCustom(
 				diags = append(diags, diag.FromErr(err)...)
 			}
 		}
-	} else if len(initialization) > 0 {
+	case len(initialization) > 0:
 		err := d.Set(mkInitialization, []interface{}{initialization})
 		diags = append(diags, diag.FromErr(err)...)
-	} else {
+	default:
 		err := d.Set(mkInitialization, []interface{}{})
 		diags = append(diags, diag.FromErr(err)...)
 	}
@@ -2637,11 +2637,9 @@ func vmReadCustom(
 		if len(clone) > 0 {
 			err = d.Set(mkNetworkDevice, networkDeviceList[:networkDeviceLast+1])
 			diags = append(diags, diag.FromErr(err)...)
-		} else {
+		} else if len(currentNetworkDeviceList) > 0 || networkDeviceLast > -1 {
-			if len(currentNetworkDeviceList) > 0 || networkDeviceLast > -1 {
+			err := d.Set(mkNetworkDevice, networkDeviceList[:networkDeviceLast+1])
-				err := d.Set(mkNetworkDevice, networkDeviceList[:networkDeviceLast+1])
+			diags = append(diags, diag.FromErr(err)...)
-				diags = append(diags, diag.FromErr(err)...)
-			}
 		}
 	}
 
@@ -2656,7 +2654,8 @@ func vmReadCustom(
 
 	currentOperatingSystem := d.Get(mkOperatingSystem).([]interface{})
 
-	if len(clone) > 0 {
+	switch {
+	case len(clone) > 0:
 		if len(currentOperatingSystem) > 0 {
 			err := d.Set(
 				mkOperatingSystem,
@@ -2664,11 +2663,11 @@ func vmReadCustom(
 			)
 			diags = append(diags, diag.FromErr(err)...)
 		}
-	} else if len(currentOperatingSystem) > 0 ||
+	case len(currentOperatingSystem) > 0 ||
-		operatingSystem[mkOperatingSystemType] != dvOperatingSystemType {
+		operatingSystem[mkOperatingSystemType] != dvOperatingSystemType:
 		err := d.Set(mkOperatingSystem, []interface{}{operatingSystem})
 		diags = append(diags, diag.FromErr(err)...)
-	} else {
+	default:
 		err := d.Set(mkOperatingSystem, []interface{}{})
 		diags = append(diags, diag.FromErr(err)...)
 	}
@@ -2875,18 +2874,19 @@ func vmReadCustom(
 
 	currentVGA := d.Get(mkVGA).([]interface{})
 
-	if len(clone) > 0 {
+	switch {
+	case len(clone) > 0:
 		if len(currentVGA) > 0 {
 			err := d.Set(mkVGA, []interface{}{vga})
 			diags = append(diags, diag.FromErr(err)...)
 		}
-	} else if len(currentVGA) > 0 ||
+	case len(currentVGA) > 0 ||
 		vga[mkVGAEnabled] != dvVGAEnabled ||
 		vga[mkVGAMemory] != dvVGAMemory ||
-		vga[mkVGAType] != dvVGAType {
+		vga[mkVGAType] != dvVGAType:
 		err := d.Set(mkVGA, []interface{}{vga})
 		diags = append(diags, diag.FromErr(err)...)
-	} else {
+	default:
 		err := d.Set(mkVGA, []interface{}{})
 		diags = append(diags, diag.FromErr(err)...)
 	}
@@ -3524,7 +3524,7 @@ func vmUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
 		rebootRequired = true
 	}
 
-	err := updateDisk(d, vmConfig, updateBody)
+	err := updateDisk(d, updateBody)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -3777,6 +3777,7 @@ func vmUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
 			if e := vmShutdown(ctx, vmAPI, d); e != nil {
 				return e
 			}
+
 			rebootRequired = false
 		}
 	}