Merge pull request #752 from Juniper/737-wrong-attr-type

bug/#737: Set mutable values `null` in `Read()` method of resource pool resources

apstra/resource_asn_pool.go

@@ -154,6 +154,11 @@ func (o *resourceAsnPool) Read(ctx context.Context, req resource.ReadRequest, re
 		return
 	}
 
+	newState.SetMutablesToNull(ctx, &resp.Diagnostics)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
 	// set state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &newState)...)
 }

apstra/resource_integer_pool.go

@@ -154,6 +154,11 @@ func (o *resourceIntegerPool) Read(ctx context.Context, req resource.ReadRequest
 		return
 	}
 
+	newState.SetMutablesToNull(ctx, &resp.Diagnostics)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
 	// set state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &newState)...)
 }

apstra/resource_ipv4_pool.go

@@ -149,6 +149,11 @@ func (o *resourceIpv4Pool) Read(ctx context.Context, req resource.ReadRequest, r
 		return
 	}
 
+	newState.SetMutablesToNull(ctx, &resp.Diagnostics)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
 	// set state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &newState)...)
 }

apstra/resource_ipv4_pool_integration_test.go

@@ -79,6 +79,9 @@ func (o resourceIpv4Pool) testChecks(t testing.TB, rType, rName string) testChec
 		})
 	}
 
+	// -----------------------------
+	// DATA SOURCE "by_id" checks below here
+	// -----------------------------
 	checks.setPath("data." + rType + "." + rName + "_by_id")
 	var total int
 	for _, subnet := range o.subnets {
@@ -103,6 +106,9 @@ func (o resourceIpv4Pool) testChecks(t testing.TB, rType, rName string) testChec
 	checks.append(t, "TestCheckResourceAttr", "used", "0")
 	checks.append(t, "TestCheckResourceAttr", "used_percentage", "0")
 
+	// -----------------------------
+	// DATA SOURCE "by_name" checks below here
+	// -----------------------------
 	checks.setPath("data." + rType + "." + rName + "_by_name")
 	for _, subnet := range o.subnets {
 		ones, _ := subnet.Mask.Size()
@@ -148,29 +154,29 @@ func TestAccResourceIpv4Pool(t *testing.T) {
 					config: resourceIpv4Pool{
 						name: acctest.RandString(6),
 						subnets: []net.IPNet{
-							randomSlash31(t, "10.0.0.0/24"),
-							randomSlash31(t, "10.0.1.0/24"),
-							randomSlash31(t, "10.0.2.0/24"),
+							randomPrefix(t, "10.0.0.0/16", 24),
+							randomPrefix(t, "10.1.0.0/16", 25),
+							randomPrefix(t, "10.2.0.0/16", 26),
+							randomPrefix(t, "10.3.0.0/16", 27),
 						},
 					},
 				},
 				{
 					config: resourceIpv4Pool{
 						name: acctest.RandString(6),
 						subnets: []net.IPNet{
-							randomSlash31(t, "10.1.0.0/24"),
-							randomSlash31(t, "10.1.1.0/24"),
-							randomSlash31(t, "10.1.2.0/24"),
+							randomPrefix(t, "10.4.0.0/16", 24),
+							randomPrefix(t, "10.5.0.0/16", 25),
 						},
 					},
 				},
 				{
 					config: resourceIpv4Pool{
 						name: acctest.RandString(6),
 						subnets: []net.IPNet{
-							randomSlash31(t, "10.2.0.0/24"),
-							randomSlash31(t, "10.2.1.0/24"),
-							randomSlash31(t, "10.2.2.0/24"),
+							randomPrefix(t, "10.6.0.0/16", 24),
+							randomPrefix(t, "10.7.0.0/16", 25),
+							randomPrefix(t, "10.8.0.0/16", 26),
 						},
 					},
 				},
@@ -196,7 +202,7 @@ func TestAccResourceIpv4Pool(t *testing.T) {
 				chkLog := checks.string()
 				stepName := fmt.Sprintf("test case %q step %d", tName, i+1)
 
-				t.Logf("\n// ------ begin config for %s ------\n%s// -------- end config for %s ------\n\n", stepName, config, stepName)
+				t.Logf("\n// ------ begin config for %s ------%s// -------- end config for %s ------\n\n", stepName, config, stepName)
 				t.Logf("\n// ------ begin checks for %s ------\n%s// -------- end checks for %s ------\n\n", stepName, chkLog, stepName)
 
 				steps[i] = resource.TestStep{

apstra/resource_ipv6_pool.go

@@ -149,6 +149,11 @@ func (o *resourceIpv6Pool) Read(ctx context.Context, req resource.ReadRequest, r
 		return
 	}
 
+	newState.SetMutablesToNull(ctx, &resp.Diagnostics)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
 	// set state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &newState)...)
 }

apstra/resource_ipv6_pool_integration_test.go

@@ -79,10 +79,14 @@ func (o resourceIpv6Pool) testChecks(t testing.TB, rType, rName string) testChec
 		})
 	}
 
