diff --git a/fwprovider/attribute/planmodifiers/use_unknown_for_null_config_list.go b/fwprovider/attribute/planmodifiers/use_unknown_for_null_config_list.go new file mode 100644 index 00000000..0911bfdc --- /dev/null +++ b/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) + } +} diff --git a/fwprovider/attribute/planmodifiers/use_unknown_for_null_config_string.go b/fwprovider/attribute/planmodifiers/use_unknown_for_null_config_string.go new file mode 100644 index 00000000..23888aba --- /dev/null +++ b/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() + } +} diff --git a/fwprovider/vm/cloudinit/resource.go b/fwprovider/vm/cloudinit/resource.go index c14cd250..02cd1b92 100644 --- a/fwprovider/vm/cloudinit/resource.go +++ b/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, diff --git a/fwprovider/vm/cloudinit/resource_schema.go b/fwprovider/vm/cloudinit/resource_schema.go index 999d31d8..3e89759d 100644 --- a/fwprovider/vm/cloudinit/resource_schema.go +++ b/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{}), + }, }, }, }, diff --git a/fwprovider/vm/cloudinit/resource_test.go b/fwprovider/vm/cloudinit/resource_test.go index ce4958ec..d18b899b 100644 --- a/fwprovider/vm/cloudinit/resource_test.go +++ b/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 TestAccResourceVM2CloudInit(t *testing.T) { +func TestResource_VM2_CloudInit_Create(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( - test.ResourceAttributes("proxmox_virtual_environment_vm2.test_vm", map[string]string{ - "initialization.datastore_id": te.DatastoreID, - "initialization.interface": "ide2", - }), - ), + Check: test.ResourceAttributes("proxmox_virtual_environment_vm2.test_vm", map[string]string{ + "initialization.datastore_id": te.DatastoreID, + "initialization.interface": "ide2", + }), }}}, - {"update VM with cloud-init", []resource.TestStep{ - //{ - // 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(` + {"domain can't be empty", []resource.TestStep{{ + Config: te.RenderConfig(` resource "proxmox_virtual_environment_vm2" "test_vm" { node_name = "{{.NodeName}}" - id = {{.UpdateVMID}} - name = "test-cloudinit" + id = {{.RandomVMID}} + name = "test-ci" initialization = { dns = { - servers = [ - "1.1.1.1" - ] + domain = "" } } }`), - Destroy: false, - Check: resource.ComposeTestCheckFunc( - test.NoResourceAttributesSet("proxmox_virtual_environment_vm2.test_vm", []string{ - "initialization.dns.domain", - }), - test.ResourceAttributes("proxmox_virtual_environment_vm2.test_vm", map[string]string{ - "initialization.dns.servers.#": "1", - }), - ), - }, - { - Config: te.RenderConfig(` + ExpectError: regexp.MustCompile(`string length must be at least 1, got: 0`), + }}}, + {"servers can't be empty", []resource.TestStep{{ + Config: te.RenderConfig(` resource "proxmox_virtual_environment_vm2" "test_vm" { node_name = "{{.NodeName}}" - id = {{.UpdateVMID}} - name = "test-cloudinit" + id = {{.RandomVMID}} + name = "test-ci" initialization = { dns = { - //servers = [] + servers = [] } } }`), - Destroy: false, - Check: resource.ComposeTestCheckFunc( - test.NoResourceAttributesSet("proxmox_virtual_environment_vm2.test_vm", []string{ - "initialization.dns.servers", - }), - ), - }, - { - Config: te.RenderConfig(` - resource "proxmox_virtual_environment_vm2" "test_vm" { - node_name = "{{.NodeName}}" - id = {{.UpdateVMID}} - name = "test-cloudinit" - initialization = { - dns = {} - } - }`), - Destroy: false, - }, - { - Config: te.RenderConfig(` - resource "proxmox_virtual_environment_vm2" "test_vm" { - node_name = "{{.NodeName}}" - id = {{.UpdateVMID}} - name = "test-cloudinit" - initialization = {} - }`), - }, - }}, + ExpectError: regexp.MustCompile(`list must contain at least 1 elements`), + }}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resource.ParallelTest(t, resource.TestCase{ + ProtoV6ProviderFactories: te.AccProviders, + Steps: tt.steps, + }) + }) + } +} + +func TestResource_VM2_CloudInit_Update(t *testing.T) { + t.Parallel() + + te := test.InitEnvironment(t) + + tests := []struct { + name string + steps []resource.TestStep + }{ + {"add 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" + } + } + }`), + }, + { + 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 {