From 21d0e9e12e71a546bc02eb879a52709d211806c7 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Mon, 30 Dec 2024 13:11:15 +0100 Subject: [PATCH] feat: validate node name (#1217) * feat: validate node name * fix: duplicate imports --- go.mod | 2 +- go.sum | 4 +- .../Internal/resource/guest/node/schema.go | 63 +++++++++++++++++++ proxmox/provider.go | 4 +- proxmox/resource_lxc.go | 14 ++--- proxmox/resource_vm_qemu.go | 51 +++++++-------- 6 files changed, 95 insertions(+), 43 deletions(-) create mode 100644 proxmox/Internal/resource/guest/node/schema.go diff --git a/go.mod b/go.mod index 1b27e5bd..664bcc82 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.21 toolchain go1.21.0 require ( - github.com/Telmate/proxmox-api-go v0.0.0-20241205214358-976ef5098918 + github.com/Telmate/proxmox-api-go v0.0.0-20241228220045-c829269b575d github.com/google/uuid v1.6.0 github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 github.com/hashicorp/terraform-plugin-sdk/v2 v2.34.0 diff --git a/go.sum b/go.sum index 4933ca7e..1656fc2e 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/ProtonMail/go-crypto v1.1.0-alpha.2-proton h1:HKz85FwoXx86kVtTvFke7rgHvq/HoloSUvW5semjFWs= github.com/ProtonMail/go-crypto v1.1.0-alpha.2-proton/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE= -github.com/Telmate/proxmox-api-go v0.0.0-20241205214358-976ef5098918 h1:dfs2cVaIHtia0uq/Vo6y8pF0McWadzFPW7mAB46d1JI= -github.com/Telmate/proxmox-api-go v0.0.0-20241205214358-976ef5098918/go.mod h1:Gu6n6vEn1hlyFUkjrvU+X1fdgaSXLoM9HKYYJqy1fsY= +github.com/Telmate/proxmox-api-go v0.0.0-20241228220045-c829269b575d h1:iFAi3XR4GFHo4jiD8qH0wbbYU7/KFLk5J9dLi4+L6ac= +github.com/Telmate/proxmox-api-go v0.0.0-20241228220045-c829269b575d/go.mod h1:Gu6n6vEn1hlyFUkjrvU+X1fdgaSXLoM9HKYYJqy1fsY= github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo= github.com/agext/levenshtein v1.2.3/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= diff --git a/proxmox/Internal/resource/guest/node/schema.go b/proxmox/Internal/resource/guest/node/schema.go new file mode 100644 index 00000000..2d2d4c43 --- /dev/null +++ b/proxmox/Internal/resource/guest/node/schema.go @@ -0,0 +1,63 @@ +package node + +import ( + pveAPI "github.com/Telmate/proxmox-api-go/proxmox" + + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +const ( + RootNode string = "target_node" + RootNodes string = "target_nodes" +) + +func SchemaNode(s schema.Schema, guestType string) *schema.Schema { + s.Type = schema.TypeString + s.Description = "The node the " + guestType + " guest goes to." + s.ValidateDiagFunc = func(i interface{}, path cty.Path) diag.Diagnostics { + v, ok := i.(string) + if !ok { + return diag.Diagnostics{diag.Diagnostic{ + Severity: diag.Error, + Summary: "Invalid " + RootNode, + Detail: RootNode + " must be a string", + AttributePath: path}} + } + if err := pveAPI.NodeName(v).Validate(); err != nil { + return diag.Diagnostics{diag.Diagnostic{ + Severity: diag.Error, + Summary: "Invalid " + RootNode, + AttributePath: path}} + } + return nil + } + return &s +} + +func SchemaNodes() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeList, // TODO should be `schema.TypeSet` + Optional: true, + Description: "A list of nodes the qemu guest goes to.", + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateDiagFunc: func(i interface{}, path cty.Path) diag.Diagnostics { + v, ok := i.(string) + if !ok { + return diag.Diagnostics{diag.Diagnostic{ + Severity: diag.Error, + Summary: "Invalid " + RootNodes, + Detail: RootNodes + " must be a string", + AttributePath: path}} + } + if err := pveAPI.NodeName(v).Validate(); err != nil { + return diag.Diagnostics{diag.Diagnostic{ + Severity: diag.Error, + Summary: "Invalid " + RootNodes, + AttributePath: path}} + } + return nil + }}} +} diff --git a/proxmox/provider.go b/proxmox/provider.go index 7d90d10e..1090d03f 100644 --- a/proxmox/provider.go +++ b/proxmox/provider.go @@ -417,8 +417,8 @@ func pmParallelBegin(pconf *providerConfiguration) *pmApiLockHolder { return lock } -func resourceId(targetNode string, resType string, vmId int) string { - return fmt.Sprintf("%s/%s/%d", targetNode, resType, vmId) +func resourceId(targetNode pxapi.NodeName, resType string, vmId int) string { + return fmt.Sprintf("%s/%s/%d", targetNode.String(), resType, vmId) } func parseResourceId(resId string) (targetNode string, resType string, vmId int, err error) { diff --git a/proxmox/resource_lxc.go b/proxmox/resource_lxc.go index 9d3f0c59..9524449d 100644 --- a/proxmox/resource_lxc.go +++ b/proxmox/resource_lxc.go @@ -10,6 +10,7 @@ import ( pxapi "github.com/Telmate/proxmox-api-go/proxmox" "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/pxapi/guest/tags" + "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/resource/guest/node" "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/util" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -422,11 +423,8 @@ func resourceLxc() *schema.Resource { Type: schema.TypeString, }, }, - "target_node": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, + node.RootNode: node.SchemaNode(schema.Schema{ + Required: true, ForceNew: true}, "lxc"), "vmid": { Type: schema.TypeInt, Optional: true, @@ -496,7 +494,7 @@ func resourceLxcCreate(ctx context.Context, d *schema.ResourceData, meta interfa config.Unique = d.Get("unique").(bool) config.Unprivileged = d.Get("unprivileged").(bool) - targetNode := d.Get("target_node").(string) + targetNode := pxapi.NodeName(d.Get(node.RootNode).(string)) // proxmox api allows multiple network sets, // having a unique 'id' parameter foreach set @@ -532,7 +530,7 @@ func resourceLxcCreate(ctx context.Context, d *schema.ResourceData, meta interfa } vmr := pxapi.NewVmRef(nextid) - vmr.SetNode(targetNode) + vmr.SetNode(targetNode.String()) if d.Get("clone").(string) != "" { @@ -785,7 +783,7 @@ func _resourceLxcRead(ctx context.Context, d *schema.ResourceData, meta interfac return err } d.SetId(resourceId(vmr.Node(), "lxc", vmr.VmId())) - d.Set("target_node", vmr.Node()) + d.Set(node.RootNode, vmr.Node()) // Read Features defaultFeatures := d.Get("features").(*schema.Set) diff --git a/proxmox/resource_vm_qemu.go b/proxmox/resource_vm_qemu.go index 4528c130..76edfdab 100755 --- a/proxmox/resource_vm_qemu.go +++ b/proxmox/resource_vm_qemu.go @@ -28,6 +28,7 @@ import ( "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/pxapi/dns/nameservers" "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/pxapi/guest/sshkeys" "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/pxapi/guest/tags" + "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/resource/guest/node" "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/resource/guest/qemu/cpu" "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/resource/guest/qemu/disk" "github.com/Telmate/terraform-provider-proxmox/v2/proxmox/Internal/resource/guest/qemu/network" @@ -146,19 +147,9 @@ func resourceVmQemu() *schema.Resource { Default: defaultDescription, Description: "The VM description", }, - "target_node": { - Type: schema.TypeString, - Optional: true, - Description: "The node where VM goes to", - }, - "target_nodes": { - Type: schema.TypeList, - Optional: true, - Elem: &schema.Schema{ - Type: schema.TypeString, - }, - Description: "A list of nodes where VM goes to", - }, + node.RootNode: node.SchemaNode(schema.Schema{ + Optional: true}, "qemu"), + node.RootNodes: node.SchemaNodes(), "bios": { Type: schema.TypeString, Optional: true, @@ -658,7 +649,7 @@ func resourceVmQemu() *schema.Resource { return thisResource } -func getSourceVmr(ctx context.Context, client *pxapi.Client, name string, id int, targetNode string) (*pxapi.VmRef, error) { +func getSourceVmr(ctx context.Context, client *pxapi.Client, name string, id int, targetNode pxapi.NodeName) (*pxapi.VmRef, error) { if name != "" { sourceVmrs, err := client.GetVmRefsByName(ctx, name) if err != nil { @@ -762,27 +753,27 @@ func resourceVmQemuCreate(ctx context.Context, d *schema.ResourceData, meta inte forceCreate := d.Get("force_create").(bool) - targetNodesRaw := d.Get("target_nodes").([]interface{}) + targetNodesRaw := d.Get(node.RootNodes).([]interface{}) var targetNodes = make([]string, len(targetNodesRaw)) for i, raw := range targetNodesRaw { targetNodes[i] = raw.(string) } - var targetNode string + var targetNode pxapi.NodeName if len(targetNodes) == 0 { - targetNode = d.Get("target_node").(string) + targetNode = pxapi.NodeName(d.Get(node.RootNode).(string)) } else { - targetNode = targetNodes[rand.Intn(len(targetNodes))] + targetNode = pxapi.NodeName(targetNodes[rand.Intn(len(targetNodes))]) } if targetNode == "" { - return diag.FromErr(fmt.Errorf("VM name (%s) has no target node! Please use target_node or target_nodes to set a specific node! %v", vmName, targetNodes)) + return diag.FromErr(fmt.Errorf("VM name (%s) has no target node! Please use "+node.RootNode+" or "+node.RootNodes+" to set a specific node! %v", vmName, targetNodes)) } if dupVmr != nil && forceCreate { return diag.FromErr(fmt.Errorf("duplicate VM name (%s) with vmId: %d. Set force_create=false to recycle", vmName, dupVmr.VmId())) } else if dupVmr != nil && dupVmr.Node() != targetNode { - return diag.FromErr(fmt.Errorf("duplicate VM name (%s) with vmId: %d on different target_node=%s", vmName, dupVmr.VmId(), dupVmr.Node())) + return diag.FromErr(fmt.Errorf("duplicate VM name (%s) with vmId: %d on different %s=%s", vmName, dupVmr.VmId(), node.RootNodes, dupVmr.Node())) } vmr := dupVmr @@ -803,7 +794,7 @@ func resourceVmQemuCreate(ctx context.Context, d *schema.ResourceData, meta inte } vmr = pxapi.NewVmRef(nextid) - vmr.SetNode(targetNode) + vmr.SetNode(targetNode.String()) config.Node = targetNode vmr.SetPool(d.Get("pool").(string)) @@ -940,9 +931,9 @@ func resourceVmQemuUpdate(ctx context.Context, d *schema.ResourceData, meta inte qemuVgaList := vga.List() d.Partial(true) - if d.HasChange("target_node") { + if d.HasChange(node.RootNode) { // Update target node when it must be migrated manually. Don't if it has been migrated by the proxmox high availability system. - vmr.SetNode(d.Get("target_node").(string)) + vmr.SetNode(d.Get(node.RootNode).(string)) } d.Partial(false) @@ -1179,11 +1170,11 @@ func resourceVmQemuRead(ctx context.Context, d *schema.ResourceData, meta interf // by calling a SetId("") // loop through all virtual servers...? - var targetNodeVMR string = "" - targetNodesRaw := d.Get("target_nodes").([]interface{}) - var targetNodes = make([]string, len(targetNodesRaw)) - for i, raw := range targetNodesRaw { - targetNodes[i] = raw.(string) + var targetNodeVMR pxapi.NodeName + targetNodesRaw := d.Get(node.RootNodes).([]interface{}) + targetNodes := make([]pxapi.NodeName, len(targetNodesRaw)) + for i := range targetNodesRaw { + targetNodes[i] = pxapi.NodeName(targetNodesRaw[i].(string)) } if len(targetNodes) == 0 { @@ -1196,14 +1187,14 @@ func resourceVmQemuRead(ctx context.Context, d *schema.ResourceData, meta interf targetNodeVMR = vmr.Node() } else { for _, targetNode := range targetNodes { - vmr.SetNode(targetNode) + vmr.SetNode(targetNode.String()) _, err = client.GetVmInfo(ctx, vmr) if err != nil { d.SetId("") } d.SetId(resourceId(vmr.Node(), "qemu", vmr.VmId())) - logger.Debug().Any("Setting node id to", d.Get(vmr.Node())) + logger.Debug().Any("Setting node id to", d.Get(vmr.Node().String())) targetNodeVMR = targetNode } }