Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: parse custom run step output in policy_check #3502

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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