don't ignore partial containers in diffs

Containers (maps, lists, sets) in an InstanceDiff need to be handled in
their entirety.  Unchanged values cannot be filtered out from diffs, as
providers expect attribute containers to be complete.

If a value in ignore_changes maps to a single key in an attribute
container, and there are other changes present, that ignored value must
be included in the diff as well.
This commit is contained in:
James Bardin 2018-01-17 15:35:02 -05:00
parent c19fb49bda
commit 8d1e479fc7
3 changed files with 87 additions and 51 deletions

View File

@ -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

View File

@ -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
}
}

View File

@ -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)
}
}
})
}
}