Skip to content
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

fix: Handles resource deletion in ReadResource #2268

Merged
merged 15 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changelog/2268.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
```release-note:bug
Deletes resource outside of Terraform for the following:
- resource/cloud_backup_schedule
- resource/encryption_at_rest
- resource/push_based_log_export
- resource/search_deployment
- resource/stream_connection
- resource/stream_instance
```
EspenAlbert marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"net/http"
"strings"

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
Expand Down Expand Up @@ -343,8 +344,12 @@ func resourceRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Di
projectID := ids["project_id"]
clusterName := ids["cluster_name"]

backupPolicy, _, err := connV2.CloudBackupsApi.GetBackupSchedule(context.Background(), projectID, clusterName).Execute()
backupPolicy, resp, err := connV2.CloudBackupsApi.GetBackupSchedule(context.Background(), projectID, clusterName).Execute()
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
d.SetId("")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@lantoli lantoli May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

@EspenAlbert EspenAlbert May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return nil
}
return diag.Errorf(errorSnapshotBackupScheduleRead, clusterName, err)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resp is already defined in the method signature:

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

if err != nil {
if getResp != nil && getResp.StatusCode == http.StatusNotFound {
resp.State.RemoveResource(ctx)
return
}
resp.Diagnostics.AddError("error when getting encryption at rest resource during read", fmt.Sprintf(errorReadEncryptionAtRest, err.Error()))
return
}
Expand Down
2 changes: 1 addition & 1 deletion internal/service/project/resource_project.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ func (r *projectRS) Read(ctx context.Context, req resource.ReadRequest, resp *re
// get project
projectRes, atlasResp, err := connV2.ProjectsApi.GetProject(ctx, projectID).Execute()
if err != nil {
if resp != nil && atlasResp.StatusCode == http.StatusNotFound {
if atlasResp != nil && atlasResp.StatusCode == http.StatusNotFound {
resp.State.RemoveResource(ctx)
return
}
Expand Down
7 changes: 6 additions & 1 deletion internal/service/pushbasedlogexport/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package pushbasedlogexport

import (
"context"
"net/http"
"time"

"github.com/hashicorp/terraform-plugin-framework/path"
Expand Down Expand Up @@ -84,8 +85,12 @@ func (r *pushBasedLogExportRS) Read(ctx context.Context, req resource.ReadReques

connV2 := r.Client.AtlasV2
projectID := tfState.ProjectID.ValueString()
logConfig, _, err := connV2.PushBasedLogExportApi.GetPushBasedLogConfiguration(ctx, projectID).Execute()
logConfig, getResp, err := connV2.PushBasedLogExportApi.GetPushBasedLogConfiguration(ctx, projectID).Execute()
if err != nil {
if getResp != nil && getResp.StatusCode == http.StatusNotFound {
resp.State.RemoveResource(ctx)
return
}
resp.Diagnostics.AddError("Error when getting push-based log export configuration", err.Error())
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package searchdeployment
import (
"context"
"errors"
"net/http"
"regexp"
"time"

Expand Down Expand Up @@ -90,8 +91,12 @@ func (r *searchDeploymentRS) Read(ctx context.Context, req resource.ReadRequest,
connV2 := r.Client.AtlasV2
projectID := searchDeploymentPlan.ProjectID.ValueString()
clusterName := searchDeploymentPlan.ClusterName.ValueString()
deploymentResp, _, err := connV2.AtlasSearchApi.GetAtlasSearchDeployment(ctx, projectID, clusterName).Execute()
deploymentResp, getResp, err := connV2.AtlasSearchApi.GetAtlasSearchDeployment(ctx, projectID, clusterName).Execute()
if err != nil {
if getResp != nil && getResp.StatusCode == http.StatusNotFound {
resp.State.RemoveResource(ctx)
return
}
resp.Diagnostics.AddError("error getting search deployment information", err.Error())
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package streamconnection
import (
"context"
"errors"
"net/http"
"regexp"

"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
Expand Down Expand Up @@ -208,8 +209,12 @@ func (r *streamConnectionRS) Read(ctx context.Context, req resource.ReadRequest,
projectID := streamConnectionState.ProjectID.ValueString()
instanceName := streamConnectionState.InstanceName.ValueString()
connectionName := streamConnectionState.ConnectionName.ValueString()
apiResp, _, err := connV2.StreamsApi.GetStreamConnection(ctx, projectID, instanceName, connectionName).Execute()
apiResp, getResp, err := connV2.StreamsApi.GetStreamConnection(ctx, projectID, instanceName, connectionName).Execute()
if err != nil {
if getResp != nil && getResp.StatusCode == http.StatusNotFound {
resp.State.RemoveResource(ctx)
return
}
resp.Diagnostics.AddError("error fetching resource", err.Error())
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package streaminstance
import (
"context"
"errors"
"net/http"
"regexp"

"github.com/hashicorp/terraform-plugin-framework/attr"
Expand Down Expand Up @@ -144,8 +145,12 @@ func (r *streamInstanceRS) Read(ctx context.Context, req resource.ReadRequest, r
connV2 := r.Client.AtlasV2
projectID := streamInstanceState.ProjectID.ValueString()
instanceName := streamInstanceState.InstanceName.ValueString()
apiResp, _, err := connV2.StreamsApi.GetStreamInstance(ctx, projectID, instanceName).Execute()
apiResp, getResp, err := connV2.StreamsApi.GetStreamInstance(ctx, projectID, instanceName).Execute()
if err != nil {
if getResp != nil && getResp.StatusCode == http.StatusNotFound {
resp.State.RemoveResource(ctx)
return
}
resp.Diagnostics.AddError("error fetching resource", err.Error())
return
}
Expand Down