From 8eff169fc72559cf02e7e7e3ab8fd7c36288d247 Mon Sep 17 00:00:00 2001 From: Mark DeCrane Date: Thu, 6 Feb 2025 15:31:51 -0500 Subject: [PATCH 1/4] Do the apply if there is a saved plan file, regardless of whether -auto-approve is set --- internal/cloud/backend_apply.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cloud/backend_apply.go b/internal/cloud/backend_apply.go index 26afd2b134..862bfae482 100644 --- a/internal/cloud/backend_apply.go +++ b/internal/cloud/backend_apply.go @@ -83,8 +83,8 @@ func (b *Cloud) opApply(stopCtx, cancelCtx context.Context, op *backendrun.Opera var r *tfe.Run var err error - - if cp, ok := op.PlanFile.Cloud(); ok { + cp, hasSavePlanFile := op.PlanFile.Cloud() + if hasSavePlanFile { log.Printf("[TRACE] Loading saved cloud plan for apply") // Check hostname first, for a more actionable error than a generic 404 later if cp.Hostname != b.Hostname { @@ -182,7 +182,7 @@ func (b *Cloud) opApply(stopCtx, cancelCtx context.Context, op *backendrun.Opera } // Do the apply! - if !op.AutoApprove && err != errRunApproved { + if (!op.AutoApprove || hasSavePlanFile) && err != errRunApproved { if err = b.client.Runs.Apply(stopCtx, r.ID, tfe.RunApplyOptions{}); err != nil { return r, b.generalError("Failed to approve the apply command", err) } From 739c35cabb375e6f5c10f868bb6666e1a5112fd5 Mon Sep 17 00:00:00 2001 From: Mark DeCrane Date: Thu, 6 Feb 2025 15:32:13 -0500 Subject: [PATCH 2/4] Add test --- internal/cloud/backend_apply_test.go | 67 ++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/internal/cloud/backend_apply_test.go b/internal/cloud/backend_apply_test.go index b3b73e5a24..969b8df31a 100644 --- a/internal/cloud/backend_apply_test.go +++ b/internal/cloud/backend_apply_test.go @@ -508,6 +508,73 @@ func TestCloud_applyWithCloudPlan(t *testing.T) { } } +func TestCloud_applyAutoApprove_with_CloudPlan(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + op, configCleanup, done := testOperationApply(t, "./testdata/apply-json") + defer configCleanup() + defer done(t) + + op.AutoApprove = true + op.UIOut = b.CLI + op.Workspace = testBackendSingleWorkspaceName + + mockSROWorkspace(t, b, op.Workspace) + + ws, err := b.client.Workspaces.Read(context.Background(), b.Organization, b.WorkspaceMapping.Name) + if err != nil { + t.Fatalf("Couldn't read workspace: %s", err) + } + + planRun, err := b.plan(context.Background(), context.Background(), op, ws) + if err != nil { + t.Fatalf("Couldn't perform plan: %s", err) + } + + // Synthesize a cloud plan file with the plan's run ID + pf := &cloudplan.SavedPlanBookmark{ + RemotePlanFormat: 1, + RunID: planRun.ID, + Hostname: b.Hostname, + } + op.PlanFile = planfile.NewWrappedCloud(pf) + + // Start spying on the apply output (now that the plan's done) + stream, close := terminal.StreamsForTesting(t) + + b.renderer = &jsonformat.Renderer{ + Streams: stream, + Colorize: mockColorize(), + } + + // Try apply + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + + <-run.Done() + output := close(t) + if run.Result != backendrun.OperationSuccess { + t.Fatal("expected apply operation to succeed") + } + if run.PlanEmpty { + t.Fatalf("expected plan to not be empty") + } + + gotOut := output.Stdout() + if !strings.Contains(gotOut, "1 added, 0 changed, 0 destroyed") { + t.Fatalf("expected apply summary in output: %s", gotOut) + } + + stateMgr, _ := b.StateMgr(testBackendSingleWorkspaceName) + // An error suggests that the state was not unlocked after apply + if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil { + t.Fatalf("unexpected error locking state after apply: %s", err.Error()) + } +} + func TestCloud_applyWithoutRefresh(t *testing.T) { b, bCleanup := testBackendWithName(t) defer bCleanup() From 0ede1f91a349eb69fe9df588ff12b92f27b3c627 Mon Sep 17 00:00:00 2001 From: Mark DeCrane Date: Thu, 6 Feb 2025 15:51:23 -0500 Subject: [PATCH 3/4] add CHANGELOG entry --- .changes/unreleased/BUG FIXES-20250206-155025.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/unreleased/BUG FIXES-20250206-155025.yaml diff --git a/.changes/unreleased/BUG FIXES-20250206-155025.yaml b/.changes/unreleased/BUG FIXES-20250206-155025.yaml new file mode 100644 index 0000000000..2c6384ba58 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20250206-155025.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: Fixes hanging behavior seen when applying a saved plan with -auto-approve using the cloud backend +time: 2025-02-06T15:50:25.767607-05:00 +custom: + Issue: "36453" From 50f2018c16ca639f45acfb49b5ae8a214f075839 Mon Sep 17 00:00:00 2001 From: Mark DeCrane Date: Fri, 7 Feb 2025 11:25:30 -0500 Subject: [PATCH 4/4] Move changelog entry to backport section, added an explanatory comment --- .../BUG FIXES-20250206-155025.yaml | 0 internal/cloud/backend_apply.go | 8 +++++--- 2 files changed, 5 insertions(+), 3 deletions(-) rename .changes/{unreleased => backported}/BUG FIXES-20250206-155025.yaml (100%) diff --git a/.changes/unreleased/BUG FIXES-20250206-155025.yaml b/.changes/backported/BUG FIXES-20250206-155025.yaml similarity index 100% rename from .changes/unreleased/BUG FIXES-20250206-155025.yaml rename to .changes/backported/BUG FIXES-20250206-155025.yaml diff --git a/internal/cloud/backend_apply.go b/internal/cloud/backend_apply.go index 862bfae482..27efb6951a 100644 --- a/internal/cloud/backend_apply.go +++ b/internal/cloud/backend_apply.go @@ -83,8 +83,8 @@ func (b *Cloud) opApply(stopCtx, cancelCtx context.Context, op *backendrun.Opera var r *tfe.Run var err error - cp, hasSavePlanFile := op.PlanFile.Cloud() - if hasSavePlanFile { + cp, hasSavedPlanFile := op.PlanFile.Cloud() + if hasSavedPlanFile { log.Printf("[TRACE] Loading saved cloud plan for apply") // Check hostname first, for a more actionable error than a generic 404 later if cp.Hostname != b.Hostname { @@ -182,7 +182,9 @@ func (b *Cloud) opApply(stopCtx, cancelCtx context.Context, op *backendrun.Opera } // Do the apply! - if (!op.AutoApprove || hasSavePlanFile) && err != errRunApproved { + // If we have a saved plan file, we proceed to apply the run without confirmation + // regardless of the value of AutoApprove. + if (!op.AutoApprove || hasSavedPlanFile) && err != errRunApproved { if err = b.client.Runs.Apply(stopCtx, r.ID, tfe.RunApplyOptions{}); err != nil { return r, b.generalError("Failed to approve the apply command", err) }