Merge pull request #843 from Juniper/task/842-integer-value-validatio…

…n-by-type Introduce type-specific integer value validation to `apstra_freeform_resource`
Juniper · Aug 30, 2024 · 0c1dcab · 0c1dcab
2 parents f15daa3 + 68511a3
commit 0c1dcab
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 22 deletions.
diff --git a/apstra/blueprint/datacenter_routing_zone.go b/apstra/blueprint/datacenter_routing_zone.go
@@ -3,11 +3,13 @@ package blueprint
 import (
 	"context"
 	"fmt"
+	"net"
+	"regexp"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	apstravalidator "github.com/Juniper/terraform-provider-apstra/apstra/apstra_validator"
 	"github.com/Juniper/terraform-provider-apstra/apstra/constants"
 	"github.com/Juniper/terraform-provider-apstra/apstra/design"
-	"github.com/Juniper/terraform-provider-apstra/apstra/resources"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
 	"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/setvalidator"
@@ -20,8 +22,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
-	"net"
-	"regexp"
 )
 
 var junosEvpnIrbModeDefault = apstra.JunosEvpnIrbModeAsymmetric.Value
@@ -236,7 +236,7 @@ func (o DatacenterRoutingZone) ResourceAttributes() map[string]resourceSchema.At
 				"automatically assigned from an allocated resource pool, or enter a specific value.",
 			Optional:   true,
 			Computed:   true,
