From 194863db4e7d8a78f1d0def0c981cc36eedc4984 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Mon, 8 Oct 2018 12:31:21 +0200 Subject: [PATCH] Properly handle workspaces that auto apply changes This commit adds logic to check if a workspace is configured to auto apply changes. And if it does, we make sure we output all steps correctly. --- backend/remote/backend_apply.go | 53 ++++++----- backend/remote/backend_apply_test.go | 130 +++++++++++++++++++++++++-- backend/remote/backend_mock.go | 7 ++ backend/remote/backend_plan_test.go | 18 ++-- 4 files changed, 167 insertions(+), 41 deletions(-) diff --git a/backend/remote/backend_apply.go b/backend/remote/backend_apply.go index ab5c1271b5..9e6c585d73 100644 --- a/backend/remote/backend_apply.go +++ b/backend/remote/backend_apply.go @@ -90,14 +90,14 @@ func (b *Remote) opApply(stopCtx, cancelCtx context.Context, op *backend.Operati } // Return if the run cannot be confirmed. - if !r.Actions.IsConfirmable { + if !w.AutoApply && !r.Actions.IsConfirmable { return r, nil } // Since we already checked the permissions before creating the run // this should never happen. But it doesn't hurt to keep this in as // a safeguard for any unexpected situations. - if !r.Permissions.CanApply { + if !w.AutoApply && !r.Permissions.CanApply { // Make sure we discard the run if possible. if r.Actions.IsDiscardable { err = b.client.Runs.Discard(stopCtx, r.ID, tfe.RunDiscardOptions{}) @@ -112,35 +112,40 @@ func (b *Remote) opApply(stopCtx, cancelCtx context.Context, op *backend.Operati fmt.Sprintf(applyErrNoApplyRights, b.hostname, b.organization, op.Workspace))) } - hasUI := op.UIIn != nil && op.UIOut != nil - mustConfirm := hasUI && + mustConfirm := (op.UIIn != nil && op.UIOut != nil) && ((op.Destroy && (!op.DestroyForce && !op.AutoApprove)) || (!op.Destroy && !op.AutoApprove)) - if mustConfirm { - opts := &terraform.InputOpts{Id: "approve"} - if op.Destroy { - opts.Query = "\nDo you really want to destroy all resources in workspace \"" + op.Workspace + "\"?" - opts.Description = "Terraform will destroy all your managed infrastructure, as shown above.\n" + - "There is no undo. Only 'yes' will be accepted to confirm." - } else { - opts.Query = "\nDo you want to perform these actions in workspace \"" + op.Workspace + "\"?" - opts.Description = "Terraform will perform the actions described above.\n" + - "Only 'yes' will be accepted to approve." + if !w.AutoApply { + if mustConfirm { + opts := &terraform.InputOpts{Id: "approve"} + + if op.Destroy { + opts.Query = "\nDo you really want to destroy all resources in workspace \"" + op.Workspace + "\"?" + opts.Description = "Terraform will destroy all your managed infrastructure, as shown above.\n" + + "There is no undo. Only 'yes' will be accepted to confirm." + } else { + opts.Query = "\nDo you want to perform these actions in workspace \"" + op.Workspace + "\"?" + opts.Description = "Terraform will perform the actions described above.\n" + + "Only 'yes' will be accepted to approve." + } + + if err = b.confirm(stopCtx, op, opts, r, "yes"); err != nil { + return r, err + } } - if err = b.confirm(stopCtx, op, opts, r, "yes"); err != nil { - return r, err - } - } else { - if b.CLI != nil { - // Insert a blank line to separate the ouputs. - b.CLI.Output("") + err = b.client.Runs.Apply(stopCtx, r.ID, tfe.RunApplyOptions{}) + if err != nil { + return r, generalError("error approving the apply command", err) } } - err = b.client.Runs.Apply(stopCtx, r.ID, tfe.RunApplyOptions{}) - if err != nil { - return r, generalError("error approving the apply command", err) + // If we don't need to ask for confirmation, insert a blank + // line to separate the ouputs. + if w.AutoApply || !mustConfirm { + if b.CLI != nil { + b.CLI.Output("") + } } logs, err := b.client.Applies.Logs(stopCtx, r.Apply.ID) diff --git a/backend/remote/backend_apply_test.go b/backend/remote/backend_apply_test.go index 2dbf00fdd9..b3c4eddc00 100644 --- a/backend/remote/backend_apply_test.go +++ b/backend/remote/backend_apply_test.go @@ -63,6 +63,61 @@ func TestRemote_applyBasic(t *testing.T) { } } +func TestRemote_applyWithAutoApply(t *testing.T) { + b := testBackendNoDefault(t) + + // Create a named workspace that auto applies. + _, err := b.client.Workspaces.Create( + context.Background(), + b.organization, + tfe.WorkspaceCreateOptions{ + AutoApply: tfe.Bool(true), + Name: tfe.String(b.prefix + "prod"), + }, + ) + if err != nil { + t.Fatalf("error creating named workspace: %v", err) + } + + mod, modCleanup := module.TestTree(t, "./test-fixtures/apply") + defer modCleanup() + + input := testInput(t, map[string]string{ + "approve": "yes", + }) + + op := testOperationApply() + op.Module = mod + op.UIIn = input + op.UIOut = b.CLI + op.Workspace = "prod" + + 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.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } + + if len(input.answers) != 1 { + t.Fatalf("expected an unused answer, got: %v", input.answers) + } + + 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) + } + if !strings.Contains(output, "1 added, 0 changed, 0 destroyed") { + t.Fatalf("missing apply summery in output: %s", output) + } +} + func TestRemote_applyWithoutPermissions(t *testing.T) { b := testBackendNoDefault(t) @@ -90,8 +145,8 @@ func TestRemote_applyWithoutPermissions(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } @@ -127,8 +182,8 @@ func TestRemote_applyWithVCS(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } @@ -152,8 +207,8 @@ func TestRemote_applyWithParallelism(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } @@ -177,8 +232,8 @@ func TestRemote_applyWithPlan(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } @@ -202,8 +257,8 @@ func TestRemote_applyWithoutRefresh(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } @@ -227,8 +282,8 @@ func TestRemote_applyWithTarget(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } @@ -252,8 +307,8 @@ func TestRemote_applyWithVariables(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } @@ -273,8 +328,8 @@ func TestRemote_applyNoConfig(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an apply error, got: %v", run.Err) } @@ -651,6 +706,65 @@ func TestRemote_applyPolicySoftFail(t *testing.T) { } } +func TestRemote_applyPolicySoftFailAutoApply(t *testing.T) { + b := testBackendDefault(t) + + // Create a named workspace that auto applies. + _, err := b.client.Workspaces.Create( + context.Background(), + b.organization, + tfe.WorkspaceCreateOptions{ + AutoApply: tfe.Bool(true), + Name: tfe.String(b.prefix + "prod"), + }, + ) + if err != nil { + t.Fatalf("error creating named workspace: %v", err) + } + + mod, modCleanup := module.TestTree(t, "./test-fixtures/apply-policy-soft-failed") + defer modCleanup() + + input := testInput(t, map[string]string{ + "override": "override", + "approve": "yes", + }) + + op := testOperationApply() + op.Module = mod + op.UIIn = input + op.UIOut = b.CLI + op.Workspace = "prod" + + 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.PlanEmpty { + t.Fatalf("expected a non-empty plan") + } + + if len(input.answers) != 1 { + t.Fatalf("expected an unused answer, got: %v", input.answers) + } + + 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) + } + if !strings.Contains(output, "Sentinel Result: false") { + t.Fatalf("missing policy check result in output: %s", output) + } + if !strings.Contains(output, "1 added, 0 changed, 0 destroyed") { + t.Fatalf("missing apply summery in output: %s", output) + } +} + func TestRemote_applyPolicySoftFailAutoApprove(t *testing.T) { b := testBackendDefault(t) diff --git a/backend/remote/backend_mock.go b/backend/remote/backend_mock.go index 93fd2558e7..a1e1f5e2c2 100644 --- a/backend/remote/backend_mock.go +++ b/backend/remote/backend_mock.go @@ -82,6 +82,10 @@ func (m *mockApplies) create(cvID, workspaceID string) (*tfe.Apply, error) { return nil, tfe.ErrResourceNotFound } + if w.AutoApply { + a.Status = tfe.ApplyRunning + } + m.logs[url] = filepath.Join( m.client.ConfigurationVersions.uploadPaths[cvID], w.WorkingDirectory, @@ -858,6 +862,9 @@ func (m *mockWorkspaces) Create(ctx context.Context, organization string, option CanUpdate: true, }, } + if options.AutoApply != nil { + w.AutoApply = *options.AutoApply + } if options.VCSRepo != nil { w.VCSRepo = &tfe.VCSRepo{} } diff --git a/backend/remote/backend_plan_test.go b/backend/remote/backend_plan_test.go index 68cd6431b0..6ac6c40dba 100644 --- a/backend/remote/backend_plan_test.go +++ b/backend/remote/backend_plan_test.go @@ -78,8 +78,8 @@ func TestRemote_planWithoutPermissions(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } @@ -103,8 +103,8 @@ func TestRemote_planWithModuleDepth(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } @@ -128,8 +128,8 @@ func TestRemote_planWithParallelism(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } @@ -153,8 +153,8 @@ func TestRemote_planWithPlan(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } @@ -178,8 +178,8 @@ func TestRemote_planWithPath(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } @@ -203,8 +203,8 @@ func TestRemote_planWithoutRefresh(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } @@ -228,8 +228,8 @@ func TestRemote_planWithTarget(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) } @@ -253,8 +253,8 @@ func TestRemote_planWithVariables(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected an plan error, got: %v", run.Err) } @@ -274,8 +274,8 @@ func TestRemote_planNoConfig(t *testing.T) { if err != nil { t.Fatalf("error starting operation: %v", err) } - <-run.Done() + <-run.Done() if run.Err == nil { t.Fatalf("expected a plan error, got: %v", run.Err) }