diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 5faf0160f..f7914bc0c 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -490,7 +490,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso return resp, nil } - newStateVal = normalizeNullValues(newStateVal, stateVal, true) + newStateVal = normalizeNullValues(newStateVal, stateVal, false) newStateVal = copyTimeoutValues(newStateVal, stateVal) newStateMP, err := msgpack.Marshal(newStateVal, blockForCore.ImpliedType()) @@ -614,7 +614,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl return resp, nil } - plannedStateVal = normalizeNullValues(plannedStateVal, proposedNewStateVal, true) + plannedStateVal = normalizeNullValues(plannedStateVal, proposedNewStateVal, false) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -839,7 +839,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A return resp, nil } - newStateVal = normalizeNullValues(newStateVal, plannedStateVal, false) + newStateVal = normalizeNullValues(newStateVal, plannedStateVal, true) newStateVal = copyTimeoutValues(newStateVal, plannedStateVal) @@ -1108,15 +1108,13 @@ func stripSchema(s *schema.Schema) *schema.Schema { // however it sees fit. This however means that a CustomizeDiffFunction may not // be able to change a null to an empty value or vice versa, but that should be // very uncommon nor was it reliable before 0.12 either. -func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value { +func normalizeNullValues(dst, src cty.Value, apply bool) cty.Value { ty := dst.Type() if !src.IsNull() && !src.IsKnown() { - // While this seems backwards to return src when preferDst is set, it - // means this might be a plan scenario, and it must retain unknown - // interpolated placeholders, which could be lost if we're only updating - // a resource. If this is a read scenario, then there shouldn't be any - // unknowns all. - if dst.IsNull() && preferDst { + // Return src during plan to retain unknown interpolated placeholders, + // which could be lost if we're only updating a resource. If this is a + // read scenario, then there shouldn't be any unknowns all. + if dst.IsNull() && !apply { return src } return dst @@ -1154,26 +1152,26 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value { srcMap := src.AsValueMap() for key, v := range srcMap { dstVal, ok := dstMap[key] - if !ok && !preferDst && ty.IsMapType() { + if !ok && apply && ty.IsMapType() { // don't transfer old map values to dst during apply continue } if dstVal == cty.NilVal { - if preferDst && ty.IsMapType() { + if !apply && ty.IsMapType() { // let plan shape this map however it wants continue } dstVal = cty.NullVal(v.Type()) } - dstMap[key] = normalizeNullValues(dstVal, v, preferDst) + dstMap[key] = normalizeNullValues(dstVal, v, apply) } // you can't call MapVal/ObjectVal with empty maps, but nothing was // copied in anyway. If the dst is nil, and the src is known, assume the // src is correct. if len(dstMap) == 0 { - if dst.IsNull() && src.IsWhollyKnown() && !preferDst { + if dst.IsNull() && src.IsWhollyKnown() && apply { return src } return dst @@ -1207,7 +1205,7 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value { // If the original was wholly known, then we expect that is what the // provider applied. The apply process loses too much information to // reliably re-create the set. - if src.IsWhollyKnown() && !preferDst { + if src.IsWhollyKnown() && apply { return src } @@ -1215,14 +1213,13 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value { // If the dst is null, and the src is known, then we lost an empty value // so take the original. if dst.IsNull() { - if src.IsWhollyKnown() && src.LengthInt() == 0 && !preferDst { + if src.IsWhollyKnown() && src.LengthInt() == 0 && apply { return src } // if dst is null and src only contains unknown values, then we lost - // those during a plan (which is when preferDst is set, there would - // be no unknowns during read). - if preferDst && !src.IsNull() { + // those during a read or plan. + if !apply && !src.IsNull() { allUnknown := true for _, v := range src.AsValueSlice() { if v.IsKnown() { @@ -1246,7 +1243,7 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value { dsts := dst.AsValueSlice() for i := 0; i < srcLen; i++ { - dsts[i] = normalizeNullValues(dsts[i], srcs[i], preferDst) + dsts[i] = normalizeNullValues(dsts[i], srcs[i], apply) } if ty.IsTupleType() { @@ -1256,7 +1253,7 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value { } case ty.IsPrimitiveType(): - if dst.IsNull() && src.IsWhollyKnown() && !preferDst { + if dst.IsNull() && src.IsWhollyKnown() && apply { return src } } diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index 10850f40c..e70827e15 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -667,7 +667,7 @@ func TestGetSchemaTimeouts(t *testing.T) { func TestNormalizeNullValues(t *testing.T) { for i, tc := range []struct { Src, Dst, Expect cty.Value - Plan bool + Apply bool }{ { // The known set value is copied over the null set value @@ -690,6 +690,7 @@ func TestNormalizeNullValues(t *testing.T) { }), }), }), + Apply: true, }, { // A zero set value is kept @@ -702,7 +703,6 @@ func TestNormalizeNullValues(t *testing.T) { Expect: cty.ObjectVal(map[string]cty.Value{ "set": cty.SetValEmpty(cty.String), }), - Plan: true, }, { // The known set value is copied over the null set value @@ -724,7 +724,6 @@ func TestNormalizeNullValues(t *testing.T) { "foo": cty.String, }))), }), - Plan: true, }, { // The empty map is copied over the null map @@ -737,6 +736,7 @@ func TestNormalizeNullValues(t *testing.T) { Expect: cty.ObjectVal(map[string]cty.Value{ "map": cty.MapValEmpty(cty.String), }), + Apply: true, }, { // A zero value primitive is copied over a null primitive @@ -749,6 +749,7 @@ func TestNormalizeNullValues(t *testing.T) { Expect: cty.ObjectVal(map[string]cty.Value{ "string": cty.StringVal(""), }), + Apply: true, }, { // Plan primitives are kept @@ -761,7 +762,6 @@ func TestNormalizeNullValues(t *testing.T) { Expect: cty.ObjectVal(map[string]cty.Value{ "string": cty.NullVal(cty.String), }), - Plan: true, }, { // The null map is retained, because the src was unknown @@ -774,6 +774,7 @@ func TestNormalizeNullValues(t *testing.T) { Expect: cty.ObjectVal(map[string]cty.Value{ "map": cty.NullVal(cty.Map(cty.String)), }), + Apply: true, }, { // the nul set is retained, because the src set contains an unknown value @@ -794,6 +795,7 @@ func TestNormalizeNullValues(t *testing.T) { "foo": cty.String, }))), }), + Apply: true, }, { // Retain don't re-add unexpected planned values in a map @@ -813,7 +815,6 @@ func TestNormalizeNullValues(t *testing.T) { "a": cty.StringVal("a"), }), }), - Plan: true, }, { // Remove extra values after apply @@ -833,7 +834,7 @@ func TestNormalizeNullValues(t *testing.T) { "a": cty.StringVal("a"), }), }), - Plan: false, + Apply: true, }, { Src: cty.ObjectVal(map[string]cty.Value{ @@ -843,7 +844,6 @@ func TestNormalizeNullValues(t *testing.T) { Expect: cty.ObjectVal(map[string]cty.Value{ "a": cty.NullVal(cty.String), }), - Plan: true, }, // a list in an object in a list, going from null to empty @@ -877,6 +877,7 @@ func TestNormalizeNullValues(t *testing.T) { }), }), }), + Apply: true, }, // a list in an object in a list, going from empty to null @@ -910,6 +911,7 @@ func TestNormalizeNullValues(t *testing.T) { }), }), }), + Apply: true, }, // the empty list should be transferred, but the new unknown should not be overridden { @@ -942,7 +944,6 @@ func TestNormalizeNullValues(t *testing.T) { }), }), }), - Plan: true, }, { // fix unknowns added to a map @@ -964,7 +965,6 @@ func TestNormalizeNullValues(t *testing.T) { "b": cty.StringVal(""), }), }), - Plan: true, }, { // fix unknowns lost from a list @@ -1001,11 +1001,10 @@ func TestNormalizeNullValues(t *testing.T) { }), }), }), - Plan: true, }, } { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - got := normalizeNullValues(tc.Dst, tc.Src, tc.Plan) + got := normalizeNullValues(tc.Dst, tc.Src, tc.Apply) if !got.RawEquals(tc.Expect) { t.Fatalf("\nexpected: %#v\ngot: %#v\n", tc.Expect, got) }