Skip to content

Commit 8a609d4

Browse files
authored
fix(ske): read only attributes produces noise (#1081)
* fix(ske): read-only attributes produces noise * feat: add new planmodifier `UseStateForUnknownIf`
1 parent 0674775 commit 8a609d4

File tree

3 files changed

+249
-0
lines changed

3 files changed

+249
-0
lines changed

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,9 @@ func (r *clusterResource) Schema(_ context.Context, _ resource.SchemaRequest, re
368368
"kubernetes_version_used": schema.StringAttribute{
369369
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,
370370
Computed: true,
371+
PlanModifiers: []planmodifier.String{
372+
utils.UseStateForUnknownIf(hasKubernetesMinChanged, "sets `UseStateForUnknown` only if `kubernetes_min_version` has not changed"),
373+
},
371374
},
372375
"egress_address_ranges": schema.ListAttribute{
373376
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
454457
"os_version_used": schema.StringAttribute{
455458
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,
456459
Computed: true,
460+
PlanModifiers: []planmodifier.String{
461+
utils.UseStateForUnknownIf(hasOsVersionMinChanged, "sets `UseStateForUnknown` only if `os_version_min` has not changed"),
462+
},
457463
},
458464
"volume_type": schema.StringAttribute{
459465
Description: "Specifies the volume type. Defaults to `storage_premium_perf1`.",
@@ -2105,6 +2111,52 @@ func getLatestSupportedKubernetesVersion(versions []ske.KubernetesVersion) (*str
21052111
return latestVersion, nil
21062112
}
21072113

2114+
func hasKubernetesMinChanged(ctx context.Context, request planmodifier.StringRequest, response *utils.UseStateForUnknownFuncResponse) { // nolint:gocritic // function signature required by Terraform
2115+
dependencyPath := path.Root("kubernetes_version_min")
2116+
2117+
var minVersionPlan types.String
2118+
diags := request.Plan.GetAttribute(ctx, dependencyPath, &minVersionPlan)
2119+
response.Diagnostics.Append(diags...)
2120+
if response.Diagnostics.HasError() {
2121+
return
2122+
}
2123+
2124+
var minVersionState types.String
2125+
diags = request.State.GetAttribute(ctx, dependencyPath, &minVersionState)
2126+
response.Diagnostics.Append(diags...)
2127+
if response.Diagnostics.HasError() {
2128+
return
2129+
}
2130+
2131+
if minVersionState == minVersionPlan {
2132+
response.UseStateForUnknown = true
2133+
return
2134+
}
2135+
}
2136+
2137+
func hasOsVersionMinChanged(ctx context.Context, request planmodifier.StringRequest, response *utils.UseStateForUnknownFuncResponse) { // nolint:gocritic // function signature required by Terraform
2138+
dependencyPath := request.Path.ParentPath().AtName("os_version_min")
2139+
2140+
var minVersionPlan types.String
2141+
diags := request.Plan.GetAttribute(ctx, dependencyPath, &minVersionPlan)
2142+
response.Diagnostics.Append(diags...)
2143+
if response.Diagnostics.HasError() {
2144+
return
2145+
}
2146+
2147+
var minVersionState types.String
2148+
diags = request.State.GetAttribute(ctx, dependencyPath, &minVersionState)
2149+
response.Diagnostics.Append(diags...)
2150+
if response.Diagnostics.HasError() {
2151+
return
2152+
}
2153+
2154+
if minVersionState == minVersionPlan {
2155+
response.UseStateForUnknown = true
2156+
return
2157+
}
2158+
}
2159+
21082160
func (r *clusterResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { // nolint:gocritic // function signature required by Terraform
21092161
var state Model
21102162
diags := req.State.Get(ctx, &state)
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package utils
2+
3+
import (
4+
"context"
5+
6+
"github.com/hashicorp/terraform-plugin-framework/diag"
7+
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
8+
)
9+
10+
type UseStateForUnknownFuncResponse struct {
11+
UseStateForUnknown bool
12+
Diagnostics diag.Diagnostics
13+
}
14+
15+
// UseStateForUnknownIfFunc is a conditional function used in UseStateForUnknownIf
16+
type UseStateForUnknownIfFunc func(context.Context, planmodifier.StringRequest, *UseStateForUnknownFuncResponse)
17+
18+
type useStateForUnknownIf struct {
19+
ifFunc UseStateForUnknownIfFunc
20+
description string
21+
}
22+
23+
// UseStateForUnknownIf returns a plan modifier similar to UseStateForUnknown with a conditional
24+
func UseStateForUnknownIf(f UseStateForUnknownIfFunc, description string) planmodifier.String {
25+
return useStateForUnknownIf{
26+
ifFunc: f,
27+
description: description,
28+
}
29+
}
30+
31+
func (m useStateForUnknownIf) Description(context.Context) string {
32+
return m.description
33+
}
34+
35+
func (m useStateForUnknownIf) MarkdownDescription(ctx context.Context) string {
36+
return m.Description(ctx)
37+
}
38+
39+
func (m useStateForUnknownIf) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { // nolint:gocritic // function signature required by Terraform
40+
// Do nothing if there is no state value.
41+
if req.StateValue.IsNull() {
42+
return
43+
}
44+
45+
// Do nothing if there is a known planned value.
46+
if !req.PlanValue.IsUnknown() {
47+
return
48+
}
49+
50+
// Do nothing if there is an unknown configuration value, otherwise interpolation gets messed up.
51+
if req.ConfigValue.IsUnknown() {
52+
return
53+
}
54+
55+
// The above checks are taken from the UseStateForUnknown plan modifier implementation
56+
// (https://github.com/hashicorp/terraform-plugin-framework/blob/44348af3923c82a93c64ae7dca906d9850ba956b/resource/schema/stringplanmodifier/use_state_for_unknown.go#L38)
57+
58+
funcResponse := &UseStateForUnknownFuncResponse{}
59+
m.ifFunc(ctx, req, funcResponse)
60+
61+
resp.Diagnostics.Append(funcResponse.Diagnostics...)
62+
if resp.Diagnostics.HasError() {
63+
return
64+
}
65+
66+
if funcResponse.UseStateForUnknown {
67+
resp.PlanValue = req.StateValue
68+
}
69+
}
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package utils
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
8+
"github.com/hashicorp/terraform-plugin-framework/types"
9+
)
10+
11+
func TestUseStateForUnknownIf_PlanModifyString(t *testing.T) {
12+
ctx := context.Background()
13+
14+
tests := []struct {
15+
name string
16+
stateValue types.String
17+
planValue types.String
18+
configValue types.String
19+
ifFunc UseStateForUnknownIfFunc
20+
expectedPlanValue types.String
21+
expectedError bool
22+
}{
23+
{
24+
name: "State is Null (Creation)",
25+
stateValue: types.StringNull(),
26+
planValue: types.StringUnknown(),
27+
configValue: types.StringValue("some-config"),
28+
ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) {
29+
// This should not be reached because the state is null
30+
resp.UseStateForUnknown = true
31+
},
32+
expectedPlanValue: types.StringUnknown(),
33+
},
34+
{
35+
name: "Plan is already known - (User updated the value)",
36+
stateValue: types.StringValue("old-state"),
37+
planValue: types.StringValue("new-plan"),
38+
configValue: types.StringValue("new-plan"),
39+
ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) {
40+
// This should not be reached because the plan is known
41+
resp.UseStateForUnknown = true
42+
},
43+
expectedPlanValue: types.StringValue("new-plan"),
44+
},
45+
{
46+
name: "Config is Unknown (Interpolation)",
47+
stateValue: types.StringValue("old-state"),
48+
planValue: types.StringUnknown(),
49+
configValue: types.StringUnknown(),
50+
ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) {
51+
// This should not be reached
52+
resp.UseStateForUnknown = true
53+
},
54+
expectedPlanValue: types.StringUnknown(),
55+
},
56+
{
57+
name: "Condition returns False (Do not use state)",
58+
stateValue: types.StringValue("old-state"),
59+
planValue: types.StringUnknown(),
60+
configValue: types.StringNull(), // Simulating computed only
61+
ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) {
62+
resp.UseStateForUnknown = false
63+
},
64+
expectedPlanValue: types.StringUnknown(),
65+
},
66+
{
67+
name: "Condition returns True (Use state)",
68+
stateValue: types.StringValue("old-state"),
69+
planValue: types.StringUnknown(),
70+
configValue: types.StringNull(),
71+
ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) {
72+
resp.UseStateForUnknown = true
73+
},
74+
expectedPlanValue: types.StringValue("old-state"),
75+
},
76+
{
77+
name: "Func returns Error",
78+
stateValue: types.StringValue("old-state"),
79+
planValue: types.StringUnknown(),
80+
configValue: types.StringNull(),
81+
ifFunc: func(_ context.Context, _ planmodifier.StringRequest, resp *UseStateForUnknownFuncResponse) {
82+
resp.Diagnostics.AddError("Test Error", "Something went wrong")
83+
},
84+
expectedPlanValue: types.StringUnknown(),
85+
expectedError: true,
86+
},
87+
}
88+
89+
for _, tt := range tests {
90+
t.Run(tt.name, func(t *testing.T) {
91+
// Initialize the modifier
92+
modifier := UseStateForUnknownIf(tt.ifFunc, "test description")
93+
94+
// Construct request
95+
req := planmodifier.StringRequest{
96+
StateValue: tt.stateValue,
97+
PlanValue: tt.planValue,
98+
ConfigValue: tt.configValue,
99+
}
100+
101+
// Construct response
102+
// Note: In the framework, resp.PlanValue is initialized to req.PlanValue
103+
// before the modifier is called. We must simulate this.
104+
resp := &planmodifier.StringResponse{
105+
PlanValue: tt.planValue,
106+
}
107+
108+
// Run the modifier
109+
modifier.PlanModifyString(ctx, req, resp)
110+
111+
// Check Errors
112+
if tt.expectedError {
113+
if !resp.Diagnostics.HasError() {
114+
t.Error("Expected error, got none")
115+
}
116+
} else {
117+
if resp.Diagnostics.HasError() {
118+
t.Errorf("Unexpected error: %s", resp.Diagnostics)
119+
}
120+
}
121+
122+
// Check Plan Value
123+
if !resp.PlanValue.Equal(tt.expectedPlanValue) {
124+
t.Errorf("PlanValue mismatch.\nExpected: %s\nGot: %s", tt.expectedPlanValue, resp.PlanValue)
125+
}
126+
})
127+
}
128+
}

0 commit comments

Comments
 (0)