backend/remote: lots of improvements

This commit adds:

- support for `-lock-timeout`
- custom error message when a 404 is received
- canceling a pending run when TF is Ctrl-C’ed
- discard a run when the apply is not approved

backend/remote/backend.go

@@ -394,7 +394,7 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
 	}
 
 	// Determine the function to call for our operation
-	var f func(context.Context, context.Context, *backend.Operation) error
+	var f func(context.Context, context.Context, *backend.Operation) (*tfe.Run, error)
 	switch op.Type {
 	case backend.OperationTypePlan:
 		f = b.opPlan
@@ -427,7 +427,7 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
 	cancelCtx, cancel := context.WithCancel(context.Background())
 	runningOp.Cancel = cancel
 
-	// Do it
+	// Do it.
 	go func() {
 		defer done()
 		defer stop()
@@ -435,16 +435,38 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
 
 		defer b.opLock.Unlock()
 
-		err := f(stopCtx, cancelCtx, op)
+		r, err := f(stopCtx, cancelCtx, op)
 		if err != nil && err != context.Canceled {
 			runningOp.Err = err
 		}
+
+		if r != nil && err == context.Canceled {
+			runningOp.Err = b.cancel(cancelCtx, r)
+		}
 	}()
 
-	// Return
+	// Return the running operation.
 	return runningOp, nil
 }
 
+func (b *Remote) cancel(cancelCtx context.Context, r *tfe.Run) error {
+	// Retrieve the run to get its current status.
+	r, err := b.client.Runs.Read(cancelCtx, r.ID)
+	if err != nil {
+		return generalError("error cancelling run", err)
+	}
+
+	// Make sure we cancel the run if possible.
+	if r.Status == tfe.RunPending && r.Actions.IsCancelable {
+		err = b.client.Runs.Cancel(cancelCtx, r.ID, tfe.RunCancelOptions{})
+		if err != nil {
+			return generalError("error cancelling run", err)
+		}
+	}
+
+	return nil
+}
+
 // Colorize returns the Colorize structure that can be used for colorizing
 // output. This is gauranteed to always return a non-nil value and so is useful
 // as a helper to wrap any potentially colored strings.
