Merge pull request #698 from Juniper/688-support-freeform-reference-d…

…esign-review 688 support freeform reference design review
Juniper · Jul 3, 2024 · 3d8ae19 · 3d8ae19
2 parents ad0baa3 + 2e453a5
commit 3d8ae19
Show file tree

Hide file tree

Showing 23 changed files with 239 additions and 79 deletions.
diff --git a/apstra/blueprint/freeform_config_template.go b/apstra/blueprint/freeform_config_template.go
@@ -2,6 +2,8 @@ package blueprint
 
 import (
 	"context"
+	"regexp"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
 	"github.com/hashicorp/terraform-plugin-framework-validators/setvalidator"
@@ -14,7 +16,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"
-	"regexp"
 )
 
 type FreeformConfigTemplate struct {
@@ -34,7 +35,7 @@ func (o FreeformConfigTemplate) DataSourceAttributes() map[string]dataSourceSche
 			Validators: []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"id": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "Populate this field to look up the Config Template by `id`. Required when `name` is omitted.",
+			MarkdownDescription: "Populate this field to look up the Config Template by ID. Required when `name` is omitted.",
 			Optional:            true,
 			Computed:            true,
 			Validators: []validator.String{
@@ -46,21 +47,19 @@ func (o FreeformConfigTemplate) DataSourceAttributes() map[string]dataSourceSche
 			},
 		},
 		"name": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "Populate this field to look up an imported Config Template by `name`. Required when `id` is omitted.",
+			MarkdownDescription: "Populate this field to look up an imported Config Template by Name. Required when `id` is omitted.",
 			Optional:            true,
 			Computed:            true,
 			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"text": dataSourceSchema.StringAttribute{
 			MarkdownDescription: "Configuration Jinja2 template text",
 			Computed:            true,
-			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"tags": dataSourceSchema.SetAttribute{
 			MarkdownDescription: "Set of Tag labels",
 			ElementType:         types.StringType,
-			Optional:            true,
-			Validators:          []validator.Set{setvalidator.SizeAtLeast(1)},
+			Computed:            true,
 		},
 	}
 }

