From 209a0a460a6ae553b2b728308bda3cdb50b67851 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 6 Mar 2019 14:09:01 -0500 Subject: [PATCH 1/3] normalize all objects read from the provider Use objchange.NormalizeObjectFromLegacySDK to ensure that all objects returned from the provider match what is expected based on the configuration according to the schemas. --- helper/plugin/grpc_provider.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 1b046f9d91..99877bd3d2 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -9,6 +9,8 @@ import ( "strconv" "strings" + "github.com/hashicorp/terraform/plans/objchange" + "github.com/zclconf/go-cty/cty" ctyconvert "github.com/zclconf/go-cty/cty/convert" "github.com/zclconf/go-cty/cty/msgpack" @@ -458,6 +460,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso } newStateVal = copyTimeoutValues(newStateVal, stateVal) + newStateVal = objchange.NormalizeObjectFromLegacySDK(newStateVal, block) newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType()) if err != nil { @@ -594,6 +597,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl } plannedStateVal = copyTimeoutValues(plannedStateVal, proposedNewStateVal) + plannedStateVal = objchange.NormalizeObjectFromLegacySDK(plannedStateVal, block) // The old SDK code has some imprecisions that cause it to sometimes // generate differences that the SDK itself does not consider significant @@ -832,6 +836,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A newStateVal = normalizeNullValues(newStateVal, plannedStateVal, false) newStateVal = copyTimeoutValues(newStateVal, plannedStateVal) + newStateVal = objchange.NormalizeObjectFromLegacySDK(newStateVal, block) newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType()) if err != nil { @@ -955,6 +960,7 @@ func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDa } newStateVal = copyTimeoutValues(newStateVal, configVal) + newStateVal = objchange.NormalizeObjectFromLegacySDK(newStateVal, block) newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType()) if err != nil { From 5c40d6610cf4ec77b929ba0c22920107d133d0c7 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 6 Mar 2019 16:21:32 -0500 Subject: [PATCH 2/3] remove NormalizeObjectFromLegacySDK from diff The NormalizeObjectFromLegacySDK calls have been moved into the provider shims, so all objects generated by the provider should conform now. --- command/format/diff.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/command/format/diff.go b/command/format/diff.go index 0d641c301e..049f625434 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -122,16 +122,6 @@ func ResourceChange( panic(fmt.Sprintf("failed to decode plan for %s while rendering diff: %s", addr, err)) } - // We currently have an opt-out that permits the legacy SDK to return values - // that defy our usual conventions around handling of nesting blocks. To - // avoid the rendering code from needing to handle all of these, we'll - // normalize first. - // (Ideally we'd do this as part of the SDK opt-out implementation in core, - // but we've added it here for now to reduce risk of unexpected impacts - // on other code in core.) - changeV.Change.Before = objchange.NormalizeObjectFromLegacySDK(changeV.Change.Before, schema) - changeV.Change.After = objchange.NormalizeObjectFromLegacySDK(changeV.Change.After, schema) - bodyWritten := p.writeBlockBodyDiff(schema, changeV.Before, changeV.After, 6, path) if bodyWritten { buf.WriteString("\n") From 5c09f946952d5c010ca81cae64c4a5f47667da56 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 6 Mar 2019 16:23:56 -0500 Subject: [PATCH 3/3] remove eval TODO for NormalizeObjectFromLegacySDK The normalization will take place in the provider shims, locating it with the rest of the code that attempts to match the new and legacy behavior. --- terraform/eval_apply.go | 6 ------ terraform/eval_diff.go | 6 ------ 2 files changed, 12 deletions(-) diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 49cfe80f21..1d766cda8f 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -203,12 +203,6 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) { // on the planned value, and thus get a different result during the // apply phase. This will usually lead to a "Provider produced invalid plan" // error that incorrectly blames the downstream resource for the change. - // - // TODO: Ideally we should use objchange.NormalizeObjectFromLegacySDK - // here so the rest of Terraform is shielded from some of the oddities - // of the legacy SDK, but for the moment we've done that only in - // format.ResourceChange to reduce the risk of unexpected impacts - // elsewhere. } else { for _, err := range errs { diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 88a142dfde..461a0b508b 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -229,12 +229,6 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { fmt.Fprintf(&buf, "\n - %s", tfdiags.FormatError(err)) } log.Print(buf.String()) - - // TODO: Ideally we should use objchange.NormalizeObjectFromLegacySDK - // here so the rest of Terraform is shielded from some of the oddities - // of the legacy SDK, but for the moment we've done that only in - // format.ResourceChange to reduce the risk of unexpected impacts - // elsewhere. } else { for _, err := range errs { diags = diags.Append(tfdiags.Sourceless(