fix(vm): multi-line description field is always marked as changed (#1030)

Also, fix acceptance tests

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

.vscode/settings.json

@@ -10,6 +10,7 @@
         "qcow",
         "rootfs",
         "signoff",
+        "stretchr",
         "tflog",
         "unmanaged",
         "virtio",

fwprovider/tests/resource_container_test.go

@@ -43,6 +43,12 @@ func TestAccResourceContainer(t *testing.T) {
 
 func testAccResourceContainerCreateConfig(isTemplate bool) string {
 	return fmt.Sprintf(`
+resource "proxmox_virtual_environment_download_file" "ubuntu_container_template" {
+	content_type = "vztmpl"
+	datastore_id = "local"
+	node_name    = "pve"
+	url = "http://download.proxmox.com/images/system/ubuntu-23.04-standard_23.04-1_amd64.tar.zst"
+}
 resource "proxmox_virtual_environment_container" "test_container" {
   node_name = "%s"
   vm_id     = 1100
@@ -74,8 +80,7 @@ resource "proxmox_virtual_environment_container" "test_container" {
   }
 
   operating_system {
-	# TODO: this file needs to be upload to PVE first
-    template_file_id = "local:vztmpl/ubuntu-23.04-standard_23.04-1_amd64.tar.zst"
+	template_file_id = proxmox_virtual_environment_download_file.ubuntu_container_template.id
     type             = "ubuntu"
   }
 }

fwprovider/tests/resource_file_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/bpg/terraform-provider-proxmox/proxmox/api"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/ssh"
+
 	"github.com/bpg/terraform-provider-proxmox/utils"
 )
 
@@ -144,13 +145,22 @@ func uploadSnippetFile(t *testing.T, file *os.File) {
 
 	defer f.Close()
 
-	err = sshClient.NodeUpload(context.Background(), "pve", "/var/lib/vz",
+	fname := filepath.Base(file.Name())
+	err = sshClient.NodeUpload(context.Background(), "pve", "/tmp/tfpve/testacc",
 		&api.FileUploadRequest{
 			ContentType: "snippets",
-			FileName:    filepath.Base(file.Name()),
+			FileName:    fname,
 			File:        f,
 		})
 	require.NoError(t, err)
+
+	_, err = sshClient.ExecuteNodeCommands(context.Background(), "pve", []string{
+		fmt.Sprintf(`%s; try_sudo "mv /tmp/tfpve/testacc/snippets/%s /var/lib/vz/snippets/%s" && rm -rf /tmp/tfpve/testacc/`,
+			ssh.TrySudo,
+			fname, fname,
+		),
+	})
+	require.NoError(t, err)
 }
 
 func createFile(t *testing.T, namePattern string, content string) *os.File {

proxmox/api/client.go

@@ -52,6 +52,9 @@ type Client interface {
 	// IsRootTicket returns true if the authenticator is configured to use the root directly using a login ticket.
 	// (root using token is weaker, cannot change VM arch)
 	IsRootTicket() bool
+
+	// HTTP returns a lower-level HTTP client.
+	HTTP() *http.Client
 }
 
 // Connection represents a connection to the Proxmox Virtual Environment API.
@@ -298,6 +301,10 @@ func (c *client) IsRootTicket() bool {
 	return c.auth.IsRootTicket()
 }
 
+func (c *client) HTTP() *http.Client {
+	return c.conn.httpClient
+}
+
 // validateResponseCode ensures that a response is valid.
 func validateResponseCode(res *http.Response) error {
 	if res.StatusCode < 200 || res.StatusCode >= 300 {

proxmox/ssh/client.go

@@ -28,6 +28,18 @@ import (
 	"github.com/bpg/terraform-provider-proxmox/utils"
 )
 
+const (
+	// TrySudo is a shell function that tries to execute a command with sudo if the user has sudo permissions.
+	TrySudo = `try_sudo(){ if [ $(sudo -n pvesm apiinfo 2>&1 | grep "APIVER" | wc -l) -gt 0 ]; then sudo $1; else $1; fi }`
+)
+
+// NewErrUserHasNoPermission creates a new error indicating that the SSH user does not have required permissions.
+func NewErrUserHasNoPermission(username string) error {
+	return fmt.Errorf("the SSH user '%s' does not have required permissions. "+
+		"Make sure 'sudo' is installed and the user is configured in sudoers file. "+
+		"Refer to the documentation for more details", username)
+}
+
 // Client is an interface for performing SSH requests against the Proxmox Nodes.
 type Client interface {
 	// Username returns the SSH username.

proxmoxtf/resource/container.go

@@ -283,7 +283,10 @@ func Container() *schema.Resource {
 				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)
-					return strings.ReplaceAll(strings.TrimSpace(i.(string)), "\r\n", "\n") + "\n"
+					if i.(string) != "" {
+						return strings.ReplaceAll(strings.TrimSpace(i.(string)), "\r\n", "\n") + "\n"
+					}
+					return ""
 				},
 			},
 			mkResourceVirtualEnvironmentContainerDisk: {

proxmoxtf/resource/file.go

@@ -30,6 +30,7 @@ import (
 	"golang.org/x/exp/slices"
 
 	"github.com/bpg/terraform-provider-proxmox/proxmox/api"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/ssh"
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
 	"github.com/bpg/terraform-provider-proxmox/utils"
@@ -594,7 +595,7 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 		_, err := capi.SSH().ExecuteNodeCommands(ctx, nodeName, []string{
 			// the `mv` command should be scoped to the specific directories in sudoers!
 			fmt.Sprintf(`%s; try_sudo "mv %s/%s %s/%s" && rmdir %s && rmdir %s || echo`,
-				trySudo,
+				ssh.TrySudo,
 				srcDir, *fileName,
 				dstDir, *fileName,
 				srcDir,
@@ -603,7 +604,7 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 		})
 		if err != nil {
 			if matches, e := regexp.MatchString(`cannot move .* Permission denied`, err.Error()); e == nil && matches {
-				return diag.FromErr(newErrSSHUserNoPermission(capi.SSH().Username()))
+				return diag.FromErr(ssh.NewErrUserHasNoPermission(capi.SSH().Username()))
 			}
 
 			diags = append(diags, diag.Errorf("error moving file: %s", err.Error())...)
@@ -776,7 +777,7 @@ func fileRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
 
 	readFileAttrs := readFile
 	if fileIsURL(d) {
-		readFileAttrs = readURL
+		readFileAttrs = readURL(capi.API().HTTP())
 	}
 
 	var diags diag.Diagnostics
@@ -873,50 +874,57 @@ func readFile(
 	return fileModificationDate, fileSize, fileTag, nil
 }
 
-//nolint:nonamedreturns
 func readURL(
+	httClient *http.Client,
+) func(
 	ctx context.Context,
 	sourceFilePath string,
 ) (fileModificationDate string, fileSize int64, fileTag string, err error) {
-	res, err := http.Head(sourceFilePath)
-	if err != nil {
-		return
-	}
-
-	defer utils.CloseOrLogError(ctx)(res.Body)
-
-	fileSize = res.ContentLength
-	httpLastModified := res.Header.Get("Last-Modified")
-
-	if httpLastModified != "" {
-		var timeParsed time.Time
-		timeParsed, err = time.Parse(time.RFC1123, httpLastModified)
-
+	return func(
+		ctx context.Context,
+		sourceFilePath string,
+	) (string, int64, string, error) {
+		req, err := http.NewRequestWithContext(ctx, http.MethodHead, sourceFilePath, nil)
 		if err != nil {
-			timeParsed, err = time.Parse(time.RFC1123Z, httpLastModified)
+			return "", 0, "", fmt.Errorf("failed to create a new request: %w", err)
+		}
+
+		res, err := httClient.Do(req) //nolint:bodyclose
+		if err != nil {
+			return "", 0, "", fmt.Errorf("failed to HEAD the URL: %w", err)
+		}
+
+		defer utils.CloseOrLogError(ctx)(res.Body)
+
+		fileModificationDate := ""
+		fileSize := res.ContentLength
+		fileTag := ""
+
+		httpLastModified := res.Header.Get("Last-Modified")
+		if httpLastModified != "" {
+			var timeParsed time.Time
+			timeParsed, err = time.Parse(time.RFC1123, httpLastModified)
+
 			if err != nil {
-				return
+				timeParsed, err = time.Parse(time.RFC1123Z, httpLastModified)
+				if err != nil {
+					return fileModificationDate, fileSize, fileTag, fmt.Errorf("failed to parse Last-Modified header: %w", err)
+				}
+			}
+
+			fileModificationDate = timeParsed.UTC().Format(time.RFC3339)
+		}
+
+		httpTag := res.Header.Get("ETag")
+		if httpTag != "" {
+			httpTagParts := strings.Split(httpTag, "\"")
+			if len(httpTagParts) > 1 {
+				fileTag = httpTagParts[1]
 			}
 		}
 
-		fileModificationDate = timeParsed.UTC().Format(time.RFC3339)
+		return fileModificationDate, fileSize, fileTag, nil
 	}
-
-	httpTag := res.Header.Get("ETag")
-
-	if httpTag != "" {
-		httpTagParts := strings.Split(httpTag, "\"")
-
-		if len(httpTagParts) > 1 {
-			fileTag = httpTagParts[1]
-		} else {
-			fileTag = ""
-		}
-	} else {
-		fileTag = ""
-	}
-
-	return
 }
 
 func fileDelete(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {

proxmoxtf/resource/sudo.go

@@ -1,15 +0,0 @@
-package resource
-
-import (
-	"fmt"
-)
-
-const (
-	trySudo = `try_sudo(){ if [ $(sudo -n pvesm apiinfo 2>&1 | grep "APIVER" | wc -l) -gt 0 ]; then sudo $1; else $1; fi }`
-)
-
-func newErrSSHUserNoPermission(username string) error {
-	return fmt.Errorf("the SSH user '%s' does not have required permissions. "+
-		"Make sure 'sudo' is installed and the user is configured in sudoers file. "+
-		"Refer to the documentation for more details", username)
-}

proxmoxtf/resource/vm.go

@@ -18,6 +18,8 @@ import (
 	"time"
 	"unicode"
 
+	"github.com/bpg/terraform-provider-proxmox/proxmox/ssh"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/uuid"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
@@ -619,6 +621,15 @@ func VM() *schema.Resource {
 				Description: "The description",
 				Optional:    true,
 				Default:     dvResourceVirtualEnvironmentVMDescription,
+				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") + "\n"
+					}
+
+					return ""
+				},
 			},
 			mkResourceVirtualEnvironmentVMDisk: {
 				Type:        schema.TypeList,
@@ -2975,7 +2986,7 @@ func vmCreateCustomDisks(ctx context.Context, d *schema.ResourceData, m interfac
 		commands = append(
 			commands,
 			`set -e`,
-			trySudo,
+			ssh.TrySudo,
 			fmt.Sprintf(`file_id="%s"`, fileID),
 			fmt.Sprintf(`file_format="%s"`, fileFormat),
 			fmt.Sprintf(`datastore_id_target="%s"`, datastoreID),
@@ -3009,7 +3020,7 @@ func vmCreateCustomDisks(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 diag.FromErr(newErrSSHUserNoPermission(api.SSH().Username()))
+				return diag.FromErr(ssh.NewErrUserHasNoPermission(api.SSH().Username()))
 			}
 
 			return diag.FromErr(err)