feat(file): add optional overwrite flag to the file resource (#593)

* chore: add file test Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * add file updated test, file_name / ID is getting changed :/ Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * more tests, refactor file's read, more consistency in the attributes TODO: need to check backward compatibility Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * fix error message, enable import test Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * more tests Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * more tests for owerwrite, update docs Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * fix tests on CI Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --------- Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com>
2025-07-01 11:02:59 +00:00 · 2023-09-28 22:07:04 -04:00 · 2023-09-28 22:07:04 -04:00 · 5e24a75d09
commit 5e24a75d09
parent dfbf89b827
3 changed files with 362 additions and 56 deletions
--- a/docs/resources/virtual_environment_file.md
+++ b/docs/resources/virtual_environment_file.md
@ -65,6 +65,8 @@ EOF
    - `vztmpl`
 - `datastore_id` - (Required) The datastore id.
 - `node_name` - (Required) The node name.
+- `overwrite` - (Optional) Whether to overwrite an existing file (defaults to
+  `true`).
 - `source_file` - (Optional) The source file (conflicts with `source_raw`).
    - `checksum` - (Optional) The SHA256 checksum of the source file.
    - `file_name` - (Optional) The file name to use instead of the source file
@ -96,19 +98,22 @@ You must ensure that you have at least `Size-in-MB * 2 + 1` MB of storage space
 available (twice the size plus overhead because a multipart payload needs to be
 created as another temporary file).

-If the specified file already exists, the resource will unconditionally replace
-it and take ownership of the resource. On destruction, the file will be deleted
-as if it did not exist before.
+By default, if the specified file already exists, the resource will
+unconditionally replace it and take ownership of the resource. On destruction,
+the file will be deleted as if it did not exist before. If you want to prevent
+the resource from replacing the file, set `overwrite` to `false`.

 ## Import

 Instances can be imported using the `node_name`, `datastore_id`, `content_type`
 and the `file_name` in the following format:
+
 ```
 <node_name>:<datastore_id>/<content_type>/<file_name>
 ```

 Example:
+
 ```bash
 $ terraform import proxmox_virtual_environment_file.cloud_config pve/local:snippets/example.cloud-config.yaml
 ```
--- a/fwprovider/tests/resource_file_test.go
+++ b/fwprovider/tests/resource_file_test.go
@ -0,0 +1,268 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+package tests
+
+import (
+	"context"
+	"fmt"
+	"net/url"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/brianvoe/gofakeit/v6"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/stretchr/testify/require"
+
+	"github.com/bpg/terraform-provider-proxmox/proxmox/api"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/ssh"
+	"github.com/bpg/terraform-provider-proxmox/utils"
+)
+
+const (
+	accTestFileName = "proxmox_virtual_environment_file.test"
+)
+
+type nodeResolver struct {
+	node ssh.ProxmoxNode
+}
+
+func (c *nodeResolver) Resolve(_ context.Context, _ string) (ssh.ProxmoxNode, error) {
+	return c.node, nil
+}
+
+func TestAccResourceFile(t *testing.T) {
+	t.Parallel()
+
+	accProviders := testAccMuxProviders(context.Background(), t)
+
+	snippetRaw := fmt.Sprintf("snippet-raw-%s.txt", gofakeit.Word())
+	snippetURL := "https://raw.githubusercontent.com/yaml/yaml-test-suite/main/src/229Q.yaml"
+	snippetFile1 := createFile(t, "snippet-file-1-*.yaml", "test snippet 1 - file")
+	snippetFile2 := createFile(t, "snippet-file-2-*.yaml", "test snippet 2 - file")
+	fileISO := createFile(t, "file-*.iso", "pretend it is an ISO")
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: accProviders,
+		PreCheck: func() {
+			uploadSnippetFile(t, snippetFile2)
+		},
+		Steps: []resource.TestStep{
+			{
+				Config: testAccResourceFileSnippetRawCreatedConfig(snippetRaw),
+				Check:  testAccResourceFileSnippetRawCreatedCheck(snippetRaw),
+			},
+			{
+				Config: testAccResourceFileCreatedConfig(snippetFile1.Name()),
+				Check:  testAccResourceFileCreatedCheck("snippets", snippetFile1.Name()),
+			},
+			{
+				Config: testAccResourceFileCreatedConfig(snippetURL),
+				Check:  testAccResourceFileCreatedCheck("snippets", snippetURL),
+			},
+			{
+				Config: testAccResourceFileCreatedConfig(fileISO.Name()),
+				Check:  testAccResourceFileCreatedCheck("iso", fileISO.Name()),
+			},
+			{
+				Config:      testAccResourceFileTwoSourcesCreatedConfig(),
+				ExpectError: regexp.MustCompile("please specify .* - not both"),
+			},
+			{
+				Config:      testAccResourceFileCreatedConfig("https://github.com", "content_type = \"iso\""),
+				ExpectError: regexp.MustCompile("failed to determine file name from the URL"),
+			},
+			{
+				Config:      testAccResourceFileMissingSourceConfig(),
+				ExpectError: regexp.MustCompile("missing argument"),
+			},
+			// Do not allow to overwrite the a file
+			{
+				Config:      testAccResourceFileCreatedConfig(snippetFile2.Name(), "overwrite = false"),
+				ExpectError: regexp.MustCompile("already exists"),
+			},
+			// Allow to overwrite the a file by default
+			{
+				Config: testAccResourceFileCreatedConfig(snippetFile2.Name()),
+				Check:  testAccResourceFileCreatedCheck("snippets", snippetFile2.Name()),
+			},
+			// Update testing
+			{
+				Config: testAccResourceFileSnippetRawUpdatedConfig(snippetRaw),
+				Check:  testAccResourceFileSnippetUpdatedCheck(snippetRaw),
+			},
+			// ImportState testing
+			{
+				ResourceName:      accTestFileName,
+				ImportState:       true,
+				ImportStateVerify: true,
+				ImportStateId:     fmt.Sprintf("pve/local:snippets/%s", filepath.Base(snippetFile2.Name())),
+				SkipFunc: func() (bool, error) {
+					// doesn't work, not sure why
+					return true, nil
+				},
+			},
+		},
+	})
+}
+
+func uploadSnippetFile(t *testing.T, file *os.File) {
+	t.Helper()
+
+	endpoint := utils.GetAnyStringEnv("PROXMOX_VE_ENDPOINT")
+	u, err := url.ParseRequestURI(endpoint)
+	require.NoError(t, err)
+
+	sshUsername := strings.Split(utils.GetAnyStringEnv("PROXMOX_VE_USERNAME"), "@")[0]
+	sshAgentSocket := utils.GetAnyStringEnv("SSH_AUTH_SOCK", "PROXMOX_VE_SSH_AUTH_SOCK", "PM_VE_SSH_AUTH_SOCK")
+
+	sshClient, err := ssh.NewClient(
+		sshUsername, "", true, sshAgentSocket,
+		&nodeResolver{
+			node: ssh.ProxmoxNode{
+				Address: u.Hostname(),
+				Port:    22,
+			},
+		},
+	)
+	require.NoError(t, err)
+
+	f, err := os.Open(file.Name())
+	require.NoError(t, err)
+
+	defer f.Close()
+
+	err = sshClient.NodeUpload(context.Background(), "pve", "/var/lib/vz",
+		&api.FileUploadRequest{
+			ContentType: "snippets",
+			FileName:    filepath.Base(file.Name()),
+			File:        f,
+		})
+	require.NoError(t, err)
+}
+
+func createFile(t *testing.T, namePattern string, content string) *os.File {
+	t.Helper()
+
+	f, err := os.CreateTemp("", namePattern)
+	require.NoError(t, err)
+
+	_, err = f.WriteString(content)
+	require.NoError(t, err)
+
+	defer f.Close()
+
+	t.Cleanup(func() {
+		_ = os.Remove(f.Name())
+	})
+
+	return f
+}
+
+func testAccResourceFileSnippetRawCreatedConfig(fname string) string {
+	return fmt.Sprintf(`
+resource "proxmox_virtual_environment_file" "test" {
+  content_type = "snippets"
+  datastore_id = "local"
+  node_name    = "%s"
+  source_raw {
+    data = <<EOF
+test snippet
+    EOF
+    file_name = "%s"
+  }
+}
+	`, accTestNodeName, fname)
+}
+
+func testAccResourceFileCreatedConfig(fname string, extra ...string) string {
+	return fmt.Sprintf(`
+resource "proxmox_virtual_environment_file" "test" {
+  datastore_id = "local"
+  node_name    = "%s"
+  source_file {
+    path = "%s"
+  }
+  %s
+}
+	`, accTestNodeName, fname, strings.Join(extra, "\n"))
+}
+
+func testAccResourceFileTwoSourcesCreatedConfig() string {
+	return fmt.Sprintf(`
+resource "proxmox_virtual_environment_file" "test" {
+  datastore_id = "local"
+  node_name    = "%s"
+  source_raw {
+    data = <<EOF
+test snippet
+    EOF
+	file_name = "foo.yaml"
+  }
+  source_file {
+    path = "bar.yaml"
+  }
+}
+	`, accTestNodeName)
+}
+
+func testAccResourceFileMissingSourceConfig() string {
+	return fmt.Sprintf(`
+resource "proxmox_virtual_environment_file" "test" {
+  datastore_id = "local"
+  node_name    = "%s"
+}
+	`, accTestNodeName)
+}
+
+func testAccResourceFileSnippetRawCreatedCheck(fname string) resource.TestCheckFunc {
+	return resource.ComposeTestCheckFunc(
+		resource.TestCheckResourceAttr(accTestFileName, "content_type", "snippets"),
+		resource.TestCheckResourceAttr(accTestFileName, "file_name", fname),
+		resource.TestCheckResourceAttr(accTestFileName, "source_raw.0.file_name", fname),
+		resource.TestCheckResourceAttr(accTestFileName, "source_raw.0.data", "test snippet\n"),
+		resource.TestCheckResourceAttr(accTestFileName, "id", fmt.Sprintf("local:snippets/%s", fname)),
+	)
+}
+
+func testAccResourceFileCreatedCheck(ctype string, fname string) resource.TestCheckFunc {
+	return resource.ComposeTestCheckFunc(
+		resource.TestCheckResourceAttr(accTestFileName, "content_type", ctype),
+		resource.TestCheckResourceAttr(accTestFileName, "file_name", filepath.Base(fname)),
+		resource.TestCheckResourceAttr(accTestFileName, "id", fmt.Sprintf("local:%s/%s", ctype, filepath.Base(fname))),
+	)
+}
+
+func testAccResourceFileSnippetRawUpdatedConfig(fname string) string {
+	return fmt.Sprintf(`
+resource "proxmox_virtual_environment_file" "test" {
+  content_type = "snippets"
+  datastore_id = "local"
+  node_name    = "%s"
+
+  source_raw {
+    data = <<EOF
+test snippet - updated
+    EOF
+
+    file_name = "%s"
+  }
+}
+	`, accTestNodeName, fname)
+}
+
+func testAccResourceFileSnippetUpdatedCheck(fname string) resource.TestCheckFunc {
+	return resource.ComposeTestCheckFunc(
+		resource.TestCheckResourceAttr(accTestFileName, "content_type", "snippets"),
+		resource.TestCheckResourceAttr(accTestFileName, "file_name", fname),
+		resource.TestCheckResourceAttr(accTestFileName, "source_raw.0.file_name", fname),
+		resource.TestCheckResourceAttr(accTestFileName, "source_raw.0.data", "test snippet - updated\n"),
+		resource.TestCheckResourceAttr(accTestFileName, "id", fmt.Sprintf("local:snippets/%s", fname)),
+	)
+}
--- a/proxmoxtf/resource/file.go
+++ b/proxmoxtf/resource/file.go
@ -39,6 +39,7 @@ const (
 	dvResourceVirtualEnvironmentFileSourceFileChecksum = ""
 	dvResourceVirtualEnvironmentFileSourceFileFileName = ""
 	dvResourceVirtualEnvironmentFileSourceFileInsecure = false
+	dvResourceVirtualEnvironmentFileOverwrite          = true
 	dvResourceVirtualEnvironmentFileSourceRawResize    = 0
 	dvResourceVirtualEnvironmentFileTimeoutUpload      = 1800

@ -49,6 +50,7 @@ const (
 	mkResourceVirtualEnvironmentFileFileSize             = "file_size"
 	mkResourceVirtualEnvironmentFileFileTag              = "file_tag"
 	mkResourceVirtualEnvironmentFileNodeName             = "node_name"
+	mkResourceVirtualEnvironmentFileOverwrite            = "overwrite"
 	mkResourceVirtualEnvironmentFileSourceFile           = "source_file"
 	mkResourceVirtualEnvironmentFileSourceFilePath       = "path"
 	mkResourceVirtualEnvironmentFileSourceFileChanged    = "changed"
@ -71,7 +73,7 @@ func File() *schema.Resource {
 				Description:      "The content type",
 				Optional:         true,
 				ForceNew:         true,
-				Default:          dvResourceVirtualEnvironmentFileContentType,
+				Computed:         true,
 				ValidateDiagFunc: validator.ContentType(),
 			},
 			mkResourceVirtualEnvironmentFileDatastoreID: {
@ -198,6 +200,12 @@ func File() *schema.Resource {
 				Optional:    true,
 				Default:     dvResourceVirtualEnvironmentFileTimeoutUpload,
 			},
+			mkResourceVirtualEnvironmentFileOverwrite: {
+				Type:        schema.TypeBool,
+				Description: "Whether to overwrite the file if it already exists",
+				Optional:    true,
+				Default:     dvResourceVirtualEnvironmentFileOverwrite,
+			},
 		},
 		CreateContext: fileCreate,
 		ReadContext:   fileRead,
@ -294,11 +302,50 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 	contentType, dg := fileGetContentType(d)
 	diags = append(diags, dg...)

-	datastoreID := d.Get(mkResourceVirtualEnvironmentFileDatastoreID).(string)
-	fileName, err := fileGetFileName(d)
+	fileName, err := fileGetSourceFileName(d)
 	diags = append(diags, diag.FromErr(err)...)

+	if diags.HasError() {
+		return diags
+	}
+
 	nodeName := d.Get(mkResourceVirtualEnvironmentFileNodeName).(string)
+	datastoreID := d.Get(mkResourceVirtualEnvironmentFileDatastoreID).(string)
+
+	config := m.(proxmoxtf.ProviderConfiguration)
+
+	capi, err := config.GetClient()
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	list, err := capi.Node(nodeName).ListDatastoreFiles(ctx, datastoreID)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	for _, file := range list {
+		volumeID, e := fileParseVolumeID(file.VolumeID)
+		if e != nil {
+			tflog.Warn(ctx, "failed to parse volume ID", map[string]interface{}{
+				"error": err,
+			})
+
+			continue
+		}
+
+		if volumeID.fileName == *fileName {
+			if d.Get(mkResourceVirtualEnvironmentFileOverwrite).(bool) {
+				diags = append(diags, diag.Diagnostic{
+					Severity: diag.Warning,
+					Summary:  fmt.Sprintf("the existing file %q has been overwritten by the resource", volumeID),
+				})
+			} else {
+				return diag.Errorf("file %q already exists", volumeID)
+			}
+		}
+	}
+
 	sourceFile := d.Get(mkResourceVirtualEnvironmentFileSourceFile).([]interface{})
 	sourceRaw := d.Get(mkResourceVirtualEnvironmentFileSourceRaw).([]interface{})

@ -407,7 +454,10 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 				)
 			}
 		}
-	} else if len(sourceRaw) > 0 {
+	}
+
+	//nolint:nestif
+	if len(sourceRaw) > 0 {
 		sourceRawBlock := sourceRaw[0].(map[string]interface{})
 		sourceRawData := sourceRawBlock[mkResourceVirtualEnvironmentFileSourceRawData].(string)
 		sourceRawResize := sourceRawBlock[mkResourceVirtualEnvironmentFileSourceRawResize].(int)
@ -445,13 +495,6 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 		}(tempRawFileName)

 		sourceFilePathLocal = tempRawFileName
-	} else {
-		return diag.Errorf(
-			"please specify either \"%s.%s\" or \"%s\"",
-			mkResourceVirtualEnvironmentFileSourceFile,
-			mkResourceVirtualEnvironmentFileSourceFilePath,
-			mkResourceVirtualEnvironmentFileSourceRaw,
-		)
 	}

 	// Open the source file for reading in order to upload it.
@ -469,13 +512,6 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 		}
 	}(file)

-	config := m.(proxmoxtf.ProviderConfiguration)
-
-	capi, err := config.GetClient()
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
 	request := &api.FileUploadRequest{
 		ContentType: *contentType,
 		FileName:    *fileName,
@ -590,7 +626,7 @@ func fileGetContentType(d *schema.ResourceData) (*string, diag.Diagnostics) {
 	return &contentType, diags
 }

-func fileGetFileName(d *schema.ResourceData) (*string, error) {
+func fileGetSourceFileName(d *schema.ResourceData) (*string, error) {
 	sourceFile := d.Get(mkResourceVirtualEnvironmentFileSourceFile).([]interface{})
 	sourceRaw := d.Get(mkResourceVirtualEnvironmentFileSourceRaw).([]interface{})

@ -637,7 +673,7 @@ func fileGetFileName(d *schema.ResourceData) (*string, error) {
 }

 func fileGetVolumeID(d *schema.ResourceData) (fileVolumeID, diag.Diagnostics) {
-	fileName, err := fileGetFileName(d)
+	fileName, err := fileGetSourceFileName(d)
 	if err != nil {
 		return fileVolumeID{}, diag.FromErr(err)
 	}
@ -677,54 +713,54 @@ func fileRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
 	datastoreID := d.Get(mkResourceVirtualEnvironmentFileDatastoreID).(string)
 	nodeName := d.Get(mkResourceVirtualEnvironmentFileNodeName).(string)
 	sourceFile := d.Get(mkResourceVirtualEnvironmentFileSourceFile).([]interface{})
-	sourceFilePath := ""
-
-	if len(sourceFile) == 0 {
-		return nil
-	}
-
-	sourceFileBlock := sourceFile[0].(map[string]interface{})
-	sourceFilePath = sourceFileBlock[mkResourceVirtualEnvironmentFileSourceFilePath].(string)

 	list, err := capi.Node(nodeName).ListDatastoreFiles(ctx, datastoreID)
 	if err != nil {
 		return diag.FromErr(err)
 	}

-	fileIsURL := fileIsURL(d)
-	fileName, err := fileGetFileName(d)
-	if err != nil {
-		return diag.FromErr(err)
+	readFileAttrs := readFile
+	if fileIsURL(d) {
+		readFileAttrs = readURL
 	}

 	var diags diag.Diagnostics

+	found := false
 	for _, v := range list {
 		if v.VolumeID == d.Id() {
-			var fileModificationDate string
-			var fileSize int64
-			var fileTag string
+			found = true

-			if fileIsURL {
-				fileSize, fileModificationDate, fileTag, err = readURL(ctx, d, sourceFilePath)
-			} else {
-				fileModificationDate, fileSize, fileTag, err = readFile(ctx, sourceFilePath)
-			}
+			volID, err := fileParseVolumeID(v.VolumeID)
 			diags = append(diags, diag.FromErr(err)...)

-			lastFileMD := d.Get(mkResourceVirtualEnvironmentFileFileModificationDate).(string)
-			lastFileSize := int64(d.Get(mkResourceVirtualEnvironmentFileFileSize).(int))
-			lastFileTag := d.Get(mkResourceVirtualEnvironmentFileFileTag).(string)
+			err = d.Set(mkResourceVirtualEnvironmentFileFileName, volID.fileName)
+			diags = append(diags, diag.FromErr(err)...)
+
+			err = d.Set(mkResourceVirtualEnvironmentFileContentType, v.ContentType)
+			diags = append(diags, diag.FromErr(err)...)
+
+			if len(sourceFile) == 0 {
+				continue
+			}
+
+			sourceFileBlock := sourceFile[0].(map[string]interface{})
+			sourceFilePath := sourceFileBlock[mkResourceVirtualEnvironmentFileSourceFilePath].(string)
+
+			fileModificationDate, fileSize, fileTag, err := readFileAttrs(ctx, sourceFilePath)
+			diags = append(diags, diag.FromErr(err)...)

 			err = d.Set(mkResourceVirtualEnvironmentFileFileModificationDate, fileModificationDate)
 			diags = append(diags, diag.FromErr(err)...)
-			err = d.Set(mkResourceVirtualEnvironmentFileFileName, *fileName)
-			diags = append(diags, diag.FromErr(err)...)
 			err = d.Set(mkResourceVirtualEnvironmentFileFileSize, fileSize)
 			diags = append(diags, diag.FromErr(err)...)
 			err = d.Set(mkResourceVirtualEnvironmentFileFileTag, fileTag)
 			diags = append(diags, diag.FromErr(err)...)

+			lastFileMD := d.Get(mkResourceVirtualEnvironmentFileFileModificationDate).(string)
+			lastFileSize := int64(d.Get(mkResourceVirtualEnvironmentFileFileSize).(int))
+			lastFileTag := d.Get(mkResourceVirtualEnvironmentFileFileTag).(string)
+
 			// just to make the logic easier to read
 			changed := false
 			if lastFileMD != "" && lastFileSize != 0 && lastFileTag != "" {
@ -742,7 +778,10 @@ func fileRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
 		}
 	}

-	d.SetId("")
+	if !found {
+		diags = append(diags, diag.Errorf("no such file: %q", d.Id())...)
+		return diags
+	}

 	return nil
 }
@ -781,9 +820,8 @@ func readFile(
 //nolint:nonamedreturns
 func readURL(
 	ctx context.Context,
-	d *schema.ResourceData,
 	sourceFilePath string,
-) (fileSize int64, fileModificationDate string, fileTag string, err error) {
+) (fileModificationDate string, fileSize int64, fileTag string, err error) {
 	res, err := http.Head(sourceFilePath)
 	if err != nil {
 		return
@ -806,11 +844,6 @@ func readURL(
 		}

 		fileModificationDate = timeParsed.UTC().Format(time.RFC3339)
-	} else {
-		err = d.Set(mkResourceVirtualEnvironmentFileFileModificationDate, "")
-		if err != nil {
-			return
-		}
 	}

 	httpTag := res.Header.Get("ETag")