Skip to content

Commit 0564761

Browse files
jbardingenx7up
authored andcommitted
return early from opPlan when the plan is nil
While the returned plan is checked for nil in most cases, there was a single point where the plan was dereferenced which could panic. Rather than always guarding the dereferences, return early when the plan is nil.
1 parent 3aedb8e commit 0564761

File tree

3 files changed

+42
-11
lines changed

3 files changed

+42
-11
lines changed

internal/backend/backend.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,8 +366,9 @@ type RunningOperation struct {
366366
// operation has completed.
367367
Result OperationResult
368368

369-
// PlanEmpty is populated after a Plan operation completes without error
370-
// to note whether a plan is empty or has changes.
369+
// PlanEmpty is populated after a Plan operation completes to note whether
370+
// a plan is empty or has changes. This is only used in the CLI to determine
371+
// the exit status because the plan value is not available at that point.
371372
PlanEmpty bool
372373

373374
// State is the final state after the operation completed. Persisting

internal/backend/local/backend_plan.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,19 @@ func (b *Local) opPlan(
100100
// generate a partial saved plan file for external analysis.
101101
diags = diags.Append(planDiags)
102102

103+
// Even if there are errors we need to handle anything that may be
104+
// contained within the plan, so only exit if there is no data at all.
105+
if plan == nil {
106+
runningOp.PlanEmpty = true
107+
op.ReportResult(runningOp, diags)
108+
return
109+
}
110+
103111
// Record whether this plan includes any side-effects that could be applied.
104112
runningOp.PlanEmpty = !plan.CanApply()
105113

106114
// Save the plan to disk
107-
if path := op.PlanOutPath; path != "" && plan != nil {
115+
if path := op.PlanOutPath; path != "" {
108116
if op.PlanOutBackend == nil {
109117
// This is always a bug in the operation caller; it's not valid
110118
// to set PlanOutPath without also setting PlanOutBackend.
@@ -154,15 +162,13 @@ func (b *Local) opPlan(
154162

155163
// Render the plan, if we produced one.
156164
// (This might potentially be a partial plan with Errored set to true)
157-
if plan != nil {
158-
schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState)
159-
diags = diags.Append(moreDiags)
160-
if moreDiags.HasErrors() {
161-
op.ReportResult(runningOp, diags)
162-
return
163-
}
164-
op.View.Plan(plan, schemas)
165+
schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState)
166+
diags = diags.Append(moreDiags)
167+
if moreDiags.HasErrors() {
168+
op.ReportResult(runningOp, diags)
169+
return
165170
}
171+
op.View.Plan(plan, schemas)
166172

167173
// If we've accumulated any diagnostics along the way then we'll show them
168174
// here just before we show the summary and next steps. This can potentially

internal/backend/local/backend_plan_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,3 +880,27 @@ func planFixtureSchema() *terraform.ProviderSchema {
880880
},
881881
}
882882
}
883+
884+
func TestLocal_invalidOptions(t *testing.T) {
885+
b := TestLocal(t)
886+
TestLocalProvider(t, b, "test", planFixtureSchema())
887+
888+
op, configCleanup, done := testOperationPlan(t, "./testdata/plan")
889+
defer configCleanup()
890+
op.PlanRefresh = true
891+
op.PlanMode = plans.RefreshOnlyMode
892+
op.ForceReplace = []addrs.AbsResourceInstance{mustResourceInstanceAddr("test_instance.foo")}
893+
894+
run, err := b.Operation(context.Background(), op)
895+
if err != nil {
896+
t.Fatalf("unexpected error: %s", err)
897+
}
898+
<-run.Done()
899+
if run.Result == backend.OperationSuccess {
900+
t.Fatalf("plan operation failed")
901+
}
902+
903+
if errOutput := done(t).Stderr(); errOutput == "" {
904+
t.Fatal("expected error output")
905+
}
906+
}

0 commit comments

Comments
 (0)