fix(observability): changing a plan after creation results in an error (#1269)

STACKITTPR-504

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

Author: Fyusel

stackit/internal/services/observability/instance/resource.go

@@ -8,8 +8,8 @@ import (
 	"strings"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
 	"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils"
+	"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/int64planmodifier"
 
 	observabilityUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/observability/utils"
 
@@ -543,23 +543,23 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
 				Optional:    true,
 				Computed:    true,
 				PlanModifiers: []planmodifier.Int64{
-					int64planmodifier.UseStateForUnknown(),
+					int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days", "sets `UseStateForUnknown` only if `metrics_retention_days` has not changed"),
 				},
 			},
 			"metrics_retention_days_5m_downsampling": schema.Int64Attribute{
 				Description: "Specifies for how many days the 5m downsampled metrics are kept. must be less than the value of the general retention. Default is set to `90`.",
 				Optional:    true,
 				Computed:    true,
 				PlanModifiers: []planmodifier.Int64{
-					int64planmodifier.UseStateForUnknown(),
+					int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days_5m_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_5m_downsampling` has not changed"),
 				},
 			},
 			"metrics_retention_days_1h_downsampling": schema.Int64Attribute{
 				Description: "Specifies for how many days the 1h downsampled metrics are kept. must be less than the value of the 5m downsampling retention. Default is set to `90`.",
 				Optional:    true,
 				Computed:    true,
 				PlanModifiers: []planmodifier.Int64{
-					int64planmodifier.UseStateForUnknown(),
+					int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days_1h_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_1h_downsampling` has not changed"),
 				},
 			},
 			"metrics_url": schema.StringAttribute{

stackit/internal/services/ske/cluster/resource.go

@@ -11,6 +11,7 @@ import (
 
 	serviceenablementUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/serviceenablement/utils"
 	skeUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/ske/utils"
+	stringplanmodifierUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/stringplanmodifier"
 
 	"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
@@ -368,7 +369,7 @@ func (r *clusterResource) Schema(_ context.Context, _ resource.SchemaRequest, re
 				Description: "Full Kubernetes version used. For example, if 1.22 was set in `kubernetes_version_min`, this value may result to 1.22.15. " + SKEUpdateDoc,
 				Computed:    true,
 				PlanModifiers: []planmodifier.String{
-					utils.UseStateForUnknownIf(hasKubernetesMinChanged, "sets `UseStateForUnknown` only if `kubernetes_min_version` has not changed"),
+					stringplanmodifierUtils.UseStateForUnknownIf(utils.StringChanged, "kubernetes_version_min", "sets `UseStateForUnknown` only if `kubernetes_min_version` has not changed"),
 				},
 			},
 			"egress_address_ranges": schema.ListAttribute{
@@ -451,7 +452,7 @@ func (r *clusterResource) Schema(_ context.Context, _ resource.SchemaRequest, re
 							Description: "Full OS image version used. For example, if 3815.2 was set in `os_version_min`, this value may result to 3815.2.2. " + SKEUpdateDoc,
 							Computed:    true,
 							PlanModifiers: []planmodifier.String{
-								utils.UseStateForUnknownIf(hasOsVersionMinChanged, "sets `UseStateForUnknown` only if `os_version_min` has not changed"),
+								stringplanmodifierUtils.UseStateForUnknownIf(utils.StringChanged, "os_version_min", "sets `UseStateForUnknown` only if `os_version_min` has not changed"),
 							},
 						},
 						"volume_type": schema.StringAttribute{
@@ -2108,52 +2109,6 @@ func getLatestSupportedKubernetesVersion(versions []ske.KubernetesVersion) (*str
 	return latestVersion, nil
 }
 
-func hasKubernetesMinChanged(ctx context.Context, request planmodifier.StringRequest, response *utils.UseStateForUnknownFuncResponse) { // nolint:gocritic // function signature required by Terraform
-	dependencyPath := path.Root("kubernetes_version_min")
-
-	var minVersionPlan types.String
-	diags := request.Plan.GetAttribute(ctx, dependencyPath, &minVersionPlan)
-	response.Diagnostics.Append(diags...)
-	if response.Diagnostics.HasError() {
-		return
-	}
-
-	var minVersionState types.String
-	diags = request.State.GetAttribute(ctx, dependencyPath, &minVersionState)
-	response.Diagnostics.Append(diags...)
-	if response.Diagnostics.HasError() {
-		return
-	}
-
-	if minVersionState == minVersionPlan {
-		response.UseStateForUnknown = true
-		return
-	}
-}
-
-func hasOsVersionMinChanged(ctx context.Context, request planmodifier.StringRequest, response *utils.UseStateForUnknownFuncResponse) { // nolint:gocritic // function signature required by Terraform
-	dependencyPath := request.Path.ParentPath().AtName("os_version_min")
-
-	var minVersionPlan types.String
-	diags := request.Plan.GetAttribute(ctx, dependencyPath, &minVersionPlan)
-	response.Diagnostics.Append(diags...)
-	if response.Diagnostics.HasError() {
-		return
-	}
-
-	var minVersionState types.String
-	diags = request.State.GetAttribute(ctx, dependencyPath, &minVersionState)
-	response.Diagnostics.Append(diags...)
-	if response.Diagnostics.HasError() {
-		return
-	}
-
-	if minVersionState == minVersionPlan {
-		response.UseStateForUnknown = true
-		return
-	}
-}
-
 func (r *clusterResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { // nolint:gocritic // function signature required by Terraform
 	var state Model
 	diags := req.State.Get(ctx, &state)

stackit/internal/utils/attributes.go

@@ -7,8 +7,11 @@ import (
 
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/path"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core"
+	"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/int64planmodifier"
+	"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/stringplanmodifier"
 )
 
 type attributeGetter interface {
@@ -44,3 +47,51 @@ func GetTimeFromStringAttribute(ctx context.Context, attributePath path.Path, so
 
 	return diags
 }
+
+// Int64Changed sets UseStateForUnkown to true if the attribute's planned value matches the current state
+func Int64Changed(ctx context.Context, attributeName string, request planmodifier.Int64Request, response *int64planmodifier.UseStateForUnknownFuncResponse) { // nolint:gocritic // function signature required by Terraform
+	dependencyPath := request.Path.ParentPath().AtName(attributeName)
+
+	var attributePlan types.Int64
+	diags := request.Plan.GetAttribute(ctx, dependencyPath, &attributePlan)
+	response.Diagnostics.Append(diags...)
+	if response.Diagnostics.HasError() {
+		return
+	}
+
+	var attributeState types.Int64
+	diags = request.State.GetAttribute(ctx, dependencyPath, &attributeState)
+	response.Diagnostics.Append(diags...)
+	if response.Diagnostics.HasError() {
+		return
+	}
+
+	if attributeState == attributePlan {
+		response.UseStateForUnknown = true
+		return
+	}
+}
+
+// StringChanged sets UseStateForUnkown to true if the attribute's planned value matches the current state
+func StringChanged(ctx context.Context, attributeName string, request planmodifier.StringRequest, response *stringplanmodifier.UseStateForUnknownFuncResponse) { // nolint:gocritic // function signature required by Terraform
+	dependencyPath := request.Path.ParentPath().AtName(attributeName)
+
+	var attributePlan types.String
+	diags := request.Plan.GetAttribute(ctx, dependencyPath, &attributePlan)
+	response.Diagnostics.Append(diags...)
+	if response.Diagnostics.HasError() {
+		return
+	}
+
+	var attributeState types.String
+	diags = request.State.GetAttribute(ctx, dependencyPath, &attributeState)
+	response.Diagnostics.Append(diags...)
+	if response.Diagnostics.HasError() {
+		return
+	}
+
+	if attributeState == attributePlan {
+		response.UseStateForUnknown = true
+		return
+	}
+}

stackit/internal/utils/planmodifiers/int64planmodifier/use_state_for_unknown_if.go

@@ -0,0 +1,71 @@
+package int64planmodifier
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-framework/diag"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+)
+
+type UseStateForUnknownFuncResponse struct {
+	UseStateForUnknown bool
+	Diagnostics        diag.Diagnostics
+}
+
+// UseStateForUnknownIfFunc is a conditional function used in UseStateForUnknownIf
+type UseStateForUnknownIfFunc func(context.Context, string, planmodifier.Int64Request, *UseStateForUnknownFuncResponse)
+
+type useStateForUnknownIf struct {
+	ifFunc        UseStateForUnknownIfFunc
+	attributeName string
+	description   string
+}
+
+// UseStateForUnknownIf returns a plan modifier similar to UseStateForUnknown with a conditional
+func UseStateForUnknownIf(f UseStateForUnknownIfFunc, attributeName, description string) planmodifier.Int64 {
+	return useStateForUnknownIf{
+		ifFunc:        f,
+		attributeName: attributeName,
+		description:   description,
+	}
+}
+
+func (m useStateForUnknownIf) Description(context.Context) string {
+	return m.description
+}
+
+func (m useStateForUnknownIf) MarkdownDescription(ctx context.Context) string {
+	return m.Description(ctx)
+}
+
+func (m useStateForUnknownIf) PlanModifyInt64(ctx context.Context, req planmodifier.Int64Request, resp *planmodifier.Int64Response) { // nolint:gocritic // function signature required by Terraform
+	// Do nothing if there is no state value.
+	if req.StateValue.IsNull() {
+		return
+	}
+
+	// Do nothing if there is a known planned value.
+	if !req.PlanValue.IsUnknown() {
+		return
+	}
+
+	// Do nothing if there is an unknown configuration value, otherwise interpolation gets messed up.
+	if req.ConfigValue.IsUnknown() {
+		return
+	}
+
+	// The above checks are taken from the UseStateForUnknown plan modifier implementation
+	// (https://github.com/hashicorp/terraform-plugin-framework/blob/44348af3923c82a93c64ae7dca906d9850ba956b/resource/schema/stringplanmodifier/use_state_for_unknown.go#L38)
+
+	funcResponse := &UseStateForUnknownFuncResponse{}
+	m.ifFunc(ctx, m.attributeName, req, funcResponse)
+
+	resp.Diagnostics.Append(funcResponse.Diagnostics...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	if funcResponse.UseStateForUnknown {
+		resp.PlanValue = req.StateValue
+	}
+}

stackit/internal/utils/planmodifiers/int64planmodifier/use_state_for_unknown_if_test.go

@@ -0,0 +1,128 @@
+package int64planmodifier
+
+import (
+	"context"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+)
+
+func TestUseStateForUnknownIf_PlanModifyInt64(t *testing.T) {
+	ctx := context.Background()
+
+	tests := []struct {
+		name              string
+		stateValue        types.Int64
+		planValue         types.Int64
+		configValue       types.Int64
+		ifFunc            UseStateForUnknownIfFunc
+		expectedPlanValue types.Int64
+		expectedError     bool
+	}{
+		{
+			name:        "State is Null (Creation)",
+			stateValue:  types.Int64Null(),
+			planValue:   types.Int64Unknown(),
+			configValue: types.Int64Value(10),
+			ifFunc: func(_ context.Context, _ string, _ planmodifier.Int64Request, resp *UseStateForUnknownFuncResponse) {
+				// This should not be reached because the state is null
+				resp.UseStateForUnknown = true
+			},
+			expectedPlanValue: types.Int64Unknown(),
+		},
+		{
+			name:        "Plan is already known - (User updated the value)",
+			stateValue:  types.Int64Value(5),
+			planValue:   types.Int64Value(10),
+			configValue: types.Int64Value(10),
+			ifFunc: func(_ context.Context, _ string, _ planmodifier.Int64Request, resp *UseStateForUnknownFuncResponse) {
+				// This should not be reached because the plan is known
+				resp.UseStateForUnknown = true
+			},
+			expectedPlanValue: types.Int64Value(10),
+		},
+		{
+			name:        "Config is Unknown (Interpolation)",
+			stateValue:  types.Int64Value(5),
+			planValue:   types.Int64Unknown(),
+			configValue: types.Int64Unknown(),
+			ifFunc: func(_ context.Context, _ string, _ planmodifier.Int64Request, resp *UseStateForUnknownFuncResponse) {
+				// This should not be reached
+				resp.UseStateForUnknown = true
+			},
+			expectedPlanValue: types.Int64Unknown(),
+		},
+		{
+			name:        "Condition returns False (Do not use state)",
+			stateValue:  types.Int64Value(5),
+			planValue:   types.Int64Unknown(),
+			configValue: types.Int64Null(), // Simulating computed only
+			ifFunc: func(_ context.Context, _ string, _ planmodifier.Int64Request, resp *UseStateForUnknownFuncResponse) {
+				resp.UseStateForUnknown = false
+			},
+			expectedPlanValue: types.Int64Unknown(),
+		},
+		{
+			name:        "Condition returns True (Use state)",
+			stateValue:  types.Int64Value(5),
+			planValue:   types.Int64Unknown(),
+			configValue: types.Int64Null(),
+			ifFunc: func(_ context.Context, _ string, _ planmodifier.Int64Request, resp *UseStateForUnknownFuncResponse) {
+				resp.UseStateForUnknown = true
+			},
+			expectedPlanValue: types.Int64Value(5),
+		},
+		{
+			name:        "Func returns Error",
+			stateValue:  types.Int64Value(5),
+			planValue:   types.Int64Unknown(),
+			configValue: types.Int64Null(),
+			ifFunc: func(_ context.Context, _ string, _ planmodifier.Int64Request, resp *UseStateForUnknownFuncResponse) {
+				resp.Diagnostics.AddError("Test Error", "Something went wrong")
+			},
+			expectedPlanValue: types.Int64Unknown(),
+			expectedError:     true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Initialize the modifier
+			modifier := UseStateForUnknownIf(tt.ifFunc, "", "test description")
+
+			// Construct request
+			req := planmodifier.Int64Request{
+				StateValue:  tt.stateValue,
+				PlanValue:   tt.planValue,
+				ConfigValue: tt.configValue,
+			}
+
+			// Construct response
+			// Note: In the framework, resp.PlanValue is initialized to req.PlanValue
+			// before the modifier is called. We must simulate this.
+			resp := &planmodifier.Int64Response{
+				PlanValue: tt.planValue,
+			}
+
+			// Run the modifier
+			modifier.PlanModifyInt64(ctx, req, resp)
+
+			// Check Errors
+			if tt.expectedError {
+				if !resp.Diagnostics.HasError() {
+					t.Error("Expected error, got none")
+				}
+			} else {
+				if resp.Diagnostics.HasError() {
+					t.Errorf("Unexpected error: %s", resp.Diagnostics)
+				}
+			}
+
+			// Check Plan Value
+			if !resp.PlanValue.Equal(tt.expectedPlanValue) {
+				t.Errorf("PlanValue mismatch.\nExpected: %s\nGot:      %s", tt.expectedPlanValue, resp.PlanValue)
+			}
+		})
+	}
+}