feat: Supports `redact_client_log_data` in `mongodbatlas_cluster` #2601

lantoli · 2024-09-16T16:04:33Z

Description

Supports redact_client_log_data in mongodbatlas_cluster.

Link to any related issue(s): CLOUDP-273790

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

github-actions · 2024-09-16T16:07:23Z

APIx bot: a message has been sent to Docs Slack channel

carriecwk

LGTM

internal/service/cluster/resource_cluster.go

AgustinBettati

minor improvements but overall LGTM

.changelog/2601.txt

AgustinBettati · 2024-09-18T14:33:31Z

internal/service/cluster/new_atlas.go

+	stateConf := &retry.StateChangeConf{
+		Pending:    []string{"CREATING", "UPDATING", "REPAIRING"},
+		Target:     []string{"IDLE"},
+		Refresh:    ResourceClusterRefreshFunc(ctx, clusterName, projectID, conn),
+		Timeout:    timeout,
+		MinTimeout: 30 * time.Second,
+		Delay:      1 * time.Minute,
+	}


I assume this was copied from existing update code, could we extract to a function just to make sure we have state transitions and timeout is single place?

I guess we could use the CreateStateChangeConfig from the advancedcluster package?

thank you both, I've followed @EspenAlbert suggestion and changed here 16f12f3

internal/service/cluster/new_atlas.go

AgustinBettati · 2024-09-18T14:38:18Z

internal/service/cluster/resource_cluster_test.go

+					resource.TestCheckResourceAttr(resourceName, "redact_client_log_data", "false"),
+					resource.TestCheckResourceAttr(dataSourceName, "redact_client_log_data", "false"),
+					resource.TestCheckResourceAttr(dataSourcePluralName, "results.0.redact_client_log_data", "false"),


acc.CheckRSAndDS can let you define this in a single call

nice, thanks! changed in cef542a

internal/service/cluster/resource_cluster.go

EspenAlbert · 2024-09-18T14:47:47Z

internal/service/cluster/resource_cluster_test.go

+				Config: configRedactClientLogData(projectID, clusterName, nil),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					checkExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "redact_client_log_data", "false"),


I think if you duplicate this step and put it after step 3 the test will fail. Since it is optional-computed removing the value will see no changes and it will remain true

yes, I think that's what we're doing in these cases. Removing the field won't be flagged as a change so current value will stay

This reverts commit ce69fa1.

AgustinBettati

LGTM, thanks for follow ups

AgustinBettati · 2024-09-18T17:38:47Z

internal/service/cloudbackupsnapshot/resource_cloud_backup_snapshot.go

+	stateConf := advancedcluster.CreateStateChangeConfig(ctx, connV2, groupID, clusterName, 15*time.Minute)
+	if _, err := stateConf.WaitForStateContext(ctx); err != nil {


I know this is unrelated but very strange why we verify cluster state transition as part of creating a backup snapshot

true, I can imagine it's to support when a user defines a cluster and a snapshot without depends_on between them

EspenAlbert

Nice! Thanks for addressing all the comments!

* feat: Supports `redact_client_log_data` in `mongodbatlas_advanced_cluster` (#2600) * use SDK Preview * add redact_client_log_data to schema * plural ds * resource * tests * changelog * feat: Supports `redact_client_log_data` in `mongodbatlas_cluster` (#2601) * doc * changelog * new atlas functions * data sources * resource * tests * rename CRUD functions * wait for cluster changes * apply feedback for changelog * use acc.CheckRSAndDS * fix ret * additional comment in cluster * Revert "additional comment in cluster" This reverts commit ce69fa1. * additional comment in cluster * refactor StateChangeConf * remove ResourceClusterRefreshFunc * attribute not necessary * fix update in cluster * fix plural ds tests

doc

ca58bbb

github-actions bot added the enhancement label Sep 16, 2024

changelog

22b96af

lantoli marked this pull request as ready for review September 16, 2024 16:06

lantoli requested a review from a team as a code owner September 16, 2024 16:06

lantoli mentioned this pull request Sep 16, 2024

feat: Supports redact_client_log_data in mongodbatlas_advanced_cluster #2600

Merged

13 tasks

carriecwk approved these changes Sep 16, 2024

View reviewed changes

lantoli added 4 commits September 18, 2024 11:37

new atlas functions

09867e8

data sources

8d585a6

resource

d5b1a6c

tests

b8e3640

lantoli requested a review from a team as a code owner September 18, 2024 09:38

lantoli added 2 commits September 18, 2024 12:12

rename CRUD functions

2b1fbcb

wait for cluster changes

18b01ab

EspenAlbert reviewed Sep 18, 2024

View reviewed changes

internal/service/cluster/resource_cluster.go Show resolved Hide resolved

AgustinBettati approved these changes Sep 18, 2024

View reviewed changes

EspenAlbert reviewed Sep 18, 2024

View reviewed changes

internal/service/cluster/resource_cluster.go Outdated Show resolved Hide resolved

EspenAlbert reviewed Sep 18, 2024

View reviewed changes

lantoli added 8 commits September 18, 2024 17:36

apply feedback for changelog

fcc8fa3

use acc.CheckRSAndDS

cef542a

fix ret

9aef32a

additional comment in cluster

ce69fa1

Revert "additional comment in cluster"

1993f46

This reverts commit ce69fa1.

additional comment in cluster

511768a

refactor StateChangeConf

16f12f3

remove ResourceClusterRefreshFunc

99264f2

AgustinBettati approved these changes Sep 18, 2024

View reviewed changes

attribute not necessary

5cb42bd

EspenAlbert approved these changes Sep 19, 2024

View reviewed changes

lantoli merged commit 7ae52b0 into CLOUDP-270155-redaction-dev Sep 19, 2024
34 of 35 checks passed

lantoli deleted the CLOUDP-273790_redact_cluster branch September 19, 2024 09:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Supports `redact_client_log_data` in `mongodbatlas_cluster` #2601

feat: Supports `redact_client_log_data` in `mongodbatlas_cluster` #2601

lantoli commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024

carriecwk left a comment

AgustinBettati left a comment

AgustinBettati Sep 18, 2024

EspenAlbert Sep 18, 2024

lantoli Sep 18, 2024

AgustinBettati Sep 18, 2024

lantoli Sep 18, 2024

EspenAlbert Sep 18, 2024

lantoli Sep 18, 2024

AgustinBettati left a comment

AgustinBettati Sep 18, 2024

lantoli Sep 18, 2024

EspenAlbert left a comment

		stateConf := advancedcluster.CreateStateChangeConfig(ctx, connV2, groupID, clusterName, 15*time.Minute)
		if _, err := stateConf.WaitForStateContext(ctx); err != nil {

feat: Supports redact_client_log_data in mongodbatlas_cluster #2601

feat: Supports redact_client_log_data in mongodbatlas_cluster #2601

Conversation

lantoli commented Sep 16, 2024 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

github-actions bot commented Sep 16, 2024

carriecwk left a comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EspenAlbert left a comment

Choose a reason for hiding this comment

feat: Supports `redact_client_log_data` in `mongodbatlas_cluster` #2601

feat: Supports `redact_client_log_data` in `mongodbatlas_cluster` #2601

lantoli commented Sep 16, 2024 •

edited

Loading