diff --git a/terraform/diff.go b/terraform/diff.go index b6651c0a8..d6dc55061 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -396,11 +396,6 @@ type ResourceAttrDiff struct { Type DiffAttrType } -// Modified returns the inequality of Old and New for this attr -func (d *ResourceAttrDiff) Modified() bool { - return d.Old != d.New -} - // Empty returns true if the diff for this attr is neutral func (d *ResourceAttrDiff) Empty() bool { return d.Old == d.New && !d.NewComputed && !d.NewRemoved diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index c1def9166..26205ce51 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -256,13 +256,15 @@ func (n *EvalDiff) processIgnoreChanges(diff *InstanceDiff) error { containers := groupContainers(diff) keep := map[string]bool{} for _, v := range containers { - if v.keepDiff() { + if v.keepDiff(ignorableAttrKeys) { // At least one key has changes, so list all the sibling keys - // to keep in the diff if any values have changed + // to keep in the diff for k := range v { - if v[k].Modified() { - keep[k] = true - } + keep[k] = true + // this key may have been added by the user to ignore, but + // if it's a subkey in a container, we need to un-ignore it + // to keep the complete containter. + delete(ignorableAttrKeys, k) } } } @@ -294,10 +296,17 @@ func (n *EvalDiff) processIgnoreChanges(diff *InstanceDiff) error { // a group of key-*ResourceAttrDiff pairs from the same flatmapped container type flatAttrDiff map[string]*ResourceAttrDiff -// we need to keep all keys if any of them have a diff -func (f flatAttrDiff) keepDiff() bool { - for _, v := range f { - if !v.Empty() && !v.NewComputed { +// we need to keep all keys if any of them have a diff that's not ignored +func (f flatAttrDiff) keepDiff(ignoreChanges map[string]bool) bool { + for k, v := range f { + ignore := false + for attr := range ignoreChanges { + if strings.HasPrefix(k, attr) { + ignore = true + } + } + + if !v.Empty() && !v.NewComputed && !ignore { return true } } diff --git a/terraform/eval_diff_test.go b/terraform/eval_diff_test.go index 0da3b0ee6..b1f76ea3d 100644 --- a/terraform/eval_diff_test.go +++ b/terraform/eval_diff_test.go @@ -1,6 +1,7 @@ package terraform import ( + "fmt" "reflect" "testing" @@ -79,7 +80,7 @@ func TestEvalFilterDiff(t *testing.T) { } } -func TestProcessIgnoreChangesOnResourceIgnoredWithRequiresNew(t *testing.T) { +func TestProcessIgnoreChanges(t *testing.T) { var evalDiff *EvalDiff var instanceDiff *InstanceDiff @@ -94,53 +95,84 @@ func TestProcessIgnoreChangesOnResourceIgnoredWithRequiresNew(t *testing.T) { &InstanceDiff{ Destroy: true, Attributes: map[string]*ResourceAttrDiff{ + "resource.%": { + Old: "3", + New: "3", + }, "resource.changed": { RequiresNew: true, Type: DiffAttrInput, Old: "old", New: "new", }, - "resource.unchanged": { - Old: "unchanged", + "resource.maybe": { + Old: "", New: newAttribute, }, + "resource.same": { + Old: "same", + New: "same", + }, }, } } - evalDiff, instanceDiff = testDiffs([]string{"resource.changed"}, "unchanged") - err := evalDiff.processIgnoreChanges(instanceDiff) - if err != nil { - t.Fatalf("err: %s", err) - } - if len(instanceDiff.Attributes) > 0 { - t.Fatalf("Expected all resources to be ignored, found %d", len(instanceDiff.Attributes)) - } - - evalDiff, instanceDiff = testDiffs([]string{}, "unchanged") - err = evalDiff.processIgnoreChanges(instanceDiff) - if err != nil { - t.Fatalf("err: %s", err) - } - if len(instanceDiff.Attributes) != 2 { - t.Fatalf("Expected 2 resources to be found, found %d", len(instanceDiff.Attributes)) - } - - evalDiff, instanceDiff = testDiffs([]string{"resource.changed"}, "changed") - err = evalDiff.processIgnoreChanges(instanceDiff) - if err != nil { - t.Fatalf("err: %s", err) - } - if len(instanceDiff.Attributes) != 1 { - t.Fatalf("Expected 1 resource to be found, found %d", len(instanceDiff.Attributes)) - } - - evalDiff, instanceDiff = testDiffs([]string{}, "changed") - err = evalDiff.processIgnoreChanges(instanceDiff) - if err != nil { - t.Fatalf("err: %s", err) - } - if len(instanceDiff.Attributes) != 2 { - t.Fatalf("Expected 2 resource to be found, found %d", len(instanceDiff.Attributes)) + for i, tc := range []struct { + ignore []string + newAttr string + attrDiffs int + }{ + // attr diffs should be all (4), or nothing + { + ignore: []string{"resource.changed"}, + attrDiffs: 0, + }, + { + ignore: []string{"resource.changed"}, + newAttr: "new", + attrDiffs: 4, + }, + { + attrDiffs: 4, + }, + { + ignore: []string{"resource.maybe"}, + newAttr: "new", + attrDiffs: 4, + }, + { + newAttr: "new", + attrDiffs: 4, + }, + { + ignore: []string{"resource"}, + newAttr: "new", + attrDiffs: 0, + }, + // extra ignored values shouldn't effect the diff + { + ignore: []string{"resource.missing", "resource.maybe"}, + newAttr: "new", + attrDiffs: 4, + }, + // this isn't useful, but make sure it doesn't break + { + ignore: []string{"resource.%"}, + attrDiffs: 4, + }, + } { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + evalDiff, instanceDiff = testDiffs(tc.ignore, tc.newAttr) + err := evalDiff.processIgnoreChanges(instanceDiff) + if err != nil { + t.Fatalf("err: %s", err) + } + if len(instanceDiff.Attributes) != tc.attrDiffs { + t.Errorf("expected %d diffs, found %d", tc.attrDiffs, len(instanceDiff.Attributes)) + for k, attr := range instanceDiff.Attributes { + fmt.Printf(" %s:%#v\n", k, attr) + } + } + }) } }