From f65b7c5372cb6cf92819ecb9d7910d0a320de7ba Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 22 Jan 2019 15:11:14 -0800 Subject: [PATCH] helper/plugin: Discard meaningless differences from provider planning Due to various inprecisions in the old SDK implementation, applying the generated diff can potentially make changes to the data structure that have no real effect, such as replacing an empty list with a null list or vice-versa. Although we can't totally eliminate such diff noise, here we attempt to avoid it in situations where there are _only_ meaningless changes -- where the prior state and planned state are equivalent -- by just echoing back the prior state verbatim to ensure that Terraform will treat it as a noop change. If there _are_ some legitimate changes then the result may still contain meaningless changes alongside it, but that is just a cosmetic problem for the diff renderer, because the meaningless changes will be ignored altogether during a subsequent apply anyway. The primary goal here is just to ensure we can converge on a fixpoint when there are no explicit changes in the configuration. --- helper/plugin/grpc_provider.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 46c44ca8c..627dd947f 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -564,6 +564,20 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl plannedStateVal = copyTimeoutValues(plannedStateVal, proposedNewStateVal) + // The old SDK code has some inprecisions that cause it to sometimes + // generate differences that the SDK itself does not consider significant + // but Terraform Core would. To avoid producing weird do-nothing diffs + // in that case, we'll check if the provider as produced something we + // think is "equivalent" to the prior state and just return the prior state + // itself if so, thus ensuring that Terraform Core will treat this as + // a no-op. See the docs for ValuesSDKEquivalent for some caveats on its + // accuracy. + forceNoChanges := false + if hcl2shim.ValuesSDKEquivalent(priorStateVal, plannedStateVal) { + plannedStateVal = priorStateVal + forceNoChanges = true + } + plannedMP, err := msgpack.Marshal(plannedStateVal, block.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -600,9 +614,11 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl // collect the attributes that require instance replacement, and convert // them to cty.Paths. var requiresNew []string - for attr, d := range diff.Attributes { - if d.RequiresNew { - requiresNew = append(requiresNew, attr) + if !forceNoChanges { + for attr, d := range diff.Attributes { + if d.RequiresNew { + requiresNew = append(requiresNew, attr) + } } }