-
Notifications
You must be signed in to change notification settings - Fork 95
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: Initial ImportResourceState support #149
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
e52297c
tfsdk: Initial ImportResourceState support
bflad 907ba1b
Update CHANGELOG for #149
bflad 792a333
Apply suggestions from code review
bflad 1bca3a5
Merge branch 'main' into bflad-f-import
bflad b8d8d34
tfsdk: Use ResourceImportStateNotImplemented and add intentionally bl…
bflad e0fcc2f
tfsdk: Add ResourceImportStatePassthroughID helper function
bflad File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
```release-note:breaking-change | ||
tfsdk: `Resource` implementations must now include the `ImportState(context.Context, ImportResourceStateRequest, *ImportResourceStateResponse)` method. If import is not supported, call the `ResourceImportStateNotImplemented()` function or return an error. | ||
``` | ||
|
||
```release-note:feature | ||
tfsdk: Support resource import | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package tfsdk | ||
|
||
// ImportResourceStateRequest represents a request for the provider to import a | ||
// resource. An instance of this request struct is supplied as an argument to | ||
// the Resource's ImportState method. | ||
type ImportResourceStateRequest struct { | ||
// ID represents the import identifier supplied by the practitioner when | ||
// calling the import command. In many cases, this may align with the | ||
// unique identifier for the resource, which can optionally be stored | ||
// as an Attribute. However, this identifier can also be treated as | ||
// its own type of value and parsed during import. This value | ||
// is not stored in the state unless the provider explicitly stores it. | ||
ID string | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package tfsdk | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/hashicorp/terraform-plugin-go/tftypes" | ||
) | ||
|
||
// 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. | ||
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." | ||
} | ||
|
||
resp.Diagnostics.AddError( | ||
"Resource Import Not Implemented", | ||
details, | ||
) | ||
} | ||
|
||
// ResourceImportStatePassthroughID is a helper function to set the import | ||
// identifier to a given state attribute path. The attribute must accept a | ||
// string value. | ||
func ResourceImportStatePassthroughID(ctx context.Context, path *tftypes.AttributePath, req ImportResourceStateRequest, resp *ImportResourceStateResponse) { | ||
if path == nil || tftypes.NewAttributePath().Equal(path) { | ||
resp.Diagnostics.AddError( | ||
"Resource Import Passthrough Missing Attribute Path", | ||
"This is always an error in the provider. Please report the following to the provider developer:\n\n"+ | ||
"Resource ImportState method call to ResourceImportStatePassthroughID path must be set to a valid attribute path that can accept a string value.", | ||
) | ||
} | ||
|
||
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path, req.ID)...) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package tfsdk | ||
|
||
import ( | ||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||
) | ||
|
||
// ImportResourceStateResponse represents a response to a ImportResourceStateRequest. | ||
// An instance of this response struct is supplied as an argument to the | ||
// Resource's ImportState method, in which the provider should set values on | ||
// the ImportResourceStateResponse as appropriate. | ||
type ImportResourceStateResponse struct { | ||
// Diagnostics report errors or warnings related to importing the | ||
// resource. An empty slice indicates a successful operation with no | ||
// warnings or errors generated. | ||
Diagnostics diag.Diagnostics | ||
|
||
// State is the state of the resource following the import operation. | ||
// It must contain enough information so Terraform can successfully | ||
// refresh the resource, e.g. call the Resource Read method. | ||
State State | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
package tfsdk | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||
"github.com/hashicorp/terraform-plugin-go/tfprotov6" | ||
"github.com/hashicorp/terraform-plugin-go/tftypes" | ||
) | ||
|
||
// importedResource represents a resource that was imported. | ||
// | ||
// This type is not exported as the framework import implementation is | ||
// currently designed for the most common use case of single resource import. | ||
type importedResource struct { | ||
Private []byte | ||
State State | ||
TypeName string | ||
} | ||
|
||
func (r importedResource) toTfprotov6(ctx context.Context) (*tfprotov6.ImportedResource, diag.Diagnostics) { | ||
var diags diag.Diagnostics | ||
irProto6 := &tfprotov6.ImportedResource{ | ||
Private: r.Private, | ||
TypeName: r.TypeName, | ||
} | ||
|
||
stateProto6, err := tfprotov6.NewDynamicValue(r.State.Schema.TerraformType(ctx), r.State.Raw) | ||
|
||
if err != nil { | ||
diags.AddError( | ||
"Error converting imported resource response", | ||
"An unexpected error was encountered when converting the imported resource response to a usable type. This is always a problem with the provider. Please give the following information to the provider developer:\n\n"+err.Error(), | ||
) | ||
return nil, diags | ||
} | ||
|
||
irProto6.State = &stateProto6 | ||
|
||
return irProto6, diags | ||
} | ||
|
||
// importResourceStateResponse is a thin abstraction to allow native Diagnostics usage | ||
type importResourceStateResponse struct { | ||
Diagnostics diag.Diagnostics | ||
ImportedResources []importedResource | ||
} | ||
|
||
func (r importResourceStateResponse) toTfprotov6(ctx context.Context) *tfprotov6.ImportResourceStateResponse { | ||
resp := &tfprotov6.ImportResourceStateResponse{ | ||
Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(), | ||
} | ||
|
||
for _, ir := range r.ImportedResources { | ||
irProto6, diags := ir.toTfprotov6(ctx) | ||
resp.Diagnostics = append(resp.Diagnostics, diags.ToTfprotov6Diagnostics()...) | ||
resp.ImportedResources = append(resp.ImportedResources, irProto6) | ||
} | ||
|
||
return resp | ||
} | ||
|
||
func (s *server) importResourceState(ctx context.Context, req *tfprotov6.ImportResourceStateRequest, resp *importResourceStateResponse) { | ||
resourceType, diags := s.getResourceType(ctx, req.TypeName) | ||
resp.Diagnostics.Append(diags...) | ||
|
||
if resp.Diagnostics.HasError() { | ||
return | ||
} | ||
|
||
resourceSchema, diags := resourceType.GetSchema(ctx) | ||
resp.Diagnostics.Append(diags...) | ||
|
||
if resp.Diagnostics.HasError() { | ||
return | ||
} | ||
|
||
resource, diags := resourceType.NewResource(ctx, s.p) | ||
resp.Diagnostics.Append(diags...) | ||
|
||
if resp.Diagnostics.HasError() { | ||
return | ||
} | ||
|
||
emptyState := tftypes.NewValue(resourceSchema.TerraformType(ctx), nil) | ||
importReq := ImportResourceStateRequest{ | ||
ID: req.ID, | ||
} | ||
importResp := ImportResourceStateResponse{ | ||
State: State{ | ||
Raw: emptyState, | ||
Schema: resourceSchema, | ||
}, | ||
} | ||
|
||
resource.ImportState(ctx, importReq, &importResp) | ||
resp.Diagnostics.Append(importResp.Diagnostics...) | ||
|
||
if resp.Diagnostics.HasError() { | ||
return | ||
} | ||
|
||
if importResp.State.Raw.Equal(emptyState) { | ||
resp.Diagnostics.AddError( | ||
"Missing Resource Import State", | ||
"An unexpected error was encountered when importing the resource. This is always a problem with the provider. Please give the following information to the provider developer:\n\n"+ | ||
"Resource ImportState method returned no State in response. If import is intentionally not supported, call the ResourceImportStateNotImplemented() function or return an error.", | ||
) | ||
return | ||
} | ||
|
||
resp.ImportedResources = []importedResource{ | ||
{ | ||
State: importResp.State, | ||
TypeName: req.TypeName, | ||
}, | ||
} | ||
} | ||
|
||
// ImportResourceState satisfies the tfprotov6.ProviderServer interface. | ||
func (s *server) ImportResourceState(ctx context.Context, req *tfprotov6.ImportResourceStateRequest) (*tfprotov6.ImportResourceStateResponse, error) { | ||
ctx = s.registerContext(ctx) | ||
resp := &importResourceStateResponse{} | ||
|
||
s.importResourceState(ctx, req, resp) | ||
|
||
return resp.toTfprotov6(ctx), nil | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A thing worth thinking about here:
We need a resolution to #148 to make this work the way we want.
Another option is to have it do something like:
This matches the more-expected (imho) default state, which is an object in state with nothing filled in, instead of nothing in state.
However, this is weird during failure cases. I'm not clear on whether Terraform will persist the object to state or write nothing to state in the face of an error diagnostic. If the former, we should default to an empty state and just rely on #148. If the latter, we should confirm that will continue to be the case, and maybe defaulting to the empty object (instead of the null object) is more expected? I dunno. I don't know that it makes a huge difference, just wanted to call it out here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current logic in Terraform CLI is to return early on ImportResourceState error diagnostics, before saving state information:
https://github.com/hashicorp/terraform/blob/2afa0a5e75d76de7e47157114c579b3b1bff994f/internal/terraform/transform_import_state.go#L141-L154
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 how do we feel about just leaving it and waiting for feedback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're going to merge this in as-is for now and iterate if necessary.
If we do wind up switching the implementation here in the future -- maybe creating the above code snippet as a method such as
(Schema).newState()
would be good to try and prevent "empty" object problems here and in any other potential places that might need it. That might be something we could export if/when multiple resource import support makes sense.