Skip to content

fix(networkinterface): Recognize the removal of allowed_addresses and… #940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions stackit/internal/services/iaas/networkinterface/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var (
_ resource.Resource = &networkInterfaceResource{}
_ resource.ResourceWithConfigure = &networkInterfaceResource{}
_ resource.ResourceWithImportState = &networkInterfaceResource{}
_ resource.ResourceWithModifyPlan = &networkInterfaceResource{}
)

type Model struct {
Expand Down Expand Up @@ -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"
Expand Down
16 changes: 16 additions & 0 deletions stackit/internal/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))...)
}
}
}
123 changes: 123 additions & 0 deletions stackit/internal/utils/utils_test.go
Original file line number Diff line number Diff line change
@@ -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"
)
Expand Down Expand Up @@ -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)
}
}
})
}
}
Loading