Skip to content

Commit 8c9d6c8

Browse files
authored
chore: Avoids non-empty plan after removing advanced_configuration block removal (#3134)
* show failing test * fix: optional-only problem in advanced_configuration * not safe to use state for custom_openssl_cipher_config_tls12 when `tls_cipher_config_mode` is changed * remove: unnecessary null value, can rely on API empty list * address PR comments * chore: update comment * doc: Add more comments to clarify behavior
1 parent ba63107 commit 8c9d6c8

File tree

6 files changed

+184
-306
lines changed

6 files changed

+184
-306
lines changed

internal/service/advancedcluster/resource_advanced_cluster_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,10 @@ func TestAccMockableAdvancedCluster_replicasetAdvConfigUpdate(t *testing.T) {
12711271
Config: configBasicReplicaset(t, projectID, clusterName, fullUpdate),
12721272
Check: checksUpdate,
12731273
},
1274+
{
1275+
Config: configBasicReplicaset(t, projectID, clusterName, ""),
1276+
Check: checks,
1277+
},
12741278
acc.TestStepImportCluster(resourceName),
12751279
},
12761280
})

internal/service/advancedcluster/testdata/TestAccMockableAdvancedCluster_replicasetAdvConfigUpdate.yaml

Lines changed: 164 additions & 300 deletions
Large diffs are not rendered by default.

internal/service/advancedclustertpf/model_ClusterDescriptionProcessArgs20240805.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,16 @@ func AddAdvancedConfig(ctx context.Context, tfModel *TFModel, input *admin.Clust
5151
}
5252
}
5353

54-
advancedConfig.CustomOpensslCipherConfigTls12 = customOpensslCipherConfigTLS12(ctx, diags, input) // required to handle move state
54+
advancedConfig.CustomOpensslCipherConfigTls12 = customOpensslCipherConfigTLS12(ctx, diags, input)
5555
objType, diagsLocal := types.ObjectValueFrom(ctx, AdvancedConfigurationObjType.AttrTypes, advancedConfig)
5656
diags.Append(diagsLocal...)
5757
tfModel.AdvancedConfiguration = objType
5858
}
5959

