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

Reference: #100

Due to provider developer feedback, the framework will now perform automatic removal of state on successful (no error diagnostic) `Resource` type `Delete` executions. This matches the behavior of terraform-plugin-sdk.

Providers can still explicitly call `(State).RemoveResource()` or leave existing implementations, however it is extraneous for most `Delete` logic now.

Author: bflad

.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
+```

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.

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(

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"),

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)
 }