From 8a609d4ab86ee734dbaddef427c3623bbcc5ba0a Mon Sep 17 00:00:00 2001 From: Marcel Jacek <72880145+marceljk@users.noreply.github.com> Date: Thu, 4 Dec 2025 13:32:18 +0100 Subject: [PATCH] fix(ske): read only attributes produces noise (#1081) * fix(ske): read-only attributes produces noise * feat: add new planmodifier `UseStateForUnknownIf` --- .../internal/services/ske/cluster/resource.go | 52 +++++++ .../utils/use_state_for_unknown_if.go | 69 ++++++++++ .../utils/use_state_for_unknown_if_test.go | 128 ++++++++++++++++++ 3 files changed, 249 insertions(+) create mode 100644 stackit/internal/utils/use_state_for_unknown_if.go create mode 100644 stackit/internal/utils/use_state_for_unknown_if_test.go diff --git a/stackit/internal/services/ske/cluster/resource.go b/stackit/internal/services/ske/cluster/resource.go index e722ebfb..d6caa824 100644 --- a/stackit/internal/services/ske/cluster/resource.go +++ b/stackit/internal/services/ske/cluster/resource.go @@ -368,6 +368,9 @@ func (r *clusterResource) Schema(_ context.Context, _ resource.SchemaRequest, re "kubernetes_version_used": schema.StringAttribute{ 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"), + }, }, "egress_address_ranges": schema.ListAttribute{ Description: "The outgoing network ranges (in CIDR notation) of traffic originating from workload on the cluster.", @@ -454,6 +457,9 @@ func (r *clusterResource) Schema(_ context.Context, _ resource.SchemaRequest, re "os_version_used": schema.StringAttribute{ 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"), + }, }, "volume_type": schema.StringAttribute{ Description: "Specifies the volume type. Defaults to `storage_premium_perf1`.", @@ -2105,6 +2111,52 @@ 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) diff --git a/stackit/internal/utils/use_state_for_unknown_if.go b/stackit/internal/utils/use_state_for_unknown_if.go new file mode 100644 index 00000000..00e90c61 --- /dev/null +++ b/stackit/internal/utils/use_state_for_unknown_if.go @@ -0,0 +1,69 @@ +package utils + +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, planmodifier.StringRequest, *UseStateForUnknownFuncResponse) + +type useStateForUnknownIf struct { + ifFunc UseStateForUnknownIfFunc + description string +} + +// UseStateForUnknownIf returns a plan modifier similar to UseStateForUnknown with a conditional +func UseStateForUnknownIf(f UseStateForUnknownIfFunc, description string) planmodifier.String { + return useStateForUnknownIf{ + ifFunc: f, + 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) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { // 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, req, funcResponse) + + resp.Diagnostics.Append(funcResponse.Diagnostics...) + if resp.Diagnostics.HasError() { + return + } + + if funcResponse.UseStateForUnknown { + resp.PlanValue = req.StateValue + } +} diff --git a/stackit/internal/utils/use_state_for_unknown_if_test.go b/stackit/internal/utils/use_state_for_unknown_if_test.go new file mode 100644 index 00000000..387e270a --- /dev/null +++ b/stackit/internal/utils/use_state_for_unknown_if_test.go @@ -0,0 +1,128 @@ +package utils + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +func TestUseStateForUnknownIf_PlanModifyString(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + stateValue types.String + planValue types.String + configValue types.String + ifFunc UseStateForUnknownIfFunc + expectedPlanValue types.String + expectedError bool + }{ + { + name: "State is Null (Creation)", + stateValue: types.StringNull(), + planValue: types.StringUnknown(), + configValue: types.StringValue("some-config"), + ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) { + // This should not be reached because the state is null + resp.UseStateForUnknown = true + }, + expectedPlanValue: types.StringUnknown(), + }, + { + name: "Plan is already known - (User updated the value)", + stateValue: types.StringValue("old-state"), + planValue: types.StringValue("new-plan"), + configValue: types.StringValue("new-plan"), + ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) { + // This should not be reached because the plan is known + resp.UseStateForUnknown = true + }, + expectedPlanValue: types.StringValue("new-plan"), + }, + { + name: "Config is Unknown (Interpolation)", + stateValue: types.StringValue("old-state"), + planValue: types.StringUnknown(), + configValue: types.StringUnknown(), + ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) { + // This should not be reached + resp.UseStateForUnknown = true + }, + expectedPlanValue: types.StringUnknown(), + }, + { + name: "Condition returns False (Do not use state)", + stateValue: types.StringValue("old-state"), + planValue: types.StringUnknown(), + configValue: types.StringNull(), // Simulating computed only + ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) { + resp.UseStateForUnknown = false + }, + expectedPlanValue: types.StringUnknown(), + }, + { + name: "Condition returns True (Use state)", + stateValue: types.StringValue("old-state"), + planValue: types.StringUnknown(), + configValue: types.StringNull(), + ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) { + resp.UseStateForUnknown = true + }, + expectedPlanValue: types.StringValue("old-state"), + }, + { + name: "Func returns Error", + stateValue: types.StringValue("old-state"), + planValue: types.StringUnknown(), + configValue: types.StringNull(), + ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) { + resp.Diagnostics.AddError("Test Error", "Something went wrong") + }, + expectedPlanValue: types.StringUnknown(), + 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.StringRequest{ + 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.StringResponse{ + PlanValue: tt.planValue, + } + + // Run the modifier + modifier.PlanModifyString(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) + } + }) + } +}