fix(firewall): add VM / container ID validation to firewall rules (#424)

bpg · Jul 12, 2023 · 6a3bc03 · 6a3bc03
1 parent 041c71e
commit 6a3bc03
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 35 deletions.
diff --git a/proxmoxtf/resource/container.go b/proxmoxtf/resource/container.go
@@ -20,6 +20,7 @@ import (
 	"github.com/bpg/terraform-provider-proxmox/internal/types"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/containers"
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
+	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
 )
 
 const (
@@ -149,7 +150,7 @@ func Container() *schema.Resource {
 							Description:      "The ID of the source container",
 							Required:         true,
 							ForceNew:         true,
-							ValidateDiagFunc: getVMIDValidator(),
+							ValidateDiagFunc: validator.VMID(),
 						},
 					},
 				},
@@ -636,7 +637,7 @@ func Container() *schema.Resource {
 				Optional:         true,
 				ForceNew:         true,
 				Default:          dvResourceVirtualEnvironmentContainerVMID,
-				ValidateDiagFunc: getVMIDValidator(),
+				ValidateDiagFunc: validator.VMID(),
 			},
 		},
 		CreateContext: containerCreate,

diff --git a/proxmoxtf/resource/firewall/selector.go b/proxmoxtf/resource/firewall/selector.go
@@ -14,6 +14,7 @@ import (
 
 	"github.com/bpg/terraform-provider-proxmox/proxmox/firewall"
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
+	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
 )
 
 const (
@@ -30,16 +31,18 @@ func selectorSchema() map[string]*schema.Schema {
 			Description: "The name of the node.",
 		},
 		mkSelectorVMID: {
-			Type:         schema.TypeInt,
-			Optional:     true,
-			Description:  "The ID of the VM to manage the firewall for.",
-			RequiredWith: []string{mkSelectorNodeName},
+			Type:             schema.TypeInt,
+			Optional:         true,
+			Description:      "The ID of the VM to manage the firewall for.",
+			RequiredWith:     []string{mkSelectorNodeName},
+			ValidateDiagFunc: validator.VMID(),
 		},
 		mkSelectorContainerID: {
-			Type:         schema.TypeInt,
-			Optional:     true,
-			Description:  "The ID of the container to manage the firewall for.",
-			RequiredWith: []string{mkSelectorNodeName},
+			Type:             schema.TypeInt,
+			Optional:         true,
+			Description:      "The ID of the container to manage the firewall for.",
+			RequiredWith:     []string{mkSelectorNodeName},
+			ValidateDiagFunc: validator.VMID(),
 		},
 	}
 }

diff --git a/proxmoxtf/resource/utils.go b/proxmoxtf/resource/utils.go
@@ -400,29 +400,6 @@ func getSCSIHardwareValidator() schema.SchemaValidateDiagFunc {
 	}, false))
 }
 
-func getVMIDValidator() schema.SchemaValidateDiagFunc {
-	return validation.ToDiagFunc(func(i interface{}, k string) (ws []string, es []error) {
-		min := 100
-		max := 2147483647
-
-		v, ok := i.(int)
-
-		if !ok {
-			es = append(es, fmt.Errorf("expected type of %s to be int", k))
-			return
-		}
-
-		if v != -1 {
-			if v < min || v > max {
-				es = append(es, fmt.Errorf("expected %s to be in the range (%d - %d), got %d", k, min, max, v))
-				return
-			}
-		}
-
-		return
-	})
-}
-
 // suppressIfListsAreEqualIgnoringOrder is a customdiff.SuppressionFunc that suppresses
 // changes to a list if the old and new lists are equal, ignoring the order of the
 // elements.

diff --git a/proxmoxtf/resource/validator/vm.go b/proxmoxtf/resource/validator/vm.go
@@ -0,0 +1,41 @@
+/*
+ * 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 validator
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
+)
+
+// VMID returns a schema validation function for a VM ID.
+func VMID() schema.SchemaValidateDiagFunc {
+	return validation.ToDiagFunc(func(i interface{}, k string) ([]string, []error) {
+		min := 100
+		max := 2147483647
+
+		var ws []string
+		var es []error
+
+		v, ok := i.(int)
+
+		if !ok {
+			es = append(es, fmt.Errorf("expected type of %s to be int", k))
+			return ws, es
+		}
+
+		if v != -1 {
+			if v < min || v > max {
+				es = append(es, fmt.Errorf("expected %s to be in the range (%d - %d), got %d", k, min, max, v))
+				return ws, es
+			}
+		}
+
+		return ws, es
+	})
+}
diff --git a/proxmoxtf/resource/vm.go b/proxmoxtf/resource/vm.go
@@ -24,6 +24,7 @@ import (
 	"github.com/bpg/terraform-provider-proxmox/proxmox/cluster"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/vms"
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
+	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
 )
 
 const (
@@ -436,7 +437,7 @@ func VM() *schema.Resource {
 							Description:      "The ID of the source VM",
 							Required:         true,
 							ForceNew:         true,
-							ValidateDiagFunc: getVMIDValidator(),
+							ValidateDiagFunc: validator.VMID(),
 						},
 						mkResourceVirtualEnvironmentVMCloneFull: {
 							Type:        schema.TypeBool,
@@ -1297,7 +1298,7 @@ func VM() *schema.Resource {
 				Computed:    true,
 				// "ForceNew: true" handled in CustomizeDiff, making sure VMs with legacy configs with vm_id = -1
 				// do not require re-creation.
-				ValidateDiagFunc: getVMIDValidator(),
+				ValidateDiagFunc: validator.VMID(),
 			},
 			mkResourceVirtualEnvironmentVMSCSIHardware: {
 				Type:             schema.TypeString,