feat: validate node name (#1217)

* feat: validate node name * fix: duplicate imports
Telmate · Dec 30, 2024 · 21d0e9e · 21d0e9e
1 parent 7c91155
commit 21d0e9e
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 43 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
 		}
 	}