Merge pull request #19010 from hashicorp/f-revert-18980

backend/remote: revert pull request #18980

CHANGELOG.md

@@ -9,7 +9,6 @@ IMPROVEMENTS:
 * backend/remote: Implement the state.Locker interface to support state locking [GH-18826]
 * backend/remote: Add initial support for the apply command [GH-18950]
 * backend/remote: Ask to cancel pending remote operations when Ctrl-C is pressed [GH-18979]
-* backend/remote: Only show the full policy output when the check failed [GH-18980]
 * backend/remote: Add support for the `-no-color` command line flag [GH-19002]
 
 BUG FIXES:
@@ -17,7 +16,7 @@ BUG FIXES:
 * backend/migrations: Check all workspaces for existing non-empty states [GH-18757]
 * provider/terraform: Always call the backend validation method to prevent a possible panic [GH-18759]
 * backend/remote: Take working directories (optional on workspaces) into account [GH-18773]
-* backend/remote: Use pagination when retrieving states (workspaces) [GH-18817] 
+* backend/remote: Use pagination when retrieving states (workspaces) [GH-18817]
 * backend/remote: Add the run ID to associate state when being used in TFE [GH-18818]
 * core: Make sure the state is locked before it is used when `(un)tainting` [GH-18894]
 
@@ -25,7 +24,7 @@ BUG FIXES:
 
 NEW FEATURES:
 
-* **New `remote` backend**: Inital release of the `remote` backend for use with Terraform Enterprise and Private Terraform Enterprise [[#18596](https://github.com/hashicorp/terraform/issues/18596)] 
+* **New `remote` backend**: Inital release of the `remote` backend for use with Terraform Enterprise and Private Terraform Enterprise [[#18596](https://github.com/hashicorp/terraform/issues/18596)]
 
 IMPROVEMENTS:
 
@@ -142,7 +141,7 @@ BACKWARDS INCOMPATIBILITIES / NOTES:
 NEW FEATURES:
 
 * **[Habitat](https://www.habitat.sh/) Provisioner** allowing automatic installation of the Habitat agent ([#16280](https://github.com/hashicorp/terraform/issues/16280))
- 
+
 IMPROVEMENTS:
 
 * core: removed duplicate prompts and clarified working when migration backend configurations ([#16939](https://github.com/hashicorp/terraform/issues/16939))

backend/remote/backend_apply.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"log"
 	"strings"
-	"time"
 
 	tfe "github.com/hashicorp/go-tfe"
 	"github.com/hashicorp/terraform/backend"
@@ -147,30 +146,21 @@ func (b *Remote) opApply(stopCtx, cancelCtx context.Context, op *backend.Operati
 	return r, nil
 }
 
-func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Operation, r *tfe.Run) (err error) {
+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 {
-		// Loop until the context is canceled or the policy check is finished.
+		logs, err := b.client.PolicyChecks.Logs(stopCtx, pc.ID)
-		for {
+		if err != nil {
-			pc, err = b.client.PolicyChecks.Read(stopCtx, pc.ID)
+			return generalError("error retrieving policy check logs", err)
-			if err != nil {
+		}
-				return generalError("error retrieving policy check", err)
+		scanner := bufio.NewScanner(logs)
-			}
 
-			switch pc.Status {
+		// Retrieve the policy check to get its current status.
-			case tfe.PolicyPending, tfe.PolicyQueued:
+		pc, err := b.client.PolicyChecks.Read(stopCtx, pc.ID)
-				select {
+		if err != nil {
-				case <-stopCtx.Done():
+			return generalError("error retrieving policy check", err)
-					return generalError("error retrieving policy check", stopCtx.Err())
-				case <-time.After(500 * time.Millisecond):
-					continue
-				}
-			}
-
-			// Break if the policy check is finished.
-			break
 		}
 
 		var msgPrefix string
@@ -183,25 +173,10 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope
 			msgPrefix = fmt.Sprintf("Unknown policy check (%s)", pc.Scope)
 		}
 
-		// Don't show the full policy output if the policy passed.
-		if pc.Status == tfe.PolicyPasses {
-			if b.CLI != nil {
-				b.CLI.Output(b.Colorize().Color(msgPrefix + ": passed\n"))
-				b.CLI.Output("------------------------------------------------------------------------")
-			}
-			continue
-		}
-
 		if b.CLI != nil {
 			b.CLI.Output(b.Colorize().Color(msgPrefix + ":\n"))
 		}
 
-		logs, err := b.client.PolicyChecks.Logs(stopCtx, pc.ID)
-		if err != nil {
-			return generalError("error retrieving policy check logs", err)
-		}
-		scanner := bufio.NewScanner(logs)
-
 		for scanner.Scan() {
 			if b.CLI != nil {
 				b.CLI.Output(b.Colorize().Color(scanner.Text()))
@@ -212,6 +187,11 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope
 		}
 
 		switch pc.Status {
+		case tfe.PolicyPasses:
+			if b.CLI != nil {
+				b.CLI.Output("\n------------------------------------------------------------------------")
+			}
+			continue
 		case tfe.PolicyErrored:
 			return fmt.Errorf(msgPrefix + " errored.")
 		case tfe.PolicyHardFailed:

backend/remote/backend_apply_test.go

@@ -442,7 +442,7 @@ func TestRemote_applyPolicyPass(t *testing.T) {
 	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, "policy check: passed") {
+	if !strings.Contains(output, "Sentinel Result: true") {
 		t.Fatalf("missing polic check result in output: %s", output)
 	}
 	if !strings.Contains(output, "1 added, 0 changed, 0 destroyed") {

backend/remote/backend_mock.go

@@ -527,7 +527,7 @@ func (m *mockPolicyChecks) Logs(ctx context.Context, policyCheckID string) (io.R
 	}
 
 	if _, err := os.Stat(logfile); os.IsNotExist(err) {
-		return nil, fmt.Errorf("logfile does not exist")
+		return bytes.NewBufferString("logfile does not exist"), nil
 	}
 
 	logs, err := ioutil.ReadFile(logfile)
@@ -535,6 +535,23 @@ func (m *mockPolicyChecks) Logs(ctx context.Context, policyCheckID string) (io.R
 		return nil, err
 	}
 
+	switch {
+	case bytes.Contains(logs, []byte("Sentinel Result: true")):
+		pc.Status = tfe.PolicyPasses
+	case bytes.Contains(logs, []byte("Sentinel Result: false")):
+		switch {
+		case bytes.Contains(logs, []byte("hard-mandatory")):
+			pc.Status = tfe.PolicyHardFailed
+		case bytes.Contains(logs, []byte("soft-mandatory")):
+			pc.Actions.IsOverridable = true
+			pc.Permissions.CanOverride = true
+			pc.Status = tfe.PolicySoftFailed
+		}
+	default:
+		// As this is an unexpected state, we say the policy errored.
+		pc.Status = tfe.PolicyErrored
+	}
+
 	return bytes.NewBuffer(logs), nil
 }
 

backend/remote/test-fixtures/apply-policy-passed/policy.log

@@ -1,2 +1,12 @@
-# This line is here only for the mock!
 Sentinel Result: true
+
+This result means that Sentinel policies returned true and the protected
+behavior is allowed by Sentinel policies.
+
+1 policies evaluated.
+
+## Policy 1: Passthrough.sentinel (soft-mandatory)
+
+Result: true
+
+TRUE - Passthrough.sentinel:1:1 - Rule "main"