From c12f0355a773c9ec8d1363a27e866bf75e87d1be Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 4 Oct 2018 23:03:40 +0200 Subject: [PATCH] Revert "Merge pull request #18980 from hashicorp/f-policy-output" This reverts commit f09c2db8d2e7e8125b012b4a38f51af82ca2f0c5, reversing changes made to 8394dc797d0356f33d85364e2836d64659b0b316. --- CHANGELOG.md | 7 ++- backend/remote/backend_apply.go | 50 ++++++------------- backend/remote/backend_apply_test.go | 2 +- backend/remote/backend_mock.go | 19 ++++++- .../apply-policy-passed/policy.log | 12 ++++- 5 files changed, 48 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07cb8c28d..ed8fcfc1a 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/backend/remote/backend_apply.go b/backend/remote/backend_apply.go index 657641a5f..a260138fb 100644 --- a/backend/remote/backend_apply.go +++ b/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. - for { - pc, err = b.client.PolicyChecks.Read(stopCtx, pc.ID) - if err != nil { - return generalError("error retrieving policy check", err) - } + logs, err := b.client.PolicyChecks.Logs(stopCtx, pc.ID) + if err != nil { + return generalError("error retrieving policy check logs", err) + } + scanner := bufio.NewScanner(logs) - switch pc.Status { - case tfe.PolicyPending, tfe.PolicyQueued: - select { - case <-stopCtx.Done(): - return generalError("error retrieving policy check", stopCtx.Err()) - case <-time.After(500 * time.Millisecond): - continue - } - } - - // Break if the policy check is finished. - break + // 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 @@ -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: diff --git a/backend/remote/backend_apply_test.go b/backend/remote/backend_apply_test.go index 460b2124d..1857141e3 100644 --- a/backend/remote/backend_apply_test.go +++ b/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") { diff --git a/backend/remote/backend_mock.go b/backend/remote/backend_mock.go index d30e7eed1..55011aad6 100644 --- a/backend/remote/backend_mock.go +++ b/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 } diff --git a/backend/remote/test-fixtures/apply-policy-passed/policy.log b/backend/remote/test-fixtures/apply-policy-passed/policy.log index eb4527a79..b0cb1e598 100644 --- a/backend/remote/test-fixtures/apply-policy-passed/policy.log +++ b/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"