diff --git a/apstra/blueprint/freeform_property_set.go b/apstra/blueprint/freeform_property_set.go
@@ -3,6 +3,7 @@ package blueprint
 import (
 	"context"
 	"encoding/json"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
@@ -33,7 +34,7 @@ func (o FreeformPropertySet) DataSourceAttributes() map[string]dataSourceSchema.
 			Validators: []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"id": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "Populate this field to look up a Freeform Property Set by `id`. Required when `name` is omitted.",
+			MarkdownDescription: "Populate this field to look up a Freeform Property Set by ID. Required when `name` is omitted.",
 			Optional:            true,
 			Computed:            true,
 			Validators: []validator.String{
@@ -50,7 +51,7 @@ func (o FreeformPropertySet) DataSourceAttributes() map[string]dataSourceSchema.
 			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"name": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "Populate this field to look up an imported Property Set by `name`. Required when `id` is omitted.",
+			MarkdownDescription: "Populate this field to look up an imported Property Set by Name. Required when `id` is omitted.",
 			Optional:            true,
 			Computed:            true,
 			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
@@ -74,6 +75,7 @@ func (o FreeformPropertySet) ResourceAttributes() map[string]resourceSchema.Attr
 		"id": resourceSchema.StringAttribute{
 			MarkdownDescription: "ID of the Property Set.",
 			Computed:            true,
+			PlanModifiers:       []planmodifier.String{stringplanmodifier.UseStateForUnknown()},
 		},
 		"system_id": resourceSchema.StringAttribute{
 			MarkdownDescription: "The system ID where the Property Set is associated.",

diff --git a/apstra/blueprint/freeform_system.go b/apstra/blueprint/freeform_system.go
@@ -2,6 +2,8 @@ package blueprint
 
 import (
 	"context"
+	"fmt"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
 	"github.com/hashicorp/terraform-plugin-framework-validators/setvalidator"
@@ -37,7 +39,7 @@ func (o FreeformSystem) DataSourceAttributes() map[string]dataSourceSchema.Attri
 			Validators: []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"id": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "Populate this field to look up the Freeform System by `id`. Required when `name` is omitted.",
+			MarkdownDescription: "Populate this field to look up the Freeform System by ID. Required when `name` is omitted.",
 			Optional:            true,
 			Computed:            true,
 			Validators: []validator.String{
@@ -49,7 +51,7 @@ func (o FreeformSystem) DataSourceAttributes() map[string]dataSourceSchema.Attri
 			},
 		},
 		"name": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "Populate this field to look up System by `name`. Required when `id` is omitted.",
+			MarkdownDescription: "Populate this field to look up System by Name. Required when `id` is omitted.",
 			Optional:            true,
 			Computed:            true,
 			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
@@ -77,7 +79,7 @@ func (o FreeformSystem) DataSourceAttributes() map[string]dataSourceSchema.Attri
 		"tags": dataSourceSchema.SetAttribute{
 			MarkdownDescription: "Set of Tag labels",
 			ElementType:         types.StringType,
-			Optional:            true,
+			Computed:            true,
 		},
 	}
 }
@@ -98,34 +100,30 @@ func (o FreeformSystem) ResourceAttributes() map[string]resourceSchema.Attribute
 		"name": resourceSchema.StringAttribute{
 			MarkdownDescription: "Freeform System name as shown in the Web UI.",
 			Required:            true,
-			Validators: []validator.String{
-				stringvalidator.LengthAtLeast(1),
-			},
+			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"hostname": resourceSchema.StringAttribute{
 			MarkdownDescription: "Hostname of the Freeform System.",
 			Optional:            true,
-			Validators: []validator.String{
-				stringvalidator.LengthAtLeast(1),
-			},
+			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"deploy_mode": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "deploy mode of the System",
+			MarkdownDescription: "Deploy mode of the System",
 			Optional:            true,
 			Validators:          []validator.String{stringvalidator.OneOf(utils.AllNodeDeployModes()...)},
 		},
 		"type": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "type of the System, either Internal or External",
+			MarkdownDescription: fmt.Sprintf("Type of the System. Must be one of `%s` or `%s`", apstra.SystemTypeInternal, apstra.SystemTypeExternal),
 			Required:            true,
 			Validators:          []validator.String{stringvalidator.OneOf(apstra.SystemTypeInternal.String(), apstra.SystemTypeExternal.String())},
 		},
 		"device_profile_id": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "device profile ID of the System",
+			MarkdownDescription: "Device profile ID of the System",
 			Optional:            true,
 			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
 		},
 		"system_id": dataSourceSchema.StringAttribute{
-			MarkdownDescription: "Device System ID assigned to the System",
+			MarkdownDescription: "ID (usually serial number) of the Managed Device to associate with this System",
 			Optional:            true,
 			Validators:          []validator.String{stringvalidator.LengthAtLeast(1)},
 		},

