sorting out delete of optional + computed attrs

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

fwprovider/attribute/planmodifiers/use_unknown_for_null_config_list.go

@@ -0,0 +1,53 @@
+/*
+ * 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 planmodifiers
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-framework/attr"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+)
+
+// UseUnknownForNullConfigList returns a plan modifier sets the value of an attribute
+// to Unknown if the attribute is missing from the plan and the config is null.
+// Use this for optional computed attributes that can be reset / removed by the user.
+//
+// The behavior for Terraform for Optional + Computed attributes is to copy the prior state
+// if there is no configuration for it. This plan modifier will instead set the value to Unknown,
+// so the provider can handle the attribute as needed.
+func UseUnknownForNullConfigList(elementType attr.Type) planmodifier.List {
+	return useUnknownForNullConfigList{elementType}
+}
+
+// useUnknownForNullConfigList implements the plan modifier.
+type useUnknownForNullConfigList struct {
+	elementType attr.Type
+}
+
+// Description returns a human-readable description of the plan modifier.
+func (m useUnknownForNullConfigList) Description(_ context.Context) string {
+	return "Value of this attribute will be set to Unknown if missing from the plan."
+}
+
+// MarkdownDescription returns a markdown description of the plan modifier.
+func (m useUnknownForNullConfigList) MarkdownDescription(_ context.Context) string {
+	return "Value of this attribute will be set to Unknown if missing from the plan. " +
+		"Use for optional computed attributes that can be reset / removed by user."
+}
+
+// PlanModifyList implements the plan modification logic.
+func (m useUnknownForNullConfigList) PlanModifyList(
+	_ context.Context,
+	req planmodifier.ListRequest,
+	resp *planmodifier.ListResponse,
+) {
+	if !req.PlanValue.IsNull() && req.ConfigValue.IsNull() {
+		resp.PlanValue = types.ListUnknown(m.elementType)
+	}
+}

fwprovider/attribute/planmodifiers/use_unknown_for_null_config_string.go

@@ -0,0 +1,50 @@
+/*
+ * 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 planmodifiers
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+)
+
+// UseUnknownForNullConfigString returns a plan modifier sets the value of an attribute
+// to Unknown if the attribute is missing from the plan and the config is null.
+// Use this for optional computed attributes that can be reset / removed by the user.
+//
+// The behavior for Terraform for Optional + Computed attributes is to copy the prior state
+// if there is no configuration for it. This plan modifier will instead set the value to Unknown,
+// so the provider can handle the attribute as needed.
+func UseUnknownForNullConfigString() planmodifier.String {
+	return useUnknownForNullConfigString{}
+}
+
+// useUnknownForNullConfigString implements the plan modifier.
+type useUnknownForNullConfigString struct{}
+
+// Description returns a human-readable description of the plan modifier.
+func (m useUnknownForNullConfigString) Description(_ context.Context) string {
+	return "Value of this attribute will be set to Unknown if missing from the plan."
+}
+
+// MarkdownDescription returns a markdown description of the plan modifier.
+func (m useUnknownForNullConfigString) MarkdownDescription(_ context.Context) string {
+	return "Value of this attribute will be set to Unknown if missing from the plan. " +
+		"Use for optional computed attributes that can be reset / removed by user."
+}
+
+// PlanModifyString implements the plan modification logic.
+func (m useUnknownForNullConfigString) PlanModifyString(
+	_ context.Context,
+	req planmodifier.StringRequest,
+	resp *planmodifier.StringResponse,
+) {
+	if !req.PlanValue.IsNull() && req.ConfigValue.IsNull() {
+		resp.PlanValue = types.StringUnknown()
+	}
+}

fwprovider/vm/cloudinit/resource.go

@@ -40,7 +40,7 @@ func NewValue(ctx context.Context, config *vms.GetResponseData, vmID int, diags
 		dns := ModelDNS{}
 		dns.Domain = types.StringPointerValue(config.CloudInitDNSDomain)
 
-		if config.CloudInitDNSServer != nil {
+		if config.CloudInitDNSServer != nil && strings.Trim(*config.CloudInitDNSServer, " ") != "" {
 			dnsServers := strings.Split(*config.CloudInitDNSServer, " ")
 			servers, d := types.ListValueFrom(ctx, customtypes.IPAddrType{}, dnsServers)
 			diags.Append(d...)
@@ -95,7 +95,7 @@ func FillCreateBody(ctx context.Context, plan *Model, body *vms.CreateRequestBod
 	body.AddCustomStorageDevice(plan.Interface.ValueString(), device)
 }
 
-// FillUpdateBody fills the UpdateRequestBody with the CPU settings from the Value.
+// FillUpdateBody fills the UpdateRequestBody with the Cloud-Init settings from the Value.
 func FillUpdateBody(
 	ctx context.Context,
 	plan, state *Model,

fwprovider/vm/cloudinit/resource_schema.go

@@ -7,6 +7,7 @@
 package cloudinit
 
 import (
+	"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
@@ -14,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 
+	"github.com/bpg/terraform-provider-proxmox/fwprovider/attribute/planmodifiers"
 	customtypes "github.com/bpg/terraform-provider-proxmox/fwprovider/types"
 	"github.com/bpg/terraform-provider-proxmox/fwprovider/validators"
 )
@@ -23,6 +25,7 @@ func ResourceSchema() schema.Attribute {
 	return schema.SingleNestedAttribute{
 		Description: "The cloud-init configuration.",
 		Optional:    true,
+		Computed:    true,
 		Attributes: map[string]schema.Attribute{
 			"datastore_id": schema.StringAttribute{
 				Description: "The identifier for the datastore to create the cloud-init disk in (defaults to `local-lvm`)",
@@ -57,15 +60,30 @@ func ResourceSchema() schema.Attribute {
 			"dns": schema.SingleNestedAttribute{
 				Description: "The DNS configuration.",
 				Optional:    true,
+				Computed:    true,
 				Attributes: map[string]schema.Attribute{
 					"domain": schema.StringAttribute{
 						Description: "The domain name to use for the VM.",
 						Optional:    true,
+						Computed:    true,
+						Validators: []validator.String{
+							stringvalidator.LengthAtLeast(1),
+						},
+						PlanModifiers: []planmodifier.String{
+							planmodifiers.UseUnknownForNullConfigString(),
+						},
 					},
 					"servers": schema.ListAttribute{
 						Description: "The list of DNS servers to use.",
 						ElementType: customtypes.IPAddrType{},
 						Optional:    true,
+						Computed:    true,
+						Validators: []validator.List{
+							listvalidator.SizeAtLeast(1),
+						},
+						PlanModifiers: []planmodifier.List{
+							planmodifiers.UseUnknownForNullConfigList(customtypes.IPAddrType{}),
+						},
 					},
 				},
 			},

fwprovider/vm/cloudinit/resource_test.go

@@ -7,6 +7,7 @@
 package cloudinit_test
 
 import (
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -14,15 +15,10 @@ import (
 	"github.com/bpg/terraform-provider-proxmox/fwprovider/test"
 )
 
-const resourceName = "proxmox_virtual_environment_vm2.test_vm"
+func TestResource_VM2_CloudInit_Create(t *testing.T) {
-
-func TestAccResourceVM2CloudInit(t *testing.T) {
 	t.Parallel()
 
 	te := test.InitEnvironment(t)
-	te.AddTemplateVars(map[string]interface{}{
-		"UpdateVMID": te.RandomVMID(),
-	})
 
 	tests := []struct {
 		name  string
@@ -33,136 +29,241 @@ func TestAccResourceVM2CloudInit(t *testing.T) {
 			resource "proxmox_virtual_environment_vm2" "test_vm" {
 				node_name = "{{.NodeName}}"
 				id = {{.RandomVMID}}
-				name = "test-cloudinit"
+				name = "test-ci"
 				initialization = {
 					dns = {
 						domain = "example.com"
 			        }
 				}
 			}`),
-			Check: resource.ComposeTestCheckFunc(
+			Check: test.ResourceAttributes("proxmox_virtual_environment_vm2.test_vm", map[string]string{
-				test.ResourceAttributes("proxmox_virtual_environment_vm2.test_vm", map[string]string{
+				"initialization.datastore_id": te.DatastoreID,
-					"initialization.datastore_id": te.DatastoreID,
+				"initialization.interface":    "ide2",
-					"initialization.interface":    "ide2",
+			}),
-				}),
-			),
 		}}},
-		{"update VM with cloud-init", []resource.TestStep{
+		{"domain can't be empty", []resource.TestStep{{
-			//{
+			Config: te.RenderConfig(`
-			//	Config: te.RenderConfig(`
-			//	resource "proxmox_virtual_environment_vm2" "test_vm" {
-			//		node_name = "{{.NodeName}}"
-			//		id = {{.UpdateVMID}}
-			//		name = "test-cloudinit"
-			//		initialization = {
-			//			dns = {
-			//				domain = "example.com"
-			//			}
-			//		}
-			//	}`),
-			//	Destroy: false,
-			//},
-			//{
-			//	Config: te.RenderConfig(`
-			//	resource "proxmox_virtual_environment_vm2" "test_vm" {
-			//		node_name = "{{.NodeName}}"
-			//		id = {{.UpdateVMID}}
-			//		name = "test-cloudinit"
-			//		initialization = {
-			//			dns = {
-			//				domain = "example.com"
-			//				servers = [
-			//					"1.1.1.1",
-			//					"8.8.8.8"
-			//				]
-			//			}
-			//		}
-			//	}`),
-			//	Destroy: false,
-			//},
-			//{
-			//	Config: te.RenderConfig(`
-			//	resource "proxmox_virtual_environment_vm2" "test_vm" {
-			//		node_name = "{{.NodeName}}"
-			//		id = {{.UpdateVMID}}
-			//		name = "test-cloudinit"
-			//		initialization = {
-			//			dns = {
-			//				domain = "another.domain.com"
-			//				servers = [
-			//					"8.8.8.8",
-			//					"1.1.1.1"
-			//				]
-			//			}
-			//		}
-			//	}`),
-			//	Destroy: false,
-			//},
-			{
-				Config: te.RenderConfig(`
 				resource "proxmox_virtual_environment_vm2" "test_vm" {
 					node_name = "{{.NodeName}}"
-					id = {{.UpdateVMID}}
+					id = {{.RandomVMID}}
-					name = "test-cloudinit"
+					name = "test-ci"
 					initialization = {
 						dns = {
-							servers = [
+							domain = ""
-								"1.1.1.1"
-							]
 						}
 					}
 				}`),
-				Destroy: false,
+			ExpectError: regexp.MustCompile(`string length must be at least 1, got: 0`),
-				Check: resource.ComposeTestCheckFunc(
+		}}},
-					test.NoResourceAttributesSet("proxmox_virtual_environment_vm2.test_vm", []string{
+		{"servers can't be empty", []resource.TestStep{{
-						"initialization.dns.domain",
+			Config: te.RenderConfig(`
-					}),
-					test.ResourceAttributes("proxmox_virtual_environment_vm2.test_vm", map[string]string{
-						"initialization.dns.servers.#": "1",
-					}),
-				),
-			},
-			{
-				Config: te.RenderConfig(`
 				resource "proxmox_virtual_environment_vm2" "test_vm" {
 					node_name = "{{.NodeName}}"
-					id = {{.UpdateVMID}}
+					id = {{.RandomVMID}}
-					name = "test-cloudinit"
+					name = "test-ci"
 					initialization = {
 						dns = {
-							//servers = []
+							servers = []
 						}
 					}
 				}`),
-				Destroy: false,
+			ExpectError: regexp.MustCompile(`list must contain at least 1 elements`),
-				Check: resource.ComposeTestCheckFunc(
+		}}},
-					test.NoResourceAttributesSet("proxmox_virtual_environment_vm2.test_vm", []string{
+	}
-						"initialization.dns.servers",
+
-					}),
+	for _, tt := range tests {
-				),
+		t.Run(tt.name, func(t *testing.T) {
-			},
+			resource.ParallelTest(t, resource.TestCase{
-			{
+				ProtoV6ProviderFactories: te.AccProviders,
-				Config: te.RenderConfig(`
+				Steps:                    tt.steps,
-				resource "proxmox_virtual_environment_vm2" "test_vm" {
+			})
-					node_name = "{{.NodeName}}"
+		})
-					id = {{.UpdateVMID}}
+	}
-					name = "test-cloudinit"
+}
-					initialization = {
+
-						dns = {}
+func TestResource_VM2_CloudInit_Update(t *testing.T) {
-					}
+	t.Parallel()
-				}`),
+
-				Destroy: false,
+	te := test.InitEnvironment(t)
-			},
+
-			{
+	tests := []struct {
-				Config: te.RenderConfig(`
+		name  string
-				resource "proxmox_virtual_environment_vm2" "test_vm" {
+		steps []resource.TestStep
-					node_name = "{{.NodeName}}"
+	}{
-					id = {{.UpdateVMID}}
+		{"add servers", []resource.TestStep{
-					name = "test-cloudinit"
+			{
-					initialization = {}
+				Config: te.RenderConfig(`
-				}`),
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
-			},
+					node_name = "{{.NodeName}}"
-		}},
+					id = {{.RandomVMID}}
+					name = "test-ci"
+					initialization = {
+						dns = {
+							domain = "example.com"
+						}
+					}
+				}`),
+			},
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					name = "test-ci"
+					initialization = {
+						dns = {
+							domain = "example.com"
+							servers = [
+								"1.1.1.1",
+								"8.8.8.8"
+							]
+						}
+					}
+				}`),
+			},
+		}},
+		{"change domain and servers", []resource.TestStep{
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					id = {{.RandomVMID}}
+					name = "test-ci"
+					initialization = {
+						dns = {
+							domain = "example.com"
+							servers = [
+								"1.1.1.1",
+								"8.8.8.8"
+							]
+						}
+					}
+				}`),
+			},
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					name = "test-ci"
+					initialization = {
+						dns = {
+							domain = "another.domain.com"
+							servers = [
+								"8.8.8.8",
+								"1.1.1.1"
+							]
+						}
+					}
+				}`),
+			},
+		}},
+		{"update VM: delete dns.domain", []resource.TestStep{
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					id = {{.RandomVMID}}
+					name = "test-ci"
+					initialization = {
+						dns = {
+							domain = "example.com"
+						}
+					}
+				}`),
+			},
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					name = "test-ci"
+					initialization = {
+						dns = {}
+					}
+				}`),
+				Check: test.NoResourceAttributesSet("proxmox_virtual_environment_vm2.test_vm", []string{
+					"initialization.dns.domain",
+				}),
+			},
+		}},
+		{"delete one of the servers", []resource.TestStep{
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					id = {{.RandomVMID}}
+					name = "test-ci"
+					initialization = {
+						dns = {
+							servers = [
+								"1.1.1.1",
+								"8.8.8.8"
+							]
+						}
+					}
+				}`),
+			},
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					name = "test-ci"
+					initialization = {
+						dns = {
+							domain = "another.domain.com"
+							servers = [
+								"1.1.1.1"
+							]
+						}
+					}
+				}`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("proxmox_virtual_environment_vm2.test_vm", "initialization.dns.servers.#", "1"),
+				),
+			},
+		}},
+		{"delete servers", []resource.TestStep{
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					id = {{.RandomVMID}}
+					name = "test-ci"
+					initialization = {
+						dns = {
+							servers = [
+								"1.1.1.1",
+								"8.8.8.8"
+							]
+						}
+					}
+				}`),
+			},
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					name = "test-ci"
+					initialization = {
+						dns = {
+							// remove, or set to servers = null
+						}
+					}
+				}`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("proxmox_virtual_environment_vm2.test_vm", "initialization.dns.servers.#", "0"),
+				),
+			},
+		}},
+		//	{
+		//		// step 9: update the VM: remove the dns block
+		//		Config: te.RenderConfig(`
+		//		resource "proxmox_virtual_environment_vm2" "test_vm" {
+		//			node_name = "{{.NodeName}}"
+		//			name = "test-ci"
+		//			initialization = {}
+		//		}`),
+		//	},
+		//}},
+
 	}
 
 	for _, tt := range tests {