diff --git a/internal/command/format/diff.go b/internal/command/format/diff.go index 982b95980..e111485e0 100644 --- a/internal/command/format/diff.go +++ b/internal/command/format/diff.go @@ -72,13 +72,27 @@ func ResourceChange( // Some extra context about this unusual situation. buf.WriteString(color.Color("\n # (left over from a partially-failed replacement of this instance)")) } + case plans.NoOp: + if change.Moved() { + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] has moved to [bold]%s[reset]", change.PrevRunAddr.String(), dispAddr))) + break + } + fallthrough default: // should never happen, since the above is exhaustive buf.WriteString(fmt.Sprintf("%s has an action the plan renderer doesn't support (this is a bug)", dispAddr)) } buf.WriteString(color.Color("[reset]\n")) - buf.WriteString(color.Color(DiffActionSymbol(change.Action)) + " ") + if change.Moved() && change.Action != plans.NoOp { + buf.WriteString(color.Color(fmt.Sprintf("[bold] # [reset]([bold]%s[reset] has moved to [bold]%s[reset])\n", change.PrevRunAddr.String(), dispAddr))) + } + + if change.Moved() && change.Action == plans.NoOp { + buf.WriteString(" ") + } else { + buf.WriteString(color.Color(DiffActionSymbol(change.Action)) + " ") + } switch addr.Resource.Resource.Mode { case addrs.ManagedResourceMode: diff --git a/internal/command/format/diff_test.go b/internal/command/format/diff_test.go index 31cb62d7d..7ca289c79 100644 --- a/internal/command/format/diff_test.go +++ b/internal/command/format/diff_test.go @@ -4448,6 +4448,79 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { runTestCases(t, testCases) } +func TestResourceChange_moved(t *testing.T) { + prevRunAddr := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "previous", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) + + testCases := map[string]testCase{ + "moved and updated": { + PrevRunAddr: prevRunAddr, + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("12345"), + "foo": cty.StringVal("hello"), + "bar": cty.StringVal("baz"), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("12345"), + "foo": cty.StringVal("hello"), + "bar": cty.StringVal("boop"), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + "foo": {Type: cty.String, Optional: true}, + "bar": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + # (test_instance.previous has moved to test_instance.example) + ~ resource "test_instance" "example" { + ~ bar = "baz" -> "boop" + id = "12345" + # (1 unchanged attribute hidden) + } +`, + }, + "moved without changes": { + PrevRunAddr: prevRunAddr, + Action: plans.NoOp, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("12345"), + "foo": cty.StringVal("hello"), + "bar": cty.StringVal("baz"), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("12345"), + "foo": cty.StringVal("hello"), + "bar": cty.StringVal("baz"), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + "foo": {Type: cty.String, Optional: true}, + "bar": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.previous has moved to test_instance.example + resource "test_instance" "example" { + id = "12345" + # (2 unchanged attributes hidden) + } +`, + }, + } + + runTestCases(t, testCases) +} + type testCase struct { Action plans.Action ActionReason plans.ResourceInstanceChangeActionReason @@ -4460,6 +4533,7 @@ type testCase struct { Schema *configschema.Block RequiredReplace cty.PathSet ExpectedOutput string + PrevRunAddr addrs.AbsResourceInstance } func runTestCases(t *testing.T, testCases map[string]testCase) { @@ -4493,13 +4567,23 @@ func runTestCases(t *testing.T, testCases map[string]testCase) { t.Fatal(err) } + addr := addrs.Resource{ + Mode: tc.Mode, + Type: "test_instance", + Name: "example", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) + + prevRunAddr := tc.PrevRunAddr + // If no previous run address is given, reuse the current address + // to make initialization easier + if prevRunAddr.Resource.Resource.Type == "" { + prevRunAddr = addr + } + change := &plans.ResourceInstanceChangeSrc{ - Addr: addrs.Resource{ - Mode: tc.Mode, - Type: "test_instance", - Name: "example", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - DeposedKey: tc.DeposedKey, + Addr: addr, + PrevRunAddr: prevRunAddr, + DeposedKey: tc.DeposedKey, ProviderAddr: addrs.AbsProviderConfig{ Provider: addrs.NewDefaultProvider("test"), Module: addrs.RootModule, diff --git a/internal/command/views/plan.go b/internal/command/views/plan.go index 5d1798d2f..ff794933b 100644 --- a/internal/command/views/plan.go +++ b/internal/command/views/plan.go @@ -116,7 +116,7 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) { counts := map[plans.Action]int{} var rChanges []*plans.ResourceInstanceChangeSrc for _, change := range plan.Changes.Resources { - if change.Action == plans.NoOp { + if change.Action == plans.NoOp && !change.Moved() { continue // We don't show anything for no-op changes } if change.Action == plans.Delete && change.Addr.Resource.Resource.Mode == addrs.DataResourceMode { @@ -125,7 +125,11 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) { } rChanges = append(rChanges, change) - counts[change.Action]++ + + // Don't count move-only changes + if change.Action != plans.NoOp { + counts[change.Action]++ + } } var changedRootModuleOutputs []*plans.OutputChangeSrc for _, output := range plan.Changes.Outputs { @@ -138,7 +142,7 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) { changedRootModuleOutputs = append(changedRootModuleOutputs, output) } - if len(counts) == 0 && len(changedRootModuleOutputs) == 0 { + if len(rChanges) == 0 && len(changedRootModuleOutputs) == 0 { // If we didn't find any changes to report at all then this is a // "No changes" plan. How we'll present this depends on whether // the plan is "applyable" and, if so, whether it had refresh changes @@ -225,7 +229,7 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) { view.streams.Println("") } - if len(counts) != 0 { + if len(counts) > 0 { headerBuf := &bytes.Buffer{} fmt.Fprintf(headerBuf, "\n%s\n", strings.TrimSpace(format.WordWrap(planHeaderIntro, view.outputColumns()))) if counts[plans.Create] > 0 { @@ -247,9 +251,11 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) { fmt.Fprintf(headerBuf, "%s read (data resources)\n", format.DiffActionSymbol(plans.Read)) } - view.streams.Println(view.colorize.Color(headerBuf.String())) + view.streams.Print(view.colorize.Color(headerBuf.String())) + } - view.streams.Printf("Terraform will perform the following actions:\n\n") + if len(rChanges) > 0 { + view.streams.Printf("\nTerraform will perform the following actions:\n\n") // Note: we're modifying the backing slice of this plan object in-place // here. The ordering of resource changes in a plan is not significant, @@ -265,7 +271,7 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) { }) for _, rcs := range rChanges { - if rcs.Action == plans.NoOp { + if rcs.Action == plans.NoOp && !rcs.Moved() { continue } diff --git a/internal/command/views/plan_test.go b/internal/command/views/plan_test.go index 13cd3c4c3..757c7162d 100644 --- a/internal/command/views/plan_test.go +++ b/internal/command/views/plan_test.go @@ -63,12 +63,15 @@ func testPlan(t *testing.T) *plans.Plan { } changes := plans.NewChanges() + addr := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) + changes.SyncWrapper().AppendResourceInstanceChange(&plans.ResourceInstanceChangeSrc{ - Addr: addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_resource", - Name: "foo", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + Addr: addr, + PrevRunAddr: addr, ProviderAddr: addrs.AbsProviderConfig{ Provider: addrs.NewDefaultProvider("test"), Module: addrs.RootModule, diff --git a/internal/plans/changes.go b/internal/plans/changes.go index ba06244cb..c9aa38fd3 100644 --- a/internal/plans/changes.go +++ b/internal/plans/changes.go @@ -33,7 +33,7 @@ func NewChanges() *Changes { func (c *Changes) Empty() bool { for _, res := range c.Resources { - if res.Action != NoOp { + if res.Action != NoOp || res.Moved() { return false } } diff --git a/internal/plans/changes_src.go b/internal/plans/changes_src.go index 69330a21d..396493956 100644 --- a/internal/plans/changes_src.go +++ b/internal/plans/changes_src.go @@ -125,6 +125,10 @@ func (rcs *ResourceInstanceChangeSrc) DeepCopy() *ResourceInstanceChangeSrc { return &ret } +func (rcs *ResourceInstanceChangeSrc) Moved() bool { + return !rcs.Addr.Equal(rcs.PrevRunAddr) +} + // OutputChangeSrc describes a change to an output value. type OutputChangeSrc struct { // Addr is the absolute address of the output value that the change diff --git a/internal/plans/changes_test.go b/internal/plans/changes_test.go index 16062429b..5dbe10f08 100644 --- a/internal/plans/changes_test.go +++ b/internal/plans/changes_test.go @@ -4,10 +4,127 @@ import ( "fmt" "testing" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/zclconf/go-cty/cty" ) +func TestChangesEmpty(t *testing.T) { + testCases := map[string]struct { + changes *Changes + want bool + }{ + "no changes": { + &Changes{}, + true, + }, + "resource change": { + &Changes{ + Resources: []*ResourceInstanceChangeSrc{ + { + Addr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "woot", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + PrevRunAddr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "woot", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ChangeSrc: ChangeSrc{ + Action: Update, + }, + }, + }, + }, + false, + }, + "resource change with no-op action": { + &Changes{ + Resources: []*ResourceInstanceChangeSrc{ + { + Addr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "woot", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + PrevRunAddr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "woot", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ChangeSrc: ChangeSrc{ + Action: NoOp, + }, + }, + }, + }, + true, + }, + "resource moved with no-op change": { + &Changes{ + Resources: []*ResourceInstanceChangeSrc{ + { + Addr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "woot", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + PrevRunAddr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "toot", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ChangeSrc: ChangeSrc{ + Action: NoOp, + }, + }, + }, + }, + false, + }, + "output change": { + &Changes{ + Outputs: []*OutputChangeSrc{ + { + Addr: addrs.OutputValue{ + Name: "result", + }.Absolute(addrs.RootModuleInstance), + ChangeSrc: ChangeSrc{ + Action: Update, + }, + }, + }, + }, + false, + }, + "output change no-op": { + &Changes{ + Outputs: []*OutputChangeSrc{ + { + Addr: addrs.OutputValue{ + Name: "result", + }.Absolute(addrs.RootModuleInstance), + ChangeSrc: ChangeSrc{ + Action: NoOp, + }, + }, + }, + }, + true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + if got, want := tc.changes.Empty(), tc.want; got != want { + t.Fatalf("unexpected result: got %v, want %v", got, want) + } + }) + } +} + func TestChangeEncodeSensitive(t *testing.T) { testVals := []cty.Value{ cty.ObjectVal(map[string]cty.Value{