fix(networkinterface): Recognize the removal of allowed_addresses and security_group_ids from terraform config (#940)

Signed-off-by: Alexander Dahmen <alexander.dahmen@inovex.de>

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"

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))...)
+		}
+	}
+}

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)
+				}
+			}
+		})
+	}
+}