Skip to content

Commit c3d98ff

Browse files
rubenhoenlerubenhoenle
authored
fix(postgresflex): prevent state drifts on cron strings (#1328)
relates to STACKITSDK-413
1 parent 2977dba commit c3d98ff

File tree

17 files changed

+380
-179
lines changed

17 files changed

+380
-179
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: new("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: new("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: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,22 @@ import (
99
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
1010
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
1111
"github.com/hashicorp/terraform-plugin-testing/terraform"
12-
"github.com/stackitcloud/stackit-sdk-go/core/utils"
12+
sdkUtils "github.com/stackitcloud/stackit-sdk-go/core/utils"
1313

1414
"github.com/stackitcloud/stackit-sdk-go/core/config"
1515
"github.com/stackitcloud/stackit-sdk-go/services/postgresflex"
1616
"github.com/stackitcloud/stackit-sdk-go/services/postgresflex/wait"
1717
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core"
1818
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/testutil"
19+
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils"
1920
)
2021

2122
// Instance resource data
2223
var instanceResource = map[string]string{
2324
"project_id": testutil.ProjectId,
2425
"name": fmt.Sprintf("tf-acc-%s", acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum)),
2526
"acl": "192.168.0.0/16",
26-
"backup_schedule": "00 16 * * *",
27+
"backup_schedule": "00 16 * * *", // ensure it works properly with leading zeros
2728
"backup_schedule_updated": "00 12 * * *",
2829
"flavor_cpu": "2",
2930
"flavor_ram": "4",
@@ -202,7 +203,7 @@ func TestAccPostgresFlexFlexResource(t *testing.T) {
202203

203204
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "acl.#", "1"),
204205
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "acl.0", instanceResource["acl"]),
205-
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "backup_schedule", instanceResource["backup_schedule"]),
206+
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "backup_schedule", "0 16 * * *"),
206207
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "flavor.id", instanceResource["flavor_id"]),
207208
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "flavor.description", instanceResource["flavor_description"]),
208209
resource.TestCheckResourceAttr("data.stackit_postgresflex_instance.instance", "flavor.cpu", instanceResource["flavor_cpu"]),
@@ -248,7 +249,18 @@ func TestAccPostgresFlexFlexResource(t *testing.T) {
248249
},
249250
ImportState: true,
250251
ImportStateVerify: true,
251-
ImportStateVerifyIgnore: []string{"password"},
252+
ImportStateVerifyIgnore: []string{"password", "backup_schedule"},
253+
ImportStateCheck: func(s []*terraform.InstanceState) error {
254+
if len(s) != 1 {
255+
return fmt.Errorf("expected 1 state, got %d", len(s))
256+
}
257+
258+
if utils.SimplifyCronString(s[0].Attributes["backup_schedule"]) != utils.SimplifyCronString(instanceResource["backup_schedule"]) {
259+
return fmt.Errorf("expected backup_schedule %s, got %s", instanceResource["backup_schedule"], s[0].Attributes["backup_schedule"])
260+
}
261+
262+
return nil
263+
},
252264
},
253265
{
254266
ResourceName: "stackit_postgresflex_user.user",
@@ -354,7 +366,7 @@ func testAccCheckPostgresFlexDestroy(s *terraform.State) error {
354366
if items[i].Id == nil {
355367
continue
356368
}
357-
if utils.Contains(instancesToDestroy, *items[i].Id) {
369+
if sdkUtils.Contains(instancesToDestroy, *items[i].Id) {
358370
err := client.ForceDeleteInstanceExecute(ctx, testutil.ProjectId, testutil.Region, *items[i].Id)
359371
if err != nil {
360372
return fmt.Errorf("deleting instance %s during CheckDestroy: %w", *items[i].Id, err)

‎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 & 4 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"
@@ -231,6 +232,7 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
231232
Optional: true,
232233
Computed: true,
233234
PlanModifiers: []planmodifier.String{
235+
stringplanmodifierCustom.CronNormalizationModifier{},
234236
stringplanmodifier.UseStateForUnknown(),
235237
},
236238
},
@@ -783,10 +785,11 @@ func mapFields(ctx context.Context, resp *sqlserverflex.GetInstanceResponse, mod
783785
return fmt.Errorf("creating options: %w", core.DiagsToError(diags))
784786
}
785787

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() {
788+
// If the API returned "0 0 * * *" but user defined "00 00 * * *" in its config,
789+
// we keep the user's "00 00 * * *" in the state to satisfy Terraform.
790+
backupScheduleApiResp := types.StringPointerValue(instance.BackupSchedule)
791+
if utils.SimplifyCronString(model.BackupSchedule.ValueString()) != utils.SimplifyCronString(backupScheduleApiResp.ValueString()) {
792+
// If the API actually changed it to something else, use the API value
790793
model.BackupSchedule = types.StringPointerValue(instance.BackupSchedule)
791794
}
792795

‎stackit/internal/services/sqlserverflex/sqlserverflex_acc_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ var testConfigVarsMin = config.Variables{
3131
"name": config.StringVariable(fmt.Sprintf("tf-acc-%s", acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum))),
3232
"flavor_cpu": config.IntegerVariable(4),
3333
"flavor_ram": config.IntegerVariable(16),
34-
"flavor_description": config.StringVariable("SQLServer-Flex-4.16-Standard-EU01"),
34+
"flavor_description": config.StringVariable("SQLServer-Flex-4.16-Single-Standard-EU01"),
3535
"replicas": config.IntegerVariable(1),
3636
"flavor_id": config.StringVariable("4.16-Single"),
3737
"username": config.StringVariable(fmt.Sprintf("tf-acc-user-%s", acctest.RandStringFromCharSet(7, acctest.CharSetAlpha))),

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

0 commit comments

Comments
 (0)