tfsdk: Move Resource ImportState method to optional ResourceWithImpor…

…tState interface (#297)

Reference: #160

Due to provider developer feedback, it has been suggested to make the current `Resource` interface `ImportState` method optional. This change accomplishes that by moving the existing method signature to a new `ResourceWithImportState` interface.

This also deprecates the `ResourceImportStateNotImplemented()` function. Providers can now signal that a resource does not support import by omitting the `ImportState` method and the framework will automatically return an error diagnostic. From a quick GitHub search, it appeared there was only one usage of a custom error message outside the framework testing. However, it was only being used to include the resource type in the message and was of no real utility over the generic messaging. A code comment is left with a suggested implementation, should there be a feature request for customized messaging.

.changelog/297.txt

@@ -0,0 +1,11 @@
+```release-note:note
+tfsdk: The `Resource` interface no longer requires the `ImportState` method. A separate `ResourceWithImportState` interface now defines the same `ImportState` method.
+```
+
+```release-note:note
+tfsdk: The `ResourceImportStateNotImplemented()` function has been deprecated. Instead, the `ImportState` method can be removed from the `Resource` and the framework will automatically return an error diagnostic if import is attempted.
+```
+
+```release-note:enhancement
+tfsdk: Added `ResourceWithImportState` interface, which allows `Resource` implementations to optionally define the `ImportState` method.
+```

tfsdk/resource.go

@@ -19,6 +19,10 @@ type ResourceType interface {
 
 // Resource represents a resource instance. This is the core interface that all
 // resources must implement.
+//
+// It is also conventional for resources to implement the
+// ResourceWithImportState interface, which enables practitioners to import
+// existing infrastructure into Terraform.
 type Resource interface {
 	// Create is called when the provider must create a new resource. Config
 	// and planned state values should be read from the
@@ -45,15 +49,6 @@ type Resource interface {
 	// 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.
-	//
-	// If import is not supported, it is recommended to use the
-	// ResourceImportStateNotImplemented() call in this method.
-	//
-	// If setting an attribute with the import identifier, it is recommended
-	// to use the ResourceImportStatePassthroughID() call in this method.
-	ImportState(context.Context, ImportResourceStateRequest, *ImportResourceStateResponse)
 }
 
 // ResourceWithModifyPlan represents a resource instance with a ModifyPlan

tfsdk/resource_import.go

@@ -6,10 +6,26 @@ import (
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
 )
 
+// Optional interface on top of Resource that enables provider control over
+// the ImportResourceState RPC. This RPC is called by Terraform when the
+// `terraform import` command is executed. Afterwards, the ReadResource RPC
+// is executed to allow providers to fully populate the resource state.
+type ResourceWithImportState interface {
+	// ImportState is called when the provider must import the state of a
+	// resource instance. This method must return enough state so the Read
+	// method can properly refresh the full resource.
+	//
+	// If setting an attribute with the import identifier, it is recommended
+	// to use the ResourceImportStatePassthroughID() call in this method.
+	ImportState(context.Context, ImportResourceStateRequest, *ImportResourceStateResponse)
+}
+
 // ResourceImportStateNotImplemented is a helper function to return an error
 // diagnostic about the resource not supporting import. The details defaults
 // to a generic message to contact the provider developer, but can be
 // customized to provide specific information or recommendations.
+//
+// Deprecated: Remove the ImportState method from the Resource instead.
 func ResourceImportStateNotImplemented(ctx context.Context, details string, resp *ImportResourceStateResponse) {
 	if details == "" {
 		details = "This resource does not support import. Please contact the provider developer for additional information."

tfsdk/serve_import.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/hashicorp/terraform-plugin-framework/diag"
+	"github.com/hashicorp/terraform-plugin-framework/internal/logging"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
 )
@@ -85,6 +86,26 @@ func (s *server) importResourceState(ctx context.Context, req *tfprotov6.ImportR
 		return
 	}
 
+	resourceWithImportState, ok := resource.(ResourceWithImportState)
+
+	if !ok {
+		// If there is a feature request for customizing this messaging,
+		// provider developers can implement a ImportState method that
+		// immediately returns a custom error diagnostic.
+		//
+		// However, implementing the ImportState method could cause issues
+		// with automated documentation generation, which likely would check
+		// if the resource implements the ResourceWithImportState interface.
+		// Instead, a separate "ResourceWithoutImportState" interface could be
+		// created with a method such as:
+		//    ImportNotImplementedMessage(context.Context) string.
+		resp.Diagnostics.AddError(
+			"Resource Import Not Implemented",
+			"This resource does not support import. Please contact the provider developer for additional information.",
+		)
+		return
+	}
+
 	emptyState := tftypes.NewValue(resourceSchema.TerraformType(ctx), nil)
 	importReq := ImportResourceStateRequest{
 		ID: req.ID,
@@ -96,7 +117,9 @@ func (s *server) importResourceState(ctx context.Context, req *tfprotov6.ImportR
 		},
 	}
 
-	resource.ImportState(ctx, importReq, &importResp)
+	logging.FrameworkDebug(ctx, "Calling provider defined ImportState")
+	resourceWithImportState.ImportState(ctx, importReq, &importResp)
+	logging.FrameworkDebug(ctx, "Called provider defined ImportState")
 	resp.Diagnostics.Append(importResp.Diagnostics...)
 
 	if resp.Diagnostics.HasError() {

tfsdk/serve_import_test.go

@@ -155,6 +155,21 @@ func TestServerImportResourceState(t *testing.T) {
 				},
 			},
 		},
+		"TypeName-ImportState-not-implemented": {
+			req: &tfprotov6.ImportResourceStateRequest{
+				ID:       "test",
+				TypeName: "test_import_state_not_implemented",
+			},
+			resp: &tfprotov6.ImportResourceStateResponse{
+				Diagnostics: []*tfprotov6.Diagnostic{
+					{
+						Summary:  "Resource Import Not Implemented",
+						Severity: tfprotov6.DiagnosticSeverityError,
+						Detail:   "This resource does not support import. Please contact the provider developer for additional information.",
+					},
+				},
+			},
+		},
 	}
 
 	for name, tc := range tests {
@@ -177,7 +192,7 @@ func TestServerImportResourceState(t *testing.T) {
 				return
 			}
 
-			if s.importResourceStateCalledResourceType != tc.req.TypeName {
+			if tc.req.TypeName == "test_import_state" && s.importResourceStateCalledResourceType != tc.req.TypeName {
 				t.Errorf("Called wrong resource. Expected to call %q, actually called %q", tc.req.TypeName, s.importResourceStateCalledResourceType)
 				return
 			}

tfsdk/serve_provider_test.go

@@ -639,6 +639,7 @@ func (t *testServeProvider) GetResources(_ context.Context) (map[string]Resource
 		"test_attribute_plan_modifiers":      testServeResourceTypeAttributePlanModifiers{},
 		"test_config_validators":             testServeResourceTypeConfigValidators{},
 		"test_import_state":                  testServeResourceTypeImportState{},
+		"test_import_state_not_implemented":  testServeResourceTypeImportStateNotImplemented{},
 		"test_upgrade_state":                 testServeResourceTypeUpgradeState{},
 		"test_upgrade_state_empty":           testServeResourceTypeUpgradeStateEmpty{},
 		"test_upgrade_state_not_implemented": testServeResourceTypeUpgradeStateNotImplemented{},

tfsdk/serve_resource_attribute_plan_modifiers_test.go

@@ -342,7 +342,3 @@ func (r testServeAttributePlanModifiers) Update(ctx context.Context, req UpdateR
 func (r testServeAttributePlanModifiers) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
 	// Intentionally blank. Not expected to be called during testing.
 }
-
-func (r testServeAttributePlanModifiers) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
-	ResourceImportStateNotImplemented(ctx, "Not expected to be called during testing.", resp)
-}

tfsdk/serve_resource_config_validators_test.go

@@ -71,9 +71,6 @@ func (r testServeResourceConfigValidators) Update(ctx context.Context, req Updat
 func (r testServeResourceConfigValidators) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
 	// Intentionally blank. Not expected to be called during testing.
 }
-func (r testServeResourceConfigValidators) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
-	ResourceImportStateNotImplemented(ctx, "Not expected to be called during testing.", resp)
-}
 
 func (r testServeResourceConfigValidators) ConfigValidators(ctx context.Context) []ResourceConfigValidator {
 	r.provider.validateResourceConfigCalledResourceType = "test_config_validators"

tfsdk/serve_resource_import_state_not_implemented_test.go

@@ -0,0 +1,67 @@
+package tfsdk
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/hashicorp/terraform-plugin-framework/diag"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
+	"github.com/hashicorp/terraform-plugin-go/tftypes"
+)
+
+type testServeResourceTypeImportStateNotImplemented struct{}
+
+func (dt testServeResourceTypeImportStateNotImplemented) GetSchema(_ context.Context) (Schema, diag.Diagnostics) {
+	return Schema{
+		Attributes: map[string]Attribute{
+			"id": {
+				Type:     types.StringType,
+				Computed: true,
+			},
+		},
+	}, nil
+}
+
+func (dt testServeResourceTypeImportStateNotImplemented) NewResource(_ context.Context, p Provider) (Resource, diag.Diagnostics) {
+	provider, ok := p.(*testServeProvider)
+	if !ok {
+		prov, ok := p.(*testServeProviderWithMetaSchema)
+		if !ok {
+			panic(fmt.Sprintf("unexpected provider type %T", p))
+		}
+		provider = prov.testServeProvider
+	}
+	return testServeResourceImportStateNotImplemented{
+		provider: provider,
+	}, nil
+}
+
+var testServeResourceTypeImportStateNotImplementedSchema = &tfprotov6.Schema{
+	Block: &tfprotov6.SchemaBlock{
+		Attributes: []*tfprotov6.SchemaAttribute{
+			{
+				Name:     "id",
+				Computed: true,
+				Type:     tftypes.String,
+			},
+		},
+	},
+}
+
+type testServeResourceImportStateNotImplemented struct {
+	provider *testServeProvider
+}
+
+func (r testServeResourceImportStateNotImplemented) Create(ctx context.Context, req CreateResourceRequest, resp *CreateResourceResponse) {
+	// Intentionally blank. Not expected to be called during testing.
+}
+func (r testServeResourceImportStateNotImplemented) Read(ctx context.Context, req ReadResourceRequest, resp *ReadResourceResponse) {
+	// Intentionally blank. Not expected to be called during testing.
+}
+func (r testServeResourceImportStateNotImplemented) Update(ctx context.Context, req UpdateResourceRequest, resp *UpdateResourceResponse) {
+	// Intentionally blank. Not expected to be called during testing.
+}
+func (r testServeResourceImportStateNotImplemented) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
+	// Intentionally blank. Not expected to be called during testing.
+}

tfsdk/serve_resource_one_test.go

@@ -125,7 +125,3 @@ func (r testServeResourceOne) Delete(ctx context.Context, req DeleteResourceRequ
 	r.provider.applyResourceChangeCalledAction = "delete"
 	r.provider.deleteFunc(ctx, req, resp)
 }
-
-func (r testServeResourceOne) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
-	ResourceImportStateNotImplemented(ctx, "Not expected to be called during testing.", resp)
-}

tfsdk/serve_resource_three_test.go

@@ -136,7 +136,3 @@ func (r testServeResourceThree) Update(ctx context.Context, req UpdateResourceRe
 func (r testServeResourceThree) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
 	// Intentionally blank. Not expected to be called during testing.
 }
-
-func (r testServeResourceThree) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
-	ResourceImportStateNotImplemented(ctx, "Not expected to be called during testing.", resp)
-}

