Merge pull request #21806 from hashicorp/jbardin/lost-empty-strings

prevent an empty string from being lost

builtin/providers/test/resource_defaults_test.go

@@ -174,3 +174,22 @@ resource "test_resource_defaults" "foo" {
 		},
 	})
 }
+
+func TestDefaults_emptyString(t *testing.T) {
+	config := `
+resource "test_resource_defaults" "test" {
+  default_string = ""
+}
+`
+	resource.UnitTest(t, resource.TestCase{
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: config,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("test_resource_defaults.test", "default_string", ""),
+				),
+			},
+		},
+	})
+}

helper/plugin/grpc_provider.go

@@ -1223,6 +1223,8 @@ func normalizeNullValues(dst, src cty.Value, apply bool) cty.Value {
 		}
 	}
 
+	// check the invariants that we need below, to ensure we are working with
+	// non-null and known values.
 	if src.IsNull() || !src.IsKnown() || !dst.IsKnown() {
 		return dst
 	}
@@ -1341,8 +1343,12 @@ func normalizeNullValues(dst, src cty.Value, apply bool) cty.Value {
 			return cty.ListVal(dsts)
 		}
 
-	case ty.IsPrimitiveType():
-		if dst.IsNull() && src.IsWhollyKnown() && apply {
+	case ty == cty.String:
+		// The legacy SDK should not be able to remove a value during plan or
+		// apply, however we are only going to overwrite this if the source was
+		// an empty string, since that is what is often equated with unset and
+		// lost in the diff process.
+		if dst.IsNull() && src.AsString() == "" {
 			return src
 		}
 	}

helper/plugin/grpc_provider_test.go

@@ -984,6 +984,18 @@ func TestNormalizeNullValues(t *testing.T) {
 		},
 		{
 			// Plan primitives are kept
+			Src: cty.ObjectVal(map[string]cty.Value{
+				"string": cty.NumberIntVal(0),
+			}),
+			Dst: cty.ObjectVal(map[string]cty.Value{
+				"string": cty.NullVal(cty.Number),
+			}),
+			Expect: cty.ObjectVal(map[string]cty.Value{
+				"string": cty.NullVal(cty.Number),
+			}),
+		},
+		{
+			// Neither plan nor apply should remove empty strings
 			Src: cty.ObjectVal(map[string]cty.Value{
 				"string": cty.StringVal(""),
 			}),
@@ -991,8 +1003,21 @@ func TestNormalizeNullValues(t *testing.T) {
 				"string": cty.NullVal(cty.String),
 			}),
 			Expect: cty.ObjectVal(map[string]cty.Value{
+				"string": cty.StringVal(""),
+			}),
+		},
+		{
+			// Neither plan nor apply should remove empty strings
+			Src: cty.ObjectVal(map[string]cty.Value{
+				"string": cty.StringVal(""),
+			}),
+			Dst: cty.ObjectVal(map[string]cty.Value{
 				"string": cty.NullVal(cty.String),
 			}),
+			Expect: cty.ObjectVal(map[string]cty.Value{
+				"string": cty.StringVal(""),
+			}),
+			Apply: true,
 		},
 		{
 			// The null map is retained, because the src was unknown