diff --git a/apstra/blueprint/pool_allocation.go b/apstra/blueprint/pool_allocation.go
@@ -15,6 +15,8 @@ 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"
+	"sort"
+	"strings"
 )
 
 type PoolAllocation struct {
@@ -26,6 +28,9 @@ type PoolAllocation struct {
 }
 
 func (o PoolAllocation) ResourceAttributes() map[string]resourceSchema.Attribute {
+	sortedRoles := utils.AllResourceGroupNameStrings()
+	sort.Strings(sortedRoles)
+
 	return map[string]resourceSchema.Attribute{
 		"blueprint_id": resourceSchema.StringAttribute{
 			MarkdownDescription: "Apstra ID of the Blueprint to which the Resource Pool should be allocated.",
@@ -43,9 +48,10 @@ func (o PoolAllocation) ResourceAttributes() map[string]resourceSchema.Attribute
 			},
 		},
 		"role": resourceSchema.StringAttribute{
-			MarkdownDescription: "Fabric Role (Apstra Resource Group Name)",
-			Required:            true,
-			PlanModifiers:       []planmodifier.String{stringplanmodifier.RequiresReplace()},
+			MarkdownDescription: "Fabric Role (Apstra Resource Group Name) must be one of:\n\n  - " +
+				strings.Join(sortedRoles, "\n  - ") + "\n",
+			Required:      true,
+			PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()},
 			Validators: []validator.String{
 				stringvalidator.OneOf(
 					utils.AllResourceGroupNameStrings()...,

diff --git a/apstra/configure_resource.go b/apstra/configure_resource.go
@@ -3,6 +3,7 @@ package tfapstra
 import (
 	"context"
 	"fmt"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 )
@@ -70,10 +71,6 @@ func configureResource(_ context.Context, rs resource.ResourceWithConfigure, req
 		rs.setBpClientFunc(pd.getTwoStageL3ClosClient)
 	}
 
-	if rs, ok := rs.(resourceWithSetDcBpClientFunc); ok {
-		rs.setBpClientFunc(pd.getTwoStageL3ClosClient)
-	}
-
 	if rs, ok := rs.(resourceWithSetFfBpClientFunc); ok {
 		rs.setBpClientFunc(pd.getFreeformClient)
 	}

diff --git a/apstra/data_source_freeform_config_template.go b/apstra/data_source_freeform_config_template.go
@@ -3,6 +3,7 @@ package tfapstra
 import (
 	"context"
 	"fmt"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/Juniper/terraform-provider-apstra/apstra/blueprint"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
@@ -12,8 +13,10 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 )
 
-var _ datasource.DataSourceWithConfigure = &dataSourceFreeformConfigTemplate{}
-var _ datasourceWithSetFfBpClientFunc = &dataSourceFreeformConfigTemplate{}
+var (
+	_ datasource.DataSourceWithConfigure = &dataSourceFreeformConfigTemplate{}
+	_ datasourceWithSetFfBpClientFunc    = &dataSourceFreeformConfigTemplate{}
+)
 
 type dataSourceFreeformConfigTemplate struct {
 	getBpClientFunc func(context.Context, string) (*apstra.FreeformClient, error)
@@ -34,6 +37,7 @@ func (o *dataSourceFreeformConfigTemplate) Schema(_ context.Context, _ datasourc
 		Attributes: blueprint.FreeformConfigTemplate{}.DataSourceAttributes(),
 	}
 }
+
 func (o *dataSourceFreeformConfigTemplate) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) {
 	var config blueprint.FreeformConfigTemplate
 	resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)

diff --git a/apstra/data_source_freeform_property_set.go b/apstra/data_source_freeform_property_set.go
@@ -3,16 +3,20 @@ package tfapstra
 import (
 	"context"
 	"fmt"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/Juniper/terraform-provider-apstra/apstra/blueprint"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
 	"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/path"
+	"github.com/hashicorp/terraform-plugin-framework/types"
 )
 
-var _ datasource.DataSourceWithConfigure = &dataSourceFreeformPropertySet{}
-var _ datasourceWithSetFfBpClientFunc = &dataSourceFreeformPropertySet{}
+var (
+	_ datasource.DataSourceWithConfigure = &dataSourceFreeformPropertySet{}
+	_ datasourceWithSetFfBpClientFunc    = &dataSourceFreeformPropertySet{}
+)
 
 type dataSourceFreeformPropertySet struct {
 	getBpClientFunc func(context.Context, string) (*apstra.FreeformClient, error)
@@ -28,11 +32,12 @@ func (o *dataSourceFreeformPropertySet) Configure(ctx context.Context, req datas
 
 func (o *dataSourceFreeformPropertySet) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) {
 	resp.Schema = schema.Schema{
-		MarkdownDescription: docCategoryFreeform + "This data source provides details of a specific Freeform PropertySet.\n\n" +
+		MarkdownDescription: docCategoryFreeform + "This data source provides details of a specific Freeform Property Set.\n\n" +
 			"At least one optional attribute is required.",
 		Attributes: blueprint.FreeformPropertySet{}.DataSourceAttributes(),
 	}
 }
+
 func (o *dataSourceFreeformPropertySet) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) {
 	var config blueprint.FreeformPropertySet
 	resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)
@@ -81,6 +86,7 @@ func (o *dataSourceFreeformPropertySet) Read(ctx context.Context, req datasource
 		return
 	}
 
+	config.Id = types.StringValue(api.Id.String())
 	config.LoadApiData(ctx, api.Data, &resp.Diagnostics)
 	if resp.Diagnostics.HasError() {
 		return

diff --git a/apstra/data_source_freeform_system.go b/apstra/data_source_freeform_system.go
@@ -3,6 +3,7 @@ package tfapstra
 import (
 	"context"
 	"fmt"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/Juniper/terraform-provider-apstra/apstra/blueprint"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
@@ -12,8 +13,10 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 )
 
-var _ datasource.DataSourceWithConfigure = &dataSourceFreeformSystem{}
-var _ datasourceWithSetFfBpClientFunc = &dataSourceFreeformSystem{}
+var (
+	_ datasource.DataSourceWithConfigure = &dataSourceFreeformSystem{}
+	_ datasourceWithSetFfBpClientFunc    = &dataSourceFreeformSystem{}
+)
 
 type dataSourceFreeformSystem struct {
 	getBpClientFunc func(context.Context, string) (*apstra.FreeformClient, error)
@@ -29,11 +32,12 @@ func (o *dataSourceFreeformSystem) Configure(ctx context.Context, req datasource
 
 func (o *dataSourceFreeformSystem) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) {
 	resp.Schema = schema.Schema{
-		MarkdownDescription: docCategoryFreeform + "This data source provides details of a specific Freeform Config Template.\n\n" +
+		MarkdownDescription: docCategoryFreeform + "This data source provides details of a specific Freeform System.\n\n" +
 			"At least one optional attribute is required.",
 		Attributes: blueprint.FreeformSystem{}.DataSourceAttributes(),
 	}
 }
+
 func (o *dataSourceFreeformSystem) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) {
 	var config blueprint.FreeformSystem
 	resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)

diff --git a/apstra/resource_freeform_config_template.go b/apstra/resource_freeform_config_template.go
@@ -3,6 +3,7 @@ package tfapstra
 import (
 	"context"
 	"fmt"
+
 	"github.com/Juniper/apstra-go-sdk/apstra"
 	"github.com/Juniper/terraform-provider-apstra/apstra/blueprint"
 	"github.com/Juniper/terraform-provider-apstra/apstra/utils"
@@ -11,9 +12,11 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 )
 
-var _ resource.ResourceWithConfigure = &resourceFreeformConfigTemplate{}
-var _ resourceWithSetFfBpClientFunc = &resourceFreeformConfigTemplate{}
-var _ resourceWithSetBpLockFunc = &resourceFreeformConfigTemplate{}
+var (
+	_ resource.ResourceWithConfigure = &resourceFreeformConfigTemplate{}
+	_ resourceWithSetFfBpClientFunc  = &resourceFreeformConfigTemplate{}
+	_ resourceWithSetBpLockFunc      = &resourceFreeformConfigTemplate{}
+)
 
 type resourceFreeformConfigTemplate struct {
 	getBpClientFunc func(context.Context, string) (*apstra.FreeformClient, error)
@@ -118,7 +121,6 @@ func (o *resourceFreeformConfigTemplate) Read(ctx context.Context, req resource.
 	resp.Diagnostics.Append(resp.State.Set(ctx, &state)...)
 }
 
-// Update resource
 func (o *resourceFreeformConfigTemplate) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
 	// Get plan values
 	var plan blueprint.FreeformConfigTemplate
@@ -162,7 +164,6 @@ func (o *resourceFreeformConfigTemplate) Update(ctx context.Context, req resourc
 	resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
 }
 
-// Delete resource
 func (o *resourceFreeformConfigTemplate) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
 	var state blueprint.FreeformConfigTemplate
 	resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
@@ -174,8 +175,7 @@ func (o *resourceFreeformConfigTemplate) Delete(ctx context.Context, req resourc
 	bp, err := o.getBpClientFunc(ctx, state.BlueprintId.ValueString())
 	if err != nil {
 		if utils.IsApstra404(err) {
-			resp.Diagnostics.AddError(fmt.Sprintf("blueprint %s not found", state.BlueprintId), err.Error())
-			return
+			return // 404 is okay
 		}
 		resp.Diagnostics.AddError("failed to create blueprint client", err.Error())
 		return