tfsdk/serve_resource_two_test.go

@@ -207,10 +207,6 @@ func (r testServeResourceTwo) Delete(ctx context.Context, req DeleteResourceRequ
 	r.provider.deleteFunc(ctx, req, resp)
 }
 
-func (r testServeResourceTwo) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
-	ResourceImportStateNotImplemented(ctx, "Not expected to be called during testing.", resp)
-}
-
 func (r testServeResourceTwo) ModifyPlan(ctx context.Context, req ModifyResourcePlanRequest, resp *ModifyResourcePlanResponse) {
 	r.provider.planResourceChangePriorStateValue = req.State.Raw
 	r.provider.planResourceChangePriorStateSchema = req.State.Schema

tfsdk/serve_resource_upgrade_state_empty_test.go

@@ -87,9 +87,6 @@ func (r testServeResourceUpgradeStateEmpty) Update(ctx context.Context, req Upda
 func (r testServeResourceUpgradeStateEmpty) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
 	// Intentionally blank. Not expected to be called during testing.
 }
-func (r testServeResourceUpgradeStateEmpty) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
-	ResourceImportStateNotImplemented(ctx, "intentionally not implemented", resp)
-}
 
 func (r testServeResourceUpgradeStateEmpty) UpgradeState(ctx context.Context) map[int64]ResourceStateUpgrader {
 	return nil

tfsdk/serve_resource_upgrade_state_test.go

@@ -123,9 +123,6 @@ func (r testServeResourceUpgradeState) Update(ctx context.Context, req UpdateRes
 func (r testServeResourceUpgradeState) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
 	// Intentionally blank. Not expected to be called during testing.
 }
-func (r testServeResourceUpgradeState) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
-	ResourceImportStateNotImplemented(ctx, "intentionally not implemented", resp)
-}
 
 func (r testServeResourceUpgradeState) UpgradeState(ctx context.Context) map[int64]ResourceStateUpgrader {
 	r.provider.upgradeResourceStateCalledResourceType = "test_upgrade_state"

tfsdk/serve_resource_validate_config_test.go

@@ -71,9 +71,6 @@ func (r testServeResourceValidateConfig) Update(ctx context.Context, req UpdateR
 func (r testServeResourceValidateConfig) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
 	// Intentionally blank. Not expected to be called during testing.
 }
-func (r testServeResourceValidateConfig) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
-	ResourceImportStateNotImplemented(ctx, "Not expected to be called during testing.", resp)
-}
 
 func (r testServeResourceValidateConfig) ValidateConfig(ctx context.Context, req ValidateResourceConfigRequest, resp *ValidateResourceConfigResponse) {
 	r.provider.validateResourceConfigCalledResourceType = "test_validate_config"

tfsdk/serve_test.go

@@ -312,6 +312,7 @@ func TestServerGetProviderSchema(t *testing.T) {
 			"test_attribute_plan_modifiers":      testServeResourceTypeAttributePlanModifiersSchema,
 			"test_config_validators":             testServeResourceTypeConfigValidatorsSchema,
 			"test_import_state":                  testServeResourceTypeImportStateSchema,
+			"test_import_state_not_implemented":  testServeResourceTypeImportStateNotImplementedSchema,
 			"test_upgrade_state":                 testServeResourceTypeUpgradeStateSchema,
 			"test_upgrade_state_empty":           testServeResourceTypeUpgradeStateEmptySchema,
 			"test_upgrade_state_not_implemented": testServeResourceTypeUpgradeStateNotImplementedSchema,
@@ -350,6 +351,7 @@ func TestServerGetProviderSchemaWithProviderMeta(t *testing.T) {
 			"test_attribute_plan_modifiers":      testServeResourceTypeAttributePlanModifiersSchema,
 			"test_config_validators":             testServeResourceTypeConfigValidatorsSchema,
 			"test_import_state":                  testServeResourceTypeImportStateSchema,
+			"test_import_state_not_implemented":  testServeResourceTypeImportStateNotImplementedSchema,
 			"test_upgrade_state":                 testServeResourceTypeUpgradeStateSchema,
 			"test_upgrade_state_empty":           testServeResourceTypeUpgradeStateEmptySchema,
 			"test_upgrade_state_not_implemented": testServeResourceTypeUpgradeStateNotImplementedSchema,