Eliminate bad assumption about logical interface node IDs

apstra/blueprint/ip_link_addressing.go

@@ -2,21 +2,19 @@ package blueprint
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"net"
 	"strings"
 
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/Juniper/apstra-go-sdk/apstra/enum"
-	"github.com/Juniper/terraform-provider-apstra/apstra/constants"
+	"github.com/Juniper/terraform-provider-apstra/apstra/private"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
 	apstravalidator "github.com/Juniper/terraform-provider-apstra/apstra/validator"
 	"github.com/hashicorp/terraform-plugin-framework-nettypes/cidrtypes"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/path"
-	"github.com/hashicorp/terraform-plugin-framework/resource"
 	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/stringdefault"
@@ -28,8 +26,6 @@ import (
 type IpLinkAddressing struct {
 	BlueprintId     types.String         `tfsdk:"blueprint_id"`
 	LinkId          types.String         `tfsdk:"link_id"`
-	SwitchIntfId    types.String         `tfsdk:"switch_interface_id"`
-	GenericIntfId   types.String         `tfsdk:"generic_interface_id"`
 	SwitchIpv4Type  types.String         `tfsdk:"switch_ipv4_address_type"`
 	SwitchIpv4Addr  cidrtypes.IPv4Prefix `tfsdk:"switch_ipv4_address"`
 	SwitchIpv6Type  types.String         `tfsdk:"switch_ipv6_address_type"`
@@ -57,16 +53,6 @@ func (o IpLinkAddressing) ResourceAttributes() map[string]resourceSchema.Attribu
 			PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()},
 			Validators:    []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
-		"switch_interface_id": resourceSchema.StringAttribute{
-			MarkdownDescription: "Apstra graph node ID of the node to which `switch` IP information will be associated.",
-			Computed:            true,
-			PlanModifiers:       []planmodifier.String{stringplanmodifier.UseStateForUnknown()},
-		},
-		"generic_interface_id": resourceSchema.StringAttribute{
-			MarkdownDescription: "Apstra graph node ID of the node to which `generic` IP information will be associated.",
-			Computed:            true,
-			PlanModifiers:       []planmodifier.String{stringplanmodifier.UseStateForUnknown()},
-		},
 		"switch_ipv4_address_type": resourceSchema.StringAttribute{
 			MarkdownDescription: fmt.Sprintf("Allowed values: [`%s`]", strings.Join(utils.AllInterfaceNumberingIpv4Types(), "`,`")),
 			Optional:            true,
@@ -180,20 +166,11 @@ func requestEndpoint(v4type, v6type types.String, v4addr cidrtypes.IPv4Prefix, v
 	return result
 }
 
-func (o IpLinkAddressing) Request(_ context.Context, diags *diag.Diagnostics) map[apstra.ObjectId]apstra.TwoStageL3ClosSubinterface {
-	if !utils.HasValue(o.SwitchIntfId) || !utils.HasValue(o.GenericIntfId) {
-		diags.AddError(
-			constants.ErrProviderBug,
-			fmt.Sprintf("attempt to generate ip link addressing with unknown interface ID\n"+
-				"switch_interface_id: %s\n generic_interface_id: %s", o.SwitchIntfId, o.GenericIntfId),
-		)
+func (o IpLinkAddressing) Request(_ context.Context, ids private.ResourceDatacenterIpLinkAddressingInterfaceIds, diags *diag.Diagnostics) map[apstra.ObjectId]apstra.TwoStageL3ClosSubinterface {
+	return map[apstra.ObjectId]apstra.TwoStageL3ClosSubinterface{
+		ids.SwitchInterface:  requestEndpoint(o.SwitchIpv4Type, o.SwitchIpv6Type, o.SwitchIpv4Addr, o.SwitchIpv6Addr, "switch", diags),
+		ids.GenericInterface: requestEndpoint(o.GenericIpv4Type, o.GenericIpv6Type, o.GenericIpv4Addr, o.GenericIpv6Addr, "generic", diags),
 	}
-
-	result := make(map[apstra.ObjectId]apstra.TwoStageL3ClosSubinterface, 2)
-	result[apstra.ObjectId(o.SwitchIntfId.ValueString())] = requestEndpoint(o.SwitchIpv4Type, o.SwitchIpv6Type, o.SwitchIpv4Addr, o.SwitchIpv6Addr, "switch", diags)
-	result[apstra.ObjectId(o.GenericIntfId.ValueString())] = requestEndpoint(o.GenericIpv4Type, o.GenericIpv6Type, o.GenericIpv4Addr, o.GenericIpv6Addr, "generic", diags)
-
-	return result
 }
 
 func epBySubinterfaceId(siId apstra.ObjectId, eps []apstra.TwoStageL3ClosSubinterfaceLinkEndpoint, diags *diag.Diagnostics) *apstra.TwoStageL3ClosSubinterfaceLinkEndpoint {
@@ -223,46 +200,7 @@ func epBySubinterfaceId(siId apstra.ObjectId, eps []apstra.TwoStageL3ClosSubinte
 	return result
 }
 
-func epBySystemType(sysType apstra.SystemType, eps []apstra.TwoStageL3ClosSubinterfaceLinkEndpoint, diags *diag.Diagnostics) *apstra.TwoStageL3ClosSubinterfaceLinkEndpoint {
-	var systemRoles []apstra.SystemRole
-
-	switch sysType {
-	case apstra.SystemTypeSwitch:
-		systemRoles = []apstra.SystemRole{apstra.SystemRoleSuperSpine, apstra.SystemRoleSpine, apstra.SystemRoleLeaf, apstra.SystemRoleAccess}
-	case apstra.SystemTypeServer:
-		systemRoles = []apstra.SystemRole{apstra.SystemRoleGeneric}
-	default:
-		diags.AddError(constants.ErrProviderBug, fmt.Sprintf("unexpected system type %q", sysType))
-		return nil
-	}
-
-	var result *apstra.TwoStageL3ClosSubinterfaceLinkEndpoint
-	for _, ep := range eps {
-		ep := ep
-		if utils.SliceContains(ep.System.Role, systemRoles) {
-			if result != nil {
-				diags.AddError(
-					"Unexpected API response",
-					fmt.Sprintf("Logical link has multiple endpoints on systems with %q roles", sysType),
-				)
-				return nil
-			}
-
-			result = &ep
-		}
-	}
-
-	if result == nil {
-		diags.AddError(
-			"Unexpected API response",
-			fmt.Sprintf("Logical link has no endpoints on systems with %q roles", sysType),
-		)
-	}
-
-	return result
-}
-
-func (o *IpLinkAddressing) LoadApiData(_ context.Context, in *apstra.TwoStageL3ClosSubinterfaceLink, diags *diag.Diagnostics) {
+func (o *IpLinkAddressing) LoadApiData(_ context.Context, in *apstra.TwoStageL3ClosSubinterfaceLink, private private.ResourceDatacenterIpLinkAddressingInterfaceIds, diags *diag.Diagnostics) {
 	// ensure 2 endpoints
 	if len(in.Endpoints) != 2 {
 		diags.AddError("Unexpected API response", fmt.Sprintf("Logical links should have 2 endpoints, got %d", len(in.Endpoints)))
@@ -284,21 +222,9 @@ func (o *IpLinkAddressing) LoadApiData(_ context.Context, in *apstra.TwoStageL3C
 		return
 	}
 
-	// ensure the subinterface IDs have the expected values
-	if !utils.ItemInSlice(apstra.ObjectId(o.GenericIntfId.ValueString()), siIds) ||
-		!utils.ItemInSlice(apstra.ObjectId(o.SwitchIntfId.ValueString()), siIds) {
-		diags.AddError(
-			"Unexpected API response",
-			fmt.Sprintf("Logical link %s previously had subinterface IDs %s and %s.\n"+
-				"Now it has IDs %q and %q. Endpoint IDs are not expected to change.",
-				o.LinkId, o.SwitchIntfId, o.GenericIntfId, siIds[0], siIds[1]),
-		)
-		return
-	}
-
 	// extract the endpoints by subinterface ID
-	switchEp := epBySubinterfaceId(apstra.ObjectId(o.SwitchIntfId.ValueString()), in.Endpoints, diags)
-	genericEp := epBySubinterfaceId(apstra.ObjectId(o.GenericIntfId.ValueString()), in.Endpoints, diags)
+	switchEp := epBySubinterfaceId(private.SwitchInterface, in.Endpoints, diags)
+	genericEp := epBySubinterfaceId(private.GenericInterface, in.Endpoints, diags)
 	if diags.HasError() {
 		return
 	}
@@ -326,40 +252,3 @@ func (o *IpLinkAddressing) LoadApiData(_ context.Context, in *apstra.TwoStageL3C
 		o.GenericIpv6Addr = cidrtypes.NewIPv6PrefixValue(genericEp.Subinterface.Ipv6Addr.String())
 	}
 }
-
-// LoadImmutableData sets the switch and generic subinterface ID elements within o and saves the
-// initial switch/generic v4/v6 address types (probably those indicated in the connectivity template).
-// The user supplies the link ID, so we only need to do this once: at the beginning of Create(). The
-// subinterface nodes associated with a given link node should never change.
-func (o *IpLinkAddressing) LoadImmutableData(ctx context.Context, in *apstra.TwoStageL3ClosSubinterfaceLink, resp *resource.CreateResponse) {
-	switchEp := epBySystemType(apstra.SystemTypeSwitch, in.Endpoints, &resp.Diagnostics)
-	if resp.Diagnostics.HasError() {
-		return
-	}
-
-	genericEp := epBySystemType(apstra.SystemTypeServer, in.Endpoints, &resp.Diagnostics)
-	if resp.Diagnostics.HasError() {
-		return
-	}
-
-	o.SwitchIntfId = types.StringValue(switchEp.SubinterfaceId.String())
-	o.GenericIntfId = types.StringValue(genericEp.SubinterfaceId.String())
-
-	private, err := json.Marshal(struct {
-		SwitchIpv4AddressType  string `json:"switch_ipv4_address_type"`
-		SwitchIpv6AddressType  string `json:"switch_ipv6_address_type"`
-		GenericIpv4AddressType string `json:"generic_ipv4_address_type"`
-		GenericIpv6AddressType string `json:"generic_ipv6_address_type"`
-	}{
-		SwitchIpv4AddressType:  utils.StringersToFriendlyString(switchEp.Subinterface.Ipv4AddrType),
-		SwitchIpv6AddressType:  utils.StringersToFriendlyString(switchEp.Subinterface.Ipv6AddrType),
-		GenericIpv4AddressType: utils.StringersToFriendlyString(genericEp.Subinterface.Ipv4AddrType),
-		GenericIpv6AddressType: utils.StringersToFriendlyString(genericEp.Subinterface.Ipv6AddrType),
-	})
-	if err != nil {
-		resp.Diagnostics.AddError("failed marshaling private data", err.Error())
-		return
-	}
-
-	resp.Private.SetKey(ctx, "ep_addr_types", private)
-}

apstra/private/resource_datacenter_ip_link_addressing.go

@@ -0,0 +1,156 @@
+package private
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+
+	"github.com/Juniper/apstra-go-sdk/apstra"
+	"github.com/Juniper/apstra-go-sdk/apstra/enum"
+	"github.com/Juniper/terraform-provider-apstra/apstra/constants"
+	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
+	"github.com/hashicorp/terraform-plugin-framework/diag"
+)
+
+// ResourceDatacenterIpLinkAddressingInterfaceAddressing is stored in private state by
+// ResourceDatacenterIpLinkAddressing.Create(). It is the record of the original numbering scheme
+// on a logical link, and is restored by ResourceDatacenterIpLinkAddressing.Delete()
+type ResourceDatacenterIpLinkAddressingInterfaceAddressing struct {
+	SwitchIpv4  enum.InterfaceNumberingIpv4Type `json:"switch_ipv4"`
+	SwitchIpv6  enum.InterfaceNumberingIpv6Type `json:"switch_ipv6"`
+	GenericIpv4 enum.InterfaceNumberingIpv4Type `json:"generic_ipv4"`
+	GenericIpv6 enum.InterfaceNumberingIpv6Type `json:"generic_ipv6"`
+}
+
+func (o *ResourceDatacenterIpLinkAddressingInterfaceAddressing) LoadApiData(_ context.Context, link *apstra.TwoStageL3ClosSubinterfaceLink, diags *diag.Diagnostics) {
+	switchEp := epBySystemType(apstra.SystemTypeSwitch, link.Endpoints, diags)
+	if diags.HasError() {
+		return
+	}
+
+	genericEp := epBySystemType(apstra.SystemTypeServer, link.Endpoints, diags)
+	if diags.HasError() {
+		return
+	}
+
+	o.SwitchIpv4 = switchEp.Subinterface.Ipv4AddrType
+	o.SwitchIpv6 = switchEp.Subinterface.Ipv6AddrType
+	o.GenericIpv4 = genericEp.Subinterface.Ipv4AddrType
+	o.GenericIpv6 = genericEp.Subinterface.Ipv6AddrType
+}
+
+func (o *ResourceDatacenterIpLinkAddressingInterfaceAddressing) LoadPrivateState(ctx context.Context, ps State, diags *diag.Diagnostics) {
+	b, d := ps.GetKey(ctx, "ResourceDatacenterIpLinkAddressingInterfaceAddressing")
+	diags.Append(d...)
+	if diags.HasError() {
+		return
+	}
+
+	err := json.Unmarshal(b, &o)
+	if err != nil {
+		diags.AddError("failed to unmarshal private state", err.Error())
+		return
+	}
+}
+
+func (o *ResourceDatacenterIpLinkAddressingInterfaceAddressing) SetPrivateState(ctx context.Context, ps State, diags *diag.Diagnostics) {
+	b, err := json.Marshal(o)
+	if err != nil {
+		diags.AddError("failed to marshal private state", err.Error())
+		return
+	}
+
+	diags.Append(ps.SetKey(ctx, "ResourceDatacenterIpLinkAddressingInterfaceAddressing", b)...)
+}
+
+// ResourceDatacenterIpLinkAddressingInterfaceIds contains the logical interfaces associated with
+// a logical link. It turns out that these interface IDs are NOT immutable. The interfaces, along
+// with the logical link are created as a side-effect of associating a CT containing IP Link
+// primitives with a physical switch port. Modifying the CT may cause the logical link and logical
+// interface pair to be replaced. The logical link is indistinguishable from immutable because
+// its ID is constructed by encoding other information. The interfaces, on the other hand, use
+// random IDs, so they may be found to have changed from one run to the next. As a result, this
+// "private data" struct is no longer committed to private state. It's now relegated to merely
+// unpacking the API response via the LoadApiData() method.
+type ResourceDatacenterIpLinkAddressingInterfaceIds struct {
+	SwitchInterface  apstra.ObjectId `json:"switch_interface"`
+	GenericInterface apstra.ObjectId `json:"generic_interface"`
+}
+
+func (o *ResourceDatacenterIpLinkAddressingInterfaceIds) LoadApiData(_ context.Context, link *apstra.TwoStageL3ClosSubinterfaceLink, diags *diag.Diagnostics) {
+	switchEp := epBySystemType(apstra.SystemTypeSwitch, link.Endpoints, diags)
+	if diags.HasError() {
+		return
+	}
+
+	genericEp := epBySystemType(apstra.SystemTypeServer, link.Endpoints, diags)
+	if diags.HasError() {
+		return
+	}
+
+	o.SwitchInterface = switchEp.SubinterfaceId
+	o.GenericInterface = genericEp.SubinterfaceId
+}
+
+func (o *ResourceDatacenterIpLinkAddressingInterfaceIds) LoadPrivateState(ctx context.Context, ps State, diags *diag.Diagnostics) {
+	b, d := ps.GetKey(ctx, "ResourceDatacenterIpLinkAddressingInterfaceIds")
+	diags.Append(d...)
+	if diags.HasError() {
+		return
+	}
+
+	err := json.Unmarshal(b, o)
+	if err != nil {
+		diags.AddError("failed to unmarshal private state", err.Error())
+		return
+	}
+}
+
+func (o *ResourceDatacenterIpLinkAddressingInterfaceIds) SetPrivateState(ctx context.Context, ps State, diags *diag.Diagnostics) {
+	b, err := json.Marshal(o)
+	if err != nil {
+		diags.AddError("failed to marshal private state", err.Error())
+		return
+	}
+
+	diags.Append(ps.SetKey(ctx, "ResourceDatacenterIpLinkAddressingInterfaceIds", b)...)
+}
+
+func epBySystemType(sysType apstra.SystemType, eps []apstra.TwoStageL3ClosSubinterfaceLinkEndpoint, diags *diag.Diagnostics) *apstra.TwoStageL3ClosSubinterfaceLinkEndpoint {
+	var systemRoles []apstra.SystemRole
+
+	switch sysType {
+	case apstra.SystemTypeSwitch:
+		systemRoles = []apstra.SystemRole{apstra.SystemRoleSuperSpine, apstra.SystemRoleSpine, apstra.SystemRoleLeaf, apstra.SystemRoleAccess}
+	case apstra.SystemTypeServer:
+		systemRoles = []apstra.SystemRole{apstra.SystemRoleGeneric}
+	default:
+		diags.AddError(constants.ErrProviderBug, fmt.Sprintf("unexpected system type %q", sysType))
+		return nil
+	}
+
+	var result *apstra.TwoStageL3ClosSubinterfaceLinkEndpoint
+	for _, ep := range eps {
+		ep := ep
+		if utils.SliceContains(ep.System.Role, systemRoles) {
+			if result != nil {
+				diags.AddError(
+					"Unexpected API response",
+					fmt.Sprintf("Logical link has multiple endpoints on systems with %q roles", sysType),
+				)
+				return nil
+			}
+
+			result = &ep
+		}
+	}
+
+	if result == nil {
+		diags.AddError(
+			"Unexpected API response",
+			fmt.Sprintf("Logical link has no endpoints on systems with %q roles", sysType),
+		)
+	}
+
+	return result
+}

apstra/private/state.go

@@ -0,0 +1,14 @@
+package private
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-framework/diag"
+)
+
+// State is intended as a stand-in for ProviderData from the not-import-able
+// github.com/hashicorp/terraform-plugin-framework/internal/privatestate package.
+type State interface {
+	GetKey(ctx context.Context, key string) ([]byte, diag.Diagnostics)
+	SetKey(ctx context.Context, key string, value []byte) diag.Diagnostics
+}