diff --git a/helper/logging/logging_http_transport_test.go b/helper/logging/logging_http_transport_test.go index a7b84e110dc..73c3515cb60 100644 --- a/helper/logging/logging_http_transport_test.go +++ b/helper/logging/logging_http_transport_test.go @@ -31,7 +31,7 @@ func TestNewLoggingHTTPTransport(t *testing.T) { reqBody := `An example multiline request body` - req, _ := http.NewRequest("GET", "https://www.terraform.io", bytes.NewBufferString(reqBody)) + req, _ := http.NewRequest("GET", "https://developer.hashicorp.com/terraform", bytes.NewBufferString(reqBody)) res, err := client.Do(req.WithContext(ctx)) if err != nil { t.Fatalf("request failed: %v", err) @@ -40,7 +40,7 @@ func TestNewLoggingHTTPTransport(t *testing.T) { entries, err := tflogtest.MultilineJSONDecode(loggerOutput) if err != nil { - t.Fatalf("log outtput parsing failed: %v", err) + t.Fatalf("log output parsing failed: %v", err) } if len(entries) != 2 { @@ -67,12 +67,12 @@ func TestNewLoggingHTTPTransport(t *testing.T) { "@module": "provider", "tf_http_op_type": "request", "tf_http_req_method": "GET", - "tf_http_req_uri": "/", + "tf_http_req_uri": "/terraform", "tf_http_req_version": "HTTP/1.1", "tf_http_req_body": "An example multiline request body", "tf_http_trans_id": transId, "Accept-Encoding": "gzip", - "Host": "www.terraform.io", + "Host": "developer.hashicorp.com", "User-Agent": "Go-http-client/1.1", "Content-Length": "37", }); diff != "" { @@ -122,7 +122,7 @@ func TestNewSubsystemLoggingHTTPTransport(t *testing.T) { reqBody := `An example multiline request body` - req, _ := http.NewRequest("GET", "https://www.terraform.io", bytes.NewBufferString(reqBody)) + req, _ := http.NewRequest("GET", "https://developer.hashicorp.com/terraform", bytes.NewBufferString(reqBody)) res, err := client.Do(req.WithContext(ctx)) if err != nil { t.Fatalf("request failed: %v", err) @@ -158,12 +158,12 @@ func TestNewSubsystemLoggingHTTPTransport(t *testing.T) { "@module": "provider.test-subsystem", "tf_http_op_type": "request", "tf_http_req_method": "GET", - "tf_http_req_uri": "/", + "tf_http_req_uri": "/terraform", "tf_http_req_version": "HTTP/1.1", "tf_http_req_body": "An example multiline request body", "tf_http_trans_id": transId, "Accept-Encoding": "gzip", - "Host": "www.terraform.io", + "Host": "developer.hashicorp.com", "User-Agent": "Go-http-client/1.1", "Content-Length": "37", }); diff != "" { @@ -201,7 +201,7 @@ func TestNewSubsystemLoggingHTTPTransport(t *testing.T) { func TestNewLoggingHTTPTransport_LogMasking(t *testing.T) { ctx, loggerOutput := setupRootLogger() ctx = tflog.MaskFieldValuesWithFieldKeys(ctx, "tf_http_op_type") - ctx = tflog.MaskAllFieldValuesRegexes(ctx, regexp.MustCompile(`(?s).*`)) + ctx = tflog.MaskAllFieldValuesRegexes(ctx, regexp.MustCompile(`(?s).*`)) ctx = tflog.MaskMessageStrings(ctx, "Request", "Response") transport := logging.NewLoggingHTTPTransport(http.DefaultTransport) @@ -210,7 +210,7 @@ func TestNewLoggingHTTPTransport_LogMasking(t *testing.T) { Timeout: 10 * time.Second, } - req, _ := http.NewRequest("GET", "https://www.terraform.io", nil) + req, _ := http.NewRequest("GET", "https://developer.hashicorp.com/terraform", nil) res, err := client.Do(req.WithContext(ctx)) if err != nil { t.Fatalf("request failed: %v", err) @@ -266,7 +266,7 @@ func TestNewLoggingHTTPTransport_LogOmitting(t *testing.T) { Timeout: 10 * time.Second, } - req, _ := http.NewRequest("GET", "https://www.terraform.io", nil) + req, _ := http.NewRequest("GET", "https://developer.hashicorp.com/terraform", nil) res, err := client.Do(req.WithContext(ctx)) if err != nil { t.Fatalf("request failed: %v", err) diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go index 843314f3ba6..c1424f10770 100644 --- a/helper/schema/grpc_provider.go +++ b/helper/schema/grpc_provider.go @@ -1566,7 +1566,27 @@ func (s *GRPCProviderServer) ImportResourceState(ctx context.Context, req *tfpro return resp, nil } - newInstanceStates, err := s.provider.ImportState(ctx, info, req.ID) + var identity map[string]string + // parse identity data if available + if req.Identity != nil && req.Identity.IdentityData != nil { + // convert req.Identity to flat map identity structure + // Step 1: Turn JSON into cty.Value based on schema + identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName) + if err != nil { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", req.TypeName, err)) + return resp, nil + } + + identityVal, err := msgpack.Unmarshal(req.Identity.IdentityData.MsgPack, identityBlock.ImpliedType()) + if err != nil { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) + return resp, nil + } + // Step 2: Turn cty.Value into flatmap representation + identity = hcl2shim.FlatmapValueFromHCL2(identityVal) + } + + newInstanceStates, err := s.provider.ImportStateWithIdentity(ctx, info, req.ID, identity) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) return resp, nil @@ -1622,12 +1642,40 @@ func (s *GRPCProviderServer) ImportResourceState(ctx context.Context, req *tfpro return resp, nil } + var identityData *tfprotov5.ResourceIdentityData + if is.Identity != nil { + identityBlock, err := s.getResourceIdentitySchemaBlock(resourceType) + if err != nil { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", req.TypeName, err)) + return resp, nil + } + + newIdentityVal, err := hcl2shim.HCL2ValueFromFlatmap(is.Identity, identityBlock.ImpliedType()) + if err != nil { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) + return resp, nil + } + + newIdentityMP, err := msgpack.Marshal(newIdentityVal, identityBlock.ImpliedType()) + if err != nil { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) + return resp, nil + } + + identityData = &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: newIdentityMP, + }, + } + } + importedResource := &tfprotov5.ImportedResource{ TypeName: resourceType, State: &tfprotov5.DynamicValue{ MsgPack: newStateMP, }, - Private: meta, + Private: meta, + Identity: identityData, } resp.ImportedResources = append(resp.ImportedResources, importedResource) diff --git a/helper/schema/grpc_provider_test.go b/helper/schema/grpc_provider_test.go index 600a4ade34e..cdf97a30f13 100644 --- a/helper/schema/grpc_provider_test.go +++ b/helper/schema/grpc_provider_test.go @@ -7715,6 +7715,169 @@ func TestImportResourceState(t *testing.T) { }, }, }, + "basic-import-from-identity": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test_string": { + Type: TypeString, + Computed: true, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "id": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + Importer: &ResourceImporter{ + StateContext: func(ctx context.Context, d *ResourceData, meta interface{}) ([]*ResourceData, error) { + identity, err := d.Identity() + if err != nil { + t.Fatalf("failed to get identity: %v", err) + } + result, exists := identity.GetOk("id") + if !exists { + t.Fatalf("expected id to exist in identity") + } + + err = d.Set("test_string", "new-imported-val") + if err != nil { + return nil, err + } + + d.SetId(result.(string)) + + return []*ResourceData{d}, nil + }, + }, + }, + }, + }), + req: &tfprotov5.ImportResourceStateRequest{ + TypeName: "test", + Identity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("imported-id"), + }), + ), + }, + }, + }, + expected: &tfprotov5.ImportResourceStateResponse{ + ImportedResources: []*tfprotov5.ImportedResource{ + { + TypeName: "test", + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test_string": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("imported-id"), + "test_string": cty.StringVal("new-imported-val"), + }), + ), + }, + Private: []byte(`{"schema_version":"1"}`), + Identity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("imported-id"), + }), + ), + }, + }, + }, + }, + }, + }, + "basic-import-from-identity-no-id": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test_string": { + Type: TypeString, + Computed: true, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "id": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + Importer: &ResourceImporter{ + // Note: this does not set the Id on the ResourceData which results in an error that this test expects + StateContext: func(ctx context.Context, d *ResourceData, meta interface{}) ([]*ResourceData, error) { + err := d.Set("test_string", "new-imported-val") + if err != nil { + return nil, err + } + + return []*ResourceData{d}, nil + }, + }, + }, + }, + }), + req: &tfprotov5.ImportResourceStateRequest{ + TypeName: "test", + Identity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("imported-id"), + }), + ), + }, + }, + }, + expected: &tfprotov5.ImportResourceStateResponse{ + ImportedResources: nil, + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "The provider returned a resource missing an identifier during ImportResourceState. This is generally a bug in the resource implementation for import. Resource import code should not call d.SetId(\"\") or create an empty ResourceData. If the resource is missing, instead return an error. Please report this to the provider developers.", + }, + }, + }, + }, "resource-doesnt-exist": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ diff --git a/helper/schema/provider.go b/helper/schema/provider.go index ad258517eac..cb7e2a89813 100644 --- a/helper/schema/provider.go +++ b/helper/schema/provider.go @@ -476,6 +476,14 @@ func (p *Provider) ImportState( ctx context.Context, info *terraform.InstanceInfo, id string) ([]*terraform.InstanceState, error) { + return p.ImportStateWithIdentity(ctx, info, id, nil) +} + +func (p *Provider) ImportStateWithIdentity( + ctx context.Context, + info *terraform.InstanceInfo, + id string, + identity map[string]string) ([]*terraform.InstanceState, error) { // Find the resource r, ok := p.ResourcesMap[info.Type] if !ok { @@ -492,6 +500,16 @@ func (p *Provider) ImportState( data.SetId(id) data.SetType(info.Type) + if data.identitySchema != nil { + identityData, err := data.Identity() + if err != nil { + return nil, err // this should not happen, as we checked above + } + identityData.raw = identity // is this too hacky / unexpected? + } else if identity != nil { + return nil, fmt.Errorf("resource %s doesn't support identity import", info.Type) + } + // Call the import function results := []*ResourceData{data} if r.Importer.State != nil || r.Importer.StateContext != nil { diff --git a/helper/schema/provider_test.go b/helper/schema/provider_test.go index 0087c61d4c0..2426a6c189a 100644 --- a/helper/schema/provider_test.go +++ b/helper/schema/provider_test.go @@ -2271,6 +2271,164 @@ func TestProviderImportState(t *testing.T) { } } +func TestProviderImportStateWithIdentity(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + provider *Provider + info *terraform.InstanceInfo + id string + identity map[string]string + expectedStates []*terraform.InstanceState + expectedErr error + }{ + "error-missing-identity-schema": { + provider: &Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + Importer: &ResourceImporter{}, + // no identity schema defined + }, + }, + }, + identity: map[string]string{ + "id": "test-id", + }, + info: &terraform.InstanceInfo{ + Type: "test_resource", + }, + expectedErr: fmt.Errorf("resource test_resource doesn't support identity import"), + }, + "error-missing-ResourceData-Id": { + provider: &Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + Importer: &ResourceImporter{ + StateContext: func(_ context.Context, d *ResourceData, _ interface{}) ([]*ResourceData, error) { + // Example for handling import based on identity but not + // setting the id even though it's still required to be set + d.SetId("") + return []*ResourceData{d}, nil + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "id": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + }, + }, + }, + info: &terraform.InstanceInfo{ + Type: "test_resource", + }, + identity: map[string]string{ + "id": "test-id", + }, + expectedErr: fmt.Errorf("The provider returned a resource missing an identifier during ImportResourceState."), + }, + "Importer-StateContext-from-identity": { + provider: &Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + Importer: &ResourceImporter{ + StateContext: func(_ context.Context, d *ResourceData, meta interface{}) ([]*ResourceData, error) { + if d.Id() != "" { + return nil, fmt.Errorf("expected d.Id() to be empty, got: %s", d.Id()) + } + + identity, err := d.Identity() + if err != nil { + return nil, fmt.Errorf("error getting identity: %s", err) + } + id, exists := identity.GetOk("id") + if !exists { + return nil, fmt.Errorf("expected identity to contain key id") + } + if id != "test-id" { + return nil, fmt.Errorf("expected identity id %q, got: %s", "test-id", id) + } + + // set region as we act as if it's derived from provider defaults + err = identity.Set("region", "eu-central-1") + if err != nil { + return nil, fmt.Errorf("error setting identity region: %s", err) + } + // set the id as well + d.SetId(id.(string)) + + return []*ResourceData{d}, nil + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "id": { + Type: TypeString, + RequiredForImport: true, + }, + "region": { + Type: TypeString, + OptionalForImport: true, + }, + } + }, + }, + }, + }, + }, + info: &terraform.InstanceInfo{ + Type: "test_resource", + }, + identity: map[string]string{ + "id": "test-id", + }, + expectedStates: []*terraform.InstanceState{ + { + Attributes: map[string]string{"id": "test-id"}, + Ephemeral: terraform.EphemeralState{Type: "test_resource"}, + ID: "test-id", + Identity: map[string]string{"id": "test-id", "region": "eu-central-1"}, + Meta: map[string]interface{}{"schema_version": "0"}, + }, + }, + }, + } + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + states, err := testCase.provider.ImportStateWithIdentity(context.Background(), testCase.info, testCase.id, testCase.identity) + + if err != nil { + if testCase.expectedErr == nil { + t.Fatalf("unexpected error: %s", err) + } + + if !strings.Contains(err.Error(), testCase.expectedErr.Error()) { + t.Fatalf("expected error %q, got: %s", testCase.expectedErr, err) + } + } + + if err == nil && testCase.expectedErr != nil { + t.Fatalf("expected error %q, got none", testCase.expectedErr) + } + + if diff := cmp.Diff(states, testCase.expectedStates); diff != "" { + t.Fatalf("unexpected states difference: %s", diff) + } + }) + } +} + func TestProviderMeta(t *testing.T) { p := new(Provider) if v := p.Meta(); v != nil { diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 077a6ddb09b..c206df6c15e 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -1398,7 +1398,7 @@ func isReservedResourceFieldName(name string) bool { // // This function is useful for unit tests and ResourceImporter functions. func (r *Resource) Data(s *terraform.InstanceState) *ResourceData { - result, err := schemaMap(r.SchemaMap()).Data(s, nil) + result, err := schemaMapWithIdentity{r.SchemaMap(), r.Identity.SchemaMap()}.Data(s, nil) if err != nil { // At the time of writing, this isn't possible (Data never returns // non-nil errors). We panic to find this in the future if we have to. diff --git a/helper/schema/resource_importer.go b/helper/schema/resource_importer.go index ad9a5c3b9c6..e2c4d08b4f7 100644 --- a/helper/schema/resource_importer.go +++ b/helper/schema/resource_importer.go @@ -77,6 +77,9 @@ func ImportStatePassthrough(d *ResourceData, m interface{}) ([]*ResourceData, er // ImportStatePassthroughContext is an implementation of StateContextFunc that can be // used to simply pass the ID directly through. This should be used only // in the case that an ID-only refresh is possible. +// Please note that this implementation does not work when using resource identity as +// an Id still has to be set and the identity might contain multiple fields +// that are not the same as the ID. func ImportStatePassthroughContext(ctx context.Context, d *ResourceData, m interface{}) ([]*ResourceData, error) { return []*ResourceData{d}, nil }