helper/schema,terraform: handle computed primtives in diffs

Fixes #3309

There are two primary changes, one to how helper/schema creates diffs
and one to how Terraform compares diffs. Both require careful
understanding.

== 1. helper/schema Changes

helper/schema, given any primitive field (string, int, bool, etc.)
_used to_ create a basic diff when given a computed new value (i.e. from
an unkown interpolation). This would put in the plan that the old value
is whatever the old value was, and the new value was the actual
interpolation. For example, from #3309, the diff showed the following:

```
~ module.test.aws_eip.test-instance.0
    instance: "<INSTANCE ID>" => "${element(aws_instance.test-instance.*.id, count.index)}"
```

Then, when running `apply`, the diff would be realized and you would get
a diff mismatch error because it would realize the final value is the
same and remove it from the diff.

**The change:** `helper/schema` now marks unknown primitive values with
`NewComputed` set to true. Semantically this is correct for the diff to
have this information.

== 2. Terraform Diff.Same Changes

Next, the way Terraform compares diffs needed to be updated

Specifically, the case where the diff from the plan had a NewComputed
primitive and the diff from the apply _no longer has that value_. This
is possible if the computed value ended up being the same as the old
value. This is allowed to pass through.

Together, these fix #3309.
This commit is contained in:
Mitchell Hashimoto 2016-10-25 22:36:59 -04:00
parent 836d17158f
commit 95d37ea79c
No known key found for this signature in database
GPG Key ID: 744E147AA52F5B0A
4 changed files with 61 additions and 10 deletions

View File

@ -995,7 +995,7 @@ func (m schemaMap) diffString(
all bool) error {
var originalN interface{}
var os, ns string
o, n, _, _ := d.diffChange(k)
o, n, _, computed := d.diffChange(k)
if schema.StateFunc != nil && n != nil {
originalN = n
n = schema.StateFunc(n)
@ -1019,7 +1019,7 @@ func (m schemaMap) diffString(
}
// Otherwise, only continue if we're computed
if !schema.Computed {
if !schema.Computed && !computed {
return nil
}
}
@ -1037,6 +1037,7 @@ func (m schemaMap) diffString(
New: ns,
NewExtra: originalN,
NewRemoved: removed,
NewComputed: computed,
})
return nil

View File

@ -441,6 +441,7 @@ func TestSchemaMap_Diff(t *testing.T) {
"availability_zone": &terraform.ResourceAttrDiff{
Old: "",
New: "${var.foo}",
NewComputed: true,
},
},
},
@ -1677,6 +1678,7 @@ func TestSchemaMap_Diff(t *testing.T) {
"route.~1.gateway": &terraform.ResourceAttrDiff{
Old: "",
New: "${var.foo}",
NewComputed: true,
},
},
},

View File

@ -603,6 +603,13 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
continue
}
// If the last diff was a computed value then the absense of
// that value is allowed since it may mean the value ended up
// being the same.
if diffOld.NewComputed {
continue
}
// No exact match, but maybe this is a set containing computed
// values. So check if there is an approximate hash in the key
// and if so, try to match the key.

View File

@ -553,6 +553,47 @@ func TestInstanceDiffSame(t *testing.T) {
"diff RequiresNew; old: true, new: false",
},
// NewComputed on primitive
{
&InstanceDiff{
Attributes: map[string]*ResourceAttrDiff{
"foo": &ResourceAttrDiff{
Old: "",
New: "${var.foo}",
NewComputed: true,
},
},
},
&InstanceDiff{
Attributes: map[string]*ResourceAttrDiff{
"foo": &ResourceAttrDiff{
Old: "0",
New: "1",
},
},
},
true,
"",
},
// NewComputed on primitive, removed
{
&InstanceDiff{
Attributes: map[string]*ResourceAttrDiff{
"foo": &ResourceAttrDiff{
Old: "",
New: "${var.foo}",
NewComputed: true,
},
},
},
&InstanceDiff{
Attributes: map[string]*ResourceAttrDiff{},
},
true,
"",
},
{
&InstanceDiff{
Attributes: map[string]*ResourceAttrDiff{