Merge pull request #297 from Juniper/bug/295

Eliminate string match when comparing subnets in IPv4 and IPv6 address pools

apstra/resource_ipv4_pool.go

@@ -7,7 +7,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
-	"github.com/hashicorp/terraform-plugin-framework/types"
 	"net"
 	"terraform-provider-apstra/apstra/resources"
 	"terraform-provider-apstra/apstra/utils"
@@ -31,7 +30,7 @@ func (o *resourceIpv4Pool) Configure(ctx context.Context, req resource.Configure
 func (o *resourceIpv4Pool) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
 	resp.Schema = schema.Schema{
 		MarkdownDescription: "This resource creates an IPv4 resource pool",
-		Attributes:          resources.Ipv4Pool{}.ResourceAttributesWrite(),
+		Attributes:          resources.Ipv4Pool{}.ResourceAttributes(),
 	}
 }
 
@@ -53,46 +52,26 @@ func (o *resourceIpv4Pool) ValidateConfig(ctx context.Context, req resource.Vali
 		return
 	}
 
-	var jNets []*net.IPNet // Each subnet will be checked for overlap with members of jNets, then appended to jNets
+	var jNets []*net.IPNet
+	// Each subnet will be checked for overlap with members of jNets
+	// (j is inner loop iterator variable), then appended to jNets
 	for i := range subnets {
-		// setVal is used to path AttributeErrors correctly
-		setVal, d := types.ObjectValueFrom(ctx, resources.Ipv4PoolSubnet{}.AttrTypes(), &subnets[i])
-		resp.Diagnostics.Append(d...)
-		if resp.Diagnostics.HasError() {
-			return
-		}
-
+		// skip unknown values
 		if subnets[i].Network.IsUnknown() {
 			continue
 		}
 
-		// parse the subnet string
-		_, iNet, err := net.ParseCIDR(subnets[i].Network.ValueString())
-		if err != nil {
-			resp.Diagnostics.AddAttributeError(
-				path.Root("subnets").AtSetValue(setVal),
-				"failure parsing CIDR notation", fmt.Sprintf("error parsing %q - %s", subnets[i], err.Error()))
-			return
-		}
-
-		// insist the user give us the all-zeros host address: 192.168.1.0/24 not 192.168.1.50/24
-		if iNet.String() != subnets[i].Network.ValueString() {
-			resp.Diagnostics.AddAttributeError(
-				path.Root("subnets").AtSetValue(setVal),
-				errInvalidConfig,
-				fmt.Sprintf("%q doesn't specify a network base address. Did you mean %q?",
-					subnets[i].Network.ValueString(), iNet.String()),
-			)
-		}
+		// parse the subnet string; error ignored because it's already been validated
+		_, iNet, _ := net.ParseCIDR(subnets[i].Network.ValueString())
 
 		// check for overlaps with previous subnets
-		for j := range jNets {
-			if iNet.Contains(jNets[j].IP) || jNets[j].Contains(iNet.IP) {
+		for _, jNet := range jNets {
+			if iNet.Contains(jNet.IP) || jNet.Contains(iNet.IP) {
+				// no return so we catch all overlap errors
 				resp.Diagnostics.AddAttributeError(
 					path.Root("subnets"),
 					"pool has overlapping subnets",
-					fmt.Sprintf("subnets %q and %q overlap", iNet.String(), jNets[j].String()))
-				return
+					fmt.Sprintf("subnets %q and %q overlap", iNet.String(), jNet.String()))
 			}
 		}
 

apstra/resource_ipv6_pool.go

@@ -7,7 +7,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
-	"github.com/hashicorp/terraform-plugin-framework/types"
 	"net"
 	"terraform-provider-apstra/apstra/resources"
 	"terraform-provider-apstra/apstra/utils"
@@ -31,7 +30,7 @@ func (o *resourceIpv6Pool) Configure(ctx context.Context, req resource.Configure
 func (o *resourceIpv6Pool) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
 	resp.Schema = schema.Schema{
 		MarkdownDescription: "This resource creates an IPv6 resource pool",
-		Attributes:          resources.Ipv6Pool{}.ResourceAttributesWrite(),
+		Attributes:          resources.Ipv6Pool{}.ResourceAttributes(),
 	}
 }
 
@@ -53,46 +52,26 @@ func (o *resourceIpv6Pool) ValidateConfig(ctx context.Context, req resource.Vali
 		return
 	}
 
-	var jNets []*net.IPNet // Each subnet will be checked for overlap with members of jNets, then appended to jNets
+	var jNets []*net.IPNet
+	// Each subnet will be checked for overlap with members of jNets
+	// (j is inner loop iterator variable), then appended to jNets
 	for i := range subnets {
-		// setVal is used to path AttributeErrors correctly
-		setVal, d := types.ObjectValueFrom(ctx, resources.Ipv6PoolSubnet{}.AttrTypes(), &subnets[i])
-		resp.Diagnostics.Append(d...)
-		if resp.Diagnostics.HasError() {
-			return
-		}
-
+		// skip unknown values
 		if subnets[i].Network.IsUnknown() {
 			continue
 		}
 
-		// parse the subnet string
-		_, iNet, err := net.ParseCIDR(subnets[i].Network.ValueString())
-		if err != nil {
-			resp.Diagnostics.AddAttributeError(
-				path.Root("subnets").AtSetValue(setVal),
-				"failure parsing CIDR notation", fmt.Sprintf("error parsing %q - %s", subnets[i], err.Error()))
-			return
-		}
-
-		// insist the user give us the all-zeros host address: 192.168.1.0/24 not 192.168.1.50/24
-		if iNet.String() != subnets[i].Network.ValueString() {
-			resp.Diagnostics.AddAttributeError(
-				path.Root("subnets").AtSetValue(setVal),
-				errInvalidConfig,
-				fmt.Sprintf("%q doesn't specify a network base address. Did you mean %q?",
-					subnets[i].Network.ValueString(), iNet.String()),
-			)
-		}
+		// parse the subnet string; error ignored because it's already been validated
+		_, iNet, _ := net.ParseCIDR(subnets[i].Network.ValueString())
 
 		// check for overlaps with previous subnets
-		for j := range jNets {
-			if iNet.Contains(jNets[j].IP) || jNets[j].Contains(iNet.IP) {
+		for _, jNet := range jNets {
+			if iNet.Contains(jNet.IP) || jNet.Contains(iNet.IP) {
+				// no return so we catch all overlap errors
 				resp.Diagnostics.AddAttributeError(
 					path.Root("subnets"),
 					"pool has overlapping subnets",
-					fmt.Sprintf("subnets %q and %q overlap", iNet.String(), jNets[j].String()))
-				return
+					fmt.Sprintf("subnets %q and %q overlap", iNet.String(), jNet.String()))
 			}
 		}
 

apstra/resources/ipv4_pool.go

@@ -73,7 +73,7 @@ func (o Ipv4Pool) DataSourceAttributes() map[string]dataSourceSchema.Attribute {
 	}
 }
 
-func (o Ipv4Pool) ResourceAttributesWrite() map[string]resourceSchema.Attribute {
+func (o Ipv4Pool) ResourceAttributes() map[string]resourceSchema.Attribute {
 	return map[string]resourceSchema.Attribute{
 		"id": resourceSchema.StringAttribute{
 			MarkdownDescription: "Apstra ID number of the pool",
@@ -90,7 +90,7 @@ func (o Ipv4Pool) ResourceAttributesWrite() map[string]resourceSchema.Attribute
 			Required:            true,
 			Validators:          []validator.Set{setvalidator.SizeAtLeast(1)},
 			NestedObject: resourceSchema.NestedAttributeObject{
-				Attributes: Ipv4PoolSubnet{}.ResourceAttributesWrite(),
+				Attributes: Ipv4PoolSubnet{}.ResourceAttributes(),
 			},
 		},
 		"total": resourceSchema.NumberAttribute{

apstra/resources/ipv4_pool_subnet.go

@@ -3,20 +3,23 @@ package resources
 import (
 	"context"
 	"github.com/Juniper/apstra-go-sdk/apstra"
+	"github.com/hashicorp/terraform-plugin-framework-nettypes/cidrtypes"
 	"github.com/hashicorp/terraform-plugin-framework/attr"
 	dataSourceSchema "github.com/hashicorp/terraform-plugin-framework/datasource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	resourceSchema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
+	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
+	apstravalidator "terraform-provider-apstra/apstra/apstra_validator"
 	"terraform-provider-apstra/apstra/utils"
 )
 
 type Ipv4PoolSubnet struct {
-	Status         types.String  `tfsdk:"status"`
-	Network        types.String  `tfsdk:"network"`
-	Total          types.Number  `tfsdk:"total"`
-	Used           types.Number  `tfsdk:"used"`
-	UsedPercentage types.Float64 `tfsdk:"used_percentage"`
+	Status         types.String         `tfsdk:"status"`
+	Network        cidrtypes.IPv4Prefix `tfsdk:"network"`
+	Total          types.Number         `tfsdk:"total"`
+	Used           types.Number         `tfsdk:"used"`
+	UsedPercentage types.Float64        `tfsdk:"used_percentage"`
 }
 
 func (o Ipv4PoolSubnet) DataSourceAttributes() map[string]dataSourceSchema.Attribute {
@@ -27,6 +30,7 @@ func (o Ipv4PoolSubnet) DataSourceAttributes() map[string]dataSourceSchema.Attri
 		},
 		"network": dataSourceSchema.StringAttribute{
 			MarkdownDescription: "Network specification in CIDR syntax (\"10.0.0.0/8\").",
+			CustomType:          cidrtypes.IPv4PrefixType{},
 			Required:            true,
 		},
 		"total": dataSourceSchema.NumberAttribute{
@@ -44,15 +48,18 @@ func (o Ipv4PoolSubnet) DataSourceAttributes() map[string]dataSourceSchema.Attri
 	}
 }
 
-func (o Ipv4PoolSubnet) ResourceAttributesWrite() map[string]resourceSchema.Attribute {
+func (o Ipv4PoolSubnet) ResourceAttributes() map[string]resourceSchema.Attribute {
 	return map[string]resourceSchema.Attribute{
 		"status": resourceSchema.StringAttribute{
 			MarkdownDescription: "Status of the IPv4 resource pool.",
 			Computed:            true,
 		},
 		"network": resourceSchema.StringAttribute{
-			MarkdownDescription: "Network specification in CIDR syntax (\"10.0.0.0/8\").",
+			MarkdownDescription: "Network specification in CIDR syntax (\"192.0.2.0/24\").",
+			CustomType:          cidrtypes.IPv4PrefixType{},
 			Required:            true,
+			Validators:          []validator.String{apstravalidator.ParseCidr(true, false)},
+			// ParseCidr is still required because the IPv4PrefixType doesn't enforce the zero address.
 		},
 		"total": resourceSchema.NumberAttribute{
 			MarkdownDescription: "Total number of addresses in this IPv4 range.",
@@ -81,7 +88,7 @@ func (o Ipv4PoolSubnet) AttrTypes() map[string]attr.Type {
 
 func (o *Ipv4PoolSubnet) LoadApiData(_ context.Context, in *apstra.IpSubnet, _ *diag.Diagnostics) {
 	o.Status = types.StringValue(in.Status)
-	o.Network = types.StringValue(in.Network.String())
+	o.Network = cidrtypes.NewIPv4PrefixValue(in.Network.String())
 	o.Total = types.NumberValue(utils.BigIntToBigFloat(&in.Total))
 	o.Used = types.NumberValue(utils.BigIntToBigFloat(&in.Used))
 	o.UsedPercentage = types.Float64Value(float64(in.UsedPercentage))

apstra/resources/ipv6_pool.go

@@ -73,7 +73,7 @@ func (o Ipv6Pool) DataSourceAttributes() map[string]dataSourceSchema.Attribute {
 	}
 }
 
-func (o Ipv6Pool) ResourceAttributesWrite() map[string]resourceSchema.Attribute {
+func (o Ipv6Pool) ResourceAttributes() map[string]resourceSchema.Attribute {
 	return map[string]resourceSchema.Attribute{
 		"id": resourceSchema.StringAttribute{
 			MarkdownDescription: "Apstra ID number of the pool",
@@ -90,7 +90,7 @@ func (o Ipv6Pool) ResourceAttributesWrite() map[string]resourceSchema.Attribute
 			Required:            true,
 			Validators:          []validator.Set{setvalidator.SizeAtLeast(1)},
 			NestedObject: resourceSchema.NestedAttributeObject{
-				Attributes: Ipv6PoolSubnet{}.ResourceAttributesWrite(),
+				Attributes: Ipv6PoolSubnet{}.ResourceAttributes(),
 			},
 		},
 		"total": resourceSchema.NumberAttribute{

apstra/resources/ipv6_pool_subnet.go

@@ -3,20 +3,23 @@ package resources
 import (
 	"context"
 	"github.com/Juniper/apstra-go-sdk/apstra"
+	"github.com/hashicorp/terraform-plugin-framework-nettypes/cidrtypes"
 	"github.com/hashicorp/terraform-plugin-framework/attr"
 	dataSourceSchema "github.com/hashicorp/terraform-plugin-framework/datasource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	resourceSchema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
+	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
+	apstravalidator "terraform-provider-apstra/apstra/apstra_validator"
 	"terraform-provider-apstra/apstra/utils"
 )
 
 type Ipv6PoolSubnet struct {
-	Status         types.String  `tfsdk:"status"`
-	Network        types.String  `tfsdk:"network"`
-	Total          types.Number  `tfsdk:"total"`
-	Used           types.Number  `tfsdk:"used"`
-	UsedPercentage types.Float64 `tfsdk:"used_percentage"`
+	Status         types.String         `tfsdk:"status"`
+	Network        cidrtypes.IPv6Prefix `tfsdk:"network"`
+	Total          types.Number         `tfsdk:"total"`
+	Used           types.Number         `tfsdk:"used"`
+	UsedPercentage types.Float64        `tfsdk:"used_percentage"`
 }
 
 func (o Ipv6PoolSubnet) DataSourceAttributes() map[string]dataSourceSchema.Attribute {
@@ -27,6 +30,7 @@ func (o Ipv6PoolSubnet) DataSourceAttributes() map[string]dataSourceSchema.Attri
 		},
 		"network": dataSourceSchema.StringAttribute{
 			MarkdownDescription: "Network specification in CIDR syntax (\"2001:db8::/32\").",
+			CustomType:          cidrtypes.IPv6PrefixType{},
 			Required:            true,
 		},
 		"total": dataSourceSchema.NumberAttribute{
@@ -44,15 +48,18 @@ func (o Ipv6PoolSubnet) DataSourceAttributes() map[string]dataSourceSchema.Attri
 	}
 }
 
-func (o Ipv6PoolSubnet) ResourceAttributesWrite() map[string]resourceSchema.Attribute {
+func (o Ipv6PoolSubnet) ResourceAttributes() map[string]resourceSchema.Attribute {
 	return map[string]resourceSchema.Attribute{
 		"status": resourceSchema.StringAttribute{
 			MarkdownDescription: "Status of the IPv6 resource pool.",
 			Computed:            true,
 		},
 		"network": resourceSchema.StringAttribute{
-			MarkdownDescription: "Network specification in CIDR syntax (\"10.0.0.0/8\").",
+			MarkdownDescription: "Network specification in CIDR syntax (\"2001:db8::/64\").",
+			CustomType:          cidrtypes.IPv6PrefixType{},
 			Required:            true,
+			Validators:          []validator.String{apstravalidator.ParseCidr(false, true)},
+			// ParseCidr is still required because the IPv6PrefixType doesn't enforce the zero address.
 		},
 		"total": resourceSchema.NumberAttribute{
 			MarkdownDescription: "Total number of addresses in this IPv6 range.",
@@ -81,7 +88,7 @@ func (o Ipv6PoolSubnet) AttrTypes() map[string]attr.Type {
 
 func (o *Ipv6PoolSubnet) LoadApiData(_ context.Context, in *apstra.IpSubnet, _ *diag.Diagnostics) {
 	o.Status = types.StringValue(in.Status)
-	o.Network = types.StringValue(in.Network.String())
+	o.Network = cidrtypes.NewIPv6PrefixValue(in.Network.String())
 	o.Total = types.NumberValue(utils.BigIntToBigFloat(&in.Total))
 	o.Used = types.NumberValue(utils.BigIntToBigFloat(&in.Used))
 	o.UsedPercentage = types.Float64Value(float64(in.UsedPercentage))

docs/resources/ipv4_pool.md

@@ -82,7 +82,7 @@ output "example_pool_size" {
 
 Required:
 
-- `network` (String) Network specification in CIDR syntax ("10.0.0.0/8").
+- `network` (String) Network specification in CIDR syntax ("192.0.2.0/24").
 
 Read-Only:
 

docs/resources/ipv6_pool.md

@@ -82,7 +82,7 @@ locals {
 
 Required:
 
-- `network` (String) Network specification in CIDR syntax ("10.0.0.0/8").
+- `network` (String) Network specification in CIDR syntax ("2001:db8::/64").
 
 Read-Only: