diff --git a/builtin/providers/test/resource.go b/builtin/providers/test/resource.go index 09732d421..28c5c79a1 100644 --- a/builtin/providers/test/resource.go +++ b/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") } diff --git a/builtin/providers/test/resource_test.go b/builtin/providers/test/resource_test.go index 8b78bb415..afcbf7967 100644 --- a/builtin/providers/test/resource_test.go +++ b/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, diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 161af4d46..09982f96b 100644 --- a/helper/plugin/grpc_provider.go +++ b/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" @@ -833,6 +834,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 @@ -840,8 +842,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) } @@ -870,6 +874,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())