backend/local: Render CBD replacement (+/-) correctly (#19642)

* backend/local: Render CBD replacement (+/-) correctly

* command/format: Use IsReplace helper function

backend/local/backend_plan.go

@@ -204,25 +204,22 @@ func (b *Local) renderPlan(plan *plans.Plan, schemas *terraform.Schemas) {
 	headerBuf := &bytes.Buffer{}
 	fmt.Fprintf(headerBuf, "\n%s\n", strings.TrimSpace(planHeaderIntro))
 	if counts[plans.Create] > 0 {
-		fmt.Fprintf(headerBuf, "%s create\n", format.DiffActionSymbol(terraform.DiffCreate))
+		fmt.Fprintf(headerBuf, "%s create\n", format.DiffActionSymbol(plans.Create))
 	}
 	if counts[plans.Update] > 0 {
-		fmt.Fprintf(headerBuf, "%s update in-place\n", format.DiffActionSymbol(terraform.DiffUpdate))
+		fmt.Fprintf(headerBuf, "%s update in-place\n", format.DiffActionSymbol(plans.Update))
 	}
 	if counts[plans.Delete] > 0 {
-		fmt.Fprintf(headerBuf, "%s destroy\n", format.DiffActionSymbol(terraform.DiffDestroy))
+		fmt.Fprintf(headerBuf, "%s destroy\n", format.DiffActionSymbol(plans.Delete))
 	}
 	if counts[plans.DeleteThenCreate] > 0 {
-		fmt.Fprintf(headerBuf, "%s destroy and then create replacement\n", format.DiffActionSymbol(terraform.DiffDestroyCreate))
+		fmt.Fprintf(headerBuf, "%s destroy and then create replacement\n", format.DiffActionSymbol(plans.DeleteThenCreate))
 	}
 	if counts[plans.CreateThenDelete] > 0 {
-		// FIXME: This shows the wrong symbol, because our old diff action
+		fmt.Fprintf(headerBuf, "%s create replacement and then destroy\n", format.DiffActionSymbol(plans.CreateThenDelete))
-		// type can't represent CreateThenDelete. We should switch
-		// format.DiffActionSymbol over to using plans.Action instead.
-		fmt.Fprintf(headerBuf, "%s create replacement and then destroy prior\n", format.DiffActionSymbol(terraform.DiffDestroyCreate))
 	}
 	if counts[plans.Read] > 0 {
-		fmt.Fprintf(headerBuf, "%s read (data resources)\n", format.DiffActionSymbol(terraform.DiffRefresh))
+		fmt.Fprintf(headerBuf, "%s read (data resources)\n", format.DiffActionSymbol(plans.Read))
 	}
 
 	b.CLI.Output(b.Colorize().Color(headerBuf.String()))

backend/local/backend_plan_test.go

@@ -130,6 +130,132 @@ func TestLocal_planNoConfig(t *testing.T) {
 	}
 }
 
+func TestLocal_planTainted(t *testing.T) {
+	b, cleanup := TestLocal(t)
+	defer cleanup()
+	p := TestLocalProvider(t, b, "test", planFixtureSchema())
+	testStateFile(t, b.StatePath, testPlanState_tainted())
+	b.CLI = cli.NewMockUi()
+	outDir := testTempDir(t)
+	defer os.RemoveAll(outDir)
+	planPath := filepath.Join(outDir, "plan.tfplan")
+	op, configCleanup := testOperationPlan(t, "./test-fixtures/plan")
+	defer configCleanup()
+	op.PlanRefresh = true
+	op.PlanOutPath = planPath
+	cfg := cty.ObjectVal(map[string]cty.Value{
+		"path": cty.StringVal(b.StatePath),
+	})
+	cfgRaw, err := plans.NewDynamicValue(cfg, cfg.Type())
+	if err != nil {
+		t.Fatal(err)
+	}
+	op.PlanOutBackend = &plans.Backend{
+		// Just a placeholder so that we can generate a valid plan file.
+		Type:   "local",
+		Config: cfgRaw,
+	}
+	run, err := b.Operation(context.Background(), op)
+	if err != nil {
+		t.Fatalf("bad: %s", err)
+	}
+	<-run.Done()
+	if run.Result != backend.OperationSuccess {
+		t.Fatalf("plan operation failed")
+	}
+	if !p.ReadResourceCalled {
+		t.Fatal("ReadResource should be called")
+	}
+	if run.PlanEmpty {
+		t.Fatal("plan should not be empty")
+	}
+
+	expectedOutput := `An execution plan has been generated and is shown below.
+Resource actions are indicated with the following symbols:
+-/+ destroy and then create replacement
+
+Terraform will perform the following actions:
+
+  # test_instance.foo must be replaced
+-/+ resource "test_instance" "foo" {
+        ami = "bar"
+
+        network_interface {
+            description  = "Main network interface"
+            device_index = 0
+        }
+    }
+
+Plan: 1 to add, 0 to change, 1 to destroy.`
+	output := b.CLI.(*cli.MockUi).OutputWriter.String()
+	if !strings.Contains(output, expectedOutput) {
+		t.Fatalf("Unexpected output:\n%s", output)
+	}
+}
+
+func TestLocal_planTainted_createBeforeDestroy(t *testing.T) {
+	b, cleanup := TestLocal(t)
+	defer cleanup()
+	p := TestLocalProvider(t, b, "test", planFixtureSchema())
+	testStateFile(t, b.StatePath, testPlanState_tainted())
+	b.CLI = cli.NewMockUi()
+	outDir := testTempDir(t)
+	defer os.RemoveAll(outDir)
+	planPath := filepath.Join(outDir, "plan.tfplan")
+	op, configCleanup := testOperationPlan(t, "./test-fixtures/plan-cbd")
+	defer configCleanup()
+	op.PlanRefresh = true
+	op.PlanOutPath = planPath
+	cfg := cty.ObjectVal(map[string]cty.Value{
+		"path": cty.StringVal(b.StatePath),
+	})
+	cfgRaw, err := plans.NewDynamicValue(cfg, cfg.Type())
+	if err != nil {
+		t.Fatal(err)
+	}
+	op.PlanOutBackend = &plans.Backend{
+		// Just a placeholder so that we can generate a valid plan file.
+		Type:   "local",
+		Config: cfgRaw,
+	}
+	run, err := b.Operation(context.Background(), op)
+	if err != nil {
+		t.Fatalf("bad: %s", err)
+	}
+	<-run.Done()
+	if run.Result != backend.OperationSuccess {
+		t.Fatalf("plan operation failed")
+	}
+	if !p.ReadResourceCalled {
+		t.Fatal("ReadResource should be called")
+	}
+	if run.PlanEmpty {
+		t.Fatal("plan should not be empty")
+	}
+
+	expectedOutput := `An execution plan has been generated and is shown below.
+Resource actions are indicated with the following symbols:
++/- create replacement and then destroy
+
+Terraform will perform the following actions:
+
+  # test_instance.foo must be replaced
++/- resource "test_instance" "foo" {
+        ami = "bar"
+
+        network_interface {
+            description  = "Main network interface"
+            device_index = 0
+        }
+    }
+
+Plan: 1 to add, 0 to change, 1 to destroy.`
+	output := b.CLI.(*cli.MockUi).OutputWriter.String()
+	if !strings.Contains(output, expectedOutput) {
+		t.Fatalf("Unexpected output:\n%s", output)
+	}
+}
+
 func TestLocal_planRefreshFalse(t *testing.T) {
 	b, cleanup := TestLocal(t)
 	defer cleanup()
@@ -466,6 +592,32 @@ func testPlanState_withDataSource() *states.State {
 	return state
 }
 
+func testPlanState_tainted() *states.State {
+	state := states.NewState()
+	rootModule := state.RootModule()
+	rootModule.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "test_instance",
+			Name: "foo",
+		}.Instance(addrs.IntKey(0)),
+		&states.ResourceInstanceObjectSrc{
+			Status: states.ObjectTainted,
+			AttrsJSON: []byte(`{
+				"ami": "bar",
+				"network_interface": [{
+					"device_index": 0,
+					"description": "Main network interface"
+				}]
+			}`),
+		},
+		addrs.ProviderConfig{
+			Type: "test",
+		}.Absolute(addrs.RootModuleInstance),
+	)
+	return state
+}
+
 func testReadPlan(t *testing.T, path string) *plans.Plan {
 	t.Helper()
 

backend/local/test-fixtures/plan-cbd/main.tf

@@ -0,0 +1,13 @@
+resource "test_instance" "foo" {
+    ami = "bar"
+
+    # This is here because at some point it caused a test failure
+    network_interface {
+      device_index = 0
+      description = "Main network interface"
+    }
+
+    lifecycle {
+      create_before_destroy = true
+    }
+}

command/format/plan.go

@@ -29,7 +29,7 @@ type Plan struct {
 // for display, in conjunction with DisplayPlan.
 type InstanceDiff struct {
 	Addr   *terraform.ResourceAddress
-	Action terraform.DiffChangeType
+	Action plans.Action
 
 	// Attributes describes changes to the attributes of the instance.
 	//
@@ -47,7 +47,7 @@ type AttributeDiff struct {
 	// intended for display purposes only.
 	Path string
 
-	Action terraform.DiffChangeType
+	Action plans.Action
 
 	OldValue string
 	NewValue string
@@ -73,7 +73,7 @@ func NewPlan(changes *plans.Changes) *Plan {
 
 	for _, rc := range changes.Resources {
 		addr := rc.Addr
-		log.Printf("[TRACE] NewPlan found %s", addr)
+		log.Printf("[TRACE] NewPlan found %s (%s)", addr, rc.Action)
 		dataSource := addr.Resource.Resource.Mode == addrs.DataResourceMode
 
 		// We create "delete" actions for data resources so we can clean
@@ -87,32 +87,8 @@ func NewPlan(changes *plans.Changes) *Plan {
 		// TODO: Update for the new plan types, ideally also switching over to
 		// a structural diff renderer instead of a flat renderer.
 		did := &InstanceDiff{
-			Addr: terraform.NewLegacyResourceInstanceAddress(addr),
+			Addr:   terraform.NewLegacyResourceInstanceAddress(addr),
-		}
+			Action: rc.Action,
-
-		switch rc.Action {
-		case plans.NoOp:
-			continue
-		case plans.Create:
-			if dataSource {
-				// Use "refresh" as the action for display, but core
-				// currently uses Create for this internally.
-				// FIXME: Update core to generate plans.Read for this case
-				// instead.
-				did.Action = terraform.DiffRefresh
-			} else {
-				did.Action = terraform.DiffCreate
-			}
-		case plans.Read:
-			did.Action = terraform.DiffRefresh
-		case plans.Delete:
-			did.Action = terraform.DiffDestroy
-		case plans.DeleteThenCreate, plans.CreateThenDelete:
-			did.Action = terraform.DiffDestroyCreate
-		case plans.Update:
-			did.Action = terraform.DiffUpdate
-		default:
-			panic(fmt.Sprintf("unexpected change action %s", rc.Action))
 		}
 
 		if rc.DeposedKey != states.NotDeposed {
@@ -180,14 +156,14 @@ func (p *Plan) Stats() PlanStats {
 	var ret PlanStats
 	for _, r := range p.Resources {
 		switch r.Action {
-		case terraform.DiffCreate:
+		case plans.Create:
 			ret.ToAdd++
-		case terraform.DiffUpdate:
+		case plans.Update:
 			ret.ToChange++
-		case terraform.DiffDestroyCreate:
+		case plans.DeleteThenCreate, plans.CreateThenDelete:
 			ret.ToAdd++
 			ret.ToDestroy++
-		case terraform.DiffDestroy:
+		case plans.Delete:
 			ret.ToDestroy++
 		}
 	}
@@ -195,8 +171,8 @@ func (p *Plan) Stats() PlanStats {
 }
 
 // ActionCounts returns the number of diffs for each action type
-func (p *Plan) ActionCounts() map[terraform.DiffChangeType]int {
+func (p *Plan) ActionCounts() map[plans.Action]int {
-	ret := map[terraform.DiffChangeType]int{}
+	ret := map[plans.Action]int{}
 	for _, r := range p.Resources {
 		ret[r.Action]++
 	}
@@ -212,18 +188,22 @@ func (p *Plan) Empty() bool {
 // colorstring.Colorize, will produce a result that can be written
 // to a terminal to produce a symbol made of three printable
 // characters, possibly interspersed with VT100 color codes.
-func DiffActionSymbol(action terraform.DiffChangeType) string {
+func DiffActionSymbol(action plans.Action) string {
 	switch action {
-	case terraform.DiffDestroyCreate:
+	case plans.DeleteThenCreate:
 		return "[red]-[reset]/[green]+[reset]"
-	case terraform.DiffCreate:
+	case plans.CreateThenDelete:
+		return "[green]+[reset]/[red]-[reset]"
+	case plans.Create:
 		return "  [green]+[reset]"
-	case terraform.DiffDestroy:
+	case plans.Delete:
 		return "  [red]-[reset]"
-	case terraform.DiffRefresh:
+	case plans.Read:
 		return " [cyan]<=[reset]"
-	default:
+	case plans.Update:
 		return "  [yellow]~[reset]"
+	default:
+		return "  ?"
 	}
 }
 
@@ -239,14 +219,14 @@ func formatPlanInstanceDiff(buf *bytes.Buffer, r *InstanceDiff, keyLen int, colo
 	symbol := DiffActionSymbol(r.Action)
 	oldValues := true
 	switch r.Action {
-	case terraform.DiffDestroyCreate:
+	case plans.DeleteThenCreate, plans.CreateThenDelete:
 		color = "yellow"
-	case terraform.DiffCreate:
+	case plans.Create:
 		color = "green"
 		oldValues = false
-	case terraform.DiffDestroy:
+	case plans.Delete:
 		color = "red"
-	case terraform.DiffRefresh:
+	case plans.Read:
 		color = "cyan"
 		oldValues = false
 	}
@@ -258,7 +238,7 @@ func formatPlanInstanceDiff(buf *bytes.Buffer, r *InstanceDiff, keyLen int, colo
 	if r.Deposed {
 		extraStr = extraStr + " (deposed)"
 	}
-	if r.Action == terraform.DiffDestroyCreate {
+	if r.Action.IsReplace() {
 		extraStr = extraStr + colorizer.Color(" [red][bold](new resource required)")
 	}
 
@@ -284,7 +264,7 @@ func formatPlanInstanceDiff(buf *bytes.Buffer, r *InstanceDiff, keyLen int, colo
 
 		updateMsg := ""
 		switch {
-		case attr.ForcesNew && r.Action == terraform.DiffDestroyCreate:
+		case attr.ForcesNew && r.Action.IsReplace():
 			updateMsg = colorizer.Color(" [red](forces new resource)")
 		case attr.Sensitive && oldValues:
 			updateMsg = colorizer.Color(" [yellow](attribute changed)")