diff --git a/.changelog/297.txt b/.changelog/297.txt new file mode 100644 index 000000000..c10030677 --- /dev/null +++ b/.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. +``` diff --git a/tfsdk/resource.go b/tfsdk/resource.go index 99c088b1d..fd99a8c99 100644 --- a/tfsdk/resource.go +++ b/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 diff --git a/tfsdk/resource_import.go b/tfsdk/resource_import.go index 4c166c376..f22018910 100644 --- a/tfsdk/resource_import.go +++ b/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." diff --git a/tfsdk/serve_import.go b/tfsdk/serve_import.go index da8ec5c36..1eeea0ad9 100644 --- a/tfsdk/serve_import.go +++ b/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() { diff --git a/tfsdk/serve_import_test.go b/tfsdk/serve_import_test.go index c6d7ad879..0e43bec4d 100644 --- a/tfsdk/serve_import_test.go +++ b/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 } diff --git a/tfsdk/serve_provider_test.go b/tfsdk/serve_provider_test.go index 907c17d88..477e58fd9 100644 --- a/tfsdk/serve_provider_test.go +++ b/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{}, diff --git a/tfsdk/serve_resource_attribute_plan_modifiers_test.go b/tfsdk/serve_resource_attribute_plan_modifiers_test.go index ab6aab4a4..acae0ee72 100644 --- a/tfsdk/serve_resource_attribute_plan_modifiers_test.go +++ b/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) -} diff --git a/tfsdk/serve_resource_config_validators_test.go b/tfsdk/serve_resource_config_validators_test.go index d25b70814..b97009ba2 100644 --- a/tfsdk/serve_resource_config_validators_test.go +++ b/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" diff --git a/tfsdk/serve_resource_import_state_not_implemented_test.go b/tfsdk/serve_resource_import_state_not_implemented_test.go new file mode 100644 index 000000000..c8173de67 --- /dev/null +++ b/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. +} diff --git a/tfsdk/serve_resource_one_test.go b/tfsdk/serve_resource_one_test.go index 7872b9120..9348c16ae 100644 --- a/tfsdk/serve_resource_one_test.go +++ b/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) -} diff --git a/tfsdk/serve_resource_three_test.go b/tfsdk/serve_resource_three_test.go index 1c63cb7a5..0f50b725f 100644 --- a/tfsdk/serve_resource_three_test.go +++ b/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) -} diff --git a/tfsdk/serve_resource_two_test.go b/tfsdk/serve_resource_two_test.go index da9600a3b..ce8346e55 100644 --- a/tfsdk/serve_resource_two_test.go +++ b/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 diff --git a/tfsdk/serve_resource_upgrade_state_empty_test.go b/tfsdk/serve_resource_upgrade_state_empty_test.go index 993536e6b..750ad0f59 100644 --- a/tfsdk/serve_resource_upgrade_state_empty_test.go +++ b/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 diff --git a/tfsdk/serve_resource_upgrade_state_test.go b/tfsdk/serve_resource_upgrade_state_test.go index 8dc233fb8..27c72773a 100644 --- a/tfsdk/serve_resource_upgrade_state_test.go +++ b/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" diff --git a/tfsdk/serve_resource_validate_config_test.go b/tfsdk/serve_resource_validate_config_test.go index 9ba823af8..41a40fb08 100644 --- a/tfsdk/serve_resource_validate_config_test.go +++ b/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" diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go index 83bb60b22..b62b15622 100644 --- a/tfsdk/serve_test.go +++ b/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,