Skip to content

Commit

Permalink
fix: parse custom run step output in policy_check (#3502)
Browse files Browse the repository at this point in the history
* parse custom run output in policy_check

* error if no conftest output

* e2e test

* Adding other policy template.

* update markdown renderer test
  • Loading branch information
pseudomorph committed Jun 20, 2023
1 parent e271eaa commit 7545296
Show file tree
Hide file tree
Showing 17 changed files with 217 additions and 4 deletions.
15 changes: 15 additions & 0 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,21 @@ func TestGitHubWorkflowWithPolicyCheck(t *testing.T) {
{"exp-output-merge.txt"},
},
},
{
Description: "failing policy without policies passing and custom run steps",
RepoDir: "policy-checks-custom-run-steps",
ModifiedFiles: []string{"main.tf"},
ExpAutoplan: true,
Comments: []string{
"atlantis apply",
},
ExpReplies: [][]string{
{"exp-output-autoplan.txt"},
{"exp-output-auto-policy-check.txt"},
{"exp-output-apply-failed.txt"},
{"exp-output-merge.txt"},
},
},
{
Description: "failing policy additional apply requirements specified",
RepoDir: "policy-checks-apply-reqs",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
version: 3
projects:
- dir: .
workspace: default
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Ran Apply for dir: `.` workspace: `default`

**Apply Failed**: All policies must pass for project before running apply.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Ran Apply for dir: `.` workspace: `default`

```diff
null_resource.simple:
null_resource.simple:

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Outputs:

workspace = "default"

```

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Approved Policies for 1 projects:

1. dir: `.` workspace: `default`


Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Ran Policy Check for dir: `.` workspace: `default`

**Policy Check Failed**: Some policy sets did not pass.
```diff
pre-conftest output

```

#### Policy Set: `test_policy`
```diff
FAIL - <redacted plan file> - main - WARNING: Null Resource creation is prohibited.

1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions

```


```diff
post-conftest output

```

#### Policy Approval Status:
```
policy set: test_policy: requires: 1 approval(s), have: 0.
```
* :heavy_check_mark: To **approve** this project, comment:
* `atlantis approve_policies -d .`
* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
* :repeat: To re-run policies **plan** this project again by commenting:
* `atlantis plan -d .`

---
* :heavy_check_mark: To **approve** all unapplied plans from this pull request, comment:
* `atlantis approve_policies`
* :put_litter_in_its_place: To delete all plans and locks for the PR, comment:
* `atlantis unlock`
* :repeat: To re-run policies **plan** this project again by commenting:
* `atlantis plan`
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
Ran Plan for dir: `.` workspace: `default`

<details><summary>Show Output</summary>

```diff
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
+ create

Terraform will perform the following actions:

# null_resource.simple[0] will be created
+ resource "null_resource" "simple" {
+ id = (known after apply)
}

Plan: 1 to add, 0 to change, 0 to destroy.

Changes to Outputs:
+ workspace = "default"
```

* :arrow_forward: To **apply** this plan, comment:
* `atlantis apply -d .`
* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
* :repeat: To **plan** this project again, comment:
* `atlantis plan -d .`
</details>
Plan: 1 to add, 0 to change, 0 to destroy.

---
* :fast_forward: To **apply** all unapplied plans from this pull request, comment:
* `atlantis apply`
* :put_litter_in_its_place: To delete all plans and locks for the PR, comment:
* `atlantis unlock`
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Locks and plans deleted for the projects and workspaces modified in this pull request:

- dir: `.` workspace: `default`
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
resource "null_resource" "simple" {
count = 1
}

output "workspace" {
value = terraform.workspace
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package main

import input as tfplan

deny[reason] {
num_deletes.null_resource > 0
reason := "WARNING: Null Resource creation is prohibited."
}

resource_types = {"null_resource"}

resources[resource_type] = all {
some resource_type
resource_types[resource_type]
all := [name |
name := tfplan.resource_changes[_]
name.type == resource_type
]
}

# number of deletions of resources of a given type
num_deletes[resource_type] = num {
some resource_type
resource_types[resource_type]
all := resources[resource_type]
deletions := [res | res := all[_]; res.change.actions[_] == "create"]
num := count(deletions)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
policies:
owners:
users:
- runatlantis
policy_sets:
- name: test_policy
path: policies/policy.rego
source: local

workflows:
default:
policy_check:
steps:
- show
- run: "echo 'pre-conftest output'"
- policy_check:
extra_args:
- --no-fail
- run: "echo 'post-conftest output'"
4 changes: 4 additions & 0 deletions server/events/markdown_renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ type planSuccessData struct {

type policyCheckResultsData struct {
models.PolicyCheckResults
PreConftestOutput string
PostConftestOutput string
PolicyCheckSummary string
PolicyApprovalSummary string
PolicyCleared bool
Expand Down Expand Up @@ -214,6 +216,8 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult,
numPlanSuccesses++
} else if result.PolicyCheckResults != nil && common.Command == policyCheckCommandTitle {
policyCheckResults := policyCheckResultsData{
PreConftestOutput: result.PolicyCheckResults.PreConftestOutput,
PostConftestOutput: result.PolicyCheckResults.PostConftestOutput,
PolicyCheckResults: *result.PolicyCheckResults,
PolicyCheckSummary: result.PolicyCheckResults.Summary(),
PolicyApprovalSummary: result.PolicyCheckResults.PolicySummary(),
Expand Down
1 change: 0 additions & 1 deletion server/events/markdown_renderer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,6 @@ $$$
<details><summary>Show Output</summary>
#### Policy Set: $policy1$
$$$diff
line
Expand Down
2 changes: 2 additions & 0 deletions server/events/models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,8 @@ func (p PlanSuccess) Stats() PlanSuccessStats {

// PolicyCheckResults is the result of a successful policy check run.
type PolicyCheckResults struct {
PreConftestOutput string
PostConftestOutput string
// PolicySetResults is the output from policy check binary(conftest|opa)
PolicySetResults []PolicySetResult
// LockURL is the full URL to the lock held by this policy check.
Expand Down
22 changes: 19 additions & 3 deletions server/events/project_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,14 +485,30 @@ func (p *DefaultProjectCommandRunner) doPolicyCheck(ctx command.ProjectContext)
}
}

// Separate output from custom run steps
var index int
var preConftestOutput []string
var postConftestOutput []string
var policySetResults []models.PolicySetResult
err = json.Unmarshal([]byte(strings.Join(outputs, "\n")), &policySetResults)
if err != nil {
return nil, "", err
for i, output := range outputs {
index = i
err = json.Unmarshal([]byte(strings.Join([]string{output}, "\n")), &policySetResults)
if err == nil {
break
}
preConftestOutput = append(preConftestOutput, output)
}
if policySetResults == nil {
return nil, "", errors.New("unable to unmarshal conftest output")
}
if len(outputs) > 0 {
postConftestOutput = outputs[(index + 1):]
}

result := &models.PolicyCheckResults{
LockURL: p.LockURLGenerator.GenerateLockURL(lockAttempt.LockKey),
PreConftestOutput: strings.Join(preConftestOutput, "\n"),
PostConftestOutput: strings.Join(postConftestOutput, "\n"),
PolicySetResults: policySetResults,
RePlanCmd: ctx.RePlanCmd,
ApplyCmd: ctx.ApplyCmd,
Expand Down
10 changes: 10 additions & 0 deletions server/events/templates/policy_check_results_unwrapped.tmpl
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
{{ define "policyCheckResultsUnwrapped" -}}
{{- if eq .Command "Policy Check" }}
{{- if ne .PreConftestOutput "" }}
```diff
{{ .PreConftestOutput }}
```
{{- end -}}
{{ template "policyCheck" .PolicySetResults }}
{{- if ne .PostConftestOutput "" }}
```diff
{{ .PostConftestOutput }}
```
{{ end -}}
{{- end }}
{{- if .PolicyCleared }}
* :arrow_forward: To **apply** this plan, comment:
Expand Down
10 changes: 10 additions & 0 deletions server/events/templates/policy_check_results_wrapped.tmpl
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
{{ define "policyCheckResultsWrapped" -}}
<details><summary>Show Output</summary>
{{- if eq .Command "Policy Check" }}
{{- if ne .PreConftestOutput "" }}
```diff
{{ .PreConftestOutput }}
```
{{- end -}}
{{ template "policyCheck" .PolicySetResults }}
{{- if ne .PostConftestOutput "" }}
```diff
{{ .PostConftestOutput }}
```
{{ end -}}
{{- end }}
{{- if .PolicyCleared }}
* :arrow_forward: To **apply** this plan, comment:
Expand Down

0 comments on commit 7545296

Please sign in to comment.