diff --git a/backend/backend.go b/backend/backend.go index 7f9435e995..840cb06ac1 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -188,6 +188,10 @@ type RunningOperation struct { // the operation has completed. Err error + // ExitCode can be used to set a custom exit code. This enables enhanced + // backends to set specific exit codes that miror any remote exit codes. + ExitCode int + // PlanEmpty is populated after a Plan operation completes without error // to note whether a plan is empty or has changes. PlanEmpty bool diff --git a/backend/remote/backend.go b/backend/remote/backend.go index 8730904bed..2dcbc8b0d4 100644 --- a/backend/remote/backend.go +++ b/backend/remote/backend.go @@ -416,7 +416,8 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend // the runninCtx is only used to block until the operation returns. runningCtx, done := context.WithCancel(context.Background()) runningOp := &backend.RunningOperation{ - Context: runningCtx, + Context: runningCtx, + PlanEmpty: true, } // stopCtx wraps the context passed in, and is used to signal a graceful Stop. @@ -436,13 +437,30 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend defer b.opLock.Unlock() - r, err := f(stopCtx, cancelCtx, op) - if err != nil && err != context.Canceled { - runningOp.Err = err + r, opErr := f(stopCtx, cancelCtx, op) + if opErr != nil && opErr != context.Canceled { + runningOp.Err = opErr + return } - if r != nil && err == context.Canceled { - runningOp.Err = b.cancel(cancelCtx, op, r.ID) + if r != nil { + // Retrieve the run to get its current status. + r, err := b.client.Runs.Read(cancelCtx, r.ID) + if err != nil { + runningOp.Err = generalError("error retrieving run", err) + return + } + + // Record if there are any changes. + runningOp.PlanEmpty = !r.HasChanges + + if opErr == context.Canceled { + runningOp.Err = b.cancel(cancelCtx, op, r) + } + + if runningOp.Err == nil && r.Status == tfe.RunErrored { + runningOp.ExitCode = 1 + } } }() @@ -450,13 +468,7 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend return runningOp, nil } -func (b *Remote) cancel(cancelCtx context.Context, op *backend.Operation, runID string) error { - // Retrieve the run to get its current status. - r, err := b.client.Runs.Read(cancelCtx, runID) - if err != nil { - return generalError("error cancelling run", err) - } - +func (b *Remote) cancel(cancelCtx context.Context, op *backend.Operation, r *tfe.Run) error { if r.Status == tfe.RunPending && r.Actions.IsCancelable { // Only ask if the remote operation should be canceled // if the auto approve flag is not set. @@ -483,7 +495,7 @@ func (b *Remote) cancel(cancelCtx context.Context, op *backend.Operation, runID } // Try to cancel the remote operation. - err = b.client.Runs.Cancel(cancelCtx, r.ID, tfe.RunCancelOptions{}) + err := b.client.Runs.Cancel(cancelCtx, r.ID, tfe.RunCancelOptions{}) if err != nil { return generalError("error cancelling run", err) } diff --git a/backend/remote/backend_apply_test.go b/backend/remote/backend_apply_test.go index 2dbf00fdd9..d2794b795c 100644 --- a/backend/remote/backend_apply_test.go +++ b/backend/remote/backend_apply_test.go @@ -49,6 +49,9 @@ func TestRemote_applyBasic(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } if len(input.answers) > 0 { t.Fatalf("expected no unused answers, got: %v", input.answers) @@ -132,6 +135,9 @@ func TestRemote_applyWithVCS(t *testing.T) { if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "not allowed for workspaces with a VCS") { t.Fatalf("expected a VCS error, got: %v", run.Err) } @@ -182,6 +188,9 @@ func TestRemote_applyWithPlan(t *testing.T) { if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "saved plan is currently not supported") { t.Fatalf("expected a saved plan error, got: %v", run.Err) } @@ -232,6 +241,9 @@ func TestRemote_applyWithTarget(t *testing.T) { if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "targeting is currently not supported") { t.Fatalf("expected a targeting error, got: %v", run.Err) } @@ -278,6 +290,9 @@ func TestRemote_applyNoConfig(t *testing.T) { if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "configuration files found") { t.Fatalf("expected configuration files error, got: %v", run.Err) } @@ -302,6 +317,9 @@ func TestRemote_applyNoChanges(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } output := b.CLI.(*cli.MockUi).OutputWriter.String() if !strings.Contains(output, "No changes. Infrastructure is up-to-date.") { @@ -334,6 +352,9 @@ func TestRemote_applyNoApprove(t *testing.T) { if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "Apply discarded") { t.Fatalf("expected an apply discarded error, got: %v", run.Err) } @@ -368,6 +389,9 @@ func TestRemote_applyAutoApprove(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } if len(input.answers) != 1 { t.Fatalf("expected an unused answer, got: %v", input.answers) @@ -479,6 +503,9 @@ func TestRemote_applyDestroy(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } if len(input.answers) > 0 { t.Fatalf("expected no unused answers, got: %v", input.answers) @@ -516,6 +543,9 @@ func TestRemote_applyDestroyNoConfig(t *testing.T) { if run.Err != nil { t.Fatalf("unexpected apply error: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } if len(input.answers) > 0 { t.Fatalf("expected no unused answers, got: %v", input.answers) @@ -547,6 +577,9 @@ func TestRemote_applyPolicyPass(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } if len(input.answers) > 0 { t.Fatalf("expected no unused answers, got: %v", input.answers) @@ -589,6 +622,9 @@ func TestRemote_applyPolicyHardFail(t *testing.T) { if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "hard failed") { t.Fatalf("expected a policy check error, got: %v", run.Err) } @@ -634,6 +670,9 @@ func TestRemote_applyPolicySoftFail(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } if len(input.answers) > 0 { t.Fatalf("expected no unused answers, got: %v", input.answers) @@ -677,6 +716,9 @@ func TestRemote_applyPolicySoftFailAutoApprove(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } if len(input.answers) > 0 { t.Fatalf("expected no unused answers, got: %v", input.answers) @@ -693,3 +735,32 @@ func TestRemote_applyPolicySoftFailAutoApprove(t *testing.T) { t.Fatalf("missing apply summery in output: %s", output) } } + +func TestRemote_applyWithRemoteError(t *testing.T) { + b := testBackendDefault(t) + + mod, modCleanup := module.TestTree(t, "./test-fixtures/apply-with-error") + defer modCleanup() + + op := testOperationApply() + op.Module = mod + op.Workspace = backend.DefaultStateName + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + + <-run.Done() + if run.Err != nil { + t.Fatalf("error running operation: %v", run.Err) + } + if run.ExitCode != 1 { + t.Fatalf("expected exit code 1, got %d", run.ExitCode) + } + + output := b.CLI.(*cli.MockUi).OutputWriter.String() + if !strings.Contains(output, "null_resource.foo: 1 error") { + t.Fatalf("missing apply error in output: %s", output) + } +} diff --git a/backend/remote/backend_mock.go b/backend/remote/backend_mock.go index 93fd2558e7..909dcb9477 100644 --- a/backend/remote/backend_mock.go +++ b/backend/remote/backend_mock.go @@ -609,9 +609,9 @@ func (m *mockRuns) Create(ctx context.Context, options tfe.RunCreateOptions) (*t r := &tfe.Run{ ID: generateID("run-"), - Actions: &tfe.RunActions{}, + Actions: &tfe.RunActions{IsCancelable: true}, Apply: a, - HasChanges: true, + HasChanges: false, Permissions: &tfe.RunPermissions{}, Plan: p, Status: tfe.RunPending, @@ -625,14 +625,6 @@ func (m *mockRuns) Create(ctx context.Context, options tfe.RunCreateOptions) (*t r.IsDestroy = *options.IsDestroy } - logs, _ := ioutil.ReadFile(m.client.Plans.logs[p.LogReadURL]) - if r.IsDestroy || !bytes.Contains(logs, []byte("No changes. Infrastructure is up-to-date.")) { - r.Actions.IsCancelable = true - r.Actions.IsConfirmable = true - r.HasChanges = true - r.Permissions.CanApply = true - } - m.runs[r.ID] = r m.workspaces[options.Workspace.ID] = append(m.workspaces[options.Workspace.ID], r) @@ -653,12 +645,28 @@ func (m *mockRuns) Read(ctx context.Context, runID string) (*tfe.Run, error) { } } - if !pending { + if !pending && r.Status == tfe.RunPending { // Only update the status if there are no other pending runs. r.Status = tfe.RunPlanning r.Plan.Status = tfe.PlanRunning } + logs, _ := ioutil.ReadFile(m.client.Plans.logs[r.Plan.LogReadURL]) + if r.Plan.Status == tfe.PlanFinished { + if r.IsDestroy || bytes.Contains(logs, []byte("1 to add, 0 to change, 0 to destroy")) { + r.Actions.IsCancelable = false + r.Actions.IsConfirmable = true + r.HasChanges = true + r.Permissions.CanApply = true + } + + if bytes.Contains(logs, []byte("null_resource.foo: 1 error")) { + r.Actions.IsCancelable = false + r.HasChanges = false + r.Status = tfe.RunErrored + } + } + return r, nil } diff --git a/backend/remote/backend_plan_test.go b/backend/remote/backend_plan_test.go index 68cd6431b0..a9ac9a5d5f 100644 --- a/backend/remote/backend_plan_test.go +++ b/backend/remote/backend_plan_test.go @@ -44,6 +44,9 @@ func TestRemote_planBasic(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if run.PlanEmpty { + t.Fatal("expected a non-empty plan") + } output := b.CLI.(*cli.MockUi).OutputWriter.String() if !strings.Contains(output, "1 to add, 0 to change, 0 to destroy") { @@ -158,6 +161,9 @@ func TestRemote_planWithPlan(t *testing.T) { if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "saved plan is currently not supported") { t.Fatalf("expected a saved plan error, got: %v", run.Err) } @@ -183,6 +189,9 @@ func TestRemote_planWithPath(t *testing.T) { if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "generated plan is currently not supported") { t.Fatalf("expected a generated plan error, got: %v", run.Err) } @@ -233,6 +242,9 @@ func TestRemote_planWithTarget(t *testing.T) { if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "targeting is currently not supported") { t.Fatalf("expected a targeting error, got: %v", run.Err) } @@ -279,6 +291,9 @@ func TestRemote_planNoConfig(t *testing.T) { if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } if !strings.Contains(run.Err.Error(), "configuration files found") { t.Fatalf("expected configuration files error, got: %v", run.Err) } @@ -372,6 +387,9 @@ func TestRemote_planDestroy(t *testing.T) { if run.Err != nil { t.Fatalf("unexpected plan error: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } } func TestRemote_planDestroyNoConfig(t *testing.T) { @@ -391,6 +409,9 @@ func TestRemote_planDestroyNoConfig(t *testing.T) { if run.Err != nil { t.Fatalf("unexpected plan error: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } } func TestRemote_planWithWorkingDirectory(t *testing.T) { @@ -422,9 +443,41 @@ func TestRemote_planWithWorkingDirectory(t *testing.T) { if run.Err != nil { t.Fatalf("error running operation: %v", run.Err) } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } output := b.CLI.(*cli.MockUi).OutputWriter.String() if !strings.Contains(output, "1 to add, 0 to change, 0 to destroy") { t.Fatalf("missing plan summery in output: %s", output) } } + +func TestRemote_planWithRemoteError(t *testing.T) { + b := testBackendDefault(t) + + mod, modCleanup := module.TestTree(t, "./test-fixtures/plan-with-error") + defer modCleanup() + + op := testOperationPlan() + op.Module = mod + op.Workspace = backend.DefaultStateName + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + + <-run.Done() + if run.Err != nil { + t.Fatalf("error running operation: %v", run.Err) + } + if run.ExitCode != 1 { + t.Fatalf("expected exit code 1, got %d", run.ExitCode) + } + + output := b.CLI.(*cli.MockUi).OutputWriter.String() + if !strings.Contains(output, "null_resource.foo: 1 error") { + t.Fatalf("missing plan error in output: %s", output) + } +} diff --git a/backend/remote/test-fixtures/apply-with-error/main.tf b/backend/remote/test-fixtures/apply-with-error/main.tf new file mode 100644 index 0000000000..bc45f28f56 --- /dev/null +++ b/backend/remote/test-fixtures/apply-with-error/main.tf @@ -0,0 +1,5 @@ +resource "null_resource" "foo" { + triggers { + random = "${guid()}" + } +} diff --git a/backend/remote/test-fixtures/apply-with-error/plan.log b/backend/remote/test-fixtures/apply-with-error/plan.log new file mode 100644 index 0000000000..4344a37229 --- /dev/null +++ b/backend/remote/test-fixtures/apply-with-error/plan.log @@ -0,0 +1,10 @@ +Terraform v0.11.7 + +Configuring remote state backend... +Initializing Terraform configuration... + +Error: null_resource.foo: 1 error(s) occurred: + +* null_resource.foo: 1:3: unknown function called: guid in: + +${guid()} diff --git a/backend/remote/test-fixtures/plan-with-error/main.tf b/backend/remote/test-fixtures/plan-with-error/main.tf new file mode 100644 index 0000000000..bc45f28f56 --- /dev/null +++ b/backend/remote/test-fixtures/plan-with-error/main.tf @@ -0,0 +1,5 @@ +resource "null_resource" "foo" { + triggers { + random = "${guid()}" + } +} diff --git a/backend/remote/test-fixtures/plan-with-error/plan.log b/backend/remote/test-fixtures/plan-with-error/plan.log new file mode 100644 index 0000000000..4344a37229 --- /dev/null +++ b/backend/remote/test-fixtures/plan-with-error/plan.log @@ -0,0 +1,10 @@ +Terraform v0.11.7 + +Configuring remote state backend... +Initializing Terraform configuration... + +Error: null_resource.foo: 1 error(s) occurred: + +* null_resource.foo: 1:3: unknown function called: guid in: + +${guid()} diff --git a/command/apply.go b/command/apply.go index 8c1786619d..14baa5aa72 100644 --- a/command/apply.go +++ b/command/apply.go @@ -177,7 +177,7 @@ func (c *ApplyCommand) Run(args []string) int { } } - return 0 + return op.ExitCode } func (c *ApplyCommand) Help() string { diff --git a/command/plan.go b/command/plan.go index 0db56fbe4d..06b8d2ca83 100644 --- a/command/plan.go +++ b/command/plan.go @@ -121,7 +121,7 @@ func (c *PlanCommand) Run(args []string) int { return 2 } - return 0 + return op.ExitCode } func (c *PlanCommand) Help() string {