-			Validators: []validator.Int64{int64validator.Between(resources.VniMin, resources.VniMax)},
+			Validators: []validator.Int64{int64validator.Between(constants.VniMin, constants.VniMax)},
 		},
 		"had_prior_vni_config": resourceSchema.BoolAttribute{
 			MarkdownDescription: "Used to trigger plan modification when `vni` has been removed from the " +

diff --git a/apstra/blueprint/datacenter_virtual_network.go b/apstra/blueprint/datacenter_virtual_network.go
@@ -3,12 +3,15 @@ package blueprint
 import (
 	"context"
 	"fmt"
+	"net"
+	"regexp"
+	"strconv"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	apiversions "github.com/Juniper/terraform-provider-apstra/apstra/api_versions"
 	apstravalidator "github.com/Juniper/terraform-provider-apstra/apstra/apstra_validator"
 	"github.com/Juniper/terraform-provider-apstra/apstra/constants"
 	"github.com/Juniper/terraform-provider-apstra/apstra/design"
-	"github.com/Juniper/terraform-provider-apstra/apstra/resources"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
 	"github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform-plugin-framework-validators/helpers/validatordiag"
@@ -26,9 +29,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
-	"net"
-	"regexp"
-	"strconv"
 )
 
 type DatacenterVirtualNetwork struct {
@@ -340,7 +340,7 @@ func (o DatacenterVirtualNetwork) ResourceAttributes() map[string]resourceSchema
 			Optional: true,
 			Computed: true,
 			Validators: []validator.Int64{
-				int64validator.Between(resources.VniMin, resources.VniMax),
+				int64validator.Between(constants.VniMin, constants.VniMax),
 				apstravalidator.ForbiddenWhenValueIs(
 					path.MatchRelative().AtParent().AtName("type"),
 					types.StringValue(apstra.VnTypeVlan.String()),
@@ -746,7 +746,7 @@ func (o *DatacenterVirtualNetwork) Query(resultName string) apstra.QEQuery {
 	}
 
 	// not handling ipv6 subnet as a string match because of '::' expansion weirdness
-	//if !o.IPv6Subnet.IsNull() { nope! }
+	// if !o.IPv6Subnet.IsNull() { nope! }
 
 	if !o.IPv4GatewayEnabled.IsNull() {
 		nodeAttributes = append(nodeAttributes, apstra.QEEAttribute{
@@ -777,7 +777,7 @@ func (o *DatacenterVirtualNetwork) Query(resultName string) apstra.QEQuery {
 	}
 
 	// not handling ipv6 gateway as a string match because of '::' expansion weirdness
-	//if !o.IPv6Gateway.IsNull() { nope! }
+	// if !o.IPv6Gateway.IsNull() { nope! }
 
 	// Begin the query with the VN node
 	vnQuery := new(apstra.MatchQuery).Match(new(apstra.PathQuery).Node(nodeAttributes))

diff --git a/apstra/constants/numeric.go b/apstra/constants/numeric.go
@@ -26,4 +26,7 @@ const (
 
 	VlanMinUsable = 2
 	VlanMaxUsable = 4094
+
+	VniMin = 4096
+	VniMax = 16777214
 )
diff --git a/apstra/resource_freeform_resource.go b/apstra/resource_freeform_resource.go
@@ -3,8 +3,10 @@ package tfapstra
 import (
 	"context"
 	"fmt"
+	"math"
 
 	"github.com/Juniper/apstra-go-sdk/apstra"
+	"github.com/Juniper/terraform-provider-apstra/apstra/constants"
 	"github.com/Juniper/terraform-provider-apstra/apstra/freeform"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
 	"github.com/hashicorp/terraform-plugin-framework/path"
@@ -57,17 +59,66 @@ func (o *resourceFreeformResource) ValidateConfig(ctx context.Context, req resou
 
 	// Reminder Logic: the unknown state in this function means a value was passed by reference
 	switch resourceType {
-	case apstra.FFResourceTypeAsn,
-		apstra.FFResourceTypeVni,
-		apstra.FFResourceTypeVlan,
-		apstra.FFResourceTypeInt:
+	case apstra.FFResourceTypeAsn:
 		if (config.AllocatedFrom.IsNull() && config.IntValue.IsNull()) ||
 			(!config.AllocatedFrom.IsNull() && !config.IntValue.IsNull()) {
 			resp.Diagnostics.AddError(
 				errInvalidConfig,
 				"Exactly one of `allocated_from` and `integer_value` must be set when `type` is set to "+config.Type.String(),
 			)
 		}
+		if !config.IntValue.IsNull() && (config.IntValue.ValueInt64() < constants.AsnMin || config.IntValue.ValueInt64() > constants.AsnMax) {
+			resp.Diagnostics.AddAttributeError(
+				path.Root("integer_value"),
+				errInvalidConfig,
+				fmt.Sprintf("When type is %s, value must be between %d and %d, got %s", config.Type, constants.AsnMin, constants.AsnMax, config.IntValue.String()),
+			)
+		}
+	case apstra.FFResourceTypeVni:
+		if (config.AllocatedFrom.IsNull() && config.IntValue.IsNull()) ||
+			(!config.AllocatedFrom.IsNull() && !config.IntValue.IsNull()) {
+			resp.Diagnostics.AddError(
+				errInvalidConfig,
+				"Exactly one of `allocated_from` and `integer_value` must be set when `type` is set to "+config.Type.String(),
+			)
+		}
+		if !config.IntValue.IsNull() && (config.IntValue.ValueInt64() < constants.VniMin || config.IntValue.ValueInt64() > constants.VniMax) {
+			resp.Diagnostics.AddAttributeError(
+				path.Root("integer_value"),
+				errInvalidConfig,
+				fmt.Sprintf("When type is %s, value must be between %d and %d, got %s", config.Type, constants.VniMin, constants.VniMax, config.IntValue.String()),
+			)
+		}
+	case apstra.FFResourceTypeVlan:
+		if (config.AllocatedFrom.IsNull() && config.IntValue.IsNull()) ||
+			(!config.AllocatedFrom.IsNull() && !config.IntValue.IsNull()) {
+			resp.Diagnostics.AddError(
+				errInvalidConfig,
+				"Exactly one of `allocated_from` and `integer_value` must be set when `type` is set to "+config.Type.String(),
+			)
+		}
+		if !config.IntValue.IsNull() && (config.IntValue.ValueInt64() < constants.VlanMinUsable || config.IntValue.ValueInt64() > constants.VlanMaxUsable) {
+			resp.Diagnostics.AddAttributeError(
+				path.Root("integer_value"),
+				errInvalidConfig,
+				fmt.Sprintf("When type is %s, value must be between %d and %d, got %s", config.Type, constants.VlanMinUsable, constants.VlanMaxUsable, config.IntValue.String()),
+			)
+		}
+	case apstra.FFResourceTypeInt:
+		if (config.AllocatedFrom.IsNull() && config.IntValue.IsNull()) ||
+			(!config.AllocatedFrom.IsNull() && !config.IntValue.IsNull()) {
+			resp.Diagnostics.AddError(
+				errInvalidConfig,
+				"Exactly one of `allocated_from` and `integer_value` must be set when `type` is set to "+config.Type.String(),
+			)
+		}
+		if !config.IntValue.IsNull() && (config.IntValue.ValueInt64() < 1 || config.IntValue.ValueInt64() > math.MaxUint32) {
+			resp.Diagnostics.AddAttributeError(
+				path.Root("integer_value"),
+				errInvalidConfig,
+				fmt.Sprintf("When type is %s, value must be between %d and %d, got %s", config.Type, 1, math.MaxUint32, config.IntValue.String()),
+			)
+		}
 	case apstra.FFResourceTypeHostIpv4:
 		if (config.AllocatedFrom.IsNull() && config.Ipv4Value.IsNull()) ||
 			(!config.AllocatedFrom.IsNull() && !config.Ipv4Value.IsNull()) {
@@ -104,6 +155,13 @@ func (o *resourceFreeformResource) ValidateConfig(ctx context.Context, req resou
 				"`integer_value` is used to indicate the Subnet Prefix Length. It must be set when `allocated_from` is set and `type` is set to "+config.Type.String(),
 			)
 		}
+		if !config.IntValue.IsNull() && (config.IntValue.ValueInt64() < 1 || config.IntValue.ValueInt64() > 32) {
+			resp.Diagnostics.AddAttributeError(
+				path.Root("integer_value"),
+				errInvalidConfig,
+				fmt.Sprintf("When type is %s, value must be between %d and %d, got %s", config.Type, 1, 32, config.IntValue.String()),
+			)
+		}
 	case apstra.FFResourceTypeIpv6:
 		if (config.AllocatedFrom.IsNull() && config.Ipv6Value.IsNull()) ||
 			(!config.AllocatedFrom.IsNull() && !config.Ipv6Value.IsNull()) {
@@ -124,6 +182,13 @@ func (o *resourceFreeformResource) ValidateConfig(ctx context.Context, req resou
 				"`integer_value` is used to indicate the Subnet Prefix Length. It must be set when `allocated_from` is set and `type` is set to "+config.Type.String(),
 			)
 		}
+		if !config.IntValue.IsNull() && (config.IntValue.ValueInt64() < 1 || config.IntValue.ValueInt64() > 128) {
+			resp.Diagnostics.AddAttributeError(
+				path.Root("integer_value"),
+				errInvalidConfig,
+				fmt.Sprintf("When type is %s, value must be between %d and %d, got %s", config.Type, 1, 128, config.IntValue.String()),
+			)
+		}
 	}
 }
 

diff --git a/apstra/resources/vni_pool_range.go b/apstra/resources/vni_pool_range.go
@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/Juniper/apstra-go-sdk/apstra"
+	"github.com/Juniper/terraform-provider-apstra/apstra/constants"
 	"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
 	"github.com/hashicorp/terraform-plugin-framework/attr"
 	dataSourceSchema "github.com/hashicorp/terraform-plugin-framework/datasource/schema"
@@ -14,11 +15,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 )
 
-const (
-	VniMin = 4096
-	VniMax = 16777214
-)
-
 type VniPoolRange struct {
 	Status         types.String  `tfsdk:"status"`
 	First          types.Int64   `tfsdk:"first"`
@@ -72,12 +68,12 @@ func (o VniPoolRange) ResourceAttributes() map[string]resourceSchema.Attribute {
 	return map[string]resourceSchema.Attribute{
 		"first": resourceSchema.Int64Attribute{
 			Required:   true,
-			Validators: []validator.Int64{int64validator.Between(VniMin, VniMax)},
+			Validators: []validator.Int64{int64validator.Between(constants.VniMin, constants.VniMax)},
 		},
 		"last": resourceSchema.Int64Attribute{
 			Required: true,
 			Validators: []validator.Int64{
-				int64validator.Between(VniMin, VniMax),
+				int64validator.Between(constants.VniMin, constants.VniMax),
 				int64validator.AtLeastSumOf(path.MatchRelative().AtParent().AtName("first")),
 			},
 		},