From 9f6a12629386a5b06850ddd6b68e910a35ca4807 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 8 Mar 2019 10:37:25 +0100 Subject: [PATCH] backend/remote: check for external updates --- backend/remote/backend_apply.go | 12 ++- backend/remote/backend_apply_test.go | 150 +++++++++++++++++++++++++++ backend/remote/backend_common.go | 145 +++++++++++++++++++++----- backend/remote/backend_mock.go | 17 ++- backend/remote/backend_plan.go | 8 +- 5 files changed, 294 insertions(+), 38 deletions(-) diff --git a/backend/remote/backend_apply.go b/backend/remote/backend_apply.go index 5596020d7..39d26017f 100644 --- a/backend/remote/backend_apply.go +++ b/backend/remote/backend_apply.go @@ -119,7 +119,7 @@ func (b *Remote) opApply(stopCtx, cancelCtx context.Context, op *backend.Operati // This check is also performed in the plan method to determine if // the policies should be checked, but we need to check the values // here again to determine if we are done and should return. - if !r.HasChanges || r.Status == tfe.RunErrored { + if !r.HasChanges || r.Status == tfe.RunCanceled || r.Status == tfe.RunErrored { return r, nil } @@ -177,14 +177,16 @@ func (b *Remote) opApply(stopCtx, cancelCtx context.Context, op *backend.Operati "Only 'yes' will be accepted to approve." } - if err = b.confirm(stopCtx, op, opts, r, "yes"); err != nil { + err = b.confirm(stopCtx, op, opts, r, "yes") + if err != nil && err != errRunApproved { return r, err } } - err = b.client.Runs.Apply(stopCtx, r.ID, tfe.RunApplyOptions{}) - if err != nil { - return r, generalError("Failed to approve the apply command", err) + if err != errRunApproved { + if err = b.client.Runs.Apply(stopCtx, r.ID, tfe.RunApplyOptions{}); err != nil { + return r, generalError("Failed to approve the apply command", err) + } } } diff --git a/backend/remote/backend_apply_test.go b/backend/remote/backend_apply_test.go index a9e7e46d1..635c141be 100644 --- a/backend/remote/backend_apply_test.go +++ b/backend/remote/backend_apply_test.go @@ -459,6 +459,156 @@ func TestRemote_applyAutoApprove(t *testing.T) { } } +func TestRemote_applyApprovedExternally(t *testing.T) { + b, bCleanup := testBackendDefault(t) + defer bCleanup() + + op, configCleanup := testOperationApply(t, "./test-fixtures/apply") + defer configCleanup() + + input := testInput(t, map[string]string{ + "approve": "wait-for-external-update", + }) + + op.UIIn = input + op.UIOut = b.CLI + op.Workspace = backend.DefaultStateName + + ctx := context.Background() + + run, err := b.Operation(ctx, op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + + // Wait 2 seconds to make sure the run started. + time.Sleep(2 * time.Second) + + wl, err := b.client.Workspaces.List( + ctx, + b.organization, + tfe.WorkspaceListOptions{ + ListOptions: tfe.ListOptions{PageNumber: 2, PageSize: 10}, + }, + ) + if err != nil { + t.Fatalf("unexpected error listing workspaces: %v", err) + } + if len(wl.Items) != 1 { + t.Fatalf("expected 1 workspace, got %d workspaces", len(wl.Items)) + } + + rl, err := b.client.Runs.List(ctx, wl.Items[0].ID, tfe.RunListOptions{}) + if err != nil { + t.Fatalf("unexpected error listing runs: %v", err) + } + if len(rl.Items) != 1 { + t.Fatalf("expected 1 run, got %d runs", len(rl.Items)) + } + + err = b.client.Runs.Apply(context.Background(), rl.Items[0].ID, tfe.RunApplyOptions{}) + if err != nil { + t.Fatalf("unexpected error approving run: %v", err) + } + + <-run.Done() + if run.Result != backend.OperationSuccess { + t.Fatalf("operation failed: %s", b.CLI.(*cli.MockUi).ErrorWriter.String()) + } + if run.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } + + output := b.CLI.(*cli.MockUi).OutputWriter.String() + if !strings.Contains(output, "Running apply in the remote backend") { + t.Fatalf("expected remote backend header in output: %s", output) + } + if !strings.Contains(output, "1 to add, 0 to change, 0 to destroy") { + t.Fatalf("expected plan summery in output: %s", output) + } + if !strings.Contains(output, "approved using the UI or API") { + t.Fatalf("expected external approval in output: %s", output) + } + if !strings.Contains(output, "1 added, 0 changed, 0 destroyed") { + t.Fatalf("expected apply summery in output: %s", output) + } +} + +func TestRemote_applyDiscardedExternally(t *testing.T) { + b, bCleanup := testBackendDefault(t) + defer bCleanup() + + op, configCleanup := testOperationApply(t, "./test-fixtures/apply") + defer configCleanup() + + input := testInput(t, map[string]string{ + "approve": "wait-for-external-update", + }) + + op.UIIn = input + op.UIOut = b.CLI + op.Workspace = backend.DefaultStateName + + ctx := context.Background() + + run, err := b.Operation(ctx, op) + if err != nil { + t.Fatalf("error starting operation: %v", err) + } + + // Wait 2 seconds to make sure the run started. + time.Sleep(2 * time.Second) + + wl, err := b.client.Workspaces.List( + ctx, + b.organization, + tfe.WorkspaceListOptions{ + ListOptions: tfe.ListOptions{PageNumber: 2, PageSize: 10}, + }, + ) + if err != nil { + t.Fatalf("unexpected error listing workspaces: %v", err) + } + if len(wl.Items) != 1 { + t.Fatalf("expected 1 workspace, got %d workspaces", len(wl.Items)) + } + + rl, err := b.client.Runs.List(ctx, wl.Items[0].ID, tfe.RunListOptions{}) + if err != nil { + t.Fatalf("unexpected error listing runs: %v", err) + } + if len(rl.Items) != 1 { + t.Fatalf("expected 1 run, got %d runs", len(rl.Items)) + } + + err = b.client.Runs.Discard(context.Background(), rl.Items[0].ID, tfe.RunDiscardOptions{}) + if err != nil { + t.Fatalf("unexpected error discarding run: %v", err) + } + + <-run.Done() + if run.Result == backend.OperationSuccess { + t.Fatal("expected apply operation to fail") + } + if !run.PlanEmpty { + t.Fatalf("expected plan to be empty") + } + + output := b.CLI.(*cli.MockUi).OutputWriter.String() + if !strings.Contains(output, "Running apply in the remote backend") { + t.Fatalf("expected remote backend header in output: %s", output) + } + if !strings.Contains(output, "1 to add, 0 to change, 0 to destroy") { + t.Fatalf("expected plan summery in output: %s", output) + } + if !strings.Contains(output, "discarded using the UI or API") { + t.Fatalf("expected external discard output: %s", output) + } + if strings.Contains(output, "1 added, 0 changed, 0 destroyed") { + t.Fatalf("unexpected apply summery in output: %s", output) + } +} + func TestRemote_applyWithAutoApply(t *testing.T) { b, bCleanup := testBackendNoDefault(t) defer bCleanup() diff --git a/backend/remote/backend_common.go b/backend/remote/backend_common.go index 662915239..4e64830f2 100644 --- a/backend/remote/backend_common.go +++ b/backend/remote/backend_common.go @@ -15,6 +15,14 @@ import ( "github.com/hashicorp/terraform/tfdiags" ) +var ( + errApplyDiscarded = errors.New("Apply discarded.") + errDestroyDiscarded = errors.New("Destroy discarded.") + errRunApproved = errors.New("approved using the UI or API") + errRunDiscarded = errors.New("discarded using the UI or API") + errRunOverridden = errors.New("overridden using the UI or API") +) + // backoff will perform exponential backoff based on the iteration and // limited by the provided min and max (in milliseconds) durations. func backoff(min, max float64, iter int) time.Duration { @@ -296,12 +304,15 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope Description: "Only 'override' will be accepted to override.", } - if err = b.confirm(stopCtx, op, opts, r, "override"); err != nil { + err = b.confirm(stopCtx, op, opts, r, "override") + if err != nil && err != errRunOverridden { return err } - if _, err = b.client.PolicyChecks.Override(stopCtx, pc.ID); err != nil { - return generalError("Failed to override policy check", err) + if err != errRunOverridden { + if _, err = b.client.PolicyChecks.Override(stopCtx, pc.ID); err != nil { + return generalError("Failed to override policy check", err) + } } if b.CLI != nil { @@ -313,35 +324,115 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope } func (b *Remote) confirm(stopCtx context.Context, op *backend.Operation, opts *terraform.InputOpts, r *tfe.Run, keyword string) error { - v, err := op.UIIn.Input(stopCtx, opts) - if err != nil { - return fmt.Errorf("Error asking %s: %v", opts.Id, err) - } - if v != keyword { - // Retrieve the run again to get its current status. - r, err = b.client.Runs.Read(stopCtx, r.ID) - if err != nil { - return generalError("Failed to retrieve run", err) - } + doneCtx, cancel := context.WithCancel(stopCtx) + result := make(chan error, 2) - // Make sure we discard the run if possible. - if r.Actions.IsDiscardable { - err = b.client.Runs.Discard(stopCtx, r.ID, tfe.RunDiscardOptions{}) - if err != nil { - if op.Destroy { - return generalError("Failed to discard destroy", err) + go func() { + // Make sure we cancel doneCtx before we return + // so the input command is also canceled. + defer cancel() + + for { + select { + case <-doneCtx.Done(): + return + case <-stopCtx.Done(): + return + case <-time.After(3 * time.Second): + // Retrieve the run again to get its current status. + r, err := b.client.Runs.Read(stopCtx, r.ID) + if err != nil { + result <- generalError("Failed to retrieve run", err) + return + } + + switch keyword { + case "override": + if r.Status != tfe.RunPolicyOverride { + if r.Status == tfe.RunDiscarded { + err = errRunDiscarded + } else { + err = errRunOverridden + } + } + case "yes": + if !r.Actions.IsConfirmable { + if r.Status == tfe.RunDiscarded { + err = errRunDiscarded + } else { + err = errRunApproved + } + } + } + + if err != nil { + if b.CLI != nil { + b.CLI.Output(b.Colorize().Color( + fmt.Sprintf("[reset][yellow]%s[reset]", err.Error()))) + } + + if err == errRunDiscarded { + if op.Destroy { + err = errDestroyDiscarded + } + err = errApplyDiscarded + } + + result <- err + return } - return generalError("Failed to discard apply", err) } } + }() - // Even if the run was disarding successfully, we still - // return an error as the apply command was cancelled. - if op.Destroy { - return errors.New("Destroy discarded.") + result <- func() error { + v, err := op.UIIn.Input(doneCtx, opts) + if err != nil && err != context.Canceled && stopCtx.Err() != context.Canceled { + return fmt.Errorf("Error asking %s: %v", opts.Id, err) } - return errors.New("Apply discarded.") - } - return nil + // We return the error of our parent channel as we don't + // care about the error of the doneCtx which is only used + // within this function. So if the doneCtx was canceled + // because stopCtx was canceled, this will properly return + // a context.Canceled error and otherwise it returns nil. + if doneCtx.Err() == context.Canceled || stopCtx.Err() == context.Canceled { + return stopCtx.Err() + } + + // Make sure we cancel the context here so the loop that + // checks for external changes to the run is ended before + // we start to make changes ourselves. + cancel() + + if v != keyword { + // Retrieve the run again to get its current status. + r, err = b.client.Runs.Read(stopCtx, r.ID) + if err != nil { + return generalError("Failed to retrieve run", err) + } + + // Make sure we discard the run if possible. + if r.Actions.IsDiscardable { + err = b.client.Runs.Discard(stopCtx, r.ID, tfe.RunDiscardOptions{}) + if err != nil { + if op.Destroy { + return generalError("Failed to discard destroy", err) + } + return generalError("Failed to discard apply", err) + } + } + + // Even if the run was discarded successfully, we still + // return an error as the apply command was canceled. + if op.Destroy { + return errDestroyDiscarded + } + return errApplyDiscarded + } + + return nil + }() + + return <-result } diff --git a/backend/remote/backend_mock.go b/backend/remote/backend_mock.go index 1f0e1fd33..8985dd0e3 100644 --- a/backend/remote/backend_mock.go +++ b/backend/remote/backend_mock.go @@ -222,6 +222,12 @@ func (m *mockInput) Input(ctx context.Context, opts *terraform.InputOpts) (strin if !ok { return "", fmt.Errorf("unexpected input request in test: %s", opts.Id) } + if v == "wait-for-external-update" { + select { + case <-ctx.Done(): + case <-time.After(time.Minute): + } + } delete(m.answers, opts.Id) return v, nil } @@ -704,7 +710,7 @@ func (m *mockRuns) Read(ctx context.Context, runID string) (*tfe.Run, error) { } logs, _ := ioutil.ReadFile(m.client.Plans.logs[r.Plan.LogReadURL]) - if r.Plan.Status == tfe.PlanFinished { + if r.Status == tfe.RunPlanning && 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 @@ -730,6 +736,7 @@ func (m *mockRuns) Apply(ctx context.Context, runID string, options tfe.RunApply if r.Status != tfe.RunPending { // Only update the status if the run is not pending anymore. r.Status = tfe.RunApplying + r.Actions.IsConfirmable = false r.Apply.Status = tfe.ApplyRunning } return nil @@ -744,7 +751,13 @@ func (m *mockRuns) ForceCancel(ctx context.Context, runID string, options tfe.Ru } func (m *mockRuns) Discard(ctx context.Context, runID string, options tfe.RunDiscardOptions) error { - panic("not implemented") + r, ok := m.runs[runID] + if !ok { + return tfe.ErrResourceNotFound + } + r.Status = tfe.RunDiscarded + r.Actions.IsConfirmable = false + return nil } type mockStateVersions struct { diff --git a/backend/remote/backend_plan.go b/backend/remote/backend_plan.go index d9628ca7e..200f7fd8d 100644 --- a/backend/remote/backend_plan.go +++ b/backend/remote/backend_plan.go @@ -283,10 +283,10 @@ func (b *Remote) plan(stopCtx, cancelCtx context.Context, op *backend.Operation, return r, generalError("Failed to retrieve run", err) } - // Return if the run errored. We return without an error, even - // if the run errored, as the error is already displayed by the - // output of the remote run. - if r.Status == tfe.RunErrored { + // Return if the run is canceled or errored. We return without + // an error, even if the run errored, as the error is already + // displayed by the output of the remote run. + if r.Status == tfe.RunCanceled || r.Status == tfe.RunErrored { return r, nil }