diff --git a/.changelog/301.txt b/.changelog/301.txt new file mode 100644 index 000000000..522ab91d1 --- /dev/null +++ b/.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 +``` diff --git a/tfsdk/resource.go b/tfsdk/resource.go index c74eea420..99c088b1d 100644 --- a/tfsdk/resource.go +++ b/tfsdk/resource.go @@ -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. diff --git a/tfsdk/serve.go b/tfsdk/serve.go index 5a0f94c43..b647eb682 100644 --- a/tfsdk/serve.go +++ b/tfsdk/serve.go @@ -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( diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go index 32da7cc24..71add9ffe 100644 --- a/tfsdk/serve_test.go +++ b/tfsdk/serve_test.go @@ -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), }, @@ -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", @@ -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"), diff --git a/tfsdk/state.go b/tfsdk/state.go index d0340975b..8057ce5be 100644 --- a/tfsdk/state.go +++ b/tfsdk/state.go @@ -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) }