Merge pull request #20680 from hashicorp/jbardin/read-normalization

ReadResource normalization

builtin/providers/test/resource_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/helper/resource"
+	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -732,3 +733,60 @@ resource "test_resource" "foo" {
 		},
 	})
 }
+
+func TestResource_setDrift(t *testing.T) {
+	testProvider := testAccProviders["test"]
+	res := testProvider.(*schema.Provider).ResourcesMap["test_resource"]
+
+	// reset the Read function after the test
+	defer func() {
+		res.Read = testResourceRead
+	}()
+
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource" "foo" {
+  required     = "first"
+  required_map = {
+    a = "a"
+	}
+	set = ["a", "b"]
+}
+`),
+				Check: func(s *terraform.State) error {
+					return nil
+				},
+			},
+			resource.TestStep{
+				PreConfig: func() {
+					// update the Read function to return the wrong "set" attribute values.
+					res.Read = func(d *schema.ResourceData, meta interface{}) error {
+						// update as expected first
+						if err := testResourceRead(d, meta); err != nil {
+							return err
+						}
+						d.Set("set", []interface{}{"a", "x"})
+						return nil
+					}
+				},
+				// Leave the config, so we can detect the mismatched set values.
+				// Updating the config would force the test to pass even if the Read
+				// function values were ignored.
+				Config: strings.TrimSpace(`
+resource "test_resource" "foo" {
+  required     = "second"
+  required_map = {
+    a = "a"
+	}
+	set = ["a", "b"]
+}
+`),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}

helper/plugin/grpc_provider.go

@@ -448,7 +448,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
 		return resp, nil
 	}
 
-	newStateVal = normalizeNullValues(newStateVal, stateVal, false)
+	newStateVal = normalizeNullValues(newStateVal, stateVal, true)
 	newStateVal = copyTimeoutValues(newStateVal, stateVal)
 
 	newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType())
@@ -1078,7 +1078,7 @@ 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, plan bool) cty.Value {
+func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value {
 	ty := dst.Type()
 
 	if !src.IsNull() && !src.IsKnown() {
@@ -1121,20 +1121,20 @@ func normalizeNullValues(dst, src cty.Value, plan bool) cty.Value {
 
 			dstVal := dstMap[key]
 			if dstVal == cty.NilVal {
-				if plan && ty.IsMapType() {
+				if preferDst && ty.IsMapType() {
 					// let plan shape this map however it wants
 					continue
 				}
 				dstVal = cty.NullVal(v.Type())
 			}
-			dstMap[key] = normalizeNullValues(dstVal, v, plan)
+			dstMap[key] = normalizeNullValues(dstVal, v, preferDst)
 		}
 
 		// 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() && !plan {
+			if dst.IsNull() && src.IsWhollyKnown() && !preferDst {
 				return src
 			}
 			return dst
@@ -1150,7 +1150,7 @@ func normalizeNullValues(dst, src cty.Value, plan 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() && !plan {
+		if src.IsWhollyKnown() && !preferDst {
 			return src
 		}
 
@@ -1158,7 +1158,7 @@ func normalizeNullValues(dst, src cty.Value, plan bool) cty.Value {
 		// If the dst is nil, and the src is known, then we lost an empty value
 		// so take the original.
 		if dst.IsNull() {
-			if src.IsWhollyKnown() && src.LengthInt() == 0 && !plan {
+			if src.IsWhollyKnown() && src.LengthInt() == 0 && !preferDst {
 				return src
 			}
 			return dst
@@ -1172,7 +1172,7 @@ func normalizeNullValues(dst, src cty.Value, plan bool) cty.Value {
 			dsts := dst.AsValueSlice()
 
 			for i := 0; i < srcLen; i++ {
-				dsts[i] = normalizeNullValues(dsts[i], srcs[i], plan)
+				dsts[i] = normalizeNullValues(dsts[i], srcs[i], preferDst)
 			}
 
 			if ty.IsTupleType() {
@@ -1182,7 +1182,7 @@ func normalizeNullValues(dst, src cty.Value, plan bool) cty.Value {
 		}
 
 	case ty.IsPrimitiveType():
-		if dst.IsNull() && src.IsWhollyKnown() && !plan {
+		if dst.IsNull() && src.IsWhollyKnown() && !preferDst {
 			return src
 		}
 	}