6060
func customOpensslCipherConfigTLS12(ctx context.Context, diags *diag.Diagnostics, processArgs *admin.ClusterDescriptionProcessArgs20240805) types.Set {
61-
if processArgs == nil || len(*processArgs.CustomOpensslCipherConfigTls12) == 0 {
61+
if processArgs == nil {
6262
return types.SetNull(types.StringType)
6363
}
64-
6564
customOpensslCipherConfigTLS12, d := types.SetValueFrom(ctx, types.StringType, processArgs.CustomOpensslCipherConfigTls12)
6665
diags.Append(d...)
6766
return customOpensslCipherConfigTLS12

internal/service/advancedclustertpf/model_to_ClusterDescriptionProcessArgs20240805.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,13 @@ func NewAtlasReqAdvancedConfiguration(ctx context.Context, objInput *types.Objec
2222
diags.Append(localDiags...)
2323
return resp
2424
}
25+
changeStreamOptionsPreAndPostImagesExpireAfterSeconds := conversion.NilForUnknown(input.ChangeStreamOptionsPreAndPostImagesExpireAfterSeconds, conversion.Int64PtrToIntPtr(input.ChangeStreamOptionsPreAndPostImagesExpireAfterSeconds.ValueInt64Pointer()))
26+
if changeStreamOptionsPreAndPostImagesExpireAfterSeconds == nil {
27+
// in case the user removes the value, we should set it to -1, a special value used by the backend to use its default behavior
28+
changeStreamOptionsPreAndPostImagesExpireAfterSeconds = conversion.Pointer(-1)
29+
}
2530
return &admin.ClusterDescriptionProcessArgs20240805{
26-
ChangeStreamOptionsPreAndPostImagesExpireAfterSeconds: conversion.NilForUnknown(input.ChangeStreamOptionsPreAndPostImagesExpireAfterSeconds, conversion.Int64PtrToIntPtr(input.ChangeStreamOptionsPreAndPostImagesExpireAfterSeconds.ValueInt64Pointer())),
31+
ChangeStreamOptionsPreAndPostImagesExpireAfterSeconds: changeStreamOptionsPreAndPostImagesExpireAfterSeconds,
2732
DefaultWriteConcern: conversion.NilForUnknown(input.DefaultWriteConcern, input.DefaultWriteConcern.ValueStringPointer()),
2833
JavascriptEnabled: conversion.NilForUnknown(input.JavascriptEnabled, input.JavascriptEnabled.ValueBoolPointer()),
2934
MinimumEnabledTlsProtocol: conversion.NilForUnknown(input.MinimumEnabledTlsProtocol, input.MinimumEnabledTlsProtocol.ValueStringPointer()),

internal/service/advancedclustertpf/plan_modifier.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ import (
1313
)
1414

1515
var (
16+
// Change mappings uses `attribute_name`, it doesn't care about the nested level.
1617
attributeRootChangeMapping = map[string][]string{
1718
"disk_size_gb": {}, // disk_size_gb can be change at any level/spec
1819
"replication_specs": {},
1920
"mongo_db_major_version": {"mongo_db_version"},
21+
"tls_cipher_config_mode": {"custom_openssl_cipher_config_tls12"},
2022
}
2123
attributeReplicationSpecChangeMapping = map[string][]string{
2224
// All these fields can exist in specs that are computed, therefore, it is not safe to use them when they have changed.

internal/service/advancedclustertpf/schema.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,12 +440,14 @@ func AdvancedConfigurationSchema(ctx context.Context) schema.SingleNestedAttribu
440440
Computed: true,
441441
Optional: true,
442442
MarkdownDescription: "advanced_configuration", // TODO: add description
443+
// Avoid adding optional-only attributes, if the block is removed and attributes are not null in the state we get unintentional plan changes after apply.
444+
// Avoid computed-optional with Default, if the block is removed and the attribute Default != state value we get unintentional plan changes after apply.
443445
Attributes: map[string]schema.Attribute{
444446
"change_stream_options_pre_and_post_images_expire_after_seconds": schema.Int64Attribute{
445-
Optional: true,
447+
Optional: true,
448+
// Default set in NewAtlasReqAdvancedConfiguration
446449
Computed: true,
447450
MarkdownDescription: "The minimum pre- and post-image retention time in seconds.",
448-
Default: int64default.StaticInt64(-1), // in case the user removes the value, we should set it to -1, a special value used by the backend to use its default behavior
449451
PlanModifiers: []planmodifier.Int64{
450452
PlanMustUseMongoDBVersion(7.0, EqualOrHigher),
451453
},
@@ -512,13 +514,15 @@ func AdvancedConfigurationSchema(ctx context.Context) schema.SingleNestedAttribu
512514
MarkdownDescription: "fail_index_key_too_long", // TODO: add description
513515
},
514516
"default_max_time_ms": schema.Int64Attribute{
517+
Computed: true,
515518
Optional: true,
516519
MarkdownDescription: "Default time limit in milliseconds for individual read operations to complete. This parameter is supported only for MongoDB version 8.0 and above.",
517520
PlanModifiers: []planmodifier.Int64{
518521
PlanMustUseMongoDBVersion(8.0, EqualOrHigher),
519522
},
520523
},
521524
"custom_openssl_cipher_config_tls12": schema.SetAttribute{
525+
Computed: true,
522526
Optional: true,
523527
ElementType: types.StringType,
524528
MarkdownDescription: "The custom OpenSSL cipher suite list for TLS 1.2. This field is only valid when `tls_cipher_config_mode` is set to `CUSTOM`.",

0 commit comments

Comments
 (0)