From 5031767b93af3fa0a5176c0ed34533bacaf6edd2 Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Mon, 3 Jul 2017 12:59:42 -0700 Subject: [PATCH] terraform: Add more exception cases for value comparison Needed to add more cases to support value comparison exceptions that the rest of TF expects to work (this fixes tests in various places). Also moved things to a switch block so that it's a little more compact. --- terraform/diff.go | 51 ++++++++++++++++---------------------- terraform/diff_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 30 deletions(-) diff --git a/terraform/diff.go b/terraform/diff.go index 6972cacd9..5e51ff36b 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -840,36 +840,27 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) { // If our attributes are not computed, then there is no reason why we can't // check to make sure the diff values are the same. Do that now. // - // Single-value NewComputed values pass - if diffOld.NewComputed { - continue - } - - // Computed keys for sets and lists pass - if strings.Contains(k, "~") { - continue - } - - // Counts for sets need to be skipped as well as we have determined that we - // may not know the full value due to interpolation - if strings.HasSuffix(k, "#") { - continue - } - - // Lists can be skipped if they are being removed (going from n > 0 to 0) - if strings.HasSuffix(k, "%") && diffOld.New == "0" && diffOld.Old != "0" { - continue - } - - // If both old and new are RequiresNew, we are okay if both new values - // match. - if diffOld.RequiresNew && diffNew.RequiresNew && diffOld.New == diffNew.New { - continue - } - - // At this point, we should be able to just check for deep equality. - if !reflect.DeepEqual(diffOld, diffNew) { - return false, fmt.Sprintf("value mismatch: %s", k) + // There are several conditions that we need to pass here as they are + // allowed cases even if values don't match, so let's check those first. + switch { + case diffOld.NewComputed: + // NewComputed values pass + 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) + } } } diff --git a/terraform/diff_test.go b/terraform/diff_test.go index 0e64e5d85..90bf49038 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -1151,6 +1151,62 @@ func TestInstanceDiffSame(t *testing.T) { false, "value mismatch: foo", }, + + // Make sure that DestroyTainted diffs pass as well, especially when diff + // two works off of no state. + { + &InstanceDiff{ + DestroyTainted: true, + Attributes: map[string]*ResourceAttrDiff{ + "foo": &ResourceAttrDiff{ + Old: "foo", + New: "foo", + }, + }, + }, + &InstanceDiff{ + DestroyTainted: true, + Attributes: map[string]*ResourceAttrDiff{ + "foo": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + true, + "", + }, + // RequiresNew in different attribute + { + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "foo": &ResourceAttrDiff{ + Old: "foo", + New: "foo", + }, + "bar": &ResourceAttrDiff{ + Old: "bar", + New: "baz", + RequiresNew: true, + }, + }, + }, + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "foo": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + "bar": &ResourceAttrDiff{ + Old: "", + New: "baz", + RequiresNew: true, + }, + }, + }, + true, + "", + }, } for i, tc := range cases {