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

tfsdk: Added automatic RemoveResource() call after Resource type Delete method execution #301

Merged
merged 2 commits into from Apr 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changelog/301.txt
@@ -0,0 +1,7 @@
```release-note:note
tfsdk: Providers may now optionally remove `RemoveResource()` calls from `Resource` type `Delete` methods
```

```release-note:enhancement
tfsdk: Added automatic `(DeleteResourceResponse.State).RemoveResource()` call after `Resource` type `Delete` method execution if there are no errors
```
4 changes: 4 additions & 0 deletions tfsdk/resource.go
Expand Up @@ -40,6 +40,10 @@ type Resource interface {

// Delete is called when the provider must delete the resource. Config
// values may be read from the DeleteResourceRequest.
//
// If execution completes without error, the framework will automatically
// call DeleteResourceResponse.State.RemoveResource(), so it can be omitted
// from provider logic.
Delete(context.Context, DeleteResourceRequest, *DeleteResourceResponse)

// ImportState is called when the provider must import the resource.
Expand Down
6 changes: 6 additions & 0 deletions tfsdk/serve.go
Expand Up @@ -1438,6 +1438,12 @@ func (s *server) applyResourceChange(ctx context.Context, req *tfprotov6.ApplyRe
}
resource.Delete(ctx, destroyReq, &destroyResp)
resp.Diagnostics = destroyResp.Diagnostics

if !resp.Diagnostics.HasError() {
logging.FrameworkTrace(ctx, "No provider defined Delete errors detected, ensuring State is cleared")
destroyResp.State.RemoveResource(ctx)
}

newState, err := tfprotov6.NewDynamicValue(resourceSchema.TerraformType(ctx), destroyResp.State.Raw)
if err != nil {
resp.Diagnostics.AddError(
Expand Down
84 changes: 82 additions & 2 deletions tfsdk/serve_test.go
Expand Up @@ -4576,7 +4576,8 @@ func TestServerApplyResourceChange(t *testing.T) {
action: "delete",
resourceType: testServeResourceTypeOneType,
destroy: func(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
resp.State.Raw = tftypes.NewValue(testServeResourceTypeOneType, nil)
// Removing the state prior to the framework should not generate errors
resp.State.RemoveResource(ctx)
},
expectedNewState: tftypes.NewValue(testServeResourceTypeOneType, nil),
},
Expand All @@ -4592,7 +4593,8 @@ func TestServerApplyResourceChange(t *testing.T) {
action: "delete",
resourceType: testServeResourceTypeOneType,
destroy: func(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
resp.State.Raw = tftypes.NewValue(testServeResourceTypeOneType, nil)
// Removing the state prior to the framework should not generate errors
resp.State.RemoveResource(ctx)
resp.Diagnostics.AddAttributeWarning(
tftypes.NewAttributePath().WithAttributeName("created_timestamp"),
"This is a warning",
Expand Down Expand Up @@ -4641,6 +4643,84 @@ func TestServerApplyResourceChange(t *testing.T) {
},
},
},
"one_delete_automatic_removeresource": {
priorState: tftypes.NewValue(testServeResourceTypeOneType, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "hello, world"),
"favorite_colors": tftypes.NewValue(tftypes.List{ElementType: tftypes.String}, []tftypes.Value{
tftypes.NewValue(tftypes.String, "red"),
}),
"created_timestamp": tftypes.NewValue(tftypes.String, "right now I guess"),
}),
resource: "test_one",
action: "delete",
resourceType: testServeResourceTypeOneType,
destroy: func(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
// The framework should automatically call resp.State.RemoveResource()
},
expectedNewState: tftypes.NewValue(testServeResourceTypeOneType, nil),
},
"one_delete_diags_warning_automatic_removeresource": {
priorState: tftypes.NewValue(testServeResourceTypeOneType, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "hello, world"),
"favorite_colors": tftypes.NewValue(tftypes.List{ElementType: tftypes.String}, []tftypes.Value{
tftypes.NewValue(tftypes.String, "red"),
}),
"created_timestamp": tftypes.NewValue(tftypes.String, "right now I guess"),
}),
resource: "test_one",
action: "delete",
resourceType: testServeResourceTypeOneType,
destroy: func(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
// The framework should automatically call resp.State.RemoveResource()
resp.Diagnostics.AddAttributeWarning(
tftypes.NewAttributePath().WithAttributeName("created_timestamp"),
"This is a warning",
"just a warning diagnostic, no behavior changes",
)
},
expectedNewState: tftypes.NewValue(testServeResourceTypeOneType, nil),
expectedDiags: []*tfprotov6.Diagnostic{
{
Severity: tfprotov6.DiagnosticSeverityWarning,
Summary: "This is a warning",
Detail: "just a warning diagnostic, no behavior changes",
Attribute: tftypes.NewAttributePath().WithAttributeName("created_timestamp"),
},
},
},
"one_delete_diags_error_no_automatic_removeresource": {
priorState: tftypes.NewValue(testServeResourceTypeOneType, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "hello, world"),
"favorite_colors": tftypes.NewValue(tftypes.List{ElementType: tftypes.String}, []tftypes.Value{
tftypes.NewValue(tftypes.String, "red"),
}),
"created_timestamp": tftypes.NewValue(tftypes.String, "right now I guess"),
}),
resource: "test_one",
action: "delete",
resourceType: testServeResourceTypeOneType,
destroy: func(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
// The framework should NOT automatically call resp.State.RemoveResource()
resp.Diagnostics.AddError(
"This is an error",
"Something went wrong, keep the old state around",
)
},
expectedNewState: tftypes.NewValue(testServeResourceTypeOneType, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "hello, world"),
"favorite_colors": tftypes.NewValue(tftypes.List{ElementType: tftypes.String}, []tftypes.Value{
tftypes.NewValue(tftypes.String, "red"),
}),
"created_timestamp": tftypes.NewValue(tftypes.String, "right now I guess"),
}),
expectedDiags: []*tfprotov6.Diagnostic{
{
Severity: tfprotov6.DiagnosticSeverityError,
Summary: "This is an error",
Detail: "Something went wrong, keep the old state around",
},
},
},
"two_create": {
plannedState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{
"id": tftypes.NewValue(tftypes.String, "test-instance"),
Expand Down
3 changes: 3 additions & 0 deletions tfsdk/state.go
Expand Up @@ -326,6 +326,9 @@ func (s State) setAttributeTransformFunc(ctx context.Context, path *tftypes.Attr
}

// RemoveResource removes the entire resource from state.
//
// If a Resource type Delete method is completed without error, this is
// automatically called on the DeleteResourceResponse.State.
func (s *State) RemoveResource(ctx context.Context) {
s.Raw = tftypes.NewValue(s.Schema.TerraformType(ctx), nil)
}
Expand Down