Remove removed attribute from applied state

When a Diff contains a NewRemoved attribute (which would have been null
in the planned state), the final value is often the "zero" value string
for the type, which the provider itself still applies to the state.
Rather than risking a change of behavior in helper/schema by fixing the
inconsistency, we'll remove the NewRemoved attributes after apply to
prevent further issues resulting from the change in planned value.
This commit is contained in:
James Bardin 2019-06-13 16:53:36 -04:00
parent eaddf9ccf1
commit 2e2a363052
3 changed files with 73 additions and 2 deletions

View File

@ -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")
}

View File

@ -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,

View File

@ -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())