fix: Handles resource deletion in ReadResource #2268

EspenAlbert · 2024-05-10T16:30:46Z

Description

Handles resource deletion in ReadResource.
Terraform informs you about the resource deleted outside of Terraform, and the plan will re-create the resource

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

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 defined an isolated PR with a relevant title as it will be used in the auto-generated changelog.
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-05-16T00:20:56Z

This PR has gone 7 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 7 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy!

github-actions · 2024-05-20T08:40:51Z

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

marcosuma · 2024-05-20T09:59:11Z

internal/service/encryptionatrest/resource_encryption_at_rest.go

@@ -289,8 +289,12 @@ func (r *encryptionAtRestRS) Read(ctx context.Context, req resource.ReadRequest,

 	connV2 := r.Client.AtlasV2

-	encryptionResp, _, err := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.GetEncryptionAtRest(context.Background(), projectID).Execute()
+	encryptionResp, getResp, err := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.GetEncryptionAtRest(context.Background(), projectID).Execute()


Suggested change

encryptionResp, getResp, err := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.GetEncryptionAtRest(context.Background(), projectID).Execute()

encryptionResp, resp, err := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.GetEncryptionAtRest(context.Background(), projectID).Execute()

resp is already defined in the method signature:

func (r *encryptionAtRestRS) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse)

marcosuma · 2024-05-20T10:02:08Z

