fix: Handles updates of `mongodbatlas_global_cluster_config` #3060

oarbusi · 2025-02-12T11:49:52Z

Description

Reverts #2282 to be able to handle updates of mongodbatlas_global_cluster_config but with the following adaptations:

error if an existing managed_namespace entry is being modified. We can manage collection and db fields to do so.
for new managed_namespace entries call the POST for each of them
for deleted managed_namespace entries call the DELETE for each of them
add new custom_zone_mappings and call the POST for each of them
only allow custom_zone_mappings deletion if all of them are being deleted

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

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

marcosuma · 2025-02-12T12:38:46Z

internal/service/globalclusterconfig/resource_global_cluster_config.go

+			if newEntry, exists := newMap[key]; exists {
+				// Modification detected: key exists but value differs.
+				if !reflect.DeepEqual(oldEntry, newEntry) {
+					return diag.FromErr(fmt.Errorf("managed namespace for collection '%s' in db '%s' cannot be modified; remove it and add a new entry instead", oldEntry["collection"], oldEntry["db"]))


remove it and add a new entry is incorrect. They simply can't change it and we should ask upstream updates on how to handle this.

I removed that part of the error message. But I want to clarify that it is possible, even if not recommended, to delete and create if user wants to

Upstream confirmed that it's not recommended to drop and re-create to update, but possible if the user wants to do it. This is hard to avoid if we want to allow deleting and creating managed_namespaces. Which as far as I know we should allow

internal/service/globalclusterconfig/resource_global_cluster_config.go

nikhil-mongo · 2025-02-13T04:49:01Z

I have tested this from the TSE perspective and below are the results:

Adding 2 managed namespaces: WORKS
Adding 1 more managed namespace: WORKS
Removing 1 managed namespace: WORKS
- However, please note that removing a managed namespace only removes the Global Writes configuration. It still keeps the shard_key and the associated index in, which is acceptable.
Do not allow modifying any of the existing namespace(eg. modifying is_shard_key_unique): WORKS

docs/resources/global_cluster_config.md

Co-authored-by: Marco Suma <[email protected]>

github-actions · 2025-02-13T08:17:56Z

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

lantoli · 2025-02-13T10:39:58Z

.changelog/3060.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/mongodbatlas_global_cluster_config: Supports update operation


I think it would be easier to review if this was done in two PRs, one with the revert without any additional change, next PR with the changes, so we can spot better the real changes after the revert

internal/service/globalclusterconfig/resource_global_cluster_config_test.go

internal/service/globalclusterconfig/resource_global_cluster_config.go

docs/resources/global_cluster_config.md

internal/service/globalclusterconfig/resource_global_cluster_config.go

maastha · 2025-02-13T16:24:34Z

internal/service/globalclusterconfig/resource_global_cluster_config.go

+		if isShardKeyUnique, okShard := mn["is_shard_key_unique"]; okShard {
+			addManagedNamespace.IsShardKeyUnique = conversion.Pointer[bool](isShardKeyUnique.(bool))
+		}
+		_, _, err := connV2.GlobalClustersApi.CreateManagedNamespace(ctx, projectID, clusterName, addManagedNamespace).Execute()


What happens in case this errors out for a particular namespace? Right now I think it would just fail right away and not complete the loop. Should we instead capture errors from all calls and fail once?
Same for deleting namespaces and adding custom zone mappings

I think failing right away is okay, but I haven't thought about the alternative or if we should complete the loop in case of a single failure. No strong opinion from me on this

I'm okay failing right away. Only thing to consider is users might run follow-up updates in those cases, and as long as that works as expected and doesn't run into any state inconsistencies this should be fine.

internal/service/globalclusterconfig/resource_global_cluster_config.go

AgustinBettati

LGTM

internal/service/globalclusterconfig/resource_global_cluster_config.go

…onfig.go Co-authored-by: Leo Antoli <[email protected]>

docs/resources/global_cluster_config.md

Co-authored-by: John Williams <[email protected]>

* master: chore: Updates CHANGELOG.md for #3060 fix: Handles updates of `mongodbatlas_global_cluster_config` (#3060) chore: Updates CHANGELOG.md for #3069 fix: Handle the deleted on the backend case correctly for `mongodbatlas_database_user` resource (#3069) build(deps): bump go.mongodb.org/atlas-sdk (#3082)

* handle update * change error message * changelog * docs change * test changes * Update docs/resources/global_cluster_config.md Co-authored-by: Marco Suma <[email protected]> * reference API * add partial and total removal of custom_zone_mappings in test * refactor update * pr suggestions * Update internal/service/globalclusterconfig/resource_global_cluster_config.go Co-authored-by: Leo Antoli <[email protected]> * Update docs/resources/global_cluster_config.md Co-authored-by: John Williams <[email protected]> --------- Co-authored-by: Marco Suma <[email protected]> Co-authored-by: Leo Antoli <[email protected]> Co-authored-by: John Williams <[email protected]>

handle update

6e179c0

github-actions bot added the bug label Feb 12, 2025

marcosuma reviewed Feb 12, 2025

View reviewed changes

internal/service/globalclusterconfig/resource_global_cluster_config.go Show resolved Hide resolved

oarbusi added 4 commits February 12, 2025 14:56

change error message

fcc2ef2

changelog

5c697be

docs change

7cf1ee1

test changes

948af3f

marcosuma reviewed Feb 13, 2025

View reviewed changes

docs/resources/global_cluster_config.md Outdated Show resolved Hide resolved

oarbusi and others added 2 commits February 13, 2025 09:14

Update docs/resources/global_cluster_config.md

1eb3538

Co-authored-by: Marco Suma <[email protected]>

reference API

86de531

oarbusi marked this pull request as ready for review February 13, 2025 08:17

oarbusi requested review from a team as code owners February 13, 2025 08:17

lantoli reviewed Feb 13, 2025

View reviewed changes

internal/service/globalclusterconfig/resource_global_cluster_config_test.go Show resolved Hide resolved

lantoli reviewed Feb 13, 2025

View reviewed changes

internal/service/globalclusterconfig/resource_global_cluster_config.go Show resolved Hide resolved

EspenAlbert reviewed Feb 13, 2025

View reviewed changes

docs/resources/global_cluster_config.md Outdated Show resolved Hide resolved

EspenAlbert reviewed Feb 13, 2025

View reviewed changes

internal/service/globalclusterconfig/resource_global_cluster_config.go Outdated Show resolved Hide resolved

oarbusi added 2 commits February 13, 2025 12:52

add partial and total removal of custom_zone_mappings in test

8caeb9a

refactor update

878a30d

maastha reviewed Feb 13, 2025

View reviewed changes

AgustinBettati reviewed Feb 13, 2025

View reviewed changes

internal/service/globalclusterconfig/resource_global_cluster_config.go Show resolved Hide resolved

AgustinBettati reviewed Feb 14, 2025

View reviewed changes

internal/service/globalclusterconfig/resource_global_cluster_config.go Show resolved Hide resolved

AgustinBettati approved these changes Feb 14, 2025

View reviewed changes

internal/service/globalclusterconfig/resource_global_cluster_config.go Outdated Show resolved Hide resolved

internal/service/globalclusterconfig/resource_global_cluster_config.go Outdated Show resolved Hide resolved

pr suggestions

3ba3c0f

marcosuma approved these changes Feb 18, 2025

View reviewed changes

lantoli reviewed Feb 18, 2025

View reviewed changes

internal/service/globalclusterconfig/resource_global_cluster_config.go Outdated Show resolved Hide resolved

lantoli approved these changes Feb 18, 2025

View reviewed changes

Update internal/service/globalclusterconfig/resource_global_cluster_c…

2ed8856

…onfig.go Co-authored-by: Leo Antoli <[email protected]>

jwilliams-mongo reviewed Feb 18, 2025

View reviewed changes

docs/resources/global_cluster_config.md Outdated Show resolved Hide resolved

Update docs/resources/global_cluster_config.md

146e7f2

Co-authored-by: John Williams <[email protected]>

oarbusi merged commit 48c9c4e into master Feb 19, 2025
40 checks passed

oarbusi deleted the HELP-70971 branch February 19, 2025 09:49

svc-apix-Bot added a commit that referenced this pull request Feb 19, 2025

chore: Updates CHANGELOG.md for #3060

363badf

maastha pushed a commit that referenced this pull request Feb 20, 2025

chore: Updates CHANGELOG.md for #3060

cbb6cc7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Handles updates of `mongodbatlas_global_cluster_config` #3060

fix: Handles updates of `mongodbatlas_global_cluster_config` #3060

oarbusi commented Feb 12, 2025 •

edited

Loading

marcosuma Feb 12, 2025

oarbusi Feb 12, 2025

oarbusi Feb 12, 2025

nikhil-mongo commented Feb 13, 2025

github-actions bot commented Feb 13, 2025

lantoli Feb 13, 2025

maastha Feb 13, 2025 •

edited

Loading

oarbusi Feb 14, 2025

maastha Feb 18, 2025

AgustinBettati left a comment

		@@ -0,0 +1,3 @@
		```release-note:enhancement
		resource/mongodbatlas_global_cluster_config: Supports update operation

fix: Handles updates of mongodbatlas_global_cluster_config #3060

fix: Handles updates of mongodbatlas_global_cluster_config #3060

Conversation

oarbusi commented Feb 12, 2025 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

marcosuma Feb 12, 2025

Choose a reason for hiding this comment

oarbusi Feb 12, 2025

Choose a reason for hiding this comment

oarbusi Feb 12, 2025

Choose a reason for hiding this comment

nikhil-mongo commented Feb 13, 2025

github-actions bot commented Feb 13, 2025

lantoli Feb 13, 2025

Choose a reason for hiding this comment

maastha Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

oarbusi Feb 14, 2025

Choose a reason for hiding this comment

maastha Feb 18, 2025

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

fix: Handles updates of `mongodbatlas_global_cluster_config` #3060

fix: Handles updates of `mongodbatlas_global_cluster_config` #3060

oarbusi commented Feb 12, 2025 •

edited

Loading

maastha Feb 13, 2025 •

edited

Loading