Skip to content

Commit 726f181

Browse files
rubenhoenlerubenhoenle
committed
fix(postgresflex): prevent state drifts on cron strings
relates to STACKITSDK-413
1 parent 2adf8f0 commit 726f181

File tree

15 files changed

+362
-175
lines changed

15 files changed

+362
-175
lines changed

‎stackit/internal/services/mongodbflex/instance/resource.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
mongodbflexUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/mongodbflex/utils"
13+
stringplanmodifierCustom "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/stringplanmodifier"
1314

1415
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
1516
"github.com/hashicorp/terraform-plugin-framework/attr"
@@ -240,6 +241,9 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
240241
"backup_schedule": schema.StringAttribute{
241242
Description: descriptions["backup_schedule"],
242243
Required: true,
244+
PlanModifiers: []planmodifier.String{
245+
stringplanmodifierCustom.CronNormalizationModifier{},
246+
},
243247
},
244248
"flavor": schema.SingleNestedAttribute{
245249
Required: true,
@@ -852,10 +856,11 @@ func mapFields(ctx context.Context, resp *mongodbflex.InstanceResponse, model *M
852856
return fmt.Errorf("creating options: %w", core.DiagsToError(diags))
853857
}
854858

855-
simplifiedModelBackupSchedule := utils.SimplifyBackupSchedule(model.BackupSchedule.ValueString())
856-
// If the value returned by the API is different from the one in the model after simplification,
857-
// we update the model so that it causes an error in Terraform
858-
if simplifiedModelBackupSchedule != types.StringPointerValue(instance.BackupSchedule).ValueString() {
859+
// If the API returned "0 0 * * *" but user defined "00 00 * * *" in its config,
860+
// we keep the user's "00 00 * * *" in the state to satisfy Terraform.
861+
backupScheduleApiResp := types.StringPointerValue(instance.BackupSchedule)
862+
if utils.SimplifyCronString(model.BackupSchedule.ValueString()) != utils.SimplifyCronString(backupScheduleApiResp.ValueString()) {
863+
// If the API actually changed it to something else, use the API value
859864
model.BackupSchedule = types.StringPointerValue(instance.BackupSchedule)
860865
}
861866

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -534,23 +534,23 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
534534
Optional: true,
535535
Computed: true,
536536
PlanModifiers: []planmodifier.Int64{
537-
int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days", "sets `UseStateForUnknown` only if `metrics_retention_days` has not changed"),
537+
int64planmodifier.UseStateForUnknownIf(int64planmodifier.Int64Changed, "metrics_retention_days", "sets `UseStateForUnknown` only if `metrics_retention_days` has not changed"),
538538
},
539539
},
540540
"metrics_retention_days_5m_downsampling": schema.Int64Attribute{
541541
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`.",
542542
Optional: true,
543543
Computed: true,
544544
PlanModifiers: []planmodifier.Int64{
545-
int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days_5m_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_5m_downsampling` has not changed"),
545+
int64planmodifier.UseStateForUnknownIf(int64planmodifier.Int64Changed, "metrics_retention_days_5m_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_5m_downsampling` has not changed"),
546546
},
547547
},
548548
"metrics_retention_days_1h_downsampling": schema.Int64Attribute{
549549
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`.",
550550
Optional: true,
551551
Computed: true,
552552
PlanModifiers: []planmodifier.Int64{
553-
int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days_1h_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_1h_downsampling` has not changed"),
553+
int64planmodifier.UseStateForUnknownIf(int64planmodifier.Int64Changed, "metrics_retention_days_1h_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_1h_downsampling` has not changed"),
554554
},
555555
},
556556
"metrics_url": schema.StringAttribute{

‎stackit/internal/services/postgresflex/instance/resource.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/conversion"
2020
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core"
2121
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils"
22+
stringplanmodifierCustom "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/stringplanmodifier"
2223
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/validate"
2324

2425
"github.com/hashicorp/terraform-plugin-framework/resource"
@@ -209,6 +210,9 @@ func (r *instanceResource) Schema(_ context.Context, req resource.SchemaRequest,
209210
"backup_schedule": schema.StringAttribute{
210211
Description: descriptions["backup_schedule"],
211212
Required: true,
213+
PlanModifiers: []planmodifier.String{
214+
stringplanmodifierCustom.CronNormalizationModifier{},
215+
},
212216
},
213217
"flavor": schema.SingleNestedAttribute{
214218
Required: true,
@@ -652,11 +656,18 @@ func mapFields(ctx context.Context, resp *postgresflex.InstanceResponse, model *
652656
return fmt.Errorf("creating storage: %w", core.DiagsToError(diags))
653657
}
654658

659+
// If the API returned "0 0 * * *" but user defined "00 00 * * *" in its config,
660+
// we keep the user's "00 00 * * *" in the state to satisfy Terraform.
661+
backupScheduleApiResp := types.StringPointerValue(instance.BackupSchedule)
662+
if utils.SimplifyCronString(model.BackupSchedule.ValueString()) != utils.SimplifyCronString(backupScheduleApiResp.ValueString()) {
663+
// If the API actually changed it to something else, use the API value
664+
model.BackupSchedule = types.StringPointerValue(instance.BackupSchedule)
665+
}
666+
655667
model.Id = utils.BuildInternalTerraformId(model.ProjectId.ValueString(), region, instanceId)
656668
model.InstanceId = types.StringValue(instanceId)
657669
model.Name = types.StringPointerValue(instance.Name)
658670
model.ACL = aclList
659-
model.BackupSchedule = types.StringPointerValue(instance.BackupSchedule)
660671
model.Flavor = flavorObject
661672
model.Replicas = types.Int64PointerValue(instance.Replicas)
662673
model.Storage = storageObject

‎stackit/internal/services/postgresflex/instance/resource_test.go‎

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,37 @@ func (c *postgresFlexClientMocked) ListFlavorsExecute(_ context.Context, _, _ st
2727

2828
func TestMapFields(t *testing.T) {
2929
const testRegion = "region"
30+
31+
fixtureModel := func(mods ...func(*Model)) Model {
32+
m := Model{
33+
Id: types.StringValue("pid,region,iid"),
34+
InstanceId: types.StringValue("iid"),
35+
ProjectId: types.StringValue("pid"),
36+
Name: types.StringNull(),
37+
ACL: types.ListNull(types.StringType),
38+
BackupSchedule: types.StringNull(),
39+
Flavor: types.ObjectValueMust(flavorTypes, map[string]attr.Value{
40+
"id": types.StringNull(),
41+
"description": types.StringNull(),
42+
"cpu": types.Int64Null(),
43+
"ram": types.Int64Null(),
44+
}),
45+
Replicas: types.Int64Null(),
46+
Storage: types.ObjectValueMust(storageTypes, map[string]attr.Value{
47+
"class": types.StringNull(),
48+
"size": types.Int64Null(),
49+
}),
50+
Version: types.StringNull(),
51+
Region: types.StringValue(testRegion),
52+
}
53+
54+
for _, mod := range mods {
55+
mod(&m)
56+
}
57+
58+
return m
59+
}
60+
3061
tests := []struct {
3162
description string
3263
state Model
@@ -49,27 +80,7 @@ func TestMapFields(t *testing.T) {
4980
&flavorModel{},
5081
&storageModel{},
5182
testRegion,
52-
Model{
53-
Id: types.StringValue("pid,region,iid"),
54-
InstanceId: types.StringValue("iid"),
55-
ProjectId: types.StringValue("pid"),
56-
Name: types.StringNull(),
57-
ACL: types.ListNull(types.StringType),
58-
BackupSchedule: types.StringNull(),
59-
Flavor: types.ObjectValueMust(flavorTypes, map[string]attr.Value{
60-
"id": types.StringNull(),
61-
"description": types.StringNull(),
62-
"cpu": types.Int64Null(),
63-
"ram": types.Int64Null(),
64-
}),
65-
Replicas: types.Int64Null(),
66-
Storage: types.ObjectValueMust(storageTypes, map[string]attr.Value{
67-
"class": types.StringNull(),
68-
"size": types.Int64Null(),
69-
}),
70-
Version: types.StringNull(),
71-
Region: types.StringValue(testRegion),
72-
},
83+
fixtureModel(),
7384
true,
7485
},
7586
{
@@ -262,6 +273,42 @@ func TestMapFields(t *testing.T) {
262273
},
263274
true,
264275
},
276+
{
277+
description: "backup schedule - keep state value when API strips leading zeros",
278+
state: fixtureModel(func(m *Model) {
279+
m.BackupSchedule = types.StringValue("00 00 * * *")
280+
}),
281+
input: &postgresflex.InstanceResponse{
282+
Item: &postgresflex.Instance{
283+
BackupSchedule: utils.Ptr("0 0 * * *"),
284+
},
285+
},
286+
flavor: &flavorModel{},
287+
storage: &storageModel{},
288+
region: testRegion,
289+
expected: fixtureModel(func(m *Model) {
290+
m.BackupSchedule = types.StringValue("00 00 * * *")
291+
}),
292+
isValid: true,
293+
},
294+
{
295+
description: "backup schedule - use updated value from API if cron actually changed",
296+
state: fixtureModel(func(m *Model) {
297+
m.BackupSchedule = types.StringValue("00 01 * * *")
298+
}),
299+
input: &postgresflex.InstanceResponse{
300+
Item: &postgresflex.Instance{
301+
BackupSchedule: utils.Ptr("0 2 * * *"),
302+
},
303+
},
304+
flavor: &flavorModel{},
305+
storage: &storageModel{},
306+
region: testRegion,
307+
expected: fixtureModel(func(m *Model) {
308+
m.BackupSchedule = types.StringValue("0 2 * * *")
309+
}),
310+
isValid: true,
311+
},
265312
{
266313
"nil_response",
267314
Model{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ func (r *clusterResource) Schema(_ context.Context, _ resource.SchemaRequest, re
379379
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,
380380
Computed: true,
381381
PlanModifiers: []planmodifier.String{
382-
stringplanmodifierUtils.UseStateForUnknownIf(utils.StringChanged, "kubernetes_version_min", "sets `UseStateForUnknown` only if `kubernetes_min_version` has not changed"),
382+
stringplanmodifierUtils.UseStateForUnknownIf(stringplanmodifierUtils.StringChanged, "kubernetes_version_min", "sets `UseStateForUnknown` only if `kubernetes_min_version` has not changed"),
383383
},
384384
},
385385
"egress_address_ranges": schema.ListAttribute{
@@ -462,7 +462,7 @@ func (r *clusterResource) Schema(_ context.Context, _ resource.SchemaRequest, re
462462
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,
463463
Computed: true,
464464
PlanModifiers: []planmodifier.String{
465-
stringplanmodifierUtils.UseStateForUnknownIf(utils.StringChanged, "os_version_min", "sets `UseStateForUnknown` only if `os_version_min` has not changed"),
465+
stringplanmodifierUtils.UseStateForUnknownIf(stringplanmodifierUtils.StringChanged, "os_version_min", "sets `UseStateForUnknown` only if `os_version_min` has not changed"),
466466
},
467467
},
468468
"volume_type": schema.StringAttribute{

‎stackit/internal/services/sqlserverflex/instance/resource.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
sqlserverflexUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/sqlserverflex/utils"
13+
stringplanmodifierCustom "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/stringplanmodifier"
1314

1415
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
1516
"github.com/hashicorp/terraform-plugin-framework/attr"
@@ -230,9 +231,8 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
230231
"backup_schedule": schema.StringAttribute{
231232
Description: descriptions["backup_schedule"],
232233
Optional: true,
233-
Computed: true,
234234
PlanModifiers: []planmodifier.String{
235-
stringplanmodifier.UseStateForUnknown(),
235+
stringplanmodifierCustom.CronNormalizationModifier{},
236236
},
237237
},
238238
"flavor": schema.SingleNestedAttribute{
@@ -784,10 +784,11 @@ func mapFields(ctx context.Context, resp *sqlserverflex.GetInstanceResponse, mod
784784
return fmt.Errorf("creating options: %w", core.DiagsToError(diags))
785785
}
786786

787-
simplifiedModelBackupSchedule := utils.SimplifyBackupSchedule(model.BackupSchedule.ValueString())
788-
// If the value returned by the API is different from the one in the model after simplification,
789-
// we update the model so that it causes an error in Terraform
790-
if simplifiedModelBackupSchedule != types.StringPointerValue(instance.BackupSchedule).ValueString() {
787+
// If the API returned "0 0 * * *" but user defined "00 00 * * *" in its config,
788+
// we keep the user's "00 00 * * *" in the state to satisfy Terraform.
789+
backupScheduleApiResp := types.StringPointerValue(instance.BackupSchedule)
790+
if utils.SimplifyCronString(model.BackupSchedule.ValueString()) != utils.SimplifyCronString(backupScheduleApiResp.ValueString()) {
791+
// If the API actually changed it to something else, use the API value
791792
model.BackupSchedule = types.StringPointerValue(instance.BackupSchedule)
792793
}
793794

‎stackit/internal/utils/attributes.go‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,8 @@ import (
77

88
"github.com/hashicorp/terraform-plugin-framework/diag"
99
"github.com/hashicorp/terraform-plugin-framework/path"
10-
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
1110
"github.com/hashicorp/terraform-plugin-framework/types"
1211
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core"
13-
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/int64planmodifier"
14-
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/planmodifiers/stringplanmodifier"
1512
)
1613

1714
type attributeGetter interface {
@@ -47,51 +44,3 @@ func GetTimeFromStringAttribute(ctx context.Context, attributePath path.Path, so
4744

4845
return diags
4946
}
50-
51-
// Int64Changed sets UseStateForUnkown to true if the attribute's planned value matches the current state
52-
func Int64Changed(ctx context.Context, attributeName string, request planmodifier.Int64Request, response *int64planmodifier.UseStateForUnknownFuncResponse) { // nolint:gocritic // function signature required by Terraform
53-
dependencyPath := request.Path.ParentPath().AtName(attributeName)
54-
55-
var attributePlan types.Int64
56-
diags := request.Plan.GetAttribute(ctx, dependencyPath, &attributePlan)
57-
response.Diagnostics.Append(diags...)
58-
if response.Diagnostics.HasError() {
59-
return
60-
}
61-
62-
var attributeState types.Int64
63-
diags = request.State.GetAttribute(ctx, dependencyPath, &attributeState)
64-
response.Diagnostics.Append(diags...)
65-
if response.Diagnostics.HasError() {
66-
return
67-
}
68-
69-
if attributeState == attributePlan {
70-
response.UseStateForUnknown = true
71-
return
72-
}
73-
}
74-
75-
// StringChanged sets UseStateForUnkown to true if the attribute's planned value matches the current state
76-
func StringChanged(ctx context.Context, attributeName string, request planmodifier.StringRequest, response *stringplanmodifier.UseStateForUnknownFuncResponse) { // nolint:gocritic // function signature required by Terraform
77-
dependencyPath := request.Path.ParentPath().AtName(attributeName)
78-
79-
var attributePlan types.String
80-
diags := request.Plan.GetAttribute(ctx, dependencyPath, &attributePlan)
81-
response.Diagnostics.Append(diags...)
82-
if response.Diagnostics.HasError() {
83-
return
84-
}
85-
86-
var attributeState types.String
87-
diags = request.State.GetAttribute(ctx, dependencyPath, &attributeState)
88-
response.Diagnostics.Append(diags...)
89-
if response.Diagnostics.HasError() {
90-
return
91-
}
92-
93-
if attributeState == attributePlan {
94-
response.UseStateForUnknown = true
95-
return
96-
}
97-
}

‎stackit/internal/utils/cron.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package utils
2+
3+
import (
4+
"regexp"
5+
"strings"
6+
)
7+
8+
// SimplifyCronString removes leading 0s from backup schedule numbers (e.g. "00 00 * * *" becomes "0 0 * * *")
9+
// Needed as some API might do it internally and would otherwise cause inconsistent result in Terraform
10+
func SimplifyCronString(cron string) string {
11+
regex := regexp.MustCompile(`0+\d+`) // Matches series of one or more zeros followed by a series of one or more digits
12+
simplifiedCron := regex.ReplaceAllStringFunc(cron, func(match string) string {
13+
simplified := strings.TrimLeft(match, "0")
14+
if simplified == "" {
15+
simplified = "0"
16+
}
17+
return simplified
18+
})
19+
20+
whiteSpaceRegex := regexp.MustCompile(`\s+`)
21+
simplifiedCron = whiteSpaceRegex.ReplaceAllString(simplifiedCron, " ")
22+
23+
return simplifiedCron
24+
}

0 commit comments

Comments
 (0)