Skip to content

Commit

Permalink
command/format: restore "(forces new resource)" caption
Browse files Browse the repository at this point in the history
In 3ea1592 the plan rendering was refactored to add an extra indirection
of producing a display-oriented plan object first and then rendering from
that object.

There was a logic error while adapting the existing plan rendering code
to use the new display-oriented object: the core InstanceDiff object sets
the "Destroy" flag (a boolean) for both DiffDestroy and DiffDestroyCreate,
and so this code previously checked r.Destroy to recognize the
"destroy-create" case. This was incorrectly adapted to a check for the
display action being DiffDestroy, when it should actually have been
DiffDestroyCreate.

The effect of this bug was to cause the "(forces new resource)"
annotations to not be displayed on attributes, though the resource-level
information still correctly reflected that a new resource was required.

This fix restores the attribute-level annotations.
  • Loading branch information
apparentlymart committed Sep 11, 2017
1 parent 9365145 commit fe4cfd0
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
2 changes: 1 addition & 1 deletion command/format/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func formatPlanInstanceDiff(buf *bytes.Buffer, r *InstanceDiff, keyLen int, colo

updateMsg := ""
switch {
case attr.ForcesNew && r.Action == terraform.DiffDestroy:
case attr.ForcesNew && r.Action == terraform.DiffDestroyCreate:
updateMsg = colorizer.Color(" [red](forces new resource)")
case attr.Sensitive && oldValues:
updateMsg = colorizer.Color(" [yellow](attribute changed)")
Expand Down
35 changes: 35 additions & 0 deletions command/format/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,41 @@ func TestPlan_displayInterpolations(t *testing.T) {
}
}

// Ensure that (forces new resource) text is included
// https://github.com/hashicorp/terraform/issues/16035
func TestPlan_forcesNewResource(t *testing.T) {
plan := &terraform.Plan{
Diff: &terraform.Diff{
Modules: []*terraform.ModuleDiff{
&terraform.ModuleDiff{
Path: []string{"root"},
Resources: map[string]*terraform.InstanceDiff{
"test_resource.foo": &terraform.InstanceDiff{
Destroy: true,
Attributes: map[string]*terraform.ResourceAttrDiff{
"A": &terraform.ResourceAttrDiff{
New: "B",
RequiresNew: true,
},
},
},
},
},
},
},
}
dispPlan := NewPlan(plan)
actual := dispPlan.Format(disabledColorize)

expected := strings.TrimSpace(`
-/+ test_resource.foo (new resource required)
A: "" => "B" (forces new resource)
`)
if actual != expected {
t.Fatalf("expected:\n\n%s\n\ngot:\n\n%s", expected, actual)
}
}

// Test that a root level data source gets a special plan output on create
func TestPlan_rootDataSource(t *testing.T) {
plan := &terraform.Plan{
Expand Down

0 comments on commit fe4cfd0

Please sign in to comment.