From ad91123c9139c8309f853c80be245b498765493a Mon Sep 17 00:00:00 2001 From: Alexander Dahmen Date: Tue, 5 Aug 2025 16:27:20 +0200 Subject: [PATCH] fix(networkinterface): Recognize the removal of allowed_addresses and security_group_ids from terraform config Signed-off-by: Alexander Dahmen --- .../iaas/networkinterface/resource.go | 33 +++++ stackit/internal/utils/utils.go | 16 +++ stackit/internal/utils/utils_test.go | 123 ++++++++++++++++++ 3 files changed, 172 insertions(+) diff --git a/stackit/internal/services/iaas/networkinterface/resource.go b/stackit/internal/services/iaas/networkinterface/resource.go index 5e40fc1f1..1532a9b81 100644 --- a/stackit/internal/services/iaas/networkinterface/resource.go +++ b/stackit/internal/services/iaas/networkinterface/resource.go @@ -33,6 +33,7 @@ var ( _ resource.Resource = &networkInterfaceResource{} _ resource.ResourceWithConfigure = &networkInterfaceResource{} _ resource.ResourceWithImportState = &networkInterfaceResource{} + _ resource.ResourceWithModifyPlan = &networkInterfaceResource{} ) type Model struct { @@ -61,6 +62,38 @@ type networkInterfaceResource struct { client *iaas.APIClient } +// ModifyPlan implements resource.ResourceWithModifyPlan. +func (r *networkInterfaceResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { // nolint:gocritic // function signature required by Terraform + // skip initial empty configuration to avoid follow-up errors + if req.Config.Raw.IsNull() { + return + } + var configModel Model + resp.Diagnostics.Append(req.Config.Get(ctx, &configModel)...) + if resp.Diagnostics.HasError() { + return + } + + var planModel Model + resp.Diagnostics.Append(req.Plan.Get(ctx, &planModel)...) + if resp.Diagnostics.HasError() { + return + } + // If allowed_addresses were completly removed from the config this is not recognized by terraform + // since this field is optional and computed therefore this plan modifier is needed. + utils.CheckListRemoval(ctx, configModel.AllowedAddresses, planModel.AllowedAddresses, path.Root("allowed_addresses"), types.StringType, false, resp) + if resp.Diagnostics.HasError() { + return + } + + // If security_group_ids were completly removed from the config this is not recognized by terraform + // since this field is optional and computed therefore this plan modifier is needed. + utils.CheckListRemoval(ctx, configModel.SecurityGroupIds, planModel.SecurityGroupIds, path.Root("security_group_ids"), types.StringType, true, resp) + if resp.Diagnostics.HasError() { + return + } +} + // Metadata returns the resource type name. func (r *networkInterfaceResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = req.ProviderTypeName + "_network_interface" diff --git a/stackit/internal/utils/utils.go b/stackit/internal/utils/utils.go index 3368b3416..6451837d0 100644 --- a/stackit/internal/utils/utils.go +++ b/stackit/internal/utils/utils.go @@ -7,7 +7,10 @@ import ( "regexp" "strings" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" + "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" @@ -157,3 +160,16 @@ func FormatPossibleValues(values ...string) string { func BuildInternalTerraformId(idParts ...string) types.String { return types.StringValue(strings.Join(idParts, core.Separator)) } + +// If a List was completely removed from the terraform config this is not recognized by terraform. +// This helper function checks if that is the case and adjusts the plan accordingly. +func CheckListRemoval(ctx context.Context, configModelList, planModelList types.List, destination path.Path, listType attr.Type, createEmptyList bool, resp *resource.ModifyPlanResponse) { + if configModelList.IsNull() && !planModelList.IsNull() { + if createEmptyList { + emptyList, _ := types.ListValueFrom(ctx, listType, []string{}) + resp.Diagnostics.Append(resp.Plan.SetAttribute(ctx, destination, emptyList)...) + } else { + resp.Diagnostics.Append(resp.Plan.SetAttribute(ctx, destination, types.ListNull(listType))...) + } + } +} diff --git a/stackit/internal/utils/utils_test.go b/stackit/internal/utils/utils_test.go index 0a2af5c14..2df080a72 100644 --- a/stackit/internal/utils/utils_test.go +++ b/stackit/internal/utils/utils_test.go @@ -1,12 +1,17 @@ package utils import ( + "context" "fmt" "reflect" "testing" "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" ) @@ -378,3 +383,121 @@ func TestBuildInternalTerraformId(t *testing.T) { }) } } + +func TestCheckListRemoval(t *testing.T) { + type model struct { + AllowedAddresses types.List `tfsdk:"allowed_addresses"` + } + tests := []struct { + description string + configModelList types.List + planModelList types.List + path path.Path + listType attr.Type + createEmptyList bool + expectedAdjustedResp bool + }{ + { + "config and plan are the same - no change", + types.ListValueMust(types.StringType, []attr.Value{ + types.StringValue("value1"), + }), + types.ListValueMust(types.StringType, []attr.Value{ + types.StringValue("value1"), + }), + path.Root("allowed_addresses"), + types.StringType, + false, + false, + }, + { + "list was removed from config", + types.ListNull(types.StringType), + types.ListValueMust(types.StringType, []attr.Value{ + types.StringValue("value1"), + }), + path.Root("allowed_addresses"), + types.StringType, + false, + true, + }, + { + "list was added to config", + types.ListValueMust(types.StringType, []attr.Value{ + types.StringValue("value1"), + }), + types.ListNull(types.StringType), + path.Root("allowed_addresses"), + types.StringType, + false, + false, + }, + { + "no list provided at all", + types.ListNull(types.StringType), + types.ListNull(types.StringType), + path.Root("allowed_addresses"), + types.StringType, + false, + false, + }, + { + "create empty list test - list was removed from config", + types.ListNull(types.StringType), + types.ListValueMust(types.StringType, []attr.Value{ + types.StringValue("value1"), + }), + path.Root("allowed_addresses"), + types.StringType, + true, + true, + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + // create resp + plan := tfsdk.Plan{ + Schema: schema.Schema{ + Attributes: map[string]schema.Attribute{ + "allowed_addresses": schema.ListAttribute{ + ElementType: basetypes.StringType{}, + }, + }, + }, + } + + // set input planModelList to plan + if diags := plan.Set(context.Background(), model{tt.planModelList}); diags.HasError() { + t.Fatalf("cannot create test model: %v", diags) + } + resp := resource.ModifyPlanResponse{ + Plan: plan, + } + + CheckListRemoval(context.Background(), tt.configModelList, tt.planModelList, tt.path, tt.listType, tt.createEmptyList, &resp) + // check targetList + var respList types.List + resp.Plan.GetAttribute(context.Background(), tt.path, &respList) + + if tt.createEmptyList { + emptyList, _ := types.ListValueFrom(context.Background(), tt.listType, []string{}) + diffEmptyList := cmp.Diff(emptyList, respList) + if diffEmptyList != "" { + t.Fatalf("an empty list should have been created but was not: %s", diffEmptyList) + } + } + + // compare planModelList and resp list + diff := cmp.Diff(tt.planModelList, respList) + if tt.expectedAdjustedResp { + if diff == "" { + t.Fatalf("plan should be adjusted but was not") + } + } else { + if diff != "" { + t.Fatalf("plan should not be adjusted but diff is: %s", diff) + } + } + }) + } +}