internal/service/cloudbackupschedule/resource_cloud_backup_schedule.go

 	if err != nil {
+		if resp != nil && resp.StatusCode == http.StatusNotFound {
+			d.SetId("")


why in here we don't do resp.State.RemoveResource(ctx) ?

same question, what is it for and why we use it only in some resources. do we also need to call d.SetId("") when calling RemoveResource?

This is the difference between SDK v2 and TPF in how we mark a resource as deleted, see google doc

.changelog/2268.txt

kanchana-mongodb

LGTM

.changelog/2268.txt

lantoli

thanks for your patience :-)

kanchana-mongodb

1 non-blocking suggestion. LGTM.

.changelog/2268.txt

lantoli · 2024-05-21T19:06:42Z

.changelog/2268.txt

@@ -0,0 +1,23 @@
+```release-note:bug
+resource/cloud_backup_schedule: Fixes behavior when resource is deleted outside of Terraform.


@AgustinBettati should changelog sentences end in period? I see most of them don't have it, but some of them have it like https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/.changelog/2208.txt

looking at: https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/CHANGELOG.md
it looks like it would be better NOT to have periods at the end, as the after the sentence it's the associated PR link

@EspenAlbert apart from removing periods note that resources should have mongodbatlas_ as prefix, e.g.: resource/mongodbatlas_cloud_backup_schedule: ...

Forgot to refresh the PR 🤦, and merged this. I'll create a follow up PR to remove the trailing dot and ensure the prefix

@lantoli and @AgustinBettati updated in follow-up PR: #2298

EspenAlbert · 2024-05-22T06:44:26Z

Only OUT_OF_CAPACITY failure in the test so merging

EspenAlbert added 6 commits May 10, 2024 17:20

fix: removed outside terraform cloudbackupschedule

10ddf81

fix: removed outside terraform during read encryption_at_rest

98e9682

fix: removed outside terraform during read push_based_log_export

5755afb

fix: removed outside terraform during read search_deployment

fc5cb2b

fix: removed outside terraform during read stream_connection

fde2955

fix: removed outside terraform during read stream_instance

66585dd

github-actions bot added the bug label May 10, 2024

fix: typo using resp instead of atlasResp

8debf17

github-actions bot added the stale label May 16, 2024

EspenAlbert added the not_stale Not stale issue or PR label May 16, 2024

docs: add changelog file

4516a3b

EspenAlbert marked this pull request as ready for review May 20, 2024 08:40

EspenAlbert requested review from a team as code owners May 20, 2024 08:40

docs: third person in changelog entry [ci skip]

cd76be1

marcosuma reviewed May 20, 2024

View reviewed changes

lantoli approved these changes May 20, 2024

View reviewed changes

kanchana-mongodb reviewed May 20, 2024

View reviewed changes

.changelog/2268.txt Outdated Show resolved Hide resolved

EspenAlbert added 2 commits May 20, 2024 15:18

docs: apply suggestion [ci skip]

19fe459

docs: add specific resources to changelog as suggested [ci skip]

d6215c5

kanchana-mongodb approved these changes May 20, 2024

View reviewed changes

lantoli reviewed May 20, 2024

View reviewed changes

.changelog/2268.txt Show resolved Hide resolved

EspenAlbert added 2 commits May 20, 2024 16:13

docs: update changelog entry [ci skip]

d1629e0

docs: add specific changelog entry for each resource changed [ci skip]

255e4ad

lantoli approved these changes May 20, 2024

View reviewed changes

kanchana-mongodb approved these changes May 21, 2024

View reviewed changes

.changelog/2268.txt Outdated Show resolved Hide resolved

.changelog/2268.txt Outdated Show resolved Hide resolved

docs: update changelog [ci skip]

58c66a0

retrigger checks

7214409

lantoli reviewed May 21, 2024

View reviewed changes

EspenAlbert merged commit ae63d9e into master May 22, 2024
31 of 32 checks passed

EspenAlbert deleted the CLOUDP-248670_read_resource_support_404 branch May 22, 2024 06:42

svc-apix-Bot added a commit that referenced this pull request May 22, 2024

chore: Updates CHANGELOG.md for #2268

db695ed

EspenAlbert mentioned this pull request May 22, 2024

chore: Manual changelog fixes, mongodbatlas_ prefix and removing "." #2298

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Handles resource deletion in ReadResource #2268

fix: Handles resource deletion in ReadResource #2268

EspenAlbert commented May 10, 2024

github-actions bot commented May 16, 2024

github-actions bot commented May 20, 2024

marcosuma May 20, 2024

marcosuma May 20, 2024

EspenAlbert May 20, 2024

marcosuma May 20, 2024

lantoli May 20, 2024 •

edited

EspenAlbert May 20, 2024 •

edited

kanchana-mongodb left a comment

lantoli left a comment

kanchana-mongodb left a comment

lantoli May 21, 2024

lantoli May 21, 2024 •

edited

lantoli May 21, 2024 •

edited

EspenAlbert May 22, 2024

EspenAlbert May 22, 2024

EspenAlbert commented May 22, 2024

	encryptionResp, getResp, err := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.GetEncryptionAtRest(context.Background(), projectID).Execute()
	encryptionResp, resp, err := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.GetEncryptionAtRest(context.Background(), projectID).Execute()

		@@ -0,0 +1,23 @@
		```release-note:bug
		resource/cloud_backup_schedule: Fixes behavior when resource is deleted outside of Terraform.

fix: Handles resource deletion in ReadResource #2268

fix: Handles resource deletion in ReadResource #2268

Conversation

EspenAlbert commented May 10, 2024

Description

Type of change:

Required Checklist:

Further comments

github-actions bot commented May 16, 2024

github-actions bot commented May 20, 2024

marcosuma May 20, 2024

Choose a reason for hiding this comment

marcosuma May 20, 2024

Choose a reason for hiding this comment

EspenAlbert May 20, 2024

Choose a reason for hiding this comment

marcosuma May 20, 2024

Choose a reason for hiding this comment

lantoli May 20, 2024 • edited

Choose a reason for hiding this comment

EspenAlbert May 20, 2024 • edited

Choose a reason for hiding this comment

kanchana-mongodb left a comment

Choose a reason for hiding this comment

lantoli left a comment

Choose a reason for hiding this comment

kanchana-mongodb left a comment

Choose a reason for hiding this comment

lantoli May 21, 2024

Choose a reason for hiding this comment

lantoli May 21, 2024 • edited

Choose a reason for hiding this comment

lantoli May 21, 2024 • edited

Choose a reason for hiding this comment

EspenAlbert May 22, 2024

Choose a reason for hiding this comment

EspenAlbert May 22, 2024

Choose a reason for hiding this comment

EspenAlbert commented May 22, 2024

lantoli May 20, 2024 •

edited

EspenAlbert May 20, 2024 •

edited

lantoli May 21, 2024 •

edited

lantoli May 21, 2024 •

edited