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: Move Resource ImportState method to optional ResourceWithImportState interface #297

Merged
merged 4 commits into from
Apr 25, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changelog/297.txt
Original file line number Diff line number Diff line change
@@ -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.
```
13 changes: 4 additions & 9 deletions tfsdk/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 16 additions & 0 deletions tfsdk/resource_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps nit-picking, but shouldn't this interface contain Resource, which would allow for validation that an implementation is fully implementing both using a single line? e.g.

var _ ResourceWithImportState = MyResource{}
# which also implies
# var _ Resource = MyResource{}

// 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."
Expand Down
25 changes: 24 additions & 1 deletion tfsdk/serve_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Comment on lines +89 to +107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👨‍🍳 💋


emptyState := tftypes.NewValue(resourceSchema.TerraformType(ctx), nil)
importReq := ImportResourceStateRequest{
ID: req.ID,
Expand All @@ -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() {
Expand Down
17 changes: 16 additions & 1 deletion tfsdk/serve_import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions tfsdk/serve_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down
4 changes: 0 additions & 4 deletions tfsdk/serve_resource_attribute_plan_modifiers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
3 changes: 0 additions & 3 deletions tfsdk/serve_resource_config_validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
67 changes: 67 additions & 0 deletions tfsdk/serve_resource_import_state_not_implemented_test.go
Original file line number Diff line number Diff line change
@@ -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.
}
4 changes: 0 additions & 4 deletions tfsdk/serve_resource_one_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
4 changes: 0 additions & 4 deletions tfsdk/serve_resource_three_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
4 changes: 0 additions & 4 deletions tfsdk/serve_resource_two_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions tfsdk/serve_resource_upgrade_state_empty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions tfsdk/serve_resource_upgrade_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 0 additions & 3 deletions tfsdk/serve_resource_validate_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions tfsdk/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down