Merge pull request #21721 from hashicorp/jbardin/remove-new-removed

Remove removed attribute from applied state

builtin/providers/test/resource.go

@@ -202,8 +202,13 @@ func testResourceRead(d *schema.ResourceData, meta interface{}) error {
 		d.Set("set", []interface{}{})
 	}
 
+	_, ok := d.GetOk("optional_computed")
+	if !ok {
+		d.Set("optional_computed", "computed")
+	}
+
 	// This should not show as set unless it's set in the config
-	_, ok := d.GetOkExists("get_ok_exists_false")
+	_, ok = d.GetOkExists("get_ok_exists_false")
 	if ok {
 		return errors.New("get_ok_exists_false should not be set")
 	}

builtin/providers/test/resource_test.go

@@ -36,6 +36,55 @@ resource "test_resource" "foo" {
 	})
 }
 
+func TestResource_removedOptional(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource" "foo" {
+	required = "yep"
+	required_map = {
+	    key = "value"
+	}
+	int = 1
+	optional = "string"
+	optional_computed = "not computed"
+	optional_bool = true
+}
+				`),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource" "foo" {
+	required = "yep"
+	required_map = {
+	    key = "value"
+	}
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckNoResourceAttr(
+						"test_resource.foo", "int",
+					),
+					resource.TestCheckNoResourceAttr(
+						"test_resource.foo", "string",
+					),
+					resource.TestCheckNoResourceAttr(
+						"test_resource.foo", "optional_bool",
+					),
+					// while this isn't optimal, the prior config value being
+					// left for optional+computed is expected
+					resource.TestCheckResourceAttr(
+						"test_resource.foo", "optional_computed", "not computed",
+					),
+				),
+			},
+		},
+	})
+}
+
 func TestResource_changedList(t *testing.T) {
 	resource.UnitTest(t, resource.TestCase{
 		Providers:    testAccProviders,

helper/plugin/grpc_provider.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log"
 	"strconv"
+	"strings"
 
 	"github.com/zclconf/go-cty/cty"
 	ctyconvert "github.com/zclconf/go-cty/cty/convert"
@@ -845,6 +846,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 		diff.Meta = private
 	}
 
+	var newRemoved []string
 	for k, d := range diff.Attributes {
 		// We need to turn off any RequiresNew. There could be attributes
 		// without changes in here inserted by helper/schema, but if they have
@@ -852,8 +854,10 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 		d.RequiresNew = false
 
 		// Check that any "removed" attributes that don't actually exist in the
-		// prior state, or helper/schema will confuse itself
+		// prior state, or helper/schema will confuse itself, and record them
+		// to make sure they are actually removed from the state.
 		if d.NewRemoved {
+			newRemoved = append(newRemoved, k)
 			if _, ok := priorState.Attributes[k]; !ok {
 				delete(diff.Attributes, k)
 			}
@@ -882,6 +886,19 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 		return resp, nil
 	}
 
+	// Now remove any primitive zero values that were left from NewRemoved
+	// attributes. Any attempt to reconcile more complex structures to the best
+	// of our abilities happens in normalizeNullValues.
+	for _, r := range newRemoved {
+		if strings.HasSuffix(r, ".#") || strings.HasSuffix(r, ".%") {
+			continue
+		}
+		switch newInstanceState.Attributes[r] {
+		case "", "0", "false":
+			delete(newInstanceState.Attributes, r)
+		}
+	}
+
 	// We keep the null val if we destroyed the resource, otherwise build the
 	// entire object, even if the new state was nil.
 	newStateVal, err = schema.StateValueFromInstanceState(newInstanceState, schemaBlock.ImpliedType())