+	// -----------------------------
+	// DATA SOURCE "by_id" checks below here
+	// -----------------------------
 	checks.setPath("data." + rType + "." + rName + "_by_id")
 	var total int
 	for _, subnet := range o.subnets {
 		ones, _ := subnet.Mask.Size()
+		// todo: calculation of thisSubnetTotal cannot handle large values. convert to big.Int
 		thisSubnetTotal := 1 << (128 - ones)
 
 		checks.appendSetNestedCheck(t, "subnets.*", map[string]string{
@@ -103,6 +107,9 @@ func (o resourceIpv6Pool) testChecks(t testing.TB, rType, rName string) testChec
 	checks.append(t, "TestCheckResourceAttr", "used", "0")
 	checks.append(t, "TestCheckResourceAttr", "used_percentage", "0")
 
+	// -----------------------------
+	// DATA SOURCE "by_name" checks below here
+	// -----------------------------
 	checks.setPath("data." + rType + "." + rName + "_by_name")
 	for _, subnet := range o.subnets {
 		ones, _ := subnet.Mask.Size()
@@ -148,29 +155,29 @@ func TestAccResourceIpv6Pool(t *testing.T) {
 					config: resourceIpv6Pool{
 						name: acctest.RandString(6),
 						subnets: []net.IPNet{
-							randomSlash127(t, "2001:db8:0::/64"),
-							randomSlash127(t, "2001:db8:0::/64"),
-							randomSlash127(t, "2001:db8:0::/64"),
+							randomPrefix(t, "2001:db8:0::/48", 112),
+							randomPrefix(t, "2001:db8:1::/48", 112),
+							randomPrefix(t, "2001:db8:2::/48", 112),
 						},
 					},
 				},
 				{
 					config: resourceIpv6Pool{
 						name: acctest.RandString(6),
 						subnets: []net.IPNet{
-							randomSlash127(t, "2001:db8:1::/64"),
-							randomSlash127(t, "2001:db8:1::/64"),
-							randomSlash127(t, "2001:db8:1::/64"),
+							randomPrefix(t, "2001:db8:3::/48", 127),
+							randomPrefix(t, "2001:db8:4::/48", 127),
 						},
 					},
 				},
 				{
 					config: resourceIpv6Pool{
 						name: acctest.RandString(6),
 						subnets: []net.IPNet{
-							randomSlash127(t, "2001:db8:2::/64"),
-							randomSlash127(t, "2001:db8:2::/64"),
-							randomSlash127(t, "2001:db8:2::/64"),
+							randomPrefix(t, "2001:db8:5::/48", 112),
+							randomPrefix(t, "2001:db8:6::/48", 112),
+							randomPrefix(t, "2001:db8:7::/48", 112),
+							randomPrefix(t, "2001:db8:8::/48", 112),
 						},
 					},
 				},
@@ -196,7 +203,7 @@ func TestAccResourceIpv6Pool(t *testing.T) {
 				chkLog := checks.string()
 				stepName := fmt.Sprintf("test case %q step %d", tName, i+1)
 
-				t.Logf("\n// ------ begin config for %s ------\n%s// -------- end config for %s ------\n\n", stepName, config, stepName)
+				t.Logf("\n// ------ begin config for %s ------%s// -------- end config for %s ------\n\n", stepName, config, stepName)
 				t.Logf("\n// ------ begin checks for %s ------\n%s// -------- end checks for %s ------\n\n", stepName, chkLog, stepName)
 
 				steps[i] = resource.TestStep{

apstra/resource_vni_pool.go

@@ -154,6 +154,11 @@ func (o *resourceVniPool) Read(ctx context.Context, req resource.ReadRequest, re
 		return
 	}
 
+	newState.SetMutablesToNull(ctx, &resp.Diagnostics)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
 	// set state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &newState)...)
 }

apstra/resources/ipv4_pool_subnet.go

@@ -80,7 +80,7 @@ func (o Ipv4PoolSubnet) ResourceAttributes() map[string]resourceSchema.Attribute
 func (o Ipv4PoolSubnet) AttrTypes() map[string]attr.Type {
 	return map[string]attr.Type{
 		"status":          types.StringType,
-		"network":         types.StringType,
+		"network":         cidrtypes.IPv4PrefixType{},
 		"total":           types.NumberType,
 		"used":            types.NumberType,
 		"used_percentage": types.Float64Type,

apstra/resources/ipv6_pool_subnet.go

@@ -80,7 +80,7 @@ func (o Ipv6PoolSubnet) ResourceAttributes() map[string]resourceSchema.Attribute
 func (o Ipv6PoolSubnet) AttrTypes() map[string]attr.Type {
 	return map[string]attr.Type{
 		"status":          types.StringType,
-		"network":         types.StringType,
+		"network":         cidrtypes.IPv6PrefixType{},
 		"total":           types.NumberType,
 		"used":            types.NumberType,
 		"used_percentage": types.Float64Type,

apstra/test_helpers_test.go

@@ -4,6 +4,7 @@ package tfapstra_test
 
 import (
 	"context"
+	crand "crypto/rand"
 	"encoding/json"
 	"fmt"
 	"math"
@@ -265,6 +266,46 @@ func randomJson(t testing.TB, maxInt int, strLen int, count int) json.RawMessage
 	return result
 }
 
+func randomPrefix(t testing.TB, cidrBlock string, bits int) net.IPNet {
+	t.Helper()
+
+	ip, block, err := net.ParseCIDR(cidrBlock)
+	if err != nil {
+		t.Fatalf("randomPrefix cannot parse cidrBlock - %s", err)
+	}
+	if block.IP.String() != ip.String() {
+		t.Fatal("invocation of randomPrefix doesn't use a base block address")
+	}
+
+	mOnes, mBits := block.Mask.Size()
+	if mOnes >= bits {
+		t.Fatalf("cannot select a random /%d from within %s", bits, cidrBlock)
+	}
+
+	// generate a completely random address
+	randomIP := make(net.IP, mBits/8)
+	_, err = crand.Read(randomIP)
+	if err != nil {
+		t.Fatalf("rand read failed")
+	}
+
+	// mask off the "network" bits
+	for i, b := range randomIP {
+		mBitsThisByte := min(mOnes, 8)
+		mOnes -= mBitsThisByte
+		block.IP[i] = block.IP[i] | (b & byte(math.MaxUint8>>mBitsThisByte))
+	}
+
+	block.Mask = net.CIDRMask(bits, mBits)
+
+	_, result, err := net.ParseCIDR(block.String())
+	if err != nil {
+		t.Fatal("failed to parse own CIDR block")
+	}
+
+	return *result
+}
+
 func randomSlash31(t testing.TB, cidrBlock string) net.IPNet {
 	t.Helper()
 

go.mod

@@ -7,7 +7,6 @@ go 1.22.5
 require (
 	github.com/IBM/netaddr v1.5.0
 	github.com/Juniper/apstra-go-sdk v0.0.0-20240722155513-01f6e5e4e91a
-	github.com/apparentlymart/go-cidr v1.1.0
 	github.com/chrismarget-j/go-licenses v0.0.0-20240224210557-f22f3e06d3d4
 	github.com/google/go-cmp v0.6.0
 	github.com/hashicorp/go-version v1.7.0

go.sum

@@ -23,8 +23,6 @@ github.com/ProtonMail/go-crypto v1.1.0-alpha.3-proton h1:0RXAi0EJFs81j+MMsqvHNuA
 github.com/ProtonMail/go-crypto v1.1.0-alpha.3-proton/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE=
 github.com/agext/levenshtein v1.2.2 h1:0S/Yg6LYmFJ5stwQeRp6EeOcCbj7xiqQSdNelsXvaqE=
 github.com/agext/levenshtein v1.2.2/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558=
-github.com/apparentlymart/go-cidr v1.1.0 h1:2mAhrMoF+nhXqxTzSZMUzDHkLjmIHC+Zzn4tdgBZjnU=
-github.com/apparentlymart/go-cidr v1.1.0/go.mod h1:EBcsNrHc3zQeuaeCeCtQruQm+n9/YjEn/vI25Lg7Gwc=
 github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec=
 github.com/apparentlymart/go-textseg/v15 v15.0.0 h1:uYvfpb3DyLSCGWnctWKGj857c6ew1u1fNQOlOtuGxQY=
 github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmmsvpAG721bKi0joRfFdHIWJ4=