Skip to content

Commit 276189f

Browse files
rubenhoenlerubenhoenle
committed
fix(postgresflex): prevent state drifts on cron strings
relates to STACKITSDK-413
1 parent a44cfba commit 276189f

File tree

16 files changed

+363
-176
lines changed

16 files changed

+363
-176
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
@@ -533,23 +533,23 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
533533
Optional: true,
534534
Computed: true,
535535
PlanModifiers: []planmodifier.Int64{
536-
int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days", "sets `UseStateForUnknown` only if `metrics_retention_days` has not changed"),
536+
int64planmodifier.UseStateForUnknownIf(int64planmodifier.Int64Changed, "metrics_retention_days", "sets `UseStateForUnknown` only if `metrics_retention_days` has not changed"),
537537
},
538538
},
539539
"metrics_retention_days_5m_downsampling": schema.Int64Attribute{
540540
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`.",
541541
Optional: true,
542542
Computed: true,
543543
PlanModifiers: []planmodifier.Int64{
544-
int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days_5m_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_5m_downsampling` has not changed"),
544+
int64planmodifier.UseStateForUnknownIf(int64planmodifier.Int64Changed, "metrics_retention_days_5m_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_5m_downsampling` has not changed"),
545545
},
546546
},
547547
"metrics_retention_days_1h_downsampling": schema.Int64Attribute{
548548
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`.",
549549
Optional: true,
550550
Computed: true,
551551
PlanModifiers: []planmodifier.Int64{
552-
int64planmodifier.UseStateForUnknownIf(utils.Int64Changed, "metrics_retention_days_1h_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_1h_downsampling` has not changed"),
552+
int64planmodifier.UseStateForUnknownIf(int64planmodifier.Int64Changed, "metrics_retention_days_1h_downsampling", "sets `UseStateForUnknown` only if `metrics_retention_days_1h_downsampling` has not changed"),
553553
},
554554
},
555555
"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
@@ -26,6 +26,37 @@ func (c *postgresFlexClientMocked) ListFlavorsExecute(_ context.Context, _, _ st
2626

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

‎stackit/internal/services/postgresflex/postgresflex_acc_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func TestAccPostgresFlexFlexResource(t *testing.T) {
202202

203203
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "acl.#", "1"),
204204
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "acl.0", instanceResource["acl"]),
205-
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "backup_schedule", instanceResource["backup_schedule"]),
205+
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "backup_schedule", "0 16 * * *"),
206206
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "flavor.id", instanceResource["flavor_id"]),
207207
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "flavor.description", instanceResource["flavor_description"]),
208208
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "flavor.cpu", instanceResource["flavor_cpu"]),

‎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"
@@ -229,9 +230,8 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
229230
"backup_schedule": schema.StringAttribute{
230231
Description: descriptions["backup_schedule"],
231232
Optional: true,
232-
Computed: true,
233233
PlanModifiers: []planmodifier.String{
234-
stringplanmodifier.UseStateForUnknown(),
234+
stringplanmodifierCustom.CronNormalizationModifier{},
235235
},
236236
},
237237
"flavor": schema.SingleNestedAttribute{
@@ -783,10 +783,11 @@ func mapFields(ctx context.Context, resp *sqlserverflex.GetInstanceResponse, mod
783783
return fmt.Errorf("creating options: %w", core.DiagsToError(diags))
784784
}
785785

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

‎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)