diff --git a/terraform/diff.go b/terraform/diff.go index a474efad1..002f72921 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -606,14 +606,37 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) { d.mu.Lock() defer d.mu.Unlock() - if d.Destroy != d2.GetDestroy() { + // If we're going from requiring new to NOT requiring new, then we have + // to see if all required news were computed. If so, it is allowed since + // computed may also mean "same value and therefore not new". + oldNew := d.requiresNew() + newNew := d2.RequiresNew() + if oldNew && !newNew { + oldNew = false + for _, rd := range d.Attributes { + // If the field is requires new and NOT computed, then what + // we have is a diff mismatch for sure. We set that the old + // diff does REQUIRE a ForceNew. + if rd != nil && rd.RequiresNew && !rd.NewComputed { + oldNew = true + break + } + } + } + + if oldNew != newNew { + return false, fmt.Sprintf( + "diff RequiresNew; old: %t, new: %t", oldNew, newNew) + } + + // Verify that destroy matches. The second boolean here allows us to + // have mismatching Destroy if we're moving from RequiresNew true + // to false above. Therefore, the second boolean will only pass if + // we're moving from Destroy: true to false as well. + if d.Destroy != d2.GetDestroy() && d.requiresNew() == oldNew { return false, fmt.Sprintf( "diff: Destroy; old: %t, new: %t", d.Destroy, d2.GetDestroy()) } - if d.requiresNew() != d2.RequiresNew() { - return false, fmt.Sprintf( - "diff RequiresNew; old: %t, new: %t", d.requiresNew(), d2.RequiresNew()) - } // Go through the old diff and make sure the new diff has all the // same attributes. To start, build up the check map to be all the keys. diff --git a/terraform/diff_test.go b/terraform/diff_test.go index 655b165f9..ae5ce6da4 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -750,6 +750,76 @@ func TestInstanceDiffSame(t *testing.T) { "", }, + // Computed can change RequiresNew by removal, and that's okay + { + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "foo.#": &ResourceAttrDiff{ + Old: "0", + NewComputed: true, + RequiresNew: true, + }, + }, + }, + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{}, + }, + true, + "", + }, + + // Computed can change Destroy by removal, and that's okay + { + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "foo.#": &ResourceAttrDiff{ + Old: "0", + NewComputed: true, + RequiresNew: true, + }, + }, + + Destroy: true, + }, + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{}, + }, + true, + "", + }, + + // Computed can change Destroy by elements + { + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "foo.#": &ResourceAttrDiff{ + Old: "0", + NewComputed: true, + RequiresNew: true, + }, + }, + + Destroy: true, + }, + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "foo.#": &ResourceAttrDiff{ + Old: "1", + New: "1", + }, + "foo.12": &ResourceAttrDiff{ + Old: "4", + New: "12", + RequiresNew: true, + }, + }, + + Destroy: true, + }, + true, + "", + }, + // Computed sets may not contain all fields in the original diff, and // because multiple entries for the same set can compute to the same // hash before the values are computed or interpolated, the overall