From 9fb5a3285a651dd01ac3557bd44651730d13c236 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Mon, 12 May 2025 21:28:23 +0200 Subject: [PATCH 1/4] Validate that identities do not change after Terraform stores it --- helper/schema/grpc_provider.go | 102 +- helper/schema/grpc_provider_test.go | 2721 ++++++++++++++++++++------- terraform/state.go | 7 + 3 files changed, 2104 insertions(+), 726 deletions(-) diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go index c1424f10770..74cf16946a4 100644 --- a/helper/schema/grpc_provider.go +++ b/helper/schema/grpc_provider.go @@ -788,11 +788,43 @@ func (s *GRPCProviderServer) ConfigureProvider(ctx context.Context, req *tfproto func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.ReadResourceRequest) (*tfprotov5.ReadResourceResponse, error) { ctx = logging.InitContext(ctx) + readFollowingImport := false + + ReqPrivate := req.Private + + if ReqPrivate != nil { + // unmarshal the private data + if len(ReqPrivate) > 0 { + newReqPrivate := make(map[string]interface{}) + if err := json.Unmarshal(ReqPrivate, &newReqPrivate); err != nil { + return nil, err + } + // This internal private field is set on a resource during ImportResourceState to help framework determine if + // the resource has been recently imported. We only need to read this once, so we immediately clear it after. + if _, ok := newReqPrivate[terraform.ImportBeforeReadMetaKey]; ok { + readFollowingImport = true + delete(newReqPrivate, terraform.ImportBeforeReadMetaKey) + + if len(newReqPrivate) == 0 { + // if there are no other private data, set the private data to nil + ReqPrivate = nil + } else { + // set the new private data without the import key + bytes, err := json.Marshal(newReqPrivate) + if err != nil { + return nil, err + } + ReqPrivate = bytes + } + } + } + } + resp := &tfprotov5.ReadResourceResponse{ // helper/schema did previously handle private data during refresh, but // core is now going to expect this to be maintained in order to // persist it in the state. - Private: req.Private, + Private: ReqPrivate, } res, ok := s.provider.ResourcesMap[req.TypeName] @@ -832,7 +864,7 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re } instanceState.RawState = stateVal - // TODO: is there a more elegant way to do this? this requires us to look for the identity schema block again + var currentIdentityVal cty.Value if req.CurrentIdentity != nil && req.CurrentIdentity.IdentityData != nil { // convert req.CurrentIdentity to flat map identity structure @@ -843,20 +875,20 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re return resp, nil } - identityVal, err := msgpack.Unmarshal(req.CurrentIdentity.IdentityData.MsgPack, identityBlock.ImpliedType()) + currentIdentityVal, err = msgpack.Unmarshal(req.CurrentIdentity.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 - identityAttrs := hcl2shim.FlatmapValueFromHCL2(identityVal) + identityAttrs := hcl2shim.FlatmapValueFromHCL2(currentIdentityVal) // Step 3: Well, set it in the instanceState instanceState.Identity = identityAttrs } private := make(map[string]interface{}) - if len(req.Private) > 0 { - if err := json.Unmarshal(req.Private, &private); err != nil { + if len(ReqPrivate) > 0 { + if err := json.Unmarshal(ReqPrivate, &private); err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) return resp, nil } @@ -929,6 +961,15 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re return resp, nil } + // If we're refreshing the resource state (excluding a recently imported resource), validate that the new identity isn't changing + if !readFollowingImport && !currentIdentityVal.IsNull() && !currentIdentityVal.RawEquals(newIdentityVal) { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("Unexpected Identity Change: %s", "During the read operation, the Terraform Provider unexpectedly returned a different identity then the previously stored one.\n\n"+ + "This is always a problem with the provider and should be reported to the provider developer.\n\n"+ + fmt.Sprintf("Current Identity: %s\n\n", currentIdentityVal.GoString())+ + fmt.Sprintf("New Identity: %s", newIdentityVal.GoString()))) + return resp, nil + } + newIdentityMP, err := msgpack.Marshal(newIdentityVal, identityBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) @@ -1052,6 +1093,7 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot // turn the proposed state into a legacy configuration cfg := terraform.NewResourceConfigShimmed(proposedNewStateVal, schemaBlock) + var priorIdentityVal cty.Value // add identity data to priorState if req.PriorIdentity != nil && req.PriorIdentity.IdentityData != nil { // convert req.PriorIdentity to flat map identity structure @@ -1062,13 +1104,13 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot return resp, nil } - identityVal, err := msgpack.Unmarshal(req.PriorIdentity.IdentityData.MsgPack, identityBlock.ImpliedType()) + priorIdentityVal, err = msgpack.Unmarshal(req.PriorIdentity.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 - identityAttrs := hcl2shim.FlatmapValueFromHCL2(identityVal) + identityAttrs := hcl2shim.FlatmapValueFromHCL2(priorIdentityVal) // Step 3: Well, set it in the priorState priorState.Identity = identityAttrs } @@ -1257,13 +1299,26 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot return resp, nil } - newIdentityVal, err := hcl2shim.HCL2ValueFromFlatmap(diff.Identity, identityBlock.ImpliedType()) + plannedIdentityVal, err := hcl2shim.HCL2ValueFromFlatmap(diff.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 we're updating or deleting and we already have an identity stored, validate that the planned identity isn't changing + if !create && !priorIdentityVal.IsNull() && !priorIdentityVal.RawEquals(plannedIdentityVal) { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf( + "Unexpected Identity Change: During the planning operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+ + "This is always a problem with the provider and should be reported to the provider developer.\n\n"+ + "Prior Identity: %s\n\nPlanned Identity: %s", + priorIdentityVal.GoString(), + plannedIdentityVal.GoString(), + )) + + return resp, nil + } + + plannedIdentityMP, err := msgpack.Marshal(plannedIdentityVal, identityBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) return resp, nil @@ -1271,7 +1326,7 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot resp.PlannedIdentity = &tfprotov5.ResourceIdentityData{ IdentityData: &tfprotov5.DynamicValue{ - MsgPack: newIdentityMP, + MsgPack: plannedIdentityMP, }, } } @@ -1299,6 +1354,8 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro return resp, nil } + create := priorStateVal.IsNull() + plannedStateVal, err := msgpack.Unmarshal(req.PlannedState.MsgPack, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) @@ -1325,6 +1382,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro } } + var plannedIdentityVal cty.Value // add identity data to priorState if req.PlannedIdentity != nil && req.PlannedIdentity.IdentityData != nil { // convert req.PriorIdentity to flat map identity structure @@ -1335,13 +1393,13 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro return resp, nil } - identityVal, err := msgpack.Unmarshal(req.PlannedIdentity.IdentityData.MsgPack, identityBlock.ImpliedType()) + plannedIdentityVal, err = msgpack.Unmarshal(req.PlannedIdentity.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 - identityAttrs := hcl2shim.FlatmapValueFromHCL2(identityVal) + identityAttrs := hcl2shim.FlatmapValueFromHCL2(plannedIdentityVal) // Step 3: Well, set it in the priorState priorState.Identity = identityAttrs } @@ -1489,6 +1547,18 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro return resp, nil } + if !create && !plannedIdentityVal.IsNull() && !plannedIdentityVal.RawEquals(newIdentityVal) { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf( + "Unexpected Identity Change: During the update operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+ + "This is always a problem with the provider and should be reported to the provider developer.\n\n"+ + "Planned Identity: %s\n\nNew Identity: %s", + plannedIdentityVal.GoString(), + newIdentityVal.GoString(), + )) + + return resp, nil + } + newIdentityMP, err := msgpack.Marshal(newIdentityVal, identityBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) @@ -1636,6 +1706,10 @@ func (s *GRPCProviderServer) ImportResourceState(ctx context.Context, req *tfpro return resp, nil } + // Set an internal private field that will get sent alongside the imported resource. This will be cleared by + // the following ReadResource RPC and is primarily used to control validation of resource identities during refresh. + is.Meta[terraform.ImportBeforeReadMetaKey] = true + meta, err := json.Marshal(is.Meta) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) @@ -2262,7 +2336,7 @@ func (s *GRPCProviderServer) upgradeJSONIdentity(ctx context.Context, version in for _, upgrader := range res.Identity.IdentityUpgraders { if version != upgrader.Version { - continue + continue // TODO: can we replace this with an error (as we'll require all versions to be upgraded) } m, err = upgrader.Upgrade(ctx, m, s.provider.Meta()) diff --git a/helper/schema/grpc_provider_test.go b/helper/schema/grpc_provider_test.go index cdf97a30f13..643ed243083 100644 --- a/helper/schema/grpc_provider_test.go +++ b/helper/schema/grpc_provider_test.go @@ -5106,7 +5106,7 @@ func TestReadResource(t *testing.T) { if err != nil { return diag.FromErr(err) } - err = identity.Set("region", "new-region") + err = identity.Set("region", "test-region") // TODO: once we support disabling validation, this should be set to "new-region" and this test should ignore validation if err != nil { return diag.FromErr(err) } @@ -5181,7 +5181,7 @@ func TestReadResource(t *testing.T) { }), cty.ObjectVal(map[string]cty.Value{ "instance_id": cty.StringVal("test-id"), - "region": cty.StringVal("new-region"), + "region": cty.StringVal("test-region"), }), ), }, @@ -5432,409 +5432,411 @@ func TestReadResource(t *testing.T) { }, }, }, - } - - for name, testCase := range testCases { - t.Run(name, func(t *testing.T) { - t.Parallel() - resp, err := testCase.server.ReadResource(context.Background(), testCase.req) - - if err != nil { - t.Fatal(err) - } - - if diff := cmp.Diff(resp, testCase.expected, valueComparer); diff != "" { - ty := testCase.server.getResourceSchemaBlock("test").ImpliedType() - - if resp != nil && resp.NewState != nil { - t.Logf("resp.NewState.MsgPack: %s", mustMsgpackUnmarshal(ty, resp.NewState.MsgPack)) - } - - if testCase.expected != nil && testCase.expected.NewState != nil { - t.Logf("expected: %s", mustMsgpackUnmarshal(ty, testCase.expected.NewState.MsgPack)) - } - - t.Error(diff) - } - }) - } -} - -func TestPlanResourceChange(t *testing.T) { - t.Parallel() - - testCases := map[string]struct { - server *GRPCProviderServer - req *tfprotov5.PlanResourceChangeRequest - expected *tfprotov5.PlanResourceChangeResponse - }{ - "basic-plan": { + "update-resource-without-prior-identity-identity-may-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 4, + SchemaVersion: 1, Schema: map[string]*Schema{ - "foo": { - Type: TypeInt, - Optional: true, + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, }, }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + identity, err := d.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + + return nil + }, }, }, }), - req: &tfprotov5.PlanResourceChangeRequest{ - TypeName: "test", - PriorState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "foo": cty.Number, - }), - cty.NullVal( - cty.Object(map[string]cty.Type{ - "foo": cty.Number, - }), - ), - ), - }, - ProposedNewState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.Number, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), - }), - ), - }, - Config: &tfprotov5.DynamicValue{ + req: &tfprotov5.ReadResourceRequest{ + TypeName: "test", + CurrentIdentity: nil, // no prior identity because previous provider version didn't support it yet + CurrentState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.Number, + "test": cty.String, + "id": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "foo": cty.NullVal(cty.Number), + "test": cty.StringVal("hello"), + "id": cty.StringVal("initial"), }), ), }, }, - expected: &tfprotov5.PlanResourceChangeResponse{ - PlannedState: &tfprotov5.DynamicValue{ + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.Number, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), + "id": cty.StringVal("initial"), + "test": cty.StringVal("hello"), }), ), }, - RequiresReplace: []*tftypes.AttributePath{ - tftypes.NewAttributePath().WithAttributeName("id"), + NewIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), - UnsafeToUseLegacyTypeSystem: true, }, }, - "basic-plan-with-identity": { + "imported-resource-by-identity-identity-may-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 4, + SchemaVersion: 1, Schema: map[string]*Schema{ - "foo": { - Type: TypeInt, - Optional: true, + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, }, }, Identity: &ResourceIdentity{ Version: 1, SchemaFunc: func() map[string]*Schema { return map[string]*Schema{ - "name": { + "identity": { Type: TypeString, RequiredForImport: true, }, } }, }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + err := d.Set("test", "hello") + if err != nil { + return diag.FromErr(err) + } + + identity, err := d.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + + return nil + }, }, }, }), - req: &tfprotov5.PlanResourceChangeRequest{ + req: &tfprotov5.ReadResourceRequest{ TypeName: "test", - PriorState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "foo": cty.Number, - }), - cty.NullVal( + CurrentIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.Number, + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), }), ), - ), - }, - ProposedNewState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.Number, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), - }), - ), + }, }, - Config: &tfprotov5.DynamicValue{ + Private: []byte(`{".import_before_read":true}`), + CurrentState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.Number, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "foo": cty.NullVal(cty.Number), + "id": cty.StringVal("initial"), + "test": cty.UnknownVal(cty.String), }), ), }, - PriorIdentity: &tfprotov5.ResourceIdentityData{ - IdentityData: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "name": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "name": cty.StringVal("test-name"), - }), - ), - }, - }, }, - expected: &tfprotov5.PlanResourceChangeResponse{ - PlannedState: &tfprotov5.DynamicValue{ + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.Number, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), + "id": cty.StringVal("initial"), + "test": cty.StringVal("hello"), }), ), }, - RequiresReplace: []*tftypes.AttributePath{ - tftypes.NewAttributePath().WithAttributeName("id"), - }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), - UnsafeToUseLegacyTypeSystem: true, - PlannedIdentity: &tfprotov5.ResourceIdentityData{ + NewIdentity: &tfprotov5.ResourceIdentityData{ IdentityData: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "name": cty.String, + "identity": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "name": cty.StringVal("test-name"), + "identity": cty.StringVal("changed"), }), ), }, }, }, }, - "new-resource-with-identity": { + "imported-resource-by-id-identity-may-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 4, + SchemaVersion: 1, Schema: map[string]*Schema{ - "foo": { + "id": { Type: TypeString, - Optional: true, + Required: true, + }, + "test": { + Type: TypeString, }, }, Identity: &ResourceIdentity{ Version: 1, SchemaFunc: func() map[string]*Schema { return map[string]*Schema{ - "name": { + "identity": { Type: TypeString, RequiredForImport: true, }, } }, }, - CustomizeDiff: func(ctx context.Context, d *ResourceDiff, meta interface{}) error { + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + err := d.Set("test", "hello") + if err != nil { + return diag.FromErr(err) + } + identity, err := d.Identity() if err != nil { - return err + return diag.FromErr(err) } - err = identity.Set("name", "Peter") + err = identity.Set("identity", "changed") if err != nil { - return err + return diag.FromErr(err) } + return nil }, }, }, }), - req: &tfprotov5.PlanResourceChangeRequest{ - TypeName: "test", - PriorState: &tfprotov5.DynamicValue{ + req: &tfprotov5.ReadResourceRequest{ + TypeName: "test", + CurrentIdentity: nil, + Private: []byte(`{".import_before_read":true}`), + CurrentState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("baz"), - }), - ), - }, - ProposedNewState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("baz"), - }), - ), - }, - Config: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "foo": cty.StringVal("baz"), + "id": cty.StringVal("initial"), + "test": cty.UnknownVal(cty.String), }), ), }, }, - expected: &tfprotov5.PlanResourceChangeResponse{ - PlannedState: &tfprotov5.DynamicValue{ + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("baz"), + "id": cty.StringVal("initial"), + "test": cty.StringVal("hello"), }), ), }, - RequiresReplace: []*tftypes.AttributePath{ - tftypes.NewAttributePath().WithAttributeName("id"), - }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), - UnsafeToUseLegacyTypeSystem: true, - PlannedIdentity: &tfprotov5.ResourceIdentityData{ + NewIdentity: &tfprotov5.ResourceIdentityData{ IdentityData: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "name": cty.String, + "identity": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "name": cty.StringVal("Peter"), + "identity": cty.StringVal("changed"), }), ), }, }, }, }, - "no identity schema": { + "update-resource-identity-may-not-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 4, + SchemaVersion: 1, Schema: map[string]*Schema{ - "foo": { - Type: TypeInt, - Optional: true, + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, }, }, Identity: &ResourceIdentity{ Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + identity, err := d.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + + return nil }, }, }, }), - req: &tfprotov5.PlanResourceChangeRequest{ + req: &tfprotov5.ReadResourceRequest{ TypeName: "test", - PriorState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "foo": cty.Number, - }), - cty.NullVal( + CurrentIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.Number, + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), }), ), - ), + }, }, - ProposedNewState: &tfprotov5.DynamicValue{ + CurrentState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.Number, + "test": cty.String, + "id": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), + "test": cty.StringVal("hello"), + "id": cty.StringVal("initial"), }), ), }, - Config: &tfprotov5.DynamicValue{ + }, + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.Number, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "foo": cty.NullVal(cty.Number), + "id": cty.StringVal("initial"), + "test": cty.StringVal("hello"), }), ), }, - PriorIdentity: &tfprotov5.ResourceIdentityData{ - IdentityData: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "name": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "name": cty.StringVal("test-name"), - }), - ), - }, - }, - }, - expected: &tfprotov5.PlanResourceChangeResponse{ Diagnostics: []*tfprotov5.Diagnostic{ { Severity: tfprotov5.DiagnosticSeverityError, - Summary: "getting identity schema failed for resource 'test': resource does not have an identity schema", + Summary: (`Unexpected Identity Change: During the read operation, the Terraform Provider unexpectedly returned a different identity then the previously stored one. + +This is always a problem with the provider and should be reported to the provider developer. + +Current Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("initial")}) + +New Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("changed")})`), }, }, - UnsafeToUseLegacyTypeSystem: true, }, }, - "empty identity schema": { + // "destroy-resource-identity-may-not-change": not required, as same request and response as update-resource-identity-may-not-change + // "upgraded-identity-version-identity-may-not-change": not required, as same request and response as update-resource-identity-may-not-change + } + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + resp, err := testCase.server.ReadResource(context.Background(), testCase.req) + + if err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(resp, testCase.expected, valueComparer); diff != "" { + ty := testCase.server.getResourceSchemaBlock("test").ImpliedType() + + if resp != nil && resp.NewState != nil { + t.Logf("resp.NewState.MsgPack: %s", mustMsgpackUnmarshal(ty, resp.NewState.MsgPack)) + } + + if testCase.expected != nil && testCase.expected.NewState != nil { + t.Logf("expected: %s", mustMsgpackUnmarshal(ty, testCase.expected.NewState.MsgPack)) + } + + t.Error(diff) + } + }) + } +} + +func TestPlanResourceChange(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + server *GRPCProviderServer + req *tfprotov5.PlanResourceChangeRequest + expected *tfprotov5.PlanResourceChangeResponse + }{ + "basic-plan": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { @@ -5845,12 +5847,6 @@ func TestPlanResourceChange(t *testing.T) { Optional: true, }, }, - Identity: &ResourceIdentity{ - Version: 1, - SchemaFunc: func() map[string]*Schema { - return map[string]*Schema{} - }, - }, }, }, }), @@ -5892,41 +5888,49 @@ func TestPlanResourceChange(t *testing.T) { }), ), }, - PriorIdentity: &tfprotov5.ResourceIdentityData{ - IdentityData: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "name": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "name": cty.StringVal("test-name"), - }), - ), - }, - }, }, expected: &tfprotov5.PlanResourceChangeResponse{ - Diagnostics: []*tfprotov5.Diagnostic{ - { - Severity: tfprotov5.DiagnosticSeverityError, - Summary: "getting identity schema failed for resource 'test': identity schema must have at least one attribute", - }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.NullVal(cty.Number), + }), + ), + }, + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), UnsafeToUseLegacyTypeSystem: true, }, }, - "basic-plan-EnableLegacyTypeSystemPlanErrors": { + "basic-plan-with-identity": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - // Will set UnsafeToUseLegacyTypeSystem to false - EnableLegacyTypeSystemPlanErrors: true, + SchemaVersion: 4, Schema: map[string]*Schema{ "foo": { Type: TypeInt, Optional: true, }, }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "name": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, }, }, }), @@ -5968,6 +5972,18 @@ func TestPlanResourceChange(t *testing.T) { }), ), }, + PriorIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "name": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("test-name"), + }), + ), + }, + }, }, expected: &tfprotov5.PlanResourceChangeResponse{ PlannedState: &tfprotov5.DynamicValue{ @@ -5986,51 +6002,69 @@ func TestPlanResourceChange(t *testing.T) { tftypes.NewAttributePath().WithAttributeName("id"), }, PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), - UnsafeToUseLegacyTypeSystem: false, + UnsafeToUseLegacyTypeSystem: true, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "name": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("test-name"), + }), + ), + }, + }, }, }, - "deferred-with-provider-plan-modification": { + "new-resource-with-identity": { server: NewGRPCProviderServer(&Provider{ - providerDeferred: &Deferred{ - Reason: DeferredReasonProviderConfigUnknown, - }, ResourcesMap: map[string]*Resource{ "test": { - ResourceBehavior: ResourceBehavior{ - ProviderDeferred: ProviderDeferredBehavior{ - // Will ensure that CustomizeDiff is called - EnablePlanModification: true, - }, - }, SchemaVersion: 4, - CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { - return d.SetNew("foo", "new-foo-value") - }, Schema: map[string]*Schema{ "foo": { Type: TypeString, Optional: true, - Computed: true, }, }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "name": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, meta interface{}) error { + identity, err := d.Identity() + if err != nil { + return err + } + err = identity.Set("name", "Peter") + if err != nil { + return err + } + return nil + }, }, }, }), req: &tfprotov5.PlanResourceChangeRequest{ TypeName: "test", - ClientCapabilities: &tfprotov5.PlanResourceChangeClientCapabilities{ - DeferralAllowed: true, - }, PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ + "id": cty.String, "foo": cty.String, }), - cty.NullVal( - cty.Object(map[string]cty.Type{ - "foo": cty.String, - }), - ), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("baz"), + }), ), }, ProposedNewState: &tfprotov5.DynamicValue{ @@ -6041,7 +6075,7 @@ func TestPlanResourceChange(t *testing.T) { }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.UnknownVal(cty.String), + "foo": cty.StringVal("baz"), }), ), }, @@ -6053,15 +6087,12 @@ func TestPlanResourceChange(t *testing.T) { }), cty.ObjectVal(map[string]cty.Value{ "id": cty.NullVal(cty.String), - "foo": cty.NullVal(cty.String), + "foo": cty.StringVal("baz"), }), ), }, }, expected: &tfprotov5.PlanResourceChangeResponse{ - Deferred: &tfprotov5.Deferred{ - Reason: tfprotov5.DeferredReasonProviderConfigUnknown, - }, PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ @@ -6070,7 +6101,7 @@ func TestPlanResourceChange(t *testing.T) { }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("new-foo-value"), + "foo": cty.StringVal("baz"), }), ), }, @@ -6079,42 +6110,47 @@ func TestPlanResourceChange(t *testing.T) { }, PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), UnsafeToUseLegacyTypeSystem: true, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "name": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("Peter"), + }), + ), + }, + }, }, }, - "deferred-skip-plan-modification": { + "no identity schema": { server: NewGRPCProviderServer(&Provider{ - providerDeferred: &Deferred{ - Reason: DeferredReasonProviderConfigUnknown, - }, ResourcesMap: map[string]*Resource{ "test": { SchemaVersion: 4, - CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { - return errors.New("Test assertion failed: CustomizeDiff shouldn't be called") - }, Schema: map[string]*Schema{ "foo": { - Type: TypeString, + Type: TypeInt, Optional: true, - Computed: true, }, }, + Identity: &ResourceIdentity{ + Version: 1, + }, }, }, }), req: &tfprotov5.PlanResourceChangeRequest{ TypeName: "test", - ClientCapabilities: &tfprotov5.PlanResourceChangeClientCapabilities{ - DeferralAllowed: true, - }, PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.String, + "foo": cty.Number, }), cty.NullVal( cty.Object(map[string]cty.Type{ - "foo": cty.String, + "foo": cty.Number, }), ), ), @@ -6123,11 +6159,11 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.String, + "foo": cty.Number, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("from-config!"), + "foo": cty.NullVal(cty.Number), }), ), }, @@ -6135,53 +6171,52 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.String, + "foo": cty.Number, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.NullVal(cty.String), - "foo": cty.StringVal("from-config!"), + "foo": cty.NullVal(cty.Number), }), ), }, + PriorIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "name": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("test-name"), + }), + ), + }, + }, }, expected: &tfprotov5.PlanResourceChangeResponse{ - Deferred: &tfprotov5.Deferred{ - Reason: tfprotov5.DeferredReasonProviderConfigUnknown, - }, - // Returns proposed new state with deferred response - PlannedState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("from-config!"), - }), - ), + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "getting identity schema failed for resource 'test': resource does not have an identity schema", + }, }, UnsafeToUseLegacyTypeSystem: true, }, }, - "create: write-only value can be retrieved in CustomizeDiff": { + "empty identity schema": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { SchemaVersion: 4, - CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { - val := d.Get("foo") - if val != "bar" { - t.Fatalf("Incorrect write-only value") - } - - return nil - }, Schema: map[string]*Schema{ "foo": { - Type: TypeString, - Optional: true, - WriteOnly: true, + Type: TypeInt, + Optional: true, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{} }, }, }, @@ -6192,11 +6227,11 @@ func TestPlanResourceChange(t *testing.T) { PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.String, + "foo": cty.Number, }), cty.NullVal( cty.Object(map[string]cty.Type{ - "foo": cty.String, + "foo": cty.Number, }), ), ), @@ -6205,11 +6240,11 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.String, + "foo": cty.Number, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("bar"), + "foo": cty.NullVal(cty.Number), }), ), }, @@ -6217,50 +6252,47 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.String, + "foo": cty.Number, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.NullVal(cty.String), - "foo": cty.StringVal("bar"), + "foo": cty.NullVal(cty.Number), }), ), }, + PriorIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "name": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("test-name"), + }), + ), + }, + }, }, expected: &tfprotov5.PlanResourceChangeResponse{ - PlannedState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.String), - }), - ), - }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), - RequiresReplace: []*tftypes.AttributePath{ - tftypes.NewAttributePath().WithAttributeName("id"), + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "getting identity schema failed for resource 'test': identity schema must have at least one attribute", + }, }, UnsafeToUseLegacyTypeSystem: true, }, }, - "create: write-only values are nullified in PlanResourceChangeResponse": { + "basic-plan-EnableLegacyTypeSystemPlanErrors": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 4, + // Will set UnsafeToUseLegacyTypeSystem to false + EnableLegacyTypeSystemPlanErrors: true, Schema: map[string]*Schema{ "foo": { - Type: TypeString, - Optional: true, - WriteOnly: true, - }, - "bar": { - Type: TypeString, - Optional: true, - WriteOnly: true, + Type: TypeInt, + Optional: true, }, }, }, @@ -6271,13 +6303,11 @@ func TestPlanResourceChange(t *testing.T) { PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.String, - "bar": cty.String, + "foo": cty.Number, }), cty.NullVal( cty.Object(map[string]cty.Type{ - "foo": cty.String, - "bar": cty.String, + "foo": cty.Number, }), ), ), @@ -6286,13 +6316,11 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.String, - "bar": cty.String, + "foo": cty.Number, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("baz"), - "bar": cty.StringVal("boop"), + "foo": cty.NullVal(cty.Number), }), ), }, @@ -6300,13 +6328,11 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.String, - "bar": cty.String, + "foo": cty.Number, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.NullVal(cty.String), - "foo": cty.StringVal("baz"), - "bar": cty.StringVal("boop"), + "foo": cty.NullVal(cty.Number), }), ), }, @@ -6316,45 +6342,43 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.String, - "bar": cty.String, + "foo": cty.Number, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.String), - "bar": cty.NullVal(cty.String), + "foo": cty.NullVal(cty.Number), }), ), }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), RequiresReplace: []*tftypes.AttributePath{ tftypes.NewAttributePath().WithAttributeName("id"), }, - UnsafeToUseLegacyTypeSystem: true, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + UnsafeToUseLegacyTypeSystem: false, }, }, - "update: write-only value can be retrieved in CustomizeDiff": { + "deferred-with-provider-plan-modification": { server: NewGRPCProviderServer(&Provider{ + providerDeferred: &Deferred{ + Reason: DeferredReasonProviderConfigUnknown, + }, ResourcesMap: map[string]*Resource{ "test": { + ResourceBehavior: ResourceBehavior{ + ProviderDeferred: ProviderDeferredBehavior{ + // Will ensure that CustomizeDiff is called + EnablePlanModification: true, + }, + }, SchemaVersion: 4, CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { - val := d.Get("write_only") - if val != "bar" { - t.Fatalf("Incorrect write-only value") - } - - return nil + return d.SetNew("foo", "new-foo-value") }, Schema: map[string]*Schema{ - "configured": { + "foo": { Type: TypeString, Optional: true, - }, - "write_only": { - Type: TypeString, - Optional: true, - WriteOnly: true, + Computed: true, }, }, }, @@ -6362,87 +6386,167 @@ func TestPlanResourceChange(t *testing.T) { }), req: &tfprotov5.PlanResourceChangeRequest{ TypeName: "test", + ClientCapabilities: &tfprotov5.PlanResourceChangeClientCapabilities{ + DeferralAllowed: true, + }, PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_only": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "configured": cty.StringVal("prior_val"), - "write_only": cty.NullVal(cty.String), + "foo": cty.String, }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + }), + ), ), }, ProposedNewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_only": cty.String, + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "configured": cty.StringVal("updated_val"), - "write_only": cty.StringVal("bar"), + "id": cty.UnknownVal(cty.String), + "foo": cty.UnknownVal(cty.String), }), ), }, Config: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_only": cty.String, + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "configured": cty.StringVal("updated_val"), - "write_only": cty.StringVal("bar"), + "id": cty.NullVal(cty.String), + "foo": cty.NullVal(cty.String), }), ), }, }, expected: &tfprotov5.PlanResourceChangeResponse{ + Deferred: &tfprotov5.Deferred{ + Reason: tfprotov5.DeferredReasonProviderConfigUnknown, + }, PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_only": cty.String, + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "configured": cty.StringVal("updated_val"), - "write_only": cty.NullVal(cty.String), + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("new-foo-value"), }), ), }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), RequiresReplace: []*tftypes.AttributePath{ tftypes.NewAttributePath().WithAttributeName("id"), }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), UnsafeToUseLegacyTypeSystem: true, }, }, - "update: write-only values are nullified in PlanResourceChangeResponse": { + "deferred-skip-plan-modification": { server: NewGRPCProviderServer(&Provider{ + providerDeferred: &Deferred{ + Reason: DeferredReasonProviderConfigUnknown, + }, ResourcesMap: map[string]*Resource{ "test": { SchemaVersion: 4, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { + return errors.New("Test assertion failed: CustomizeDiff shouldn't be called") + }, Schema: map[string]*Schema{ - "configured": { + "foo": { Type: TypeString, Optional: true, + Computed: true, }, - "write_onlyA": { - Type: TypeString, - Optional: true, - WriteOnly: true, - }, - "write_onlyB": { + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + ClientCapabilities: &tfprotov5.PlanResourceChangeClientCapabilities{ + DeferralAllowed: true, + }, + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + }), + ), + ), + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("from-config!"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.StringVal("from-config!"), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + Deferred: &tfprotov5.Deferred{ + Reason: tfprotov5.DeferredReasonProviderConfigUnknown, + }, + // Returns proposed new state with deferred response + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("from-config!"), + }), + ), + }, + UnsafeToUseLegacyTypeSystem: true, + }, + }, + "create: write-only value can be retrieved in CustomizeDiff": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { + val := d.Get("foo") + if val != "bar" { + t.Fatalf("Incorrect write-only value") + } + + return nil + }, + Schema: map[string]*Schema{ + "foo": { Type: TypeString, Optional: true, WriteOnly: true, @@ -6456,48 +6560,36 @@ func TestPlanResourceChange(t *testing.T) { PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_onlyA": cty.String, - "write_onlyB": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "configured": cty.StringVal("prior_val"), - "write_onlyA": cty.NullVal(cty.String), - "write_onlyB": cty.NullVal(cty.String), + "foo": cty.String, }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + }), + ), ), }, ProposedNewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_onlyA": cty.String, - "write_onlyB": cty.String, + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "configured": cty.StringVal("updated_val"), - "write_onlyA": cty.StringVal("foo"), - "write_onlyB": cty.StringVal("bar"), + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("bar"), }), ), }, Config: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_onlyA": cty.String, - "write_onlyB": cty.String, + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "configured": cty.StringVal("updated_val"), - "write_onlyA": cty.StringVal("foo"), - "write_onlyB": cty.StringVal("bar"), + "id": cty.NullVal(cty.String), + "foo": cty.StringVal("bar"), }), ), }, @@ -6506,16 +6598,12 @@ func TestPlanResourceChange(t *testing.T) { PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_onlyA": cty.String, - "write_onlyB": cty.String, + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "configured": cty.StringVal("updated_val"), - "write_onlyA": cty.NullVal(cty.String), - "write_onlyB": cty.NullVal(cty.String), + "id": cty.UnknownVal(cty.String), + "foo": cty.NullVal(cty.String), }), ), }, @@ -6526,131 +6614,11 @@ func TestPlanResourceChange(t *testing.T) { UnsafeToUseLegacyTypeSystem: true, }, }, - } - - for name, testCase := range testCases { - t.Run(name, func(t *testing.T) { - t.Parallel() - - resp, err := testCase.server.PlanResourceChange(context.Background(), testCase.req) - if err != nil { - t.Fatal(err) - } - - if diff := cmp.Diff(resp, testCase.expected, valueComparer); diff != "" { - ty := testCase.server.getResourceSchemaBlock("test").ImpliedType() - - if resp != nil && resp.PlannedState != nil { - t.Logf("resp.PlannedState.MsgPack: %s", mustMsgpackUnmarshal(ty, resp.PlannedState.MsgPack)) - } - - if testCase.expected != nil && testCase.expected.PlannedState != nil { - t.Logf("expected: %s", mustMsgpackUnmarshal(ty, testCase.expected.PlannedState.MsgPack)) - } - - t.Error(diff) - } - }) - } -} - -func TestPlanResourceChange_bigint(t *testing.T) { - r := &Resource{ - UseJSONNumber: true, - Schema: map[string]*Schema{ - "foo": { - Type: TypeInt, - Required: true, - }, - }, - } - - server := NewGRPCProviderServer(&Provider{ - ResourcesMap: map[string]*Resource{ - "test": r, - }, - }) - - schema := r.CoreConfigSchema() - priorState, err := msgpack.Marshal(cty.NullVal(schema.ImpliedType()), schema.ImpliedType()) - if err != nil { - t.Fatal(err) - } - - proposedVal := cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.MustParseNumberVal("7227701560655103598"), - }) - proposedState, err := msgpack.Marshal(proposedVal, schema.ImpliedType()) - if err != nil { - t.Fatal(err) - } - - config, err := schema.CoerceValue(cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "foo": cty.MustParseNumberVal("7227701560655103598"), - })) - if err != nil { - t.Fatal(err) - } - configBytes, err := msgpack.Marshal(config, schema.ImpliedType()) - if err != nil { - t.Fatal(err) - } - - testReq := &tfprotov5.PlanResourceChangeRequest{ - TypeName: "test", - PriorState: &tfprotov5.DynamicValue{ - MsgPack: priorState, - }, - ProposedNewState: &tfprotov5.DynamicValue{ - MsgPack: proposedState, - }, - Config: &tfprotov5.DynamicValue{ - MsgPack: configBytes, - }, - } - - resp, err := server.PlanResourceChange(context.Background(), testReq) - if err != nil { - t.Fatal(err) - } - - plannedStateVal, err := msgpack.Unmarshal(resp.PlannedState.MsgPack, schema.ImpliedType()) - if err != nil { - t.Fatal(err) - } - - if !cmp.Equal(proposedVal, plannedStateVal, valueComparer) { - t.Fatal(cmp.Diff(proposedVal, plannedStateVal, valueComparer)) - } - - plannedStateFoo, acc := plannedStateVal.GetAttr("foo").AsBigFloat().Int64() - if acc != big.Exact { - t.Fatalf("Expected exact accuracy, got %s", acc) - } - if plannedStateFoo != 7227701560655103598 { - t.Fatalf("Expected %d, got %d, this represents a loss of precision in planning large numbers", 7227701560655103598, plannedStateFoo) - } -} - -func TestApplyResourceChange(t *testing.T) { - t.Parallel() - - testCases := map[string]struct { - server *GRPCProviderServer - req *tfprotov5.ApplyResourceChangeRequest - expected *tfprotov5.ApplyResourceChangeResponse - }{ - "create: write-only values are nullified in ApplyResourceChangeResponse": { + "create: write-only values are nullified in PlanResourceChangeResponse": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { SchemaVersion: 4, - CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { - rd.SetId("baz") - return nil - }, Schema: map[string]*Schema{ "foo": { Type: TypeString, @@ -6666,7 +6634,7 @@ func TestApplyResourceChange(t *testing.T) { }, }, }), - req: &tfprotov5.ApplyResourceChangeRequest{ + req: &tfprotov5.PlanResourceChangeRequest{ TypeName: "test", PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( @@ -6682,7 +6650,7 @@ func TestApplyResourceChange(t *testing.T) { ), ), }, - PlannedState: &tfprotov5.DynamicValue{ + ProposedNewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, @@ -6711,8 +6679,8 @@ func TestApplyResourceChange(t *testing.T) { ), }, }, - expected: &tfprotov5.ApplyResourceChangeResponse{ - NewState: &tfprotov5.DynamicValue{ + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, @@ -6720,30 +6688,118 @@ func TestApplyResourceChange(t *testing.T) { "bar": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("baz"), + "id": cty.UnknownVal(cty.String), "foo": cty.NullVal(cty.String), "bar": cty.NullVal(cty.String), }), ), }, - Private: []uint8(`{"schema_version":"4"}`), + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, UnsafeToUseLegacyTypeSystem: true, }, }, - "update: write-only values are nullified in ApplyResourceChangeResponse": { + "update: write-only value can be retrieved in CustomizeDiff": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { SchemaVersion: 4, - CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { - rd.SetId("baz") - s := rd.Get("configured").(string) - err := rd.Set("configured", s) - if err != nil { - return nil + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { + val := d.Get("write_only") + if val != "bar" { + t.Fatalf("Incorrect write-only value") } + return nil }, + Schema: map[string]*Schema{ + "configured": { + Type: TypeString, + Optional: true, + }, + "write_only": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_only": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("prior_val"), + "write_only": cty.NullVal(cty.String), + }), + ), + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_only": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_only": cty.StringVal("bar"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_only": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_only": cty.StringVal("bar"), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_only": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_only": cty.NullVal(cty.String), + }), + ), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, + UnsafeToUseLegacyTypeSystem: true, + }, + }, + "update: write-only values are nullified in PlanResourceChangeResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, Schema: map[string]*Schema{ "configured": { Type: TypeString, @@ -6763,7 +6819,7 @@ func TestApplyResourceChange(t *testing.T) { }, }, }), - req: &tfprotov5.ApplyResourceChangeRequest{ + req: &tfprotov5.PlanResourceChangeRequest{ TypeName: "test", PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( @@ -6781,7 +6837,7 @@ func TestApplyResourceChange(t *testing.T) { }), ), }, - PlannedState: &tfprotov5.DynamicValue{ + ProposedNewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, @@ -6806,10 +6862,1145 @@ func TestApplyResourceChange(t *testing.T) { "write_onlyB": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "configured": cty.StringVal("updated_val"), - "write_onlyA": cty.StringVal("foo"), - "write_onlyB": cty.StringVal("bar"), + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.StringVal("foo"), + "write_onlyB": cty.StringVal("bar"), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.NullVal(cty.String), + "write_onlyB": cty.NullVal(cty.String), + }), + ), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, + UnsafeToUseLegacyTypeSystem: true, + }, + }, + "create-resource-identity-may-change": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, meta interface{}) error { + identity, err := d.Identity() + if err != nil { + return err + } + err = identity.Set("identity", "changed") + if err != nil { + return err + } + return nil + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + ), + ), + }, + PriorIdentity: nil, // create! + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + UnsafeToUseLegacyTypeSystem: true, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, + }, + }, + }, + "update-resource-identity-may-not-change": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, meta interface{}) error { + identity, err := d.Identity() + if err != nil { + return err + } + err = identity.Set("identity", "changed") + if err != nil { + return err + } + return nil + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), + ), + }, + PriorIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), + }), + ), + }, + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + UnsafeToUseLegacyTypeSystem: true, + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: `Unexpected Identity Change: During the planning operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one. + +This is always a problem with the provider and should be reported to the provider developer. + +Prior Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("initial")}) + +Planned Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("changed")})`, + }, + }, + }, + }, + "update-resource-without-prior-identity-identity-may-change": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, meta interface{}) error { + identity, err := d.Identity() + if err != nil { + return err + } + err = identity.Set("identity", "changed") + if err != nil { + return err + } + return nil + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), + ), + }, + PriorIdentity: nil, // no identity yet (prior provider version didn't support it and there was an upgrade without refresh) + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), + ), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + UnsafeToUseLegacyTypeSystem: true, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, + }, + }, + }, + "destroy-resource-identity-may-not-change": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, meta interface{}) error { + identity, err := d.Identity() + if err != nil { + return err + } + // Note: this entire function won't be run anyways for destroys (as that'll short circuit and return the prior identity) + // it's still in this test so we'd see this as an error if something breaks over in the handler + err = identity.Set("identity", "changed_this_should_not_appear_anywhere!") + if err != nil { + return err + } + return nil + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), + ), + }, + PriorIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), + }), + ), + }, + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + ), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + ), + ), + }, + UnsafeToUseLegacyTypeSystem: true, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), + }), + ), + }, + }, + }, + }, + // "imported-resource-by-id-identity-may-not-change" same as update-resource-identity-may-not-change + // "imported-resource-by-identity-identity-may-not-change" same as update-resource-identity-may-not-change + // "upgraded-identity-version-identity-may-not-change" same as update-resource-identity-may-not-change + } + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + resp, err := testCase.server.PlanResourceChange(context.Background(), testCase.req) + if err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(resp, testCase.expected, valueComparer); diff != "" { + ty := testCase.server.getResourceSchemaBlock("test").ImpliedType() + + if resp != nil && resp.PlannedState != nil { + t.Logf("resp.PlannedState.MsgPack: %s", mustMsgpackUnmarshal(ty, resp.PlannedState.MsgPack)) + } + + if testCase.expected != nil && testCase.expected.PlannedState != nil { + t.Logf("expected: %s", mustMsgpackUnmarshal(ty, testCase.expected.PlannedState.MsgPack)) + } + + t.Error(diff) + } + }) + } +} + +func TestPlanResourceChange_bigint(t *testing.T) { + r := &Resource{ + UseJSONNumber: true, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Required: true, + }, + }, + } + + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": r, + }, + }) + + schema := r.CoreConfigSchema() + priorState, err := msgpack.Marshal(cty.NullVal(schema.ImpliedType()), schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + proposedVal := cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.MustParseNumberVal("7227701560655103598"), + }) + proposedState, err := msgpack.Marshal(proposedVal, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + config, err := schema.CoerceValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.MustParseNumberVal("7227701560655103598"), + })) + if err != nil { + t.Fatal(err) + } + configBytes, err := msgpack.Marshal(config, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + testReq := &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: priorState, + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: proposedState, + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: configBytes, + }, + } + + resp, err := server.PlanResourceChange(context.Background(), testReq) + if err != nil { + t.Fatal(err) + } + + plannedStateVal, err := msgpack.Unmarshal(resp.PlannedState.MsgPack, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + if !cmp.Equal(proposedVal, plannedStateVal, valueComparer) { + t.Fatal(cmp.Diff(proposedVal, plannedStateVal, valueComparer)) + } + + plannedStateFoo, acc := plannedStateVal.GetAttr("foo").AsBigFloat().Int64() + if acc != big.Exact { + t.Fatalf("Expected exact accuracy, got %s", acc) + } + if plannedStateFoo != 7227701560655103598 { + t.Fatalf("Expected %d, got %d, this represents a loss of precision in planning large numbers", 7227701560655103598, plannedStateFoo) + } +} + +func TestApplyResourceChange(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + server *GRPCProviderServer + req *tfprotov5.ApplyResourceChangeRequest + expected *tfprotov5.ApplyResourceChangeResponse + }{ + "create: write-only values are nullified in ApplyResourceChangeResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + return nil + }, + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + "bar": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + "bar": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + "bar": cty.String, + }), + ), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + "bar": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("baz"), + "bar": cty.StringVal("boop"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + "bar": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.StringVal("baz"), + "bar": cty.StringVal("boop"), + }), + ), + }, + }, + expected: &tfprotov5.ApplyResourceChangeResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + "bar": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("baz"), + "foo": cty.NullVal(cty.String), + "bar": cty.NullVal(cty.String), + }), + ), + }, + Private: []uint8(`{"schema_version":"4"}`), + UnsafeToUseLegacyTypeSystem: true, + }, + }, + "update: write-only values are nullified in ApplyResourceChangeResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + s := rd.Get("configured").(string) + err := rd.Set("configured", s) + if err != nil { + return nil + } + return nil + }, + Schema: map[string]*Schema{ + "configured": { + Type: TypeString, + Optional: true, + }, + "write_onlyA": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + "write_onlyB": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("prior_val"), + "write_onlyA": cty.NullVal(cty.String), + "write_onlyB": cty.NullVal(cty.String), + }), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.StringVal("foo"), + "write_onlyB": cty.StringVal("bar"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.StringVal("foo"), + "write_onlyB": cty.StringVal("bar"), + }), + ), + }, + }, + expected: &tfprotov5.ApplyResourceChangeResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("baz"), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.NullVal(cty.String), + "write_onlyB": cty.NullVal(cty.String), + }), + ), + }, + Private: []uint8(`{"schema_version":"4"}`), + UnsafeToUseLegacyTypeSystem: true, + }, + }, + "create: identity returned in ApplyResourceChangeResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + identity, err := rd.Identity() + if err != nil { + t.Fatal(err) + } + err = identity.Set("ident", "bazz") + if err != nil { + t.Fatal(err) + } + return nil + }, + Schema: map[string]*Schema{}, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "ident": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{}), + cty.NullVal( + cty.Object(map[string]cty.Type{}), + ), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + }), + ), + }, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "ident": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "ident": cty.UnknownVal(cty.String), // TODO: allow change during create in handler! + }), + ), + }, + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + }), + ), + }, + }, + expected: &tfprotov5.ApplyResourceChangeResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("baz"), + }), + ), + }, + Private: []uint8(`{"schema_version":"4"}`), + UnsafeToUseLegacyTypeSystem: true, + NewIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "ident": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "ident": cty.StringVal("bazz"), + }), + ), + }, + }, + }, + }, + "create: no identity schema diag in ApplyResourceChangeResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, + Schema: map[string]*Schema{}, + Identity: &ResourceIdentity{ + Version: 1, + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{}), + cty.NullVal( + cty.Object(map[string]cty.Type{}), + ), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + }), + ), + }, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "ident": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "ident": cty.UnknownVal(cty.String), + }), + ), + }, + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + }), + ), + }, + }, + expected: &tfprotov5.ApplyResourceChangeResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "getting identity schema failed for resource 'test': resource does not have an identity schema", + }, + }, + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal(cty.DynamicPseudoType, cty.NullVal(cty.DynamicPseudoType)), + }, + }, + }, + "create: empty identity schema diag in ApplyResourceChangeResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, + Schema: map[string]*Schema{}, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{} + }, + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{}), + cty.NullVal( + cty.Object(map[string]cty.Type{}), + ), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + }), + ), + }, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "ident": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "ident": cty.UnknownVal(cty.String), + }), + ), + }, + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + }), + ), + }, + }, + expected: &tfprotov5.ApplyResourceChangeResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "getting identity schema failed for resource 'test': identity schema must have at least one attribute", + }, + }, + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal(cty.DynamicPseudoType, cty.NullVal(cty.DynamicPseudoType)), + }, + }, + }, + "create-resource-identity-may-change": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + identity, err := rd.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + rd.SetId("changed") + return nil + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + ), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), + }), + ), + }, + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), }), ), }, @@ -6818,52 +8009,68 @@ func TestApplyResourceChange(t *testing.T) { NewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "configured": cty.String, - "write_onlyA": cty.String, - "write_onlyB": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("baz"), - "configured": cty.StringVal("updated_val"), - "write_onlyA": cty.NullVal(cty.String), - "write_onlyB": cty.NullVal(cty.String), + "id": cty.StringVal("changed"), + "test": cty.StringVal("initial"), }), ), }, - Private: []uint8(`{"schema_version":"4"}`), + Private: []uint8(`{"schema_version":"1"}`), UnsafeToUseLegacyTypeSystem: true, + NewIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, + }, }, }, - "create: identity returned in ApplyResourceChangeResponse": { + "update-resource-identity-may-not-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 4, - CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { - rd.SetId("baz") - identity, err := rd.Identity() - if err != nil { - t.Fatal(err) - } - err = identity.Set("ident", "bazz") - if err != nil { - t.Fatal(err) - } - return nil + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, }, - Schema: map[string]*Schema{}, Identity: &ResourceIdentity{ Version: 1, SchemaFunc: func() map[string]*Schema { return map[string]*Schema{ - "ident": { + "identity": { Type: TypeString, RequiredForImport: true, }, } }, }, + UpdateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + identity, err := rd.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + rd.SetId("changed") + return nil + }, }, }, }), @@ -6871,19 +8078,25 @@ func TestApplyResourceChange(t *testing.T) { TypeName: "test", PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{}), - cty.NullVal( - cty.Object(map[string]cty.Type{}), - ), + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), ), }, PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), }), ), }, @@ -6891,10 +8104,10 @@ func TestApplyResourceChange(t *testing.T) { IdentityData: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "ident": cty.String, + "identity": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "ident": cty.UnknownVal(cty.String), + "identity": cty.StringVal("initial"), }), ), }, @@ -6902,10 +8115,12 @@ func TestApplyResourceChange(t *testing.T) { Config: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), }), ), }, @@ -6914,37 +8129,66 @@ func TestApplyResourceChange(t *testing.T) { NewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("baz"), + "id": cty.StringVal("changed"), + "test": cty.StringVal("initial"), }), ), }, - Private: []uint8(`{"schema_version":"4"}`), - UnsafeToUseLegacyTypeSystem: true, - NewIdentity: &tfprotov5.ResourceIdentityData{ - IdentityData: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "ident": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "ident": cty.StringVal("bazz"), - }), - ), + Private: []uint8(`{"schema_version":"1"}`), + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: `Unexpected Identity Change: During the update operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one. + +This is always a problem with the provider and should be reported to the provider developer. + +Planned Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("initial")}) + +New Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("changed")})`, }, }, }, }, - "create: no identity schema diag in ApplyResourceChangeResponse": { + "update-resource-without-planned-identity-identity-may-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 4, - Schema: map[string]*Schema{}, + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, Identity: &ResourceIdentity{ Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + UpdateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + identity, err := rd.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + rd.SetId("changed") + return nil }, }, }, @@ -6953,69 +8197,108 @@ func TestApplyResourceChange(t *testing.T) { TypeName: "test", PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{}), - cty.NullVal( - cty.Object(map[string]cty.Type{}), - ), + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), ), }, PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), }), ), }, - PlannedIdentity: &tfprotov5.ResourceIdentityData{ - IdentityData: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{ - "ident": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "ident": cty.UnknownVal(cty.String), - }), - ), - }, - }, + PlannedIdentity: nil, Config: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), }), ), }, }, expected: &tfprotov5.ApplyResourceChangeResponse{ - Diagnostics: []*tfprotov5.Diagnostic{ - { - Severity: tfprotov5.DiagnosticSeverityError, - Summary: "getting identity schema failed for resource 'test': resource does not have an identity schema", - }, - }, NewState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal(cty.DynamicPseudoType, cty.NullVal(cty.DynamicPseudoType)), + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("changed"), + "test": cty.StringVal("initial"), + }), + ), + }, + Private: []uint8(`{"schema_version":"1"}`), + UnsafeToUseLegacyTypeSystem: true, + NewIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, }, }, }, - "create: empty identity schema diag in ApplyResourceChangeResponse": { + "destroy-resource-identity-may-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 4, - Schema: map[string]*Schema{}, + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, Identity: &ResourceIdentity{ Version: 1, SchemaFunc: func() map[string]*Schema { - return map[string]*Schema{} + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } }, }, + DeleteContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + identity, err := rd.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + rd.SetId("changed") + return nil + }, }, }, }), @@ -7023,30 +8306,36 @@ func TestApplyResourceChange(t *testing.T) { TypeName: "test", PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( - cty.Object(map[string]cty.Type{}), - cty.NullVal( - cty.Object(map[string]cty.Type{}), - ), + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), ), }, PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - }), - cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), + "id": cty.String, + "test": cty.String, }), + cty.NullVal(cty.Object(map[string]cty.Type{ // NullVal => destroy + "id": cty.String, + "test": cty.String, + })), ), }, PlannedIdentity: &tfprotov5.ResourceIdentityData{ IdentityData: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "ident": cty.String, + "identity": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "ident": cty.UnknownVal(cty.String), + "identity": cty.StringVal("initial"), }), ), }, @@ -7054,26 +8343,34 @@ func TestApplyResourceChange(t *testing.T) { Config: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, + "id": cty.String, + "test": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), }), ), }, }, expected: &tfprotov5.ApplyResourceChangeResponse{ - Diagnostics: []*tfprotov5.Diagnostic{ - { - Severity: tfprotov5.DiagnosticSeverityError, - Summary: "getting identity schema failed for resource 'test': identity schema must have at least one attribute", - }, - }, NewState: &tfprotov5.DynamicValue{ - MsgPack: mustMsgpackMarshal(cty.DynamicPseudoType, cty.NullVal(cty.DynamicPseudoType)), + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.NullVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + })), + ), }, }, }, + // "imported-resource-by-id-identity-may-not-change": this is effectively the same as update-resource-identity-may-not-change + // "imported-resource-by-identity-identity-may-not-change": this is effectively the same as update-resource-identity-may-not-change + // "upgraded-identity-version-identity-may-not-change": this is effectively the same as update-resource-identity-may-not-change } for name, testCase := range testCases { @@ -7710,7 +9007,7 @@ func TestImportResourceState(t *testing.T) { }), ), }, - Private: []byte(`{"schema_version":"1"}`), + Private: []byte(`{".import_before_read":true,"schema_version":"1"}`), }, }, }, @@ -7796,7 +9093,7 @@ func TestImportResourceState(t *testing.T) { }), ), }, - Private: []byte(`{"schema_version":"1"}`), + Private: []byte(`{".import_before_read":true,"schema_version":"1"}`), Identity: &tfprotov5.ResourceIdentityData{ IdentityData: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( @@ -8064,7 +9361,7 @@ func TestImportResourceState(t *testing.T) { }), ), }, - Private: []byte(`{"schema_version":"1"}`), + Private: []byte(`{".import_before_read":true,"schema_version":"1"}`), }, }, }, diff --git a/terraform/state.go b/terraform/state.go index 2004b65dc61..f905c2a93d2 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -30,6 +30,13 @@ const ( stateVersion = 3 ) +// ImportBeforeReadMetaKey is an internal private field used to indicate that the current resource state and identity +// were provided most recently by the ImportResourceState RPC. This indicates that the state is an import stub and identity +// has not been stored in state yet. +// +// When detected, this key should be cleared before returning from the ReadResource RPC. +var ImportBeforeReadMetaKey = ".import_before_read" + // rootModulePath is the path of the root module var rootModulePath = []string{"root"} From 5313f51572044c60df88d27bb77af037c89b4443 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Wed, 14 May 2025 12:27:56 +0200 Subject: [PATCH 2/4] introduce MutableIdentity resource behaviour and remove obsolete todos --- helper/schema/grpc_provider.go | 13 +++++-------- helper/schema/resource.go | 5 +++++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go index 74cf16946a4..785b0d00324 100644 --- a/helper/schema/grpc_provider.go +++ b/helper/schema/grpc_provider.go @@ -159,7 +159,7 @@ func (s *GRPCProviderServer) UpgradeResourceIdentity(ctx context.Context, req *t // now we need to turn the state into the default json representation, so // that it can be re-decoded using the actual schema. - val, err := JSONMapToStateValue(jsonMap, schemaBlock) // TODO: Find out if we need resource identity version here + val, err := JSONMapToStateValue(jsonMap, schemaBlock) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) return resp, nil @@ -962,7 +962,7 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re } // If we're refreshing the resource state (excluding a recently imported resource), validate that the new identity isn't changing - if !readFollowingImport && !currentIdentityVal.IsNull() && !currentIdentityVal.RawEquals(newIdentityVal) { + if !res.ResourceBehavior.MutableIdentity && !readFollowingImport && !currentIdentityVal.IsNull() && !currentIdentityVal.RawEquals(newIdentityVal) { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("Unexpected Identity Change: %s", "During the read operation, the Terraform Provider unexpectedly returned a different identity then the previously stored one.\n\n"+ "This is always a problem with the provider and should be reported to the provider developer.\n\n"+ fmt.Sprintf("Current Identity: %s\n\n", currentIdentityVal.GoString())+ @@ -1130,7 +1130,6 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot diff.Attributes["id"] = &terraform.ResourceAttrDiff{ NewComputed: true, } - // TODO: we could error here if a new Diff got no Identity set } if diff == nil || (len(diff.Attributes) == 0 && len(diff.Identity) == 0) { @@ -1291,7 +1290,6 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot } } - // TODO: if schema defines identity, we should error if there's none written to newInstanceState if res.Identity != nil { identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName) if err != nil { @@ -1306,7 +1304,7 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot } // If we're updating or deleting and we already have an identity stored, validate that the planned identity isn't changing - if !create && !priorIdentityVal.IsNull() && !priorIdentityVal.RawEquals(plannedIdentityVal) { + if !res.ResourceBehavior.MutableIdentity && !create && !priorIdentityVal.IsNull() && !priorIdentityVal.RawEquals(plannedIdentityVal) { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf( "Unexpected Identity Change: During the planning operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+ "This is always a problem with the provider and should be reported to the provider developer.\n\n"+ @@ -1533,7 +1531,6 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro } resp.Private = meta - // TODO: if schema defines identity, we should error if there's none written to newInstanceState if res.Identity != nil { identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName) if err != nil { @@ -1547,7 +1544,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro return resp, nil } - if !create && !plannedIdentityVal.IsNull() && !plannedIdentityVal.RawEquals(newIdentityVal) { + if !res.ResourceBehavior.MutableIdentity && !create && !plannedIdentityVal.IsNull() && !plannedIdentityVal.RawEquals(newIdentityVal) { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf( "Unexpected Identity Change: During the update operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+ "This is always a problem with the provider and should be reported to the provider developer.\n\n"+ @@ -2336,7 +2333,7 @@ func (s *GRPCProviderServer) upgradeJSONIdentity(ctx context.Context, version in for _, upgrader := range res.Identity.IdentityUpgraders { if version != upgrader.Version { - continue // TODO: can we replace this with an error (as we'll require all versions to be upgraded) + continue } m, err = upgrader.Upgrade(ctx, m, s.provider.Meta()) diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 0bf7f9597d0..83140436800 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -674,6 +674,11 @@ type ResourceBehavior struct { // NOTE: This functionality is related to deferred action support, which is currently experimental and is subject // to change or break without warning. It is not protected by version compatibility guarantees. ProviderDeferred ProviderDeferredBehavior + + // MutableIdentity indicates that the managed resource supports an identity that can change during the + // resource's lifecycle. Setting this flag to true will disable the SDK validation that ensures identity + // data doesn't change during RPC calls. + MutableIdentity bool } // ProviderDeferredBehavior enables provider-defined logic to be executed From 929b999f92ed225b10c169519dfb50c58edc45da Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Wed, 14 May 2025 12:49:00 +0200 Subject: [PATCH 3/4] add tests for MutableIdentity resource behaviour --- helper/schema/grpc_provider_test.go | 345 ++++++++++++++++++++++++++++ 1 file changed, 345 insertions(+) diff --git a/helper/schema/grpc_provider_test.go b/helper/schema/grpc_provider_test.go index 643ed243083..5b58a5c194a 100644 --- a/helper/schema/grpc_provider_test.go +++ b/helper/schema/grpc_provider_test.go @@ -5798,6 +5798,103 @@ New Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("chang }, }, }, + "update-resource-identity-may-change-if-mutable-identity-allowed": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + ResourceBehavior: ResourceBehavior{ + MutableIdentity: true, + }, + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + identity, err := d.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadResourceRequest{ + TypeName: "test", + CurrentIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), + }), + ), + }, + }, + CurrentState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "test": cty.String, + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "test": cty.StringVal("hello"), + "id": cty.StringVal("initial"), + }), + ), + }, + }, + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("hello"), + }), + ), + }, + NewIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, + }, + }, + }, // "destroy-resource-identity-may-not-change": not required, as same request and response as update-resource-identity-may-not-change // "upgraded-identity-version-identity-may-not-change": not required, as same request and response as update-resource-identity-may-not-change } @@ -7129,6 +7226,131 @@ Planned Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("c }, }, }, + "update-resource-identity-may-change-if-mutable-identity-allowed": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + ResourceBehavior: ResourceBehavior{ + MutableIdentity: true, + }, + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, meta interface{}) error { + identity, err := d.Identity() + if err != nil { + return err + } + err = identity.Set("identity", "changed") + if err != nil { + return err + } + return nil + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), + ), + }, + PriorIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), + }), + ), + }, + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + UnsafeToUseLegacyTypeSystem: true, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, + }, + }, + }, "update-resource-without-prior-identity-identity-may-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ @@ -8153,6 +8375,129 @@ New Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("chang }, }, }, + "update-resource-identity-may-change-if-mutable-identity-allowed": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + ResourceBehavior: ResourceBehavior{ + MutableIdentity: true, + }, + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + UpdateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + identity, err := rd.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + rd.SetId("changed") + return nil + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("initial"), + }), + ), + }, + PlannedIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), + }), + ), + }, + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.StringVal("initial"), + }), + ), + }, + }, + expected: &tfprotov5.ApplyResourceChangeResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("changed"), + "test": cty.StringVal("initial"), + }), + ), + }, + Private: []uint8(`{"schema_version":"1"}`), + UnsafeToUseLegacyTypeSystem: true, + NewIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, + }, + }, + }, "update-resource-without-planned-identity-identity-may-change": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ From 3f881b1adf54c4e81a2e4a7a93e8d9064dde48f8 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Wed, 14 May 2025 17:17:18 +0200 Subject: [PATCH 4/4] address review feedback --- helper/schema/grpc_provider.go | 18 ++--- helper/schema/grpc_provider_test.go | 118 +++++++++++++++++++++++++--- 2 files changed, 116 insertions(+), 20 deletions(-) diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go index 785b0d00324..e6334e924ea 100644 --- a/helper/schema/grpc_provider.go +++ b/helper/schema/grpc_provider.go @@ -790,13 +790,13 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re ctx = logging.InitContext(ctx) readFollowingImport := false - ReqPrivate := req.Private + reqPrivate := req.Private - if ReqPrivate != nil { + if reqPrivate != nil { // unmarshal the private data - if len(ReqPrivate) > 0 { + if len(reqPrivate) > 0 { newReqPrivate := make(map[string]interface{}) - if err := json.Unmarshal(ReqPrivate, &newReqPrivate); err != nil { + if err := json.Unmarshal(reqPrivate, &newReqPrivate); err != nil { return nil, err } // This internal private field is set on a resource during ImportResourceState to help framework determine if @@ -807,14 +807,14 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re if len(newReqPrivate) == 0 { // if there are no other private data, set the private data to nil - ReqPrivate = nil + reqPrivate = nil } else { // set the new private data without the import key bytes, err := json.Marshal(newReqPrivate) if err != nil { return nil, err } - ReqPrivate = bytes + reqPrivate = bytes } } } @@ -824,7 +824,7 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re // helper/schema did previously handle private data during refresh, but // core is now going to expect this to be maintained in order to // persist it in the state. - Private: ReqPrivate, + Private: reqPrivate, } res, ok := s.provider.ResourcesMap[req.TypeName] @@ -887,8 +887,8 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re } private := make(map[string]interface{}) - if len(ReqPrivate) > 0 { - if err := json.Unmarshal(ReqPrivate, &private); err != nil { + if len(reqPrivate) > 0 { + if err := json.Unmarshal(reqPrivate, &private); err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) return resp, nil } diff --git a/helper/schema/grpc_provider_test.go b/helper/schema/grpc_provider_test.go index 5b58a5c194a..ab15715a951 100644 --- a/helper/schema/grpc_provider_test.go +++ b/helper/schema/grpc_provider_test.go @@ -5076,6 +5076,9 @@ func TestReadResource(t *testing.T) { Computed: true, }, }, + ResourceBehavior: ResourceBehavior{ + MutableIdentity: true, + }, Identity: &ResourceIdentity{ Version: 1, SchemaFunc: func() map[string]*Schema { @@ -5106,7 +5109,7 @@ func TestReadResource(t *testing.T) { if err != nil { return diag.FromErr(err) } - err = identity.Set("region", "test-region") // TODO: once we support disabling validation, this should be set to "new-region" and this test should ignore validation + err = identity.Set("region", "new-region") if err != nil { return diag.FromErr(err) } @@ -5181,7 +5184,7 @@ func TestReadResource(t *testing.T) { }), cty.ObjectVal(map[string]cty.Value{ "instance_id": cty.StringVal("test-id"), - "region": cty.StringVal("test-region"), + "region": cty.StringVal("new-region"), }), ), }, @@ -5895,8 +5898,107 @@ New Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("chang }, }, }, - // "destroy-resource-identity-may-not-change": not required, as same request and response as update-resource-identity-may-not-change - // "upgraded-identity-version-identity-may-not-change": not required, as same request and response as update-resource-identity-may-not-change + "does-not-remove-user-data-from-private": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test": { + Type: TypeString, + }, + }, + Identity: &ResourceIdentity{ + Version: 1, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "identity": { + Type: TypeString, + RequiredForImport: true, + }, + } + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + err := d.Set("test", "hello") + if err != nil { + return diag.FromErr(err) + } + + identity, err := d.Identity() + if err != nil { + return diag.FromErr(err) + } + err = identity.Set("identity", "changed") + if err != nil { + return diag.FromErr(err) + } + + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadResourceRequest{ + TypeName: "test", + CurrentIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("initial"), + }), + ), + }, + }, + Private: []byte(`{".import_before_read":true,"user_defined_key":"user_defined_value"}`), + CurrentState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.UnknownVal(cty.String), + }), + ), + }, + }, + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("initial"), + "test": cty.StringVal("hello"), + }), + ), + }, + Private: []byte(`{"user_defined_key":"user_defined_value"}`), + NewIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "identity": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "identity": cty.StringVal("changed"), + }), + ), + }, + }, + }, + }, } for name, testCase := range testCases { @@ -7583,9 +7685,6 @@ Planned Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("c }, }, }, - // "imported-resource-by-id-identity-may-not-change" same as update-resource-identity-may-not-change - // "imported-resource-by-identity-identity-may-not-change" same as update-resource-identity-may-not-change - // "upgraded-identity-version-identity-may-not-change" same as update-resource-identity-may-not-change } for name, testCase := range testCases { @@ -7954,7 +8053,7 @@ func TestApplyResourceChange(t *testing.T) { "ident": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "ident": cty.UnknownVal(cty.String), // TODO: allow change during create in handler! + "ident": cty.NullVal(cty.String), }), ), }, @@ -8713,9 +8812,6 @@ New Identity: cty.ObjectVal(map[string]cty.Value{"identity":cty.StringVal("chang }, }, }, - // "imported-resource-by-id-identity-may-not-change": this is effectively the same as update-resource-identity-may-not-change - // "imported-resource-by-identity-identity-may-not-change": this is effectively the same as update-resource-identity-may-not-change - // "upgraded-identity-version-identity-may-not-change": this is effectively the same as update-resource-identity-may-not-change } for name, testCase := range testCases {