helper/schema: Always propagate NewComputed for previously zero value primative type attributes

When the following conditions were met:
* Schema attribute with a primative type (e.g. Type: TypeString) and Computed: true
* Old state of attribute set to zero value for type (e.g. "")
* Old state ID of resource set to non-empty (e.g. existing resource)

Attempting to use CustomizeDiff with SetNewComputed() would result in the difference  previously being discarded. This update ensures that previous zero values or resource existence does not influence the propagation of the computed update.

Previously:

```
--- FAIL: TestSetNewComputed (0.00s)
    --- FAIL: TestSetNewComputed/NewComputed_should_always_propagate (0.00s)
        resource_diff_test.go:684: Expected (*terraform.InstanceDiff)(0xc00051cea0)({
             mu: (sync.Mutex) {
              state: (int32) 0,
              sema: (uint32) 0
             },
             Attributes: (map[string]*terraform.ResourceAttrDiff) (len=1) {
              (string) (len=3) "foo": (*terraform.ResourceAttrDiff)(0xc0003dcec0)({
               Old: (string) "",
               New: (string) "",
               NewComputed: (bool) true,
               NewRemoved: (bool) false,
               NewExtra: (interface {}) <nil>,
               RequiresNew: (bool) false,
               Sensitive: (bool) false,
               Type: (terraform.DiffAttrType) 0
              })
             },
             Destroy: (bool) false,
             DestroyDeposed: (bool) false,
             DestroyTainted: (bool) false,
             Meta: (map[string]interface {}) <nil>
            })
            , got (*terraform.InstanceDiff)(0xc00051ce80)({
             mu: (sync.Mutex) {
              state: (int32) 0,
              sema: (uint32) 0
             },
             Attributes: (map[string]*terraform.ResourceAttrDiff) {
             },
             Destroy: (bool) false,
             DestroyDeposed: (bool) false,
             DestroyTainted: (bool) false,
             Meta: (map[string]interface {}) <nil>
            })

--- FAIL: TestSchemaMap_Diff (0.01s)
    --- FAIL: TestSchemaMap_Diff/79-NewComputed_should_always_propagate_with_CustomizeDiff (0.00s)
        schema_test.go:3289: expected:
            *terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{"foo":*terraform.ResourceAttrDiff{Old:"", New:"", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, Meta:map[string]interface {}(nil)}

            got:
            <nil>

FAIL
FAIL  github.com/hashicorp/terraform/helper/schema  0.825s
```
This commit is contained in:
Brian Flad 2018-12-04 22:48:30 -05:00
parent af9d046afb
commit 1e81a3e7fa
No known key found for this signature in database
GPG Key ID: EC6252B42B012823
3 changed files with 67 additions and 2 deletions

View File

@ -599,6 +599,37 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
}, },
}, },
}, },
resourceDiffTestCase{
Name: "NewComputed should always propagate",
Schema: map[string]*Schema{
"foo": &Schema{
Type: TypeString,
Computed: true,
},
},
State: &terraform.InstanceState{
Attributes: map[string]string{
"foo": "",
},
ID: "pre-existing",
},
Config: testConfig(t, map[string]interface{}{}),
Diff: &terraform.InstanceDiff{Attributes: map[string]*terraform.ResourceAttrDiff{}},
Key: "foo",
NewValue: "",
Expected: &terraform.InstanceDiff{
Attributes: func() map[string]*terraform.ResourceAttrDiff {
if computed {
return map[string]*terraform.ResourceAttrDiff{
"foo": &terraform.ResourceAttrDiff{
NewComputed: computed,
},
}
}
return map[string]*terraform.ResourceAttrDiff{}
}(),
},
},
} }
} }

View File

@ -1185,7 +1185,7 @@ func (m schemaMap) diffString(
return fmt.Errorf("%s: %s", k, err) return fmt.Errorf("%s: %s", k, err)
} }
if os == ns && !all { if os == ns && !all && !computed {
// They're the same value. If there old value is not blank or we // They're the same value. If there old value is not blank or we
// have an ID, then return right away since we're already setup. // have an ID, then return right away since we're already setup.
if os != "" || d.Id() != "" { if os != "" || d.Id() != "" {
@ -1193,7 +1193,7 @@ func (m schemaMap) diffString(
} }
// Otherwise, only continue if we're computed // Otherwise, only continue if we're computed
if !schema.Computed && !computed { if !schema.Computed {
return nil return nil
} }
} }

View File

@ -3126,6 +3126,40 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false, Err: false,
}, },
{
Name: "NewComputed should always propagate with CustomizeDiff",
Schema: map[string]*Schema{
"foo": &Schema{
Type: TypeString,
Computed: true,
},
},
State: &terraform.InstanceState{
Attributes: map[string]string{
"foo": "",
},
ID: "pre-existing",
},
Config: map[string]interface{}{},
CustomizeDiff: func(d *ResourceDiff, meta interface{}) error {
d.SetNewComputed("foo")
return nil
},
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"foo": &terraform.ResourceAttrDiff{
NewComputed: true,
},
},
},
Err: false,
},
{ {
Name: "vetoing a diff", Name: "vetoing a diff",
Schema: map[string]*Schema{ Schema: map[string]*Schema{