From da963a13b99af2f86c0c01d4adc2c1186f93e6dc Mon Sep 17 00:00:00 2001
From: Nick Fagerlund <nick.fagerlund@gmail.com>
Date: Wed, 21 Jun 2023 16:29:28 -0700
Subject: [PATCH] Implement plan -out for Cloud

- Don't save errored plans.
- Call op.View.PlanNextStep for terraform plan in cloud mode (We never used to
  show this footer, because we didn't support -out.)
- Create non-speculative config version if saving plan
- Rewrite TestCloud_planWithPath to expect success!
---
 internal/cloud/backend_plan.go      | 36 ++++++++++++++++++++---------
 internal/cloud/backend_plan_test.go | 32 +++++++++++++++++--------
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/internal/cloud/backend_plan.go b/internal/cloud/backend_plan.go
index b01943d410..13cff76ee2 100644
--- a/internal/cloud/backend_plan.go
+++ b/internal/cloud/backend_plan.go
@@ -23,6 +23,7 @@ import (
 	version "github.com/hashicorp/go-version"
 
 	"github.com/hashicorp/terraform/internal/backend"
+	"github.com/hashicorp/terraform/internal/cloud/cloudplan"
 	"github.com/hashicorp/terraform/internal/command/jsonformat"
 	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/genconfig"
@@ -65,15 +66,6 @@ func (b *Cloud) opPlan(stopCtx, cancelCtx context.Context, op *backend.Operation
 		))
 	}
 
-	if op.PlanOutPath != "" {
-		diags = diags.Append(tfdiags.Sourceless(
-			tfdiags.Error,
-			"Saving a generated plan is currently not supported",
-			`Terraform Cloud does not support saving the generated execution `+
-				`plan locally at this time.`,
-		))
-	}
-
 	if !op.HasConfig() && op.PlanMode != plans.DestroyMode {
 		diags = diags.Append(tfdiags.Sourceless(
 			tfdiags.Error,
@@ -95,7 +87,25 @@ func (b *Cloud) opPlan(stopCtx, cancelCtx context.Context, op *backend.Operation
 		return nil, diags.Err()
 	}
 
-	return b.plan(stopCtx, cancelCtx, op, w)
+	// If the run errored, exit before checking whether to save a plan file
+	run, err := b.plan(stopCtx, cancelCtx, op, w)
+	if err != nil {
+		return nil, err
+	}
+
+	// Save plan file if -out <FILE> was specified
+	if op.PlanOutPath != "" {
+		bookmark := cloudplan.NewSavedPlanBookmark(run.ID, b.hostname)
+		err = bookmark.Save(op.PlanOutPath)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// Everything succeded, so display next steps
+	op.View.PlanNextStep(op.PlanOutPath, op.GenerateConfigOut)
+
+	return run, nil
 }
 
 func (b *Cloud) plan(stopCtx, cancelCtx context.Context, op *backend.Operation, w *tfe.Workspace) (*tfe.Run, error) {
@@ -107,9 +117,12 @@ func (b *Cloud) plan(stopCtx, cancelCtx context.Context, op *backend.Operation,
 		b.CLI.Output(b.Colorize().Color(strings.TrimSpace(header) + "\n"))
 	}
 
+	// Plan-only means they ran terraform plan without -out.
+	planOnly := op.Type == backend.OperationTypePlan && op.PlanOutPath == ""
+
 	configOptions := tfe.ConfigurationVersionCreateOptions{
 		AutoQueueRuns: tfe.Bool(false),
-		Speculative:   tfe.Bool(op.Type == backend.OperationTypePlan),
+		Speculative:   tfe.Bool(planOnly),
 	}
 
 	cv, err := b.client.ConfigurationVersions.Create(stopCtx, w.ID, configOptions)
@@ -206,6 +219,7 @@ in order to capture the filesystem context the remote workspace expects:
 		Refresh:              tfe.Bool(op.PlanRefresh),
 		Workspace:            w,
 		AutoApply:            tfe.Bool(op.AutoApprove),
+		SavePlan:             tfe.Bool(op.PlanOutPath != ""),
 	}
 
 	switch op.PlanMode {
diff --git a/internal/cloud/backend_plan_test.go b/internal/cloud/backend_plan_test.go
index f46ed7a34b..e4d8c0e078 100644
--- a/internal/cloud/backend_plan_test.go
+++ b/internal/cloud/backend_plan_test.go
@@ -20,6 +20,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/backend"
+	"github.com/hashicorp/terraform/internal/cloud/cloudplan"
 	"github.com/hashicorp/terraform/internal/command/arguments"
 	"github.com/hashicorp/terraform/internal/command/clistate"
 	"github.com/hashicorp/terraform/internal/command/jsonformat"
@@ -366,8 +367,11 @@ func TestCloud_planWithPath(t *testing.T) {
 
 	op, configCleanup, done := testOperationPlan(t, "./testdata/plan")
 	defer configCleanup()
+	defer done(t)
 
-	op.PlanOutPath = "./testdata/plan"
+	tmpDir := t.TempDir()
+	pfPath := tmpDir + "/plan.tfplan"
+	op.PlanOutPath = pfPath
 	op.Workspace = testBackendSingleWorkspaceName
 
 	run, err := b.Operation(context.Background(), op)
@@ -376,17 +380,27 @@ func TestCloud_planWithPath(t *testing.T) {
 	}
 
 	<-run.Done()
-	output := done(t)
-	if run.Result == backend.OperationSuccess {
-		t.Fatal("expected plan operation to fail")
+	if run.Result != backend.OperationSuccess {
+		t.Fatalf("operation failed: %s", b.CLI.(*cli.MockUi).ErrorWriter.String())
 	}
-	if !run.PlanEmpty {
-		t.Fatalf("expected plan to be empty")
+	if run.PlanEmpty {
+		t.Fatal("expected a non-empty plan")
 	}
 
-	errOutput := output.Stderr()
-	if !strings.Contains(errOutput, "generated plan is currently not supported") {
-		t.Fatalf("expected a generated plan error, got: %v", errOutput)
+	output := b.CLI.(*cli.MockUi).OutputWriter.String()
+	if !strings.Contains(output, "Running plan in Terraform Cloud") {
+		t.Fatalf("expected TFC header in output: %s", output)
+	}
+	if !strings.Contains(output, "1 to add, 0 to change, 0 to destroy") {
+		t.Fatalf("expected plan summary in output: %s", output)
+	}
+
+	plan, err := cloudplan.LoadSavedPlanBookmark(pfPath)
+	if err != nil {
+		t.Fatalf("error loading cloud plan file: %v", err)
+	}
+	if !strings.Contains(plan.RunID, "run-") || plan.Hostname != "app.terraform.io" {
+		t.Fatalf("unexpected contents in saved cloud plan: %v", plan)
 	}
 }