Merge pull request #750 from Juniper/task/749-rename-pool-allocation-…

…ipv6-roles Task/749 **BREAKING CHANGE** - Change IPv6 role strings in `apstra_datacenter_resource_pool_allocation` resource
Juniper · Jul 27, 2024 · a02fb4d · a02fb4d
2 parents 9edfb51 + 2964d78
commit a02fb4d
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 46 deletions.
diff --git a/apstra/blueprint/pool_allocation.go b/apstra/blueprint/pool_allocation.go
@@ -6,12 +6,10 @@ import (
 	"strings"
 
 	"github.com/Juniper/apstra-go-sdk/apstra"
-	apstravalidator "github.com/Juniper/terraform-provider-apstra/apstra/apstra_validator"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
 	"github.com/hashicorp/terraform-plugin-framework-validators/setvalidator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
-	"github.com/hashicorp/terraform-plugin-framework/path"
 	resourceSchema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
@@ -24,10 +22,54 @@ type PoolAllocation struct {
 	Role          types.String `tfsdk:"role"`
 	PoolIds       types.Set    `tfsdk:"pool_ids"`
 	RoutingZoneId types.String `tfsdk:"routing_zone_id"`
-	// PoolAllocationId types.String `tfsdk:"pool_allocation_id"` // placeholder for freeform
 }
 
 func (o PoolAllocation) ResourceAttributes() map[string]resourceSchema.Attribute {
+	// roleSemanticEqualityFunc is a quick-and-dirty string plan modifier function intended to facilitate
+	// migration from the following API strings to the "better" strings we'd like to present to terraform
+	// users:
+	// - ipv6_spine_leaf_link_ips       -> spine_leaf_link_ips_ipv6
+	// - ipv6_spine_superspine_link_ips -> spine_superspine_link_ips_ipv6
+	// - ipv6_to_generic_link_ips       -> to_generic_link_ips_ipv6
+	roleSemanticEqualityFunc := func(ctx context.Context, req planmodifier.StringRequest, resp *stringplanmodifier.RequiresReplaceIfFuncResponse) {
+		if req.PlanValue.Equal(req.StateValue) {
+			// plan and state are equal -- no problem!
+			resp.RequiresReplace = false
+			return
+		}
+
+		var err error
+		var plan, state apstra.ResourceGroupName
+
+		// use two strategies when parsing the state value to apstra.ResourceGroupName
+		err = state.FromString(req.StateValue.ValueString())
+		if err != nil {
+			err = utils.ApiStringerFromFriendlyString(&state, req.StateValue.ValueString())
+			if err != nil {
+				resp.Diagnostics.AddError(fmt.Sprintf("failed to parse state value %q", req.StateValue.ValueString()), err.Error())
+				return
+			}
+		}
+
+		// use two strategies when parsing the plan value to apstra.ResourceGroupName
+		err = plan.FromString(req.PlanValue.ValueString())
+		if err != nil {
+			err = utils.ApiStringerFromFriendlyString(&plan, req.PlanValue.ValueString())
+			if err != nil {
+				resp.Diagnostics.AddError(fmt.Sprintf("failed to parse plan value %q", req.PlanValue.ValueString()), err.Error())
+				return
+			}
+		}
+
+		if plan != state {
+			// plan and state have different IOTA values! This is a major configuration change. Rip-and-replace the resource.
+			resp.RequiresReplace = true
+			return
+		}
+
+		resp.RequiresReplace = false
+	}
+
 	return map[string]resourceSchema.Attribute{
 		"blueprint_id": resourceSchema.StringAttribute{
 			MarkdownDescription: "Apstra ID of the Blueprint to which the Resource Pool should be allocated.",
@@ -47,13 +89,25 @@ func (o PoolAllocation) ResourceAttributes() map[string]resourceSchema.Attribute
 		"role": resourceSchema.StringAttribute{
 			MarkdownDescription: "Fabric Role (Apstra Resource Group Name) must be one of:\n  - " +
 				strings.Join(utils.AllResourceGroupNameStrings(), "\n  - ") + "\n",
-			Required:      true,
-			PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()},
-			Validators: []validator.String{
-				stringvalidator.OneOf(
-					utils.AllResourceGroupNameStrings()...,
+			Required: true,
+			PlanModifiers: []planmodifier.String{
+				//stringplanmodifier.RequiresReplace(),
+				// RequiresReplace has been swapped for RequiresReplaceIf to support migration from old
+				// role strings to new:
+				// - ipv6_spine_leaf_link_ips       -> spine_leaf_link_ips_ipv6
+				// - ipv6_spine_superspine_link_ips -> spine_superspine_link_ips_ipv6
+				// - ipv6_to_generic_link_ips       -> to_generic_link_ips_ipv6
+				//
+				// This migration ability will have gone into effect around v0.63.0 in early August 2024.
+				// We should probably keep it around for at least a year because needlessly removing/replacing
+				// resource allocations is pretty disruptive.
+				stringplanmodifier.RequiresReplaceIf(
+					roleSemanticEqualityFunc,
+					"permit nondisruptive migration from old API strings to new terraform strings",
+					"permit nondisruptive migration from old API strings to new terraform strings",
 				),
 			},
+			Validators: []validator.String{stringvalidator.OneOf(utils.AllResourceGroupNameStrings()...)},
 		},
 		"routing_zone_id": resourceSchema.StringAttribute{
 			MarkdownDescription: fmt.Sprintf("Used to allocate a Resource Pool to a "+
@@ -62,24 +116,9 @@ func (o PoolAllocation) ResourceAttributes() map[string]resourceSchema.Attribute
 				"allocaated to a specific Routing Zone. When omitted, the specified Resource "+
 				"Pools are allocated to a fabric-wide `role`.",
 				apstra.ResourceGroupNameLeafIp4, apstra.ResourceGroupNameVirtualNetworkSviIpv4),
-			Optional: true,
-			Validators: []validator.String{
-				stringvalidator.LengthAtLeast(1),
-				apstravalidator.AtMostNOf(1,
-					path.Expressions{
-						path.MatchRelative(),
-						// other blueprint objects to which pools can be assigned must be listed here
-						// path.MatchRoot("pool_allocation_id"), //placeholder for freeform
-					}...,
-				),
-			},
+			Optional:   true,
+			Validators: []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
-		// placeholder for freeform
-		//"pool_allocation_id": resourceSchema.StringAttribute{
-		//	MarkdownDescription: "",
-		//	Optional:            true,
-		//	Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
-		//},
 	}
 }
 

diff --git a/apstra/utils/rosetta.go b/apstra/utils/rosetta.go
@@ -29,9 +29,9 @@ const (
 
 	// search for todos with 'enable_rosetta_for_pools_with_leading_ipv6' to enable rosetta for these items.
 	// total 18 occurences between this file and the test file
-	// resourceGroupNameSpineLeafLinkIpv6       = "spine_leaf_link_ips_ipv6"       // todo: enable_rosetta_for_pools_with_leading_ipv6
-	// resourceGroupNameSpineSuperspineLinkIpv6 = "spine_superspine_link_ips_ipv6" // todo: enable_rosetta_for_pools_with_leading_ipv6
-	// resourceGroupNameToGenericLinkIpv6       = "to_generic_link_ips_ipv6"       // todo: enable_rosetta_for_pools_with_leading_ipv6
+	resourceGroupNameSpineLeafLinkIpv6       = "spine_leaf_link_ips_ipv6"
+	resourceGroupNameSpineSuperspineLinkIpv6 = "spine_superspine_link_ips_ipv6"
+	resourceGroupNameToGenericLinkIpv6       = "to_generic_link_ips_ipv6"
 
 	interfaceNumberingIpv4TypeNone = "none"
 	interfaceNumberingIpv6TypeNone = "none"
@@ -217,12 +217,12 @@ func resourceGroupNameToFriendlyString(in apstra.ResourceGroupName) string {
 		return resourceGroupNameLeafL3PeerLinksIpv6
 	case apstra.ResourceGroupNameVxlanVnIds:
 		return resourceGroupNameVxlanVnIds
-		// case apstra.ResourceGroupNameSpineLeafIp6: //       todo: enable_rosetta_for_pools_with_leading_ipv6
-		//	return resourceGroupNameSpineLeafLinkIpv6 //       todo: enable_rosetta_for_pools_with_leading_ipv6
-		// case apstra.ResourceGroupNameSuperspineSpineIp6: // todo: enable_rosetta_for_pools_with_leading_ipv6
-		//	return resourceGroupNameSpineSuperspineLinkIpv6 // todo: enable_rosetta_for_pools_with_leading_ipv6
-		// case apstra.ResourceGroupNameToGenericLinkIpv6: //  todo: enable_rosetta_for_pools_with_leading_ipv6
-		//	return resourceGroupNameToGenericLinkIpv6 //       todo: enable_rosetta_for_pools_with_leading_ipv6
+	case apstra.ResourceGroupNameSpineLeafIp6:
+		return resourceGroupNameSpineLeafLinkIpv6
+	case apstra.ResourceGroupNameSuperspineSpineIp6:
+		return resourceGroupNameSpineSuperspineLinkIpv6
+	case apstra.ResourceGroupNameToGenericLinkIpv6:
+		return resourceGroupNameToGenericLinkIpv6
 	}
 
 	return in.String()
@@ -372,12 +372,12 @@ func resourceGroupNameFromFriendlyString(target *apstra.ResourceGroupName, in ..
 		*target = apstra.ResourceGroupNameLeafL3PeerLinkLinkIp6
 	case resourceGroupNameVxlanVnIds:
 		*target = apstra.ResourceGroupNameVxlanVnIds
-	// case resourceGroupNameSpineLeafLinkIpv6: //              todo: enable_rosetta_for_pools_with_leading_ipv6
-	//	*target = apstra.ResourceGroupNameSpineLeafIp6 //       todo: enable_rosetta_for_pools_with_leading_ipv6
-	// case resourceGroupNameSpineSuperspineLinkIpv6: //        todo: enable_rosetta_for_pools_with_leading_ipv6
-	//	*target = apstra.ResourceGroupNameSuperspineSpineIp6 // todo: enable_rosetta_for_pools_with_leading_ipv6
-	// case resourceGroupNameToGenericLinkIpv6: //              todo: enable_rosetta_for_pools_with_leading_ipv6
-	//	*target = apstra.ResourceGroupNameToGenericLinkIpv6 //  todo: enable_rosetta_for_pools_with_leading_ipv6
+	case resourceGroupNameSpineLeafLinkIpv6:
+		*target = apstra.ResourceGroupNameSpineLeafIp6
+	case resourceGroupNameSpineSuperspineLinkIpv6:
+		*target = apstra.ResourceGroupNameSuperspineSpineIp6
+	case resourceGroupNameToGenericLinkIpv6:
+		*target = apstra.ResourceGroupNameToGenericLinkIpv6
 	default:
 		return target.FromString(in[0])
 	}

diff --git a/apstra/utils/rosetta_test.go b/apstra/utils/rosetta_test.go
@@ -42,9 +42,9 @@ func TestRosetta(t *testing.T) {
 		{string: "leaf_l3_peer_links", stringers: []fmt.Stringer{apstra.ResourceGroupNameLeafL3PeerLinkLinkIp4}},
 		{string: "leaf_l3_peer_links_ipv6", stringers: []fmt.Stringer{apstra.ResourceGroupNameLeafL3PeerLinkLinkIp6}},
 
-		//{string: "spine_leaf_link_ips_ipv6", stringers: []fmt.Stringer{apstra.ResourceGroupNameSpineLeafIp6}},             // todo: enable_rosetta_for_pools_with_leading_ipv6
-		//{string: "spine_superspine_link_ips_ipv6", stringers: []fmt.Stringer{apstra.ResourceGroupNameSuperspineSpineIp6}}, // todo: enable_rosetta_for_pools_with_leading_ipv6
-		//{string: "to_generic_link_ips_ipv6", stringers: []fmt.Stringer{apstra.ResourceGroupNameToGenericLinkIpv6}},        // todo: enable_rosetta_for_pools_with_leading_ipv6
+		{string: "spine_leaf_link_ips_ipv6", stringers: []fmt.Stringer{apstra.ResourceGroupNameSpineLeafIp6}},
+		{string: "spine_superspine_link_ips_ipv6", stringers: []fmt.Stringer{apstra.ResourceGroupNameSuperspineSpineIp6}},
+		{string: "to_generic_link_ips_ipv6", stringers: []fmt.Stringer{apstra.ResourceGroupNameToGenericLinkIpv6}},
 
 		{string: "none", stringers: []fmt.Stringer{apstra.InterfaceNumberingIpv4TypeNone}},
 		{string: "none", stringers: []fmt.Stringer{apstra.InterfaceNumberingIpv6TypeNone}},
@@ -66,6 +66,12 @@ func TestRosetta(t *testing.T) {
 		case apstra.AsnAllocationScheme:
 			x := apstra.AsnAllocationScheme(-1)
 			target = &x
+		case apstra.InterfaceNumberingIpv4Type:
+			x := apstra.InterfaceNumberingIpv4Type{}
+			target = &x
+		case apstra.InterfaceNumberingIpv6Type:
+			x := apstra.InterfaceNumberingIpv6Type{}
+			target = &x
 		case apstra.OverlayControlProtocol:
 			x := apstra.OverlayControlProtocol(-1)
 			target = &x

diff --git a/docs/resources/datacenter_resource_pool_allocation.md b/docs/resources/datacenter_resource_pool_allocation.md
@@ -73,9 +73,6 @@ resource "apstra_datacenter_resource_pool_allocation" "ipv4" {
   - generic_asns
   - generic_loopback_ips
   - generic_loopback_ips_ipv6
-  - ipv6_spine_leaf_link_ips
-  - ipv6_spine_superspine_link_ips
-  - ipv6_to_generic_link_ips
   - leaf_asns
   - leaf_l3_peer_links
   - leaf_l3_peer_links_ipv6
@@ -86,13 +83,16 @@ resource "apstra_datacenter_resource_pool_allocation" "ipv4" {
   - mlag_domain_svi_subnets_ipv6
   - spine_asns
   - spine_leaf_link_ips
+  - spine_leaf_link_ips_ipv6
   - spine_loopback_ips
   - spine_loopback_ips_ipv6
   - spine_superspine_link_ips
+  - spine_superspine_link_ips_ipv6
   - superspine_asns
   - superspine_loopback_ips
   - superspine_loopback_ips_ipv6
   - to_generic_link_ips
+  - to_generic_link_ips_ipv6
   - virtual_network_svi_subnets
   - virtual_network_svi_subnets_ipv6
   - vni_virtual_network_ids