From febf239b58b725e273397e060563b31846a0acf7 Mon Sep 17 00:00:00 2001
From: Pavel Boldyrev <627562+bpg@users.noreply.github.com>
Date: Wed, 12 Mar 2025 17:16:40 -0400
Subject: [PATCH] fix(provider): better error handling for non-existent
 resources (#1824)

* feat(provider): enhance error handling for non-existent resources
* docs(vm): clarify `local-lvm` datastore usage

---------

Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com>
---
 README.md                                |  9 +++++++++
 docs/resources/virtual_environment_vm.md |  7 +++++++
 fwprovider/test/resource_vm_test.go      | 24 ++++++++++++++++++++++++
 proxmox/api/client.go                    | 14 ++++++++------
 4 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/README.md b/README.md
index 675f1be9..a7b16f11 100644
--- a/README.md
+++ b/README.md
@@ -25,10 +25,16 @@ However, we will try to maintain backward compatibility between provider version
 
 ## Requirements
 
+### Production Requirements
+
 - [Proxmox Virtual Environment](https://www.proxmox.com/en/proxmox-virtual-environment/) 8.x
 - TLS 1.3 for the Proxmox API endpoint (legacy TLS 1.2 is optionally supported)
 - [Terraform](https://www.terraform.io/downloads.html) 1.5.x+ or [OpenTofu](https://opentofu.org) 1.6.x+
+
+### Development Requirements
+
 - [Go](https://golang.org/doc/install) 1.24 (to build the provider plugin)
+- [Docker](https://www.docker.com/products/docker-desktop/) (optional, for running dev tools)
 
 ## Using the Provider
 
@@ -51,6 +57,9 @@ They can be run using `make testacc`. The Proxmox connection can be configured u
 ## Deploying the Example Resources
 
 There are a number of TF examples in the `example` directory, which can be used to deploy a Container, VM, or other Proxmox resources in your test Proxmox environment.
+
+### Prerequisites
+
 The following assumptions are made about the test environment:
 
 - It has one node named `pve`
diff --git a/docs/resources/virtual_environment_vm.md b/docs/resources/virtual_environment_vm.md
index 78df8c1e..364faa2e 100755
--- a/docs/resources/virtual_environment_vm.md
+++ b/docs/resources/virtual_environment_vm.md
@@ -628,6 +628,13 @@ trusts the user to set `agent.enabled` correctly and waits for
 
 ## Important Notes
 
+### `local-lvm` Datastore
+
+The `local-lvm` is the **default datastore** for many configuration blocks, including `initialization` and `tpm_state`, which may not seem to be related to "storage".
+If you do not have `local-lvm` configured in your environment, you may need to explicitly set the `datastore_id` in such blocks to a different value.
+
+### Cloning
+
 When cloning an existing virtual machine, whether it's a template or not, the
 resource will only detect changes to the arguments which are not set to their
 default values.
diff --git a/fwprovider/test/resource_vm_test.go b/fwprovider/test/resource_vm_test.go
index cd397cbd..b3a246c7 100644
--- a/fwprovider/test/resource_vm_test.go
+++ b/fwprovider/test/resource_vm_test.go
@@ -680,6 +680,30 @@ func TestAccResourceVMClone(t *testing.T) {
 				}),
 			),
 		}}},
+		{"clone initialization datastore does not exist", []resource.TestStep{{
+			Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm" "template" {
+					node_name = "{{.NodeName}}"
+					started   = false
+				}
+				resource "proxmox_virtual_environment_vm" "clone" {
+					node_name = "{{.NodeName}}"
+					started   = false
+					clone {
+						vm_id = proxmox_virtual_environment_vm.template.vm_id
+					}
+					initialization {
+						datastore_id = "doesnotexist"
+						ip_config {
+						  ipv4 {
+							address = "172.16.2.57/32"
+							gateway = "172.16.2.10"
+						  }
+						}
+					}
+				}`),
+			ExpectError: regexp.MustCompile(`storage 'doesnotexist' does not exist`),
+		}}},
 	}
 
 	for _, tt := range tests {
diff --git a/proxmox/api/client.go b/proxmox/api/client.go
index a537a159..ee763d18 100644
--- a/proxmox/api/client.go
+++ b/proxmox/api/client.go
@@ -329,11 +329,6 @@ func (c *client) HTTP() *http.Client {
 // validateResponseCode ensures that a response is valid.
 func validateResponseCode(res *http.Response) error {
 	if res.StatusCode < 200 || res.StatusCode >= 300 {
-		if res.StatusCode == http.StatusNotFound ||
-			(res.StatusCode == http.StatusInternalServerError && strings.Contains(res.Status, "does not exist")) {
-			return ErrResourceDoesNotExist
-		}
-
 		msg := strings.TrimPrefix(res.Status, fmt.Sprintf("%d ", res.StatusCode))
 
 		errRes := &ErrorResponseBody{}
@@ -349,10 +344,17 @@ func validateResponseCode(res *http.Response) error {
 			msg = fmt.Sprintf("%s (%s)", msg, strings.Join(errList, " - "))
 		}
 
-		return &HTTPError{
+		httpError := &HTTPError{
 			Code:    res.StatusCode,
 			Message: msg,
 		}
+
+		if res.StatusCode == http.StatusNotFound ||
+			(res.StatusCode == http.StatusInternalServerError && strings.Contains(res.Status, "does not exist")) {
+			return errors.Join(ErrResourceDoesNotExist, httpError)
+		}
+
+		return httpError
 	}
 
 	return nil