@@ -460,12 +482,27 @@ func (b *Remote) Colorize() *colorstring.Colorize {
 }
 
 func generalError(msg string, err error) error {
-	if err != context.Canceled {
-		err = fmt.Errorf(strings.TrimSpace(fmt.Sprintf(generalErr, msg, err)))
+	if urlErr, ok := err.(*url.Error); ok {
+		err = urlErr.Err
+	}
+	switch err {
+	case context.Canceled:
+		return err
+	case tfe.ErrResourceNotFound:
+		return fmt.Errorf(strings.TrimSpace(fmt.Sprintf(notFoundErr, msg, err)))
+	default:
+		return fmt.Errorf(strings.TrimSpace(fmt.Sprintf(generalErr, msg, err)))
 	}
-	return err
 }
 
+const notFoundErr = `
+%s: %v
+
+The configured "remote" backend returns '404 Not Found' errors for resources
+that do not exist, as well as for resources that a user doesn't have access
+to. When the resource does exists, please check the rights for the used token.
+`
+
 const generalErr = `
 %s: %v
 

backend/remote/backend_apply.go

@@ -13,87 +13,143 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func (b *Remote) opApply(stopCtx, cancelCtx context.Context, op *backend.Operation) error {
+func (b *Remote) opApply(stopCtx, cancelCtx context.Context, op *backend.Operation) (*tfe.Run, error) {
 	log.Printf("[INFO] backend/remote: starting Apply operation")
 
 	// Retrieve the workspace used to run this operation in.
 	w, err := b.client.Workspaces.Read(stopCtx, b.organization, op.Workspace)
 	if err != nil {
-		return generalError("error retrieving workspace", err)
+		return nil, generalError("error retrieving workspace", err)
 	}
 
 	if w.VCSRepo != nil {
-		return fmt.Errorf(strings.TrimSpace(applyErrVCSNotSupported))
+		return nil, fmt.Errorf(strings.TrimSpace(applyErrVCSNotSupported))
 	}
 
 	if op.Plan != nil {
-		return fmt.Errorf(strings.TrimSpace(applyErrPlanNotSupported))
+		return nil, fmt.Errorf(strings.TrimSpace(applyErrPlanNotSupported))
 	}
 
 	if op.Targets != nil {
-		return fmt.Errorf(strings.TrimSpace(applyErrTargetsNotSupported))
+		return nil, fmt.Errorf(strings.TrimSpace(applyErrTargetsNotSupported))
 	}
 
 	if (op.Module == nil || op.Module.Config().Dir == "") && !op.Destroy {
-		return fmt.Errorf(strings.TrimSpace(planErrNoConfig))
+		return nil, fmt.Errorf(strings.TrimSpace(applyErrNoConfig))
 	}
 
+	// Run the plan phase.
 	r, err := b.plan(stopCtx, cancelCtx, op, w)
 	if err != nil {
-		return err
+		return r, err
 	}
 
+	// Retrieve the run to get its current status.
+	r, err = b.client.Runs.Read(stopCtx, r.ID)
+	if err != nil {
+		return r, generalError("error retrieving run", err)
+	}
+
+	// Return if there are no changes or 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.HasChanges || r.Status == tfe.RunErrored {
+		return r, nil
+	}
+
+	// Check any configured sentinel policies.
 	if len(r.PolicyChecks) > 0 {
 		err = b.checkPolicy(stopCtx, cancelCtx, op, r)
 		if err != nil {
-			return err
+			return r, err
+		}
+	}
+
+	// Retrieve the run to get its current status.
+	r, err = b.client.Runs.Read(stopCtx, r.ID)
+	if err != nil {
+		return r, generalError("error retrieving run", err)
+	}
+
+	// Return if the run cannot be confirmed.
+	if !r.Actions.IsConfirmable {
+		return r, nil
+	}
+
+	if !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{})
+			if err != nil {
+				if op.Destroy {
+					return r, generalError("error disarding destroy", err)
+				}
+				return r, generalError("error disarding apply", err)
+			}
 		}
+		return r, fmt.Errorf(strings.TrimSpace(
+			fmt.Sprint(applyErrNoApplyRights, b.hostname, b.organization, op.Workspace)))
 	}
 
 	hasUI := op.UIOut != nil && op.UIIn != nil
-	mustConfirm := hasUI && (op.Destroy && (!op.DestroyForce && !op.AutoApprove))
+	mustConfirm := hasUI &&
+		(op.Destroy && (!op.DestroyForce && !op.AutoApprove)) || (!op.Destroy && !op.AutoApprove)
 	if mustConfirm {
 		opts := &terraform.InputOpts{Id: "approve"}
 
 		if op.Destroy {
-			opts.Query = "Do you really want to destroy all resources in workspace \"" + op.Workspace + "\"?"
+			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 = "Do you want to perform these actions in workspace \"" + op.Workspace + "\"?"
+			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); err != nil {
-			return 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 generalError("error approving the apply command", err)
+		return r, generalError("error approving the apply command", err)
 	}
 
 	logs, err := b.client.Applies.Logs(stopCtx, r.Apply.ID)
 	if err != nil {
-		return generalError("error retrieving logs", err)
+		return r, generalError("error retrieving logs", err)
 	}
 	scanner := bufio.NewScanner(logs)
 
+	skip := 0
 	for scanner.Scan() {
+		// Skip the first 3 lines to prevent duplicate output.
+		if skip < 3 {
+			skip++
+			continue
+		}
 		if b.CLI != nil {
 			b.CLI.Output(b.Colorize().Color(scanner.Text()))
 		}
 	}
 	if err := scanner.Err(); err != nil {
-		return generalError("error reading logs", err)
+		return r, generalError("error reading logs", err)
 	}
 
-	return nil
+	return r, nil
 }
 
 func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Operation, r *tfe.Run) error {
+	if b.CLI != nil {
+		b.CLI.Output("\n------------------------------------------------------------------------\n")
+	}
 	for _, pc := range r.PolicyChecks {
 		logs, err := b.client.PolicyChecks.Logs(stopCtx, pc.ID)
 		if err != nil {
@@ -101,6 +157,12 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope
 		}
 		scanner := bufio.NewScanner(logs)
 
+		// Retrieve the policy check to get its current status.
+		pc, err := b.client.PolicyChecks.Read(stopCtx, pc.ID)
+		if err != nil {
+			return generalError("error retrieving policy check", err)
+		}
+
 		var msgPrefix string
 		switch pc.Scope {
 		case tfe.PolicyScopeOrganization:
@@ -112,7 +174,7 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope
 		}
 
 		if b.CLI != nil {
-			b.CLI.Output(b.Colorize().Color("\n" + msgPrefix + ":\n"))
+			b.CLI.Output(b.Colorize().Color(msgPrefix + ":\n"))
 		}
 
 		for scanner.Scan() {
@@ -124,13 +186,11 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope
 			return generalError("error reading logs", err)
 		}
 
-		pc, err := b.client.PolicyChecks.Read(stopCtx, pc.ID)
-		if err != nil {
-			return generalError("error retrieving policy check", err)
-		}
-
 		switch pc.Status {
 		case tfe.PolicyPasses:
+			if b.CLI != nil {
+				b.CLI.Output("\n------------------------------------------------------------------------")
+			}
 			continue
 		case tfe.PolicyErrored:
 			return fmt.Errorf(msgPrefix + " errored.")
@@ -147,39 +207,55 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope
 
 		opts := &terraform.InputOpts{
 			Id:          "override",
-			Query:       "Do you want to override the failed policy check?",
-			Description: "Only 'yes' will be accepted to override.",
+			Query:       "\nDo you want to override the soft failed policy check?",
+			Description: "Only 'override' will be accepted to override.",
 		}
 
-		if err = b.confirm(stopCtx, op, opts, r); err != nil {
+		if err = b.confirm(stopCtx, op, opts, r, "override"); err != nil {
 			return err
 		}
+
+		if b.CLI != nil {
+			b.CLI.Output("------------------------------------------------------------------------")
+		}
+
+		if _, err = b.client.PolicyChecks.Override(stopCtx, pc.ID); err != nil {
+			return generalError("error overriding policy check", err)
+		}
 	}
 
 	return nil
 }
 
-func (b *Remote) confirm(stopCtx context.Context, op *backend.Operation, opts *terraform.InputOpts, r *tfe.Run) error {
+func (b *Remote) confirm(stopCtx context.Context, op *backend.Operation, opts *terraform.InputOpts, r *tfe.Run, keyword string) error {
 	v, err := op.UIIn.Input(opts)
 	if err != nil {
 		return fmt.Errorf("Error asking %s: %v", opts.Id, err)
 	}
-	if v != "yes" {
-		// Make sure we discard the run.
-		err = b.client.Runs.Discard(stopCtx, r.ID, tfe.RunDiscardOptions{})
+	if v != keyword {
+		// Retrieve the run again to get its current status.
+		r, err = b.client.Runs.Read(stopCtx, r.ID)
 		if err != nil {
-			if op.Destroy {
-				return generalError("error disarding destroy", err)
+			return generalError("error retrieving 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("error disarding destroy", err)
+				}
+				return generalError("error disarding apply", err)
 			}
-			return generalError("error disarding 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 cancelled.")
+			return errors.New("Destroy discarded.")
 		}
-		return errors.New("Apply cancelled.")
+		return errors.New("Apply discarded.")
 	}
 
 	return nil
@@ -214,8 +290,19 @@ If you would like to destroy everything, please run 'terraform destroy' which
 does not require any configuration files.
 `
 
+const applyErrNoApplyRights = `
+Insufficient rights to approve the pending changes!
+
+[reset][yellow]There are pending changes, but the used credentials have insufficient rights
+to approve them. The run will be discarded to prevent it from blocking the
+queue waiting for external approval. To trigger a run that can be approved by
+someone else, please use the 'Queue Plan' button in the web UI:
+https://%s/app/%s/%s/runs[reset]
+`
+
 const applyDefaultHeader = `
 [reset][yellow]Running apply in the remote backend. Output will stream here. Pressing Ctrl-C
+will cancel the remote apply if its still pending. If the apply started it
 will stop streaming the logs, but will not stop the apply running remotely.
 To view this run in a browser, visit:
 https://%s/app/%s/%s/runs/%s[reset]

backend/remote/backend_plan.go

@@ -10,40 +10,39 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"time"
 
 	tfe "github.com/hashicorp/go-tfe"
 	"github.com/hashicorp/terraform/backend"
 )
 
-func (b *Remote) opPlan(stopCtx, cancelCtx context.Context, op *backend.Operation) error {
+func (b *Remote) opPlan(stopCtx, cancelCtx context.Context, op *backend.Operation) (*tfe.Run, error) {
 	log.Printf("[INFO] backend/remote: starting Plan operation")
 
 	if op.Plan != nil {
-		return fmt.Errorf(strings.TrimSpace(planErrPlanNotSupported))
+		return nil, fmt.Errorf(strings.TrimSpace(planErrPlanNotSupported))
 	}
 
 	if op.PlanOutPath != "" {
-		return fmt.Errorf(strings.TrimSpace(planErrOutPathNotSupported))
+		return nil, fmt.Errorf(strings.TrimSpace(planErrOutPathNotSupported))
 	}
 
 	if op.Targets != nil {
-		return fmt.Errorf(strings.TrimSpace(planErrTargetsNotSupported))
+		return nil, fmt.Errorf(strings.TrimSpace(planErrTargetsNotSupported))
 	}
 
 	if (op.Module == nil || op.Module.Config().Dir == "") && !op.Destroy {
-		return fmt.Errorf(strings.TrimSpace(planErrNoConfig))
+		return nil, fmt.Errorf(strings.TrimSpace(planErrNoConfig))
 	}
 
 	// Retrieve the workspace used to run this operation in.
 	w, err := b.client.Workspaces.Read(stopCtx, b.organization, op.Workspace)
 	if err != nil {
-		return generalError("error retrieving workspace", err)
+		return nil, generalError("error retrieving workspace", err)
 	}
 
-	_, err = b.plan(stopCtx, cancelCtx, op, w)
-
-	return err
+	return b.plan(stopCtx, cancelCtx, op, w)
 }
 
 func (b *Remote) plan(stopCtx, cancelCtx context.Context, op *backend.Operation, w *tfe.Workspace) (*tfe.Run, error) {
@@ -122,22 +121,52 @@ func (b *Remote) plan(stopCtx, cancelCtx context.Context, op *backend.Operation,
 
 	r, err := b.client.Runs.Create(stopCtx, runOptions)
 	if err != nil {
-		return nil, generalError("error creating run", err)
+		return r, generalError("error creating run", err)
+	}
+
+	// When the lock timeout is set,
+	if op.StateLockTimeout > 0 {
+		go func() {
+			select {
+			case <-stopCtx.Done():
+				return
+			case <-cancelCtx.Done():
+				return
+			case <-time.After(op.StateLockTimeout):
+				// Retrieve the run to get its current status.
+				r, err := b.client.Runs.Read(cancelCtx, r.ID)
+				if err != nil {
+					log.Printf("[ERROR] error reading run: %v", err)
+					return
+				}
+
+				if r.Status == tfe.RunPending {
+					if b.CLI != nil {
+						b.CLI.Output(b.Colorize().Color(strings.TrimSpace(lockTimeoutErr)))
+					}
+					syscall.Kill(syscall.Getpid(), syscall.SIGINT)
+				}
+			}
+		}()
 	}
 
 	r, err = b.client.Runs.Read(stopCtx, r.ID)
 	if err != nil {
-		return nil, generalError("error retrieving run", err)
+		return r, generalError("error retrieving run", err)
 	}
 
 	if b.CLI != nil {
+		header := planDefaultHeader
+		if op.Type == backend.OperationTypeApply {
+			header = applyDefaultHeader
+		}
 		b.CLI.Output(b.Colorize().Color(strings.TrimSpace(fmt.Sprintf(
-			planDefaultHeader, b.hostname, b.organization, op.Workspace, r.ID)) + "\n"))
+			header, b.hostname, b.organization, op.Workspace, r.ID)) + "\n"))
 	}
 
 	logs, err := b.client.Plans.Logs(stopCtx, r.Plan.ID)
 	if err != nil {
-		return nil, generalError("error retrieving logs", err)
+		return r, generalError("error retrieving logs", err)
 	}
 	scanner := bufio.NewScanner(logs)
 
@@ -147,7 +176,7 @@ func (b *Remote) plan(stopCtx, cancelCtx context.Context, op *backend.Operation,
 		}
 	}
 	if err := scanner.Err(); err != nil {
-		return nil, generalError("error reading logs", err)
+		return r, generalError("error reading logs", err)
 	}
 
 	return r, nil
@@ -191,3 +220,9 @@ https://%s/app/%s/%s/runs/%s[reset]
 
 Waiting for the plan to start...
 `
+
+// The newline in this error is to make it look good in the CLI!
+const lockTimeoutErr = `
+[reset][red]Lock timeout exceeded, sending interrupt to cancel the remote operation.
+[reset]
+`