Merge pull request #29523 from hashicorp/alisdair/moved-ui

command: Render "moved" annotations in plan UI

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:

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{
+				Addr:        addr,
-					Mode: tc.Mode,
+				PrevRunAddr: prevRunAddr,
-					Type: "test_instance",
+				DeposedKey:  tc.DeposedKey,
-					Name: "example",
-				}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
-				DeposedKey: tc.DeposedKey,
 				ProviderAddr: addrs.AbsProviderConfig{
 					Provider: addrs.NewDefaultProvider("test"),
 					Module:   addrs.RootModule,

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
 			}
 

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{
+		Addr:        addr,
-			Mode: addrs.ManagedResourceMode,
+		PrevRunAddr: addr,
-			Type: "test_resource",
-			Name: "foo",
-		}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
 		ProviderAddr: addrs.AbsProviderConfig{
 			Provider: addrs.NewDefaultProvider("test"),
 			Module:   addrs.RootModule,

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

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

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{