command: Prototype of handling errored plans

This is a prototype of how the CLI layer might make use of Terraform
Core's ability to produce a partial plan if it encounters an error during
planning, with two new situations:

- When using local CLI workflow, Terraform will show the partial plan
  before showing any errors.
- "terraform plan" has a new option -always-out=..., which is similar to
  the existing -out=... but additionally instructs Terraform to produce
  a plan file even if the plan is incomplete due to errors. This means
  that the plan can still be inspected by external UI implementations.

This is just a prototype to explore how these parts might fit together.
It's not a complete implementation and so should not be shipped. In
particular, it doesn't include any mention of a plan being incomplete in
the "terraform show -json" output or in the "terraform plan -json" output,
both of which would be required for a complete solution.

internal/backend/backend.go

@@ -230,6 +230,7 @@ type Operation struct {
 	PlanRefresh    bool   // PlanRefresh will do a refresh before a plan
 	PlanOutPath    string // PlanOutPath is the path to save the plan
 	PlanOutBackend *plans.Backend
+	PlanOutAlways  bool // Produce PlanOutPath even if plan is incomplete
 
 	// ConfigDir is the path to the directory containing the configuration's
 	// root module.

internal/backend/local/backend_apply.go

@@ -83,6 +83,22 @@ func (b *Local) opApply(
 		plan, moreDiags = lr.Core.Plan(lr.Config, lr.InputState, lr.PlanOpts)
 		diags = diags.Append(moreDiags)
 		if moreDiags.HasErrors() {
+			// If Terraform Core generated a partial plan despite the errors
+			// then we'll make a best effort to render it. Terraform Core
+			// promises that if it returns a non-nil plan along with errors
+			// then the plan won't necessarily contain all of the needed
+			// actions but that any it does include will be properly-formed.
+			// plan.Errored will be true in this case, which our plan
+			// renderer can rely on to tailor its messaging.
+			if plan != nil && (len(plan.Changes.Resources) != 0 || len(plan.Changes.Outputs) != 0) {
+				schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState)
+				// If schema loading returns errors then we'll just give up and
+				// ignore them to avoid distracting from the plan-time errors we're
+				// mainly trying to report here.
+				if !moreDiags.HasErrors() {
+					op.View.Plan(plan, schemas)
+				}
+			}
 			op.ReportResult(runningOp, diags)
 			return
 		}
@@ -162,6 +178,15 @@ func (b *Local) opApply(
 		}
 	} else {
 		plan = lr.Plan
+		if plan.Errored {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Cannot apply incomplete plan",
+				"Terraform encountered an error when generating this plan, so it cannot be applied.",
+			))
+			op.ReportResult(runningOp, diags)
+			return
+		}
 		for _, change := range plan.Changes.Resources {
 			if change.Action != plans.NoOp {
 				op.View.PlannedChange(change)

internal/backend/local/backend_plan.go

@@ -95,17 +95,21 @@ func (b *Local) opPlan(
 	}
 	log.Printf("[INFO] backend/local: plan operation completed")
 
+	// NOTE: We intentionally don't stop here on errors because we always want
+	// to try to present a partial plan report and, if the user chose to,
+	// generate a partial saved plan file for external analysis.
 	diags = diags.Append(planDiags)
-	if planDiags.HasErrors() {
-		op.ReportResult(runningOp, diags)
-		return
-	}
 
 	// Record whether this plan includes any side-effects that could be applied.
 	runningOp.PlanEmpty = !plan.CanApply()
 
+	planErrored := false
+	if plan != nil {
+		planErrored = plan.Errored
+	}
+
 	// Save the plan to disk
-	if path := op.PlanOutPath; path != "" {
+	if path := op.PlanOutPath; path != "" && plan != nil && (op.PlanOutAlways || !planErrored) {
 		if op.PlanOutBackend == nil {
 			// This is always a bug in the operation caller; it's not valid
 			// to set PlanOutPath without also setting PlanOutBackend.
@@ -153,19 +157,24 @@ func (b *Local) opPlan(
 		}
 	}
 
-	// Render the plan
+	// Render the plan, if we produced one.
-	schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState)
+	// (This might potentially be a partial plan with Errored set to true)
-	diags = diags.Append(moreDiags)
+	if plan != nil {
-	if moreDiags.HasErrors() {
+		schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState)
-		op.ReportResult(runningOp, diags)
+		diags = diags.Append(moreDiags)
-		return
+		if moreDiags.HasErrors() {
+			op.ReportResult(runningOp, diags)
+			return
+		}
+		op.View.Plan(plan, schemas)
 	}
-	op.View.Plan(plan, schemas)
 
-	// If we've accumulated any warnings along the way then we'll show them
+	// If we've accumulated any diagnostics along the way then we'll show them
-	// here just before we show the summary and next steps. If we encountered
+	// here just before we show the summary and next steps. This can potentially
-	// errors then we would've returned early at some other point above.
+	// include errors, because we intentionally try to show a partial plan
-	op.View.Diagnostics(diags)
+	// above even if Terraform Core encountered an error partway through
+	// creating it.
+	op.ReportResult(runningOp, diags)
 
 	if !runningOp.PlanEmpty {
 		op.View.PlanNextStep(op.PlanOutPath)

internal/command/arguments/plan.go

@@ -19,8 +19,12 @@ type Plan struct {
 	// variable and backend config values. Default is true.
 	InputEnabled bool
 
-	// OutPath contains an optional path to store the plan file
+	// OutPath contains an optional path to store the plan file, while
-	OutPath string
+	// AlwaysOut means that we'll write to OutPath even if the plan is
+	// incomplete, so that it's still possible to inspect it with
+	// "terraform show". AlwaysOut is irrelevant if OutPath isn't set.
+	OutPath   string
+	AlwaysOut bool
 
 	// ViewType specifies which output format to use
 	ViewType ViewType
@@ -37,10 +41,13 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) {
 		Vars:      &Vars{},
 	}
 
+	var outPath, alwaysOutPath string
+
 	cmdFlags := extendedFlagSet("plan", plan.State, plan.Operation, plan.Vars)
 	cmdFlags.BoolVar(&plan.DetailedExitCode, "detailed-exitcode", false, "detailed-exitcode")
 	cmdFlags.BoolVar(&plan.InputEnabled, "input", true, "input")
-	cmdFlags.StringVar(&plan.OutPath, "out", "", "out")
+	cmdFlags.StringVar(&outPath, "out", "", "out")
+	cmdFlags.StringVar(&alwaysOutPath, "always-out", "", "always-out")
 
 	var json bool
 	cmdFlags.BoolVar(&json, "json", false, "json")
@@ -53,6 +60,22 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) {
 		))
 	}
 
+	switch {
+	case outPath != "":
+		if alwaysOutPath != "" {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Incompatible command line options",
+				"The -out=... and -always-out=... options are mutually-exclusive.",
+			))
+		}
+		plan.OutPath = outPath
+		plan.AlwaysOut = false
+	case alwaysOutPath != "":
+		plan.OutPath = alwaysOutPath
+		plan.AlwaysOut = true
+	}
+
 	args = cmdFlags.Args()
 
 	if len(args) > 0 {

internal/command/arguments/plan_test.go

@@ -31,12 +31,30 @@ func TestParsePlan_basicValid(t *testing.T) {
 				},
 			},
 		},
-		"setting all options": {
+		"setting all options with -out": {
 			[]string{"-destroy", "-detailed-exitcode", "-input=false", "-out=saved.tfplan"},
 			&Plan{
 				DetailedExitCode: true,
 				InputEnabled:     false,
 				OutPath:          "saved.tfplan",
+				AlwaysOut:        false,
+				ViewType:         ViewHuman,
+				State:            &State{Lock: true},
+				Vars:             &Vars{},
+				Operation: &Operation{
+					PlanMode:    plans.DestroyMode,
+					Parallelism: 10,
+					Refresh:     true,
+				},
+			},
+		},
+		"setting all options with -always-out": {
+			[]string{"-destroy", "-detailed-exitcode", "-input=false", "-always-out=saved.tfplan"},
+			&Plan{
+				DetailedExitCode: true,
+				InputEnabled:     false,
+				OutPath:          "saved.tfplan",
+				AlwaysOut:        true,
 				ViewType:         ViewHuman,
 				State:            &State{Lock: true},
 				Vars:             &Vars{},
@@ -93,6 +111,20 @@ func TestParsePlan_invalid(t *testing.T) {
 	}
 }
 
+func TestParsePlan_conflictingOutPath(t *testing.T) {
+	got, diags := ParsePlan([]string{"-out=foo", "-always-out=bar"})
+	if len(diags) == 0 {
+		t.Fatal("expected diags but got none")
+	}
+	const wantSubstr = "Incompatible command line options: The -out=... and -always-out=... options are mutually-exclusive."
+	if got, want := diags.Err().Error(), wantSubstr; !strings.Contains(got, want) {
+		t.Fatalf("wrong diags\ngot:  %s\nwant: %s", got, want)
+	}
+	if got.ViewType != ViewHuman {
+		t.Fatalf("wrong view type\ngot:  %#v\nwant: %#v", got.ViewType, ViewHuman)
+	}
+}
+
 func TestParsePlan_tooManyArguments(t *testing.T) {
 	got, diags := ParsePlan([]string{"saved.tfplan"})
 	if len(diags) == 0 {

internal/command/plan.go

@@ -72,7 +72,7 @@ func (c *PlanCommand) Run(rawArgs []string) int {
 	}
 
 	// Build the operation request
-	opReq, opDiags := c.OperationRequest(be, view, args.Operation, args.OutPath)
+	opReq, opDiags := c.OperationRequest(be, view, args.Operation, args.OutPath, args.AlwaysOut)
 	diags = diags.Append(opDiags)
 	if diags.HasErrors() {
 		view.Diagnostics(diags)
@@ -139,6 +139,7 @@ func (c *PlanCommand) OperationRequest(
 	view views.Plan,
 	args *arguments.Operation,
 	planOutPath string,
+	planOutAlways bool,
 ) (*backend.Operation, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
@@ -149,6 +150,7 @@ func (c *PlanCommand) OperationRequest(
 	opReq.Hooks = view.Hooks()
 	opReq.PlanRefresh = args.Refresh
 	opReq.PlanOutPath = planOutPath
+	opReq.PlanOutAlways = planOutAlways
 	opReq.Targets = args.Targets
 	opReq.ForceReplace = args.ForceReplace
 	opReq.Type = backend.OperationTypePlan

internal/command/views/plan.go

@@ -138,79 +138,89 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) {
 		// the plan is "applyable" and, if so, whether it had refresh changes
 		// that we already would've presented above.
 
-		switch plan.UIMode {
+		if plan.Errored {
-		case plans.RefreshOnlyMode:
-			if haveRefreshChanges {
-				// We already generated a sufficient prompt about what will
-				// happen if applying this change above, so we don't need to
-				// say anything more.
-				return
-			}
-
-			view.streams.Print(
-				view.colorize.Color("\n[reset][bold][green]No changes.[reset][bold] Your infrastructure still matches the configuration.[reset]\n\n"),
-			)
-			view.streams.Println(format.WordWrap(
-				"Terraform has checked that the real remote objects still match the result of your most recent changes, and found no differences.",
-				view.outputColumns(),
-			))
-
-		case plans.DestroyMode:
 			if haveRefreshChanges {
 				view.streams.Print(format.HorizontalRule(view.colorize, view.outputColumns()))
 				view.streams.Println("")
 			}
 			view.streams.Print(
-				view.colorize.Color("\n[reset][bold][green]No changes.[reset][bold] No objects need to be destroyed.[reset]\n\n"),
+				view.colorize.Color("\n[reset][bold][red]Planning failed.[reset][bold] Terraform encountered an error while generating this plan.[reset]\n\n"),
 			)
-			view.streams.Println(format.WordWrap(
+		} else {
-				"Either you have not created any objects yet or the existing objects were already deleted outside of Terraform.",
+			switch plan.UIMode {
-				view.outputColumns(),
+			case plans.RefreshOnlyMode:
-			))
+				if haveRefreshChanges {
-
+					// We already generated a sufficient prompt about what will
-		default:
+					// happen if applying this change above, so we don't need to
-			if haveRefreshChanges {
+					// say anything more.
-				view.streams.Print(format.HorizontalRule(view.colorize, view.outputColumns()))
+					return
-				view.streams.Println("")
-			}
-			view.streams.Print(
-				view.colorize.Color("\n[reset][bold][green]No changes.[reset][bold] Your infrastructure matches the configuration.[reset]\n\n"),
-			)
-
-			if haveRefreshChanges {
-				if plan.CanApply() {
-					// In this case, applying this plan will not change any
-					// remote objects but _will_ update the state to match what
-					// we detected during refresh, so we'll reassure the user
-					// about that.
-					view.streams.Println(format.WordWrap(
-						"Your configuration already matches the changes detected above, so applying this plan will only update the state to include the changes detected above and won't change any real infrastructure.",
-						view.outputColumns(),
-					))
-				} else {
-					// In this case we detected changes during refresh but this isn't
-					// a planning mode where we consider those to be applyable. The
-					// user must re-run in refresh-only mode in order to update the
-					// state to match the upstream changes.
-					suggestion := "."
-					if !view.runningInAutomation {
-						// The normal message includes a specific command line to run.
-						suggestion = ":\n  terraform apply -refresh-only"
-					}
-					view.streams.Println(format.WordWrap(
-						"Your configuration already matches the changes detected above. If you'd like to update the Terraform state to match, create and apply a refresh-only plan"+suggestion,
-						view.outputColumns(),
-					))
 				}
-				return
-			}
 
-			// If we get down here then we're just in the simple situation where
+				view.streams.Print(
-			// the plan isn't applyable at all.
+					view.colorize.Color("\n[reset][bold][green]No changes.[reset][bold] Your infrastructure still matches the configuration.[reset]\n\n"),
-			view.streams.Println(format.WordWrap(
+				)
-				"Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.",
+				view.streams.Println(format.WordWrap(
-				view.outputColumns(),
+					"Terraform has checked that the real remote objects still match the result of your most recent changes, and found no differences.",
-			))
+					view.outputColumns(),
+				))
+
+			case plans.DestroyMode:
+				if haveRefreshChanges {
+					view.streams.Print(format.HorizontalRule(view.colorize, view.outputColumns()))
+					view.streams.Println("")
+				}
+				view.streams.Print(
+					view.colorize.Color("\n[reset][bold][green]No changes.[reset][bold] No objects need to be destroyed.[reset]\n\n"),
+				)
+				view.streams.Println(format.WordWrap(
+					"Either you have not created any objects yet or the existing objects were already deleted outside of Terraform.",
+					view.outputColumns(),
+				))
+
+			default:
+				if haveRefreshChanges {
+					view.streams.Print(format.HorizontalRule(view.colorize, view.outputColumns()))
+					view.streams.Println("")
+				}
+				view.streams.Print(
+					view.colorize.Color("\n[reset][bold][green]No changes.[reset][bold] Your infrastructure matches the configuration.[reset]\n\n"),
+				)
+
+				if haveRefreshChanges {
+					if plan.CanApply() {
+						// In this case, applying this plan will not change any
+						// remote objects but _will_ update the state to match what
+						// we detected during refresh, so we'll reassure the user
+						// about that.
+						view.streams.Println(format.WordWrap(
+							"Your configuration already matches the changes detected above, so applying this plan will only update the state to include the changes detected above and won't change any real infrastructure.",
+							view.outputColumns(),
+						))
+					} else {
+						// In this case we detected changes during refresh but this isn't
+						// a planning mode where we consider those to be applyable. The
+						// user must re-run in refresh-only mode in order to update the
+						// state to match the upstream changes.
+						suggestion := "."
+						if !view.runningInAutomation {
+							// The normal message includes a specific command line to run.
+							suggestion = ":\n  terraform apply -refresh-only"
+						}
+						view.streams.Println(format.WordWrap(
+							"Your configuration already matches the changes detected above. If you'd like to update the Terraform state to match, create and apply a refresh-only plan"+suggestion,
+							view.outputColumns(),
+						))
+					}
+					return
+				}
+
+				// If we get down here then we're just in the simple situation where
+				// the plan isn't applyable at all.
+				view.streams.Println(format.WordWrap(
+					"Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.",
+					view.outputColumns(),
+				))
+			}
 		}
 		return
 	}
@@ -245,7 +255,11 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) {
 	}
 
 	if len(rChanges) > 0 {
-		view.streams.Printf("\nTerraform will perform the following actions:\n\n")
+		if plan.Errored {
+			view.streams.Printf("\nTerraform planned the following actions, but then encountered a problem:\n\n")
+		} else {
+			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,
@@ -286,23 +300,27 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) {
 			))
 		}
 
-		// stats is similar to counts above, but:
+		if plan.Errored {
-		// - it considers only resource changes
+			view.streams.Printf("This plan is incomplete and therefore cannot be applied.\n\n")
-		// - it simplifies "replace" into both a create and a delete
+		} else {
-		stats := map[plans.Action]int{}
+			// stats is similar to counts above, but:
-		for _, change := range rChanges {
+			// - it considers only resource changes
-			switch change.Action {
+			// - it simplifies "replace" into both a create and a delete
-			case plans.CreateThenDelete, plans.DeleteThenCreate:
+			stats := map[plans.Action]int{}
-				stats[plans.Create]++
+			for _, change := range rChanges {
-				stats[plans.Delete]++
+				switch change.Action {
-			default:
+				case plans.CreateThenDelete, plans.DeleteThenCreate:
-				stats[change.Action]++
+					stats[plans.Create]++
+					stats[plans.Delete]++
+				default:
+					stats[change.Action]++
+				}
 			}
+			view.streams.Printf(
+				view.colorize.Color("[reset][bold]Plan:[reset] %d to add, %d to change, %d to destroy.\n"),
+				stats[plans.Create], stats[plans.Update], stats[plans.Delete],
+			)
 		}
-		view.streams.Printf(
-			view.colorize.Color("[reset][bold]Plan:[reset] %d to add, %d to change, %d to destroy.\n"),
-			stats[plans.Create], stats[plans.Update], stats[plans.Delete],
-		)
 	}
 
 	// If there is at least one planned change to the root module outputs