resilien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

core: don't compare attribute values in Diff.Same 

We previously didn't compare values but had a TODO to start doing so,
which we then recently did. Unfortunately it turns out that we _depend_
on not comparing values here, because when we use EvalCompareDiff (a key
user of Diff.Same) we pass in a diff made from a fresh re-interpolation
of the configuration and so any non-pure function results (timestamp,
uuid) have produced different values.
This commit is contained in:
Martin Atkins 2017-11-03 15:33:40 -07:00
parent 46d2b76e5f
commit c003e8f9a6
2 changed files with 9 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
terraform/diff.go
View File

@ -748,7 +748,7 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
delete(checkOld, k) delete(checkOld, k)
delete(checkNew, k) delete(checkNew, k)
diffNew, ok := d2.GetAttribute(k) _, ok := d2.GetAttribute(k)
if !ok { if !ok {
// If there's no new attribute, and the old diff expected the attribute // If there's no new attribute, and the old diff expected the attribute
// to be removed, that's just fine. // to be removed, that's just fine.
@ -837,31 +837,14 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
} }
} }
// If our attributes are not computed, then there is no reason why we can't // We don't compare the values because we can't currently actually
// check to make sure the diff values are the same. Do that now. // guarantee to generate the same value two two diffs created from
// // the same state+config: we have some pesky interpolation functions
// There are several conditions that we need to pass here as they are // that do not behave as pure functions (uuid, timestamp) and so they
// allowed cases even if values don't match, so let's check those first. // can be different each time a diff is produced.
switch { // FIXME: Re-organize our config handling so that we don't re-evaluate
case diffOld.NewComputed: // expressions when we produce a second comparison diff during
// NewComputed values pass // apply (for EvalCompareDiff).
case strings.Contains(k, "~"):
// Computed keys for sets and lists
case strings.HasSuffix(k, "#"):
// Counts for sets need to be skipped as well as we have determined that
// we may not know the full value due to interpolation
case strings.HasSuffix(k, "%") && diffOld.New == "0" && diffOld.Old != "0":
// Lists can be skipped if they are being removed (going from n > 0 to 0)
case d.DestroyTainted && d2.GetDestroyTainted() && diffOld.New == diffNew.New:
// Same for DestoryTainted
case d.requiresNew() && d2.RequiresNew() && diffOld.New == diffNew.New:
// Same for RequiresNew
default:
// Anything that gets here should be able to be checked for deep equality.
if !reflect.DeepEqual(diffOld, diffNew) {
return false, fmt.Sprintf("value mismatch: %s", k)
}
}
} }
// Check for leftover attributes // Check for leftover attributes

20
terraform/diff_test.go
View File

@ -1131,26 +1131,6 @@ func TestInstanceDiffSame(t *testing.T) {
true, true,
"", "",
}, },
{
&InstanceDiff{
Attributes: map[string]*ResourceAttrDiff{
"foo": &ResourceAttrDiff{
Old: "1",
New: "2",
},
},
},
&InstanceDiff{
Attributes: map[string]*ResourceAttrDiff{
"foo": &ResourceAttrDiff{
Old: "1",
New: "3",
},
},
},
false,
"value mismatch: foo",
},
// Make sure that DestroyTainted diffs pass as well, especially when diff // Make sure that DestroyTainted diffs pass as well, especially when diff
// two works off of no state. // two works off of no state.