-
Notifications
You must be signed in to change notification settings - Fork 190
chore: Updates mongodbatlas_advanced_cluster
(SDKv2 & TPF) to consume selected processArgs fields from the createCluster/updateCluster APIs
#3231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
6a7b365
e26f2f1
be7e3ab
265b6c0
fbb5a9e
a8435cd
5a8fd9f
1b48237
3990691
d836ff2
2c17d0e
444778f
18523bf
8e9ab05
9f85896
ff69fc8
dcf2c23
1cd04a4
8dc6f53
5262ec8
1335955
092c0f3
5e9889b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -530,7 +530,9 @@ func expandBiConnectorConfig(d *schema.ResourceData) *admin.BiConnector { | |
return nil | ||
} | ||
|
||
func flattenProcessArgs(p20240530 *admin20240530.ClusterDescriptionProcessArgs, p *admin.ClusterDescriptionProcessArgs20240805) []map[string]any { | ||
func flattenProcessArgs(p20240530 *admin20240530.ClusterDescriptionProcessArgs, | ||
p *admin.ClusterDescriptionProcessArgs20240805, | ||
clusterAdvConfig *admin.ApiAtlasClusterAdvancedConfiguration) []map[string]any { | ||
if p20240530 == nil { | ||
return nil | ||
} | ||
|
@@ -540,7 +542,6 @@ func flattenProcessArgs(p20240530 *admin20240530.ClusterDescriptionProcessArgs, | |
"default_write_concern": p20240530.GetDefaultWriteConcern(), | ||
"fail_index_key_too_long": p20240530.GetFailIndexKeyTooLong(), | ||
"javascript_enabled": p20240530.GetJavascriptEnabled(), | ||
"minimum_enabled_tls_protocol": p20240530.GetMinimumEnabledTlsProtocol(), | ||
"no_table_scan": p20240530.GetNoTableScan(), | ||
"oplog_size_mb": p20240530.GetOplogSizeMB(), | ||
"oplog_min_retention_hours": p20240530.GetOplogMinRetentionHours(), | ||
|
@@ -559,8 +560,12 @@ func flattenProcessArgs(p20240530 *admin20240530.ClusterDescriptionProcessArgs, | |
if v := p.DefaultMaxTimeMS; v != nil { | ||
flattenedProcessArgs[0]["default_max_time_ms"] = p.GetDefaultMaxTimeMS() | ||
} | ||
flattenedProcessArgs[0]["tls_cipher_config_mode"] = p.GetTlsCipherConfigMode() | ||
flattenedProcessArgs[0]["custom_openssl_cipher_config_tls12"] = p.GetCustomOpensslCipherConfigTls12() | ||
} | ||
|
||
if clusterAdvConfig != nil { | ||
flattenedProcessArgs[0]["tls_cipher_config_mode"] = clusterAdvConfig.GetTlsCipherConfigMode() | ||
flattenedProcessArgs[0]["custom_openssl_cipher_config_tls12"] = clusterAdvConfig.GetCustomOpensslCipherConfigTls12() | ||
flattenedProcessArgs[0]["minimum_enabled_tls_protocol"] = clusterAdvConfig.GetMinimumEnabledTlsProtocol() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can it happen that minimum_enabled_tls_protocol exists in p20240530 but not in clusterAdvConfig? in case it makes sense to keep reading it from p20240530, and override if present at clusterAdvConfig There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated |
||
} | ||
|
||
return flattenedProcessArgs | ||
|
@@ -843,6 +848,32 @@ func flattenAdvancedReplicationSpecAutoScaling(apiObject *admin.AdvancedAutoScal | |
return tfList | ||
} | ||
|
||
func expandClusterAdvancedConfiguration(d *schema.ResourceData) *admin.ApiAtlasClusterAdvancedConfiguration { | ||
res := admin.ApiAtlasClusterAdvancedConfiguration{} | ||
|
||
if ac, ok := d.GetOk("advanced_configuration"); ok { | ||
if aclist, ok := ac.([]any); ok && len(aclist) > 0 { | ||
p := aclist[0].(map[string]any) | ||
|
||
if _, ok := d.GetOkExists("advanced_configuration.0.minimum_enabled_tls_protocol"); ok { | ||
res.MinimumEnabledTlsProtocol = conversion.StringPtr(cast.ToString(p["minimum_enabled_tls_protocol"])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: cast |
||
} | ||
|
||
if _, ok := d.GetOkExists("advanced_configuration.0.tls_cipher_config_mode"); ok { | ||
res.TlsCipherConfigMode = conversion.StringPtr(cast.ToString(p["tls_cipher_config_mode"])) | ||
} | ||
|
||
if _, ok := d.GetOkExists("advanced_configuration.0.custom_openssl_cipher_config_tls12"); ok { | ||
tmp := conversion.ExpandStringListFromSetSchema(d.Get("advanced_configuration.0.custom_openssl_cipher_config_tls12").(*schema.Set)) | ||
res.CustomOpensslCipherConfigTls12 = &tmp | ||
} | ||
|
||
return &res | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func expandProcessArgs(d *schema.ResourceData, p map[string]any, mongodbMajorVersion *string) (admin20240530.ClusterDescriptionProcessArgs, admin.ClusterDescriptionProcessArgs20240805) { | ||
res20240530 := admin20240530.ClusterDescriptionProcessArgs{} | ||
res := admin.ClusterDescriptionProcessArgs20240805{} | ||
|
@@ -863,10 +894,6 @@ func expandProcessArgs(d *schema.ResourceData, p map[string]any, mongodbMajorVer | |
res20240530.JavascriptEnabled = conversion.Pointer(cast.ToBool(p["javascript_enabled"])) | ||
} | ||
|
||
if _, ok := d.GetOkExists("advanced_configuration.0.minimum_enabled_tls_protocol"); ok { | ||
res20240530.MinimumEnabledTlsProtocol = conversion.StringPtr(cast.ToString(p["minimum_enabled_tls_protocol"])) | ||
} | ||
|
||
if _, ok := d.GetOkExists("advanced_configuration.0.no_table_scan"); ok { | ||
res20240530.NoTableScan = conversion.Pointer(cast.ToBool(p["no_table_scan"])) | ||
} | ||
|
@@ -917,15 +944,6 @@ func expandProcessArgs(d *schema.ResourceData, p map[string]any, mongodbMajorVer | |
log.Print(ErrorDefaultMaxTimeMinVersion) | ||
} | ||
} | ||
|
||
if _, ok := d.GetOkExists("advanced_configuration.0.tls_cipher_config_mode"); ok { | ||
res.TlsCipherConfigMode = conversion.StringPtr(cast.ToString(p["tls_cipher_config_mode"])) | ||
} | ||
|
||
if _, ok := d.GetOkExists("advanced_configuration.0.custom_openssl_cipher_config_tls12"); ok { | ||
tmp := conversion.ExpandStringListFromSetSchema(d.Get("advanced_configuration.0.custom_openssl_cipher_config_tls12").(*schema.Set)) | ||
res.CustomOpensslCipherConfigTls12 = &tmp | ||
} | ||
return res20240530, res | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -458,9 +458,10 @@ func resourceCreate(ctx context.Context, d *schema.ResourceData, meta any) diag. | |
} | ||
|
||
params := &admin.ClusterDescription20240805{ | ||
Name: conversion.StringPtr(cast.ToString(d.Get("name"))), | ||
ClusterType: conversion.StringPtr(cast.ToString(d.Get("cluster_type"))), | ||
ReplicationSpecs: replicationSpecs, | ||
Name: conversion.StringPtr(cast.ToString(d.Get("name"))), | ||
ClusterType: conversion.StringPtr(cast.ToString(d.Get("cluster_type"))), | ||
ReplicationSpecs: replicationSpecs, | ||
AdvancedConfiguration: expandClusterAdvancedConfiguration(d), | ||
} | ||
|
||
if v, ok := d.GetOk("backup_enabled"); ok { | ||
|
@@ -688,7 +689,7 @@ func resourceRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Di | |
return diag.FromErr(fmt.Errorf(errorConfigRead, clusterName, err)) | ||
} | ||
|
||
if err := d.Set("advanced_configuration", flattenProcessArgs(processArgs20240530, processArgs)); err != nil { | ||
if err := d.Set("advanced_configuration", flattenProcessArgs(processArgs20240530, processArgs, cluster.AdvancedConfiguration)); err != nil { | ||
return diag.FromErr(fmt.Errorf(ErrorClusterAdvancedSetting, "advanced_configuration", clusterName, err)) | ||
} | ||
|
||
|
@@ -819,6 +820,11 @@ func setRootFields(d *schema.ResourceData, cluster *admin.ClusterDescription2024 | |
return diag.FromErr(fmt.Errorf(ErrorClusterAdvancedSetting, "pinned_fcv", clusterName, err)) | ||
} | ||
|
||
// TODO: set adv_config | ||
// if err := d.Set("pinned_fcv", FlattenPinnedFCV(cluster)); err != nil { | ||
// return diag.FromErr(fmt.Errorf(ErrorClusterAdvancedSetting, "pinned_fcv", clusterName, err)) | ||
// } | ||
|
||
return nil | ||
} | ||
|
||
|
@@ -939,7 +945,7 @@ func resourceUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag. | |
} | ||
waitOnUpdate = true | ||
} | ||
if d.HasChange("replica_set_scaling_strategy") || d.HasChange("redact_client_log_data") || d.HasChange("config_server_management_mode") { | ||
if d.HasChange("replica_set_scaling_strategy") || d.HasChange("redact_client_log_data") || d.HasChange("config_server_management_mode") || d.HasChange("advanced_configuration") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AgustinBettati can you confirm if this should be okay or we should update this in the above call (connV220240530.ClustersApi.UpdateCluster #L943)? In that case, will need the SDK for that to be updated as well to support the new AdvancedConfiguration attribute. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a concern here, as we are still respecting what the comment below mentions when calling the latest API version.
In this case I understand we only add the advancedConfiguration property but no replicationSpecs, meaning no risk. |
||
request := new(admin.ClusterDescription20240805) | ||
if d.HasChange("replica_set_scaling_strategy") { | ||
request.ReplicaSetScalingStrategy = conversion.Pointer(d.Get("replica_set_scaling_strategy").(string)) | ||
|
@@ -950,6 +956,12 @@ func resourceUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag. | |
if d.HasChange("config_server_management_mode") { | ||
request.ConfigServerManagementMode = conversion.StringPtr(d.Get("config_server_management_mode").(string)) | ||
} | ||
if d.HasChange("advanced_configuration") { | ||
if aclist, ok := d.Get("advanced_configuration").([]any); ok && len(aclist) > 0 { | ||
request.AdvancedConfiguration = expandClusterAdvancedConfiguration(d) | ||
} | ||
} | ||
|
||
// can call latest API (2024-10-23 or newer) as replications specs (with nested autoscaling property) is not specified | ||
if _, _, err := connV2.ClustersApi.UpdateCluster(ctx, projectID, clusterName, request).Execute(); err != nil { | ||
return diag.FromErr(fmt.Errorf(errorUpdate, clusterName, err)) | ||
|
@@ -1148,6 +1160,11 @@ func updateRequest(ctx context.Context, d *schema.ResourceData, projectID, clust | |
if d.HasChange("config_server_management_mode") { | ||
cluster.ConfigServerManagementMode = conversion.StringPtr(d.Get("config_server_management_mode").(string)) | ||
} | ||
if d.HasChange("advanced_configuration") { | ||
if aclist, ok := d.Get("advanced_configuration").([]any); ok && len(aclist) > 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means that removing advanced_configuration from the config will never update? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the behavior today as well right? since it's O/C, or did you mean something else? |
||
cluster.AdvancedConfiguration = expandClusterAdvancedConfiguration(d) | ||
} | ||
} | ||
|
||
return cluster, nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
although not an absolute requirement for Read, but still fetching these from cluster response (instead of /processArgs) for consistency.