From e688aa78e18be65a9edab9e163ec3015bac6d120 Mon Sep 17 00:00:00 2001 From: Chris Kim <97423717+mik-ky@users.noreply.github.com> Date: Wed, 20 Nov 2024 12:26:48 +1300 Subject: [PATCH] Fix tenant tags producing inconsistent result after apply after updating to plugin framework (#817) * fix: Tenant tags producing incosistent result after apply after updating to plugin framework * Update docs and test --- docs/data-sources/tenants.md | 2 +- docs/resources/tenant.md | 2 +- octopusdeploy_framework/resource_tenant.go | 38 +++++++++++-------- .../resource_tenant_migration_test.go | 7 ++-- octopusdeploy_framework/schemas/tenant.go | 18 ++++----- 5 files changed, 37 insertions(+), 30 deletions(-) diff --git a/docs/data-sources/tenants.md b/docs/data-sources/tenants.md index 438682077..c95b3b896 100644 --- a/docs/data-sources/tenants.md +++ b/docs/data-sources/tenants.md @@ -43,6 +43,6 @@ Read-Only: - `id` (String) The unique ID for this resource. - `name` (String) The name of this resource. - `space_id` (String) The space ID associated with this tenant. -- `tenant_tags` (List of String) A list of tenant tags associated with this resource. +- `tenant_tags` (Set of String) A list of tenant tags associated with this resource. diff --git a/docs/resources/tenant.md b/docs/resources/tenant.md index 0a62ebe36..02fb8feb7 100644 --- a/docs/resources/tenant.md +++ b/docs/resources/tenant.md @@ -21,7 +21,7 @@ This resource manages tenants in Octopus Deploy. - `cloned_from_tenant_id` (String) The ID of the tenant from which this tenant was cloned. - `description` (String) The description of this tenant. - `space_id` (String) The space ID associated with this tenant. -- `tenant_tags` (List of String) A list of tenant tags associated with this resource. +- `tenant_tags` (Set of String) A list of tenant tags associated with this resource. ### Read-Only diff --git a/octopusdeploy_framework/resource_tenant.go b/octopusdeploy_framework/resource_tenant.go index 9e038757a..3fa5c5103 100644 --- a/octopusdeploy_framework/resource_tenant.go +++ b/octopusdeploy_framework/resource_tenant.go @@ -3,17 +3,17 @@ package octopusdeploy_framework import ( "context" "fmt" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tenants" "github.com/OctopusDeploy/terraform-provider-octopusdeploy/internal" "github.com/OctopusDeploy/terraform-provider-octopusdeploy/internal/errors" "github.com/OctopusDeploy/terraform-provider-octopusdeploy/octopusdeploy_framework/schemas" "github.com/OctopusDeploy/terraform-provider-octopusdeploy/octopusdeploy_framework/util" - "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-log/tflog" - "sort" ) type tenantTypeResource struct { @@ -48,7 +48,7 @@ func (r *tenantTypeResource) Create(ctx context.Context, req resource.CreateRequ return } - tenant, err := mapStateToTenant(data) + tenant, err := mapStateToTenant(ctx, data) if err != nil { return } @@ -61,7 +61,7 @@ func (r *tenantTypeResource) Create(ctx context.Context, req resource.CreateRequ return } - mapTenantToState(data, createdTenant) + mapTenantToState(ctx, data, createdTenant) tflog.Info(ctx, fmt.Sprintf("Tenant created (%s)", data.ID)) resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) @@ -85,7 +85,7 @@ func (r *tenantTypeResource) Read(ctx context.Context, req resource.ReadRequest, return } - mapTenantToState(data, tenant) + mapTenantToState(ctx, data, tenant) tflog.Info(ctx, fmt.Sprintf("Tenant read (%s)", tenant.GetID())) resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) @@ -106,7 +106,7 @@ func (r *tenantTypeResource) Update(ctx context.Context, req resource.UpdateRequ tenantFromApi, err := tenants.GetByID(r.Config.Client, data.SpaceID.ValueString(), data.ID.ValueString()) - tenant, err := mapStateToTenant(data) + tenant, err := mapStateToTenant(ctx, data) tenant.ID = state.ID.ValueString() if err != nil { resp.Diagnostics.AddError("unable to map to tenant", err.Error()) @@ -122,7 +122,7 @@ func (r *tenantTypeResource) Update(ctx context.Context, req resource.UpdateRequ return } - mapTenantToState(data, updatedTenant) + mapTenantToState(ctx, data, updatedTenant) tflog.Info(ctx, fmt.Sprintf("Tenant updated (%s)", data.ID)) @@ -146,30 +146,36 @@ func (r *tenantTypeResource) Delete(ctx context.Context, req resource.DeleteRequ } } -func mapStateToTenant(data *schemas.TenantModel) (*tenants.Tenant, error) { +func mapStateToTenant(ctx context.Context, data *schemas.TenantModel) (*tenants.Tenant, error) { tenant := tenants.NewTenant(data.Name.ValueString()) tenant.ID = data.ID.ValueString() tenant.ClonedFromTenantID = data.ClonedFromTenantId.ValueString() tenant.Description = data.Description.ValueString() tenant.SpaceID = data.SpaceID.ValueString() - if len(data.TenantTags.Elements()) > 0 { - tenant.TenantTags = util.ExpandStringList(data.TenantTags) - } else { - tenant.TenantTags = []string{} + + convertedTenantTags, diags := util.SetToStringArray(ctx, data.TenantTags) + if diags.HasError() { + tflog.Error(ctx, fmt.Sprintf("Error converting tenant tags: %v\n", diags)) } - sort.Strings(tenant.TenantTags) + + tenant.TenantTags = convertedTenantTags return tenant, nil } -func mapTenantToState(data *schemas.TenantModel, tenant *tenants.Tenant) { +func mapTenantToState(ctx context.Context, data *schemas.TenantModel, tenant *tenants.Tenant) { data.ID = types.StringValue(tenant.ID) data.ClonedFromTenantId = types.StringValue(tenant.ClonedFromTenantID) data.Description = types.StringValue(tenant.Description) data.SpaceID = types.StringValue(tenant.SpaceID) data.Name = types.StringValue(tenant.Name) - sort.Strings(tenant.TenantTags) - data.TenantTags = util.Ternary(tenant.TenantTags != nil && len(tenant.TenantTags) > 0, util.FlattenStringList(tenant.TenantTags), types.ListValueMust(types.StringType, make([]attr.Value, 0))) + + convertedTenantTags, diags := util.SetToStringArray(ctx, data.TenantTags) + if diags.HasError() { + tflog.Error(ctx, fmt.Sprintf("Error converting tenant tags: %v\n", diags)) + } + + data.TenantTags = basetypes.SetValue(util.FlattenStringList(convertedTenantTags)) } func (*tenantTypeResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { diff --git a/octopusdeploy_framework/resource_tenant_migration_test.go b/octopusdeploy_framework/resource_tenant_migration_test.go index 0ca7404e7..4e6c95040 100644 --- a/octopusdeploy_framework/resource_tenant_migration_test.go +++ b/octopusdeploy_framework/resource_tenant_migration_test.go @@ -2,14 +2,15 @@ package octopusdeploy_framework import ( "fmt" + "os" + "sort" + "testing" + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/stretchr/testify/assert" - "os" - "sort" - "testing" ) func TestTenantResource_UpgradeFromSDK_ToPluginFramework(t *testing.T) { diff --git a/octopusdeploy_framework/schemas/tenant.go b/octopusdeploy_framework/schemas/tenant.go index ac33f10c1..5b0c9f695 100644 --- a/octopusdeploy_framework/schemas/tenant.go +++ b/octopusdeploy_framework/schemas/tenant.go @@ -5,8 +5,8 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" datasourceSchema "github.com/hashicorp/terraform-plugin-framework/datasource/schema" resourceSchema "github.com/hashicorp/terraform-plugin-framework/resource/schema" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/listplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault" "github.com/hashicorp/terraform-plugin-framework/types" ) @@ -16,7 +16,7 @@ type TenantModel struct { Description types.String `tfsdk:"description"` Name types.String `tfsdk:"name"` SpaceID types.String `tfsdk:"space_id"` - TenantTags types.List `tfsdk:"tenant_tags"` + TenantTags types.Set `tfsdk:"tenant_tags"` ResourceModel } @@ -47,7 +47,7 @@ func TenantObjectType() map[string]attr.Type { "id": types.StringType, "name": types.StringType, "space_id": types.StringType, - "tenant_tags": types.ListType{ElemType: types.StringType}, + "tenant_tags": types.SetType{ElemType: types.StringType}, } } @@ -56,7 +56,7 @@ func FlattenTenant(tenant *tenants.Tenant) attr.Value { for i, value := range tenant.TenantTags { tenantTags[i] = types.StringValue(value) } - var tenantTagsList, _ = types.ListValue(types.StringType, tenantTags) + var tenantTagsSet, _ = types.SetValue(types.StringType, tenantTags) return types.ObjectValueMust(TenantObjectType(), map[string]attr.Value{ "cloned_from_tenant_id": types.StringValue(tenant.ClonedFromTenantID), @@ -64,7 +64,7 @@ func FlattenTenant(tenant *tenants.Tenant) attr.Value { "id": types.StringValue(tenant.GetID()), "name": types.StringValue(tenant.Name), "space_id": types.StringValue(tenant.SpaceID), - "tenant_tags": tenantTagsList, + "tenant_tags": tenantTagsSet, }) } @@ -108,7 +108,7 @@ func (t TenantSchema) GetDatasourceSchema() datasourceSchema.Schema { "id": GetIdDatasourceSchema(true), "name": GetReadonlyNameDatasourceSchema(), "space_id": GetSpaceIdDatasourceSchema("tenant", true), - "tenant_tags": datasourceSchema.ListAttribute{ + "tenant_tags": datasourceSchema.SetAttribute{ Computed: true, Description: "A list of tenant tags associated with this resource.", ElementType: types.StringType, @@ -134,13 +134,13 @@ func (t TenantSchema) GetResourceSchema() resourceSchema.Schema { "id": GetIdResourceSchema(), "name": GetNameResourceSchema(true), "space_id": GetSpaceIdResourceSchema("tenant"), - "tenant_tags": resourceSchema.ListAttribute{ + "tenant_tags": resourceSchema.SetAttribute{ Description: "A list of tenant tags associated with this resource.", ElementType: types.StringType, Optional: true, Computed: true, - PlanModifiers: []planmodifier.List{ - listplanmodifier.UseStateForUnknown(), + PlanModifiers: []planmodifier.Set{ + setplanmodifier.UseStateForUnknown(), }, }, },