From 37bdeccf9b89a4b3188182f244c95c631099bfe7 Mon Sep 17 00:00:00 2001 From: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Date: Thu, 1 May 2025 23:17:17 -0400 Subject: [PATCH] file(file): handle remote file size check error in `download_file` resource (#1940) Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- fwprovider/nodes/resource_download_file.go | 49 ++++++++++++------- .../nodes/resource_download_file_test.go | 2 + fwprovider/test/resource_file_test.go | 26 ++-------- fwprovider/test/test_support.go | 23 +++++++++ 4 files changed, 58 insertions(+), 42 deletions(-) diff --git a/fwprovider/nodes/resource_download_file.go b/fwprovider/nodes/resource_download_file.go index ca011811..e0421efe 100644 --- a/fwprovider/nodes/resource_download_file.go +++ b/fwprovider/nodes/resource_download_file.go @@ -26,6 +26,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/bpg/terraform-provider-proxmox/fwprovider/attribute" "github.com/bpg/terraform-provider-proxmox/fwprovider/config" @@ -119,17 +120,29 @@ func (r sizeRequiresReplaceModifier) PlanModifyInt64( } if state.Size.ValueInt64() != urlSize { - resp.RequiresReplace = true - resp.PlanValue = types.Int64Value(urlSize) + if urlSize < 0 { + resp.Diagnostics.AddWarning( + "Could not read the file metadata from URL.", + fmt.Sprintf( + "The remote file at URL %q most likely doesn’t exist or can’t be accessed.\n"+ + "To skip the remote file check, set `overwrite` to `false`.", + plan.URL.ValueString(), + ), + ) + } else { + resp.RequiresReplace = true + resp.PlanValue = types.Int64Value(urlSize) - resp.Diagnostics.AddWarning( - "The file size from url has changed.", - fmt.Sprintf( - "Size from url %d does not match size from datastore: %d", - urlSize, - state.Size.ValueInt64(), - ), - ) + resp.Diagnostics.AddWarning( + "The file size from url has changed.", + fmt.Sprintf( + "Size %d from url %q does not match size from datastore: %d", + urlSize, + plan.URL.ValueString(), + state.Size.ValueInt64(), + ), + ) + } return } @@ -548,15 +561,13 @@ func (r *downloadFileResource) Read( &state, ) if err != nil { - resp.Diagnostics.AddError( - "Could not get file metadata from url.", - err.Error(), - ) - - return - } - - if urlMetadata.Size != nil { + tflog.Error(ctx, "Could not get file metadata from url", map[string]interface{}{ + "error": err, + "url": state.URL.ValueString(), + }) + // force size to -1, which is a special value used in sizeRequiresReplaceModifier + resp.Private.SetKey(ctx, "url_size", []byte("-1")) + } else if urlMetadata.Size != nil { setValue := []byte(strconv.FormatInt(*urlMetadata.Size, 10)) resp.Private.SetKey(ctx, "url_size", setValue) } diff --git a/fwprovider/nodes/resource_download_file_test.go b/fwprovider/nodes/resource_download_file_test.go index f8ec5dd2..953de97e 100644 --- a/fwprovider/nodes/resource_download_file_test.go +++ b/fwprovider/nodes/resource_download_file_test.go @@ -28,6 +28,7 @@ import ( "github.com/bpg/terraform-provider-proxmox/proxmox/helpers/ptr" "github.com/bpg/terraform-provider-proxmox/proxmox/nodes/storage" "github.com/bpg/terraform-provider-proxmox/proxmox/ssh" + "github.com/bpg/terraform-provider-proxmox/proxmox/types" "github.com/bpg/terraform-provider-proxmox/utils" ) @@ -163,6 +164,7 @@ func TestAccResourceDownloadFile(t *testing.T) { Node: ptr.Ptr(te.NodeName), Storage: ptr.Ptr(te.DatastoreID), URL: ptr.Ptr(fakeFileISO), + Verify: ptr.Ptr(types.CustomBool(false)), }) require.NoError(t, err) diff --git a/fwprovider/test/resource_file_test.go b/fwprovider/test/resource_file_test.go index 96ac0035..1600cd94 100644 --- a/fwprovider/test/resource_file_test.go +++ b/fwprovider/test/resource_file_test.go @@ -42,9 +42,9 @@ func TestAccResourceFile(t *testing.T) { snippetRaw := fmt.Sprintf("snippet-raw-%s.txt", gofakeit.Word()) snippetURL := "https://raw.githubusercontent.com/yaml/yaml-test-suite/main/src/229Q.yaml" - snippetFile1 := strings.ReplaceAll(createFile(t, "snippet-file-1-*.yaml", "test snippet 1 - file").Name(), `\`, `/`) - snippetFile2 := strings.ReplaceAll(createFile(t, "snippet-file-2-*.yaml", "test snippet 2 - file").Name(), `\`, `/`) - fileISO := strings.ReplaceAll(createFile(t, "file-*.iso", "pretend it is an ISO").Name(), `\`, `/`) + snippetFile1 := strings.ReplaceAll(CreateTempFile(t, "snippet-file-1-*.yaml", "test snippet 1 - file").Name(), `\`, `/`) + snippetFile2 := strings.ReplaceAll(CreateTempFile(t, "snippet-file-2-*.yaml", "test snippet 2 - file").Name(), `\`, `/`) + fileISO := strings.ReplaceAll(CreateTempFile(t, "file-*.iso", "pretend this is an ISO").Name(), `\`, `/`) te.AddTemplateVars(map[string]interface{}{ "SnippetRaw": snippetRaw, @@ -264,26 +264,6 @@ func uploadSnippetFile(t *testing.T, fileName string) { 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 func(f *os.File) { - _ = f.Close() - }(f) - - t.Cleanup(func() { - _ = os.Remove(f.Name()) - }) - - return f -} - func deleteSnippet(te *Environment, fname string) { te.t.Helper() diff --git a/fwprovider/test/test_support.go b/fwprovider/test/test_support.go index 897a98f1..a7705a56 100644 --- a/fwprovider/test/test_support.go +++ b/fwprovider/test/test_support.go @@ -8,10 +8,13 @@ package test import ( "fmt" + "os" "regexp" + "testing" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/stretchr/testify/require" ) // ResourceAttributes is a helper function to test resource attributes. @@ -69,3 +72,23 @@ func ResourceAttributesSet(res string, attrs []string) resource.TestCheckFunc { return nil } } + +func CreateTempFile(t *testing.T, namePattern string, content string) *os.File { + t.Helper() + + f, err := os.CreateTemp(t.TempDir(), namePattern) + require.NoError(t, err) + + _, err = f.WriteString(content) + require.NoError(t, err) + + defer func(f *os.File) { + _ = f.Close() + }(f) + + t.Cleanup(func() { + _ = os.Remove(f.Name()) + }) + + return f +}