From 9a39af5047675069fbde9e2cf9b57f1a2f600491 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 28 Feb 2019 17:47:11 -0500 Subject: [PATCH 1/2] 1->0 set changes non longer should happen in Read The new normalization should make preventing those changes unnecessary, and will also prevent extra empty elements from being added when resources are refreshed. --- helper/plugin/grpc_provider.go | 2 +- helper/plugin/grpc_provider_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 8a4f253c5f..371b1701a0 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -431,7 +431,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso // here we use the prior state to check for unknown/zero containers values // when normalizing the flatmap. stateAttrs := hcl2shim.FlatmapValueFromHCL2(stateVal) - newInstanceState.Attributes = normalizeFlatmapContainers(stateAttrs, newInstanceState.Attributes, true) + newInstanceState.Attributes = normalizeFlatmapContainers(stateAttrs, newInstanceState.Attributes, false) } if newInstanceState == nil || newInstanceState.ID == "" { diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index 0ccc9ac43e..9c5398eb8d 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -709,6 +709,11 @@ func TestNormalizeFlatmapContainers(t *testing.T) { attrs: map[string]string{"map.%": "0", "list.#": "0", "id": "1"}, expect: map[string]string{"id": "1", "map.%": "0", "list.#": "0"}, }, + { + prior: map[string]string{"list.#": "1", "list.0": "old value"}, + attrs: map[string]string{"list.#": "0"}, + expect: map[string]string{}, + }, } { t.Run(strconv.Itoa(i), func(t *testing.T) { got := normalizeFlatmapContainers(tc.prior, tc.attrs, false) From 49230f8198af763fa064e9e81c1dd6ab73b48283 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 28 Feb 2019 17:49:42 -0500 Subject: [PATCH 2/2] existing fields cannot become computed during plan Fields with no change can only become computed during initial creation. --- helper/plugin/grpc_provider.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 371b1701a0..b1b4b5d3d6 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -492,6 +492,8 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl return resp, nil } + create := priorStateVal.IsNull() + proposedNewStateVal, err := msgpack.Unmarshal(req.ProposedNewState.Msgpack, block.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -533,7 +535,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl } // if this is a new instance, we need to make sure ID is going to be computed - if priorStateVal.IsNull() { + if create { if diff == nil { diff = terraform.NewInstanceDiff() } @@ -556,6 +558,17 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl priorState = &terraform.InstanceState{} } + // if we're not creating a new resource, remove any new computed fields + if !create { + for attr, d := range diff.Attributes { + // If there's no change, then don't let this go through as NewComputed. + // This usually only happens when Old and New are both empty. + if d.NewComputed && d.Old == d.New { + delete(diff.Attributes, attr) + } + } + } + // now we need to apply the diff to the prior state, so get the planned state plannedAttrs, err := diff.Apply(priorState.Attributes, block) @@ -598,7 +611,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl // if this was creating the resource, we need to set any remaining computed // fields - if priorStateVal.IsNull() { + if create { plannedStateVal = SetUnknowns(plannedStateVal, block) }