diff --git a/runatlantis.io/docs/custom-workflows.md b/runatlantis.io/docs/custom-workflows.md index 8f7ebe01cf..4ddbd7523d 100644 --- a/runatlantis.io/docs/custom-workflows.md +++ b/runatlantis.io/docs/custom-workflows.md @@ -456,6 +456,8 @@ Or a custom command * `SHOWFILE` - Absolute path to the location where Atlantis expects the plan in json format to either be generated (by show) or already exist (if running policy checks). Can be used to override the built-in `plan`/`apply` commands, ex. `run: terraform show -json $PLANFILE > $SHOWFILE`. + * `POLICYCHECKFILE` - Absolute path to the location of policy check output if Atlantis runs policy checks. + See [policy checking](/docs/policy-checking.html#data-for-custom-run-steps) for information of data structure. * `BASE_REPO_NAME` - Name of the repository that the pull request will be merged into, ex. `atlantis`. * `BASE_REPO_OWNER` - Owner of the repository that the pull request will be merged into, ex. `runatlantis`. * `HEAD_REPO_NAME` - Name of the repository that is getting merged into the base repository, ex. `atlantis`. diff --git a/runatlantis.io/docs/images/policy-check-apply-failure.png b/runatlantis.io/docs/images/policy-check-apply-failure.png index 64b13d22ba..eb6095e314 100644 Binary files a/runatlantis.io/docs/images/policy-check-apply-failure.png and b/runatlantis.io/docs/images/policy-check-apply-failure.png differ diff --git a/runatlantis.io/docs/images/policy-check-apply-status-failure.png b/runatlantis.io/docs/images/policy-check-apply-status-failure.png index 22b786f6d2..e31c04f20e 100644 Binary files a/runatlantis.io/docs/images/policy-check-apply-status-failure.png and b/runatlantis.io/docs/images/policy-check-apply-status-failure.png differ diff --git a/runatlantis.io/docs/images/policy-check-approval.png b/runatlantis.io/docs/images/policy-check-approval.png index e71da2c8e9..eb9685fb19 100644 Binary files a/runatlantis.io/docs/images/policy-check-approval.png and b/runatlantis.io/docs/images/policy-check-approval.png differ diff --git a/runatlantis.io/docs/policy-checking.md b/runatlantis.io/docs/policy-checking.md index 9bea65c507..fad6693964 100644 --- a/runatlantis.io/docs/policy-checking.md +++ b/runatlantis.io/docs/policy-checking.md @@ -16,7 +16,7 @@ Enabling "policy checking" in addition to the [mergeable apply requirement](/doc ![Policy Check Apply Status Failure](./images/policy-check-apply-status-failure.png) -Any failures need to either be addressed in a successive commit, or approved by a blessed owner. This approval is independent of the approval apply requirement which can coexist in the policy checking workflow. After an approval, the apply can proceed. +Any failures need to either be addressed in a successive commit, or approved by top-level owner(s) of policies or the owner(s) of the policy set in question. Policy approvals are independent of the approval apply requirement which can coexist in the policy checking workflow. After policies are approved, the apply can proceed. ![Policy Check Approval](./images/policy-check-approval.png) @@ -44,14 +44,23 @@ policies: users: - nishkrishnan policy_sets: - - name: null_resource_warning - path: /policies/null_resource_warning/ + - name: deny_null_resource + path: /policies/deny_null_resource/ source: local + - name: deny_local_exec + path: /policies/deny_local_exec/ + source: local + approve_count: 2 + owners: + users: + - pseudomorph ``` - `name` - A name of your policy set. - `path` - Path to a policies directory. *Note: replace `` with absolute dir path to conftest policy/policies.* - `source` - Tells atlantis where to fetch the policies from. Currently you can only host policies locally by using `local`. +- `owners` - Defines the users/teams which are able to approve a specific policy set. +- `approve_count` - Defines the number of approvals needed to bypass policy checks. Defaults to the top-level policies configuration, if not specified. By default conftest is configured to only run the `main` package. If you wish to run specific/multiple policies consider passing `--namespace` or `--all-namespaces` to conftest with [`extra_args`](https://www.runatlantis.io/docs/custom-workflows.html#adding-extra-arguments-to-terraform-commands) via a custom workflow as shown in the below example. @@ -158,3 +167,21 @@ workflows: ### Quiet policy checks By default, Atlantis will add a comment to all pull requests with the policy check result - both successes and failures. Version 0.21.0 added the [`--quiet-policy-checks`](server-configuration.html#quiet-policy-checks) option, which will instead only add comments when policy checks fail, significantly reducing the number of comments when most policy check results succeed. + + +### Data for custom run steps + +When the policy check workflow runs, a file is created in the working directory which contains information about the status of each policy set tested. This data may be useful in custom run steps to generate metrics or notifications. The file contains JSON data in the following format: + +```json +[ + { + "PolicySetName": "policy1", + "ConftestOutput": "", + "Passed": false, + "ReqApprovals": 1, + "CurApprovals": 0 + } +] + +``` \ No newline at end of file diff --git a/runatlantis.io/docs/server-side-repo-config.md b/runatlantis.io/docs/server-side-repo-config.md index 417374fe02..c44cddff99 100644 --- a/runatlantis.io/docs/server-side-repo-config.md +++ b/runatlantis.io/docs/server-side-repo-config.md @@ -519,11 +519,12 @@ If you set a workflow with the key `default`, it will override this. ### Policies -| Key | Type | Default | Required | Description | -|------------------------|-----------------|---------|-----------|------------------------------------------| -| conftest_version | string | none | no | conftest version to run all policy sets | -| owners | Owners(#Owners) | none | yes | owners that can approve failing policies | -| policy_sets | []PolicySet | none | yes | set of policies to run on a plan output | +| Key | Type | Default | Required | Description | +|------------------------|-----------------|---------|-----------|----------------------------------------------------------| +| conftest_version | string | none | no | conftest version to run all policy sets | +| owners | Owners(#Owners) | none | yes | owners that can approve failing policies | +| approve_count | int | 1 | no | number of approvals required to bypass failing policies. | +| policy_sets | []PolicySet | none | yes | set of policies to run on a plan output | ### Owners | Key | Type | Default | Required | Description | diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 8b34031e36..fce6d07b8d 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -1202,6 +1202,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers Ok(t, err) projectCommandRunner := &events.DefaultProjectCommandRunner{ + VcsClient: e2eVCSClient, Locker: projectLocker, LockURLGenerator: &mockLockURLGenerator{}, InitStepRunner: &runtime.InitStepRunner{ diff --git a/server/controllers/events/testdata/test-repos/policy-checks-apply-reqs/exp-output-auto-policy-check.txt b/server/controllers/events/testdata/test-repos/policy-checks-apply-reqs/exp-output-auto-policy-check.txt index 21c1eb312a..8faa2e036b 100644 --- a/server/controllers/events/testdata/test-repos/policy-checks-apply-reqs/exp-output-auto-policy-check.txt +++ b/server/controllers/events/testdata/test-repos/policy-checks-apply-reqs/exp-output-auto-policy-check.txt @@ -1,15 +1,29 @@ Ran Policy Check for dir: `.` workspace: `default` -**Policy Check Error** -``` -exit status 1 -Checking plan against the following policies: - test_policy +**Policy Check Failed**: Some policy sets did not pass. +#### Policy Set: `test_policy` +```diff FAIL - - main - WARNING: Null Resource creation is prohibited. 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions ``` -* :heavy_check_mark: To **approve** failing policies an authorized approver can comment: + + +#### 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` -* :repeat: Or, address the policy failure by modifying the codebase and re-planning. \ No newline at end of file +* :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` \ No newline at end of file diff --git a/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-approve-policies.txt b/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-approve-policies.txt index 1b72496de1..2a8329e4a5 100644 --- a/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-approve-policies.txt +++ b/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-approve-policies.txt @@ -1,4 +1,29 @@ +Ran Approve Policies for 1 projects: + +1. dir: `.` workspace: `default` + +### 1. dir: `.` workspace: `default` **Approve Policies Error** ``` -contact policy owners to approve failing policies +1 error occurred: + * policy set: test_policy user runatlantis is not a policy owner - please contact policy owners to approve failing policies + + ``` +#### 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` \ No newline at end of file diff --git a/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-auto-policy-check.txt b/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-auto-policy-check.txt index 21c1eb312a..8faa2e036b 100644 --- a/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-auto-policy-check.txt +++ b/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-auto-policy-check.txt @@ -1,15 +1,29 @@ Ran Policy Check for dir: `.` workspace: `default` -**Policy Check Error** -``` -exit status 1 -Checking plan against the following policies: - test_policy +**Policy Check Failed**: Some policy sets did not pass. +#### Policy Set: `test_policy` +```diff FAIL - - main - WARNING: Null Resource creation is prohibited. 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions ``` -* :heavy_check_mark: To **approve** failing policies an authorized approver can comment: + + +#### 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` -* :repeat: Or, address the policy failure by modifying the codebase and re-planning. \ No newline at end of file +* :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` \ No newline at end of file diff --git a/server/controllers/events/testdata/test-repos/policy-checks-extra-args/exp-output-auto-policy-check.txt b/server/controllers/events/testdata/test-repos/policy-checks-extra-args/exp-output-auto-policy-check.txt index 6e438307aa..59f0c90346 100644 --- a/server/controllers/events/testdata/test-repos/policy-checks-extra-args/exp-output-auto-policy-check.txt +++ b/server/controllers/events/testdata/test-repos/policy-checks-extra-args/exp-output-auto-policy-check.txt @@ -1,15 +1,29 @@ Ran Policy Check for dir: `.` workspace: `default` -**Policy Check Error** -``` -exit status 1 -Checking plan against the following policies: - test_policy +**Policy Check Failed**: Some policy sets did not pass. +#### Policy Set: `test_policy` +```diff FAIL - - null_resource_policy - WARNING: Null Resource creation is prohibited. 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions ``` -* :heavy_check_mark: To **approve** failing policies an authorized approver can comment: + + +#### 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` -* :repeat: Or, address the policy failure by modifying the codebase and re-planning. \ No newline at end of file +* :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` \ No newline at end of file diff --git a/server/controllers/events/testdata/test-repos/policy-checks-multi-projects/exp-output-approve-policies.txt b/server/controllers/events/testdata/test-repos/policy-checks-multi-projects/exp-output-approve-policies.txt deleted file mode 100644 index f5e100c23e..0000000000 --- a/server/controllers/events/testdata/test-repos/policy-checks-multi-projects/exp-output-approve-policies.txt +++ /dev/null @@ -1,5 +0,0 @@ -Approved Policies for 1 projects: - -1. dir: `.` workspace: `default` - - diff --git a/server/controllers/events/testdata/test-repos/policy-checks-multi-projects/exp-output-auto-policy-check.txt b/server/controllers/events/testdata/test-repos/policy-checks-multi-projects/exp-output-auto-policy-check.txt index 829081224b..b8a346083e 100644 --- a/server/controllers/events/testdata/test-repos/policy-checks-multi-projects/exp-output-auto-policy-check.txt +++ b/server/controllers/events/testdata/test-repos/policy-checks-multi-projects/exp-output-auto-policy-check.txt @@ -4,13 +4,14 @@ Ran Policy Check for 2 projects: 1. dir: `dir2` workspace: `default` ### 1. dir: `dir1` workspace: `default` +#### Policy Set: `test_policy` ```diff -Checking plan against the following policies: - test_policy 1 test, 1 passed, 0 warnings, 0 failures, 0 exceptions + ``` + * :arrow_forward: To **apply** this plan, comment: * `atlantis apply -d dir1` * :put_litter_in_its_place: To **delete** this plan click [here](lock-url) @@ -19,22 +20,30 @@ Checking plan against the following policies: --- ### 2. dir: `dir2` workspace: `default` -**Policy Check Error** -``` -exit status 1 -Checking plan against the following policies: - test_policy +**Policy Check Failed**: Some policy sets did not pass. +#### Policy Set: `test_policy` +```diff FAIL - - main - WARNING: Forbidden Resource creation is prohibited. 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions ``` -* :heavy_check_mark: To **approve** failing policies an authorized approver can comment: - * `atlantis approve_policies` -* :repeat: Or, address the policy failure by modifying the codebase and re-planning. + + +#### 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 dir2` +* :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 dir2` --- -* :fast_forward: To **apply** all unapplied plans from this pull request, comment: - * `atlantis apply` +* :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` \ No newline at end of file + * `atlantis unlock` +* :repeat: To re-run policies **plan** this project again by commenting: + * `atlantis plan` \ No newline at end of file diff --git a/server/controllers/events/testdata/test-repos/policy-checks/exp-output-auto-policy-check.txt b/server/controllers/events/testdata/test-repos/policy-checks/exp-output-auto-policy-check.txt index 21c1eb312a..8faa2e036b 100644 --- a/server/controllers/events/testdata/test-repos/policy-checks/exp-output-auto-policy-check.txt +++ b/server/controllers/events/testdata/test-repos/policy-checks/exp-output-auto-policy-check.txt @@ -1,15 +1,29 @@ Ran Policy Check for dir: `.` workspace: `default` -**Policy Check Error** -``` -exit status 1 -Checking plan against the following policies: - test_policy +**Policy Check Failed**: Some policy sets did not pass. +#### Policy Set: `test_policy` +```diff FAIL - - main - WARNING: Null Resource creation is prohibited. 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions ``` -* :heavy_check_mark: To **approve** failing policies an authorized approver can comment: + + +#### 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` -* :repeat: Or, address the policy failure by modifying the codebase and re-planning. \ No newline at end of file +* :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` \ No newline at end of file diff --git a/server/core/config/parser_validator_test.go b/server/core/config/parser_validator_test.go index 3f0273d1ed..d041076f11 100644 --- a/server/core/config/parser_validator_test.go +++ b/server/core/config/parser_validator_test.go @@ -1466,12 +1466,14 @@ policies: "custom1": customWorkflow1, }, PolicySets: valid.PolicySets{ - Version: conftestVersion, + Version: conftestVersion, + ApproveCount: 1, PolicySets: []valid.PolicySet{ { - Name: "good-policy", - Path: "rel/path/to/policy", - Source: valid.LocalPolicySet, + Name: "good-policy", + Path: "rel/path/to/policy", + Source: valid.LocalPolicySet, + ApproveCount: 1, }, }, }, @@ -1801,12 +1803,14 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) { "custom": customWorkflow, }, PolicySets: valid.PolicySets{ - Version: conftestVersion, + Version: conftestVersion, + ApproveCount: 1, PolicySets: []valid.PolicySet{ { - Name: "good-policy", - Path: "rel/path/to/policy", - Source: valid.LocalPolicySet, + Name: "good-policy", + Path: "rel/path/to/policy", + Source: valid.LocalPolicySet, + ApproveCount: 1, }, }, }, diff --git a/server/core/config/raw/policies.go b/server/core/config/raw/policies.go index 270e314a41..aee7019ba9 100644 --- a/server/core/config/raw/policies.go +++ b/server/core/config/raw/policies.go @@ -2,15 +2,16 @@ package raw import ( validation "github.com/go-ozzo/ozzo-validation" - "github.com/hashicorp/go-version" + version "github.com/hashicorp/go-version" "github.com/runatlantis/atlantis/server/core/config/valid" ) // PolicySets is the raw schema for repo-level atlantis.yaml config. type PolicySets struct { - Version *string `yaml:"conftest_version,omitempty" json:"conftest_version,omitempty"` - Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"` - PolicySets []PolicySet `yaml:"policy_sets" json:"policy_sets"` + Version *string `yaml:"conftest_version,omitempty" json:"conftest_version,omitempty"` + Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"` + PolicySets []PolicySet `yaml:"policy_sets" json:"policy_sets"` + ApproveCount int `yaml:"approve_count,omitempty" json:"approve_count,omitempty"` } func (p PolicySets) Validate() error { @@ -27,10 +28,22 @@ func (p PolicySets) ToValid() valid.PolicySets { policySets.Version, _ = version.NewVersion(*p.Version) } + // Default number of required reviews for all policy sets should be 1. + // Negative numbers are automatically set to 1. + policySets.ApproveCount = p.ApproveCount + if policySets.ApproveCount <= 0 { + policySets.ApproveCount = 1 + } + policySets.Owners = p.Owners.ToValid() validPolicySets := make([]valid.PolicySet, 0) for _, rawPolicySet := range p.PolicySets { + // Default to top-level review count if not specified. + // Negative numbers are automatically set to the default. + if rawPolicySet.ApproveCount <= 0 { + rawPolicySet.ApproveCount = policySets.ApproveCount + } validPolicySets = append(validPolicySets, rawPolicySet.ToValid()) } policySets.PolicySets = validPolicySets @@ -57,16 +70,18 @@ func (o PolicyOwners) ToValid() valid.PolicyOwners { } type PolicySet struct { - Path string `yaml:"path" json:"path"` - Source string `yaml:"source" json:"source"` - Name string `yaml:"name" json:"name"` - Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"` + Path string `yaml:"path" json:"path"` + Source string `yaml:"source" json:"source"` + Name string `yaml:"name" json:"name"` + Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"` + ApproveCount int `yaml:"approve_count,omitempty" json:"approve_count,omitempty"` } func (p PolicySet) Validate() error { return validation.ValidateStruct(&p, validation.Field(&p.Name, validation.Required.Error("is required")), validation.Field(&p.Owners), + validation.Field(&p.ApproveCount), validation.Field(&p.Path, validation.Required.Error("is required")), validation.Field(&p.Source, validation.In(valid.LocalPolicySet, valid.GithubPolicySet).Error("only 'local' and 'github' source types are supported")), ) @@ -78,6 +93,7 @@ func (p PolicySet) ToValid() valid.PolicySet { policySet.Name = p.Name policySet.Path = p.Path policySet.Source = p.Source + policySet.ApproveCount = p.ApproveCount policySet.Owners = p.Owners.ToValid() return policySet diff --git a/server/core/config/raw/policies_test.go b/server/core/config/raw/policies_test.go index 7987d7bf57..0626119c6f 100644 --- a/server/core/config/raw/policies_test.go +++ b/server/core/config/raw/policies_test.go @@ -199,14 +199,16 @@ func TestPolicySets_ToValid(t *testing.T) { }, }, exp: valid.PolicySets{ - Version: version, + Version: version, + ApproveCount: 1, Owners: valid.PolicyOwners{ Users: []string{"test"}, Teams: []string{"testteam"}, }, PolicySets: []valid.PolicySet{ { - Name: "good-policy", + Name: "good-policy", + ApproveCount: 1, Owners: valid.PolicyOwners{ Users: []string{ "john-doe", @@ -238,7 +240,8 @@ func TestPolicySets_ToValid(t *testing.T) { }, }, exp: valid.PolicySets{ - Version: version, + Version: version, + ApproveCount: 1, PolicySets: []valid.PolicySet{ { Name: "good-policy", @@ -248,8 +251,9 @@ func TestPolicySets_ToValid(t *testing.T) { "jane-doe", }, }, - Path: "rel/path/to/source", - Source: "local", + Path: "rel/path/to/source", + Source: "local", + ApproveCount: 1, }, }, }, diff --git a/server/core/config/valid/global_cfg_test.go b/server/core/config/valid/global_cfg_test.go index 0ae99d4601..818c744c62 100644 --- a/server/core/config/valid/global_cfg_test.go +++ b/server/core/config/valid/global_cfg_test.go @@ -668,12 +668,14 @@ policies: StateRm: valid.DefaultStateRmStage, }, PolicySets: valid.PolicySets{ - Version: nil, + Version: nil, + ApproveCount: 1, PolicySets: []valid.PolicySet{ { - Name: "good-policy", - Path: "rel/path/to/source", - Source: "local", + Name: "good-policy", + Path: "rel/path/to/source", + Source: "local", + ApproveCount: 1, }, }, }, @@ -714,12 +716,14 @@ policies: StateRm: valid.DefaultStateRmStage, }, PolicySets: valid.PolicySets{ - Version: version, + Version: version, + ApproveCount: 1, PolicySets: []valid.PolicySet{ { - Name: "good-policy", - Path: "rel/path/to/source", - Source: "local", + Name: "good-policy", + Path: "rel/path/to/source", + Source: "local", + ApproveCount: 1, }, }, }, diff --git a/server/core/config/valid/policies.go b/server/core/config/valid/policies.go index 48d166cea2..8fb6cfdc91 100644 --- a/server/core/config/valid/policies.go +++ b/server/core/config/valid/policies.go @@ -3,7 +3,7 @@ package valid import ( "strings" - "github.com/hashicorp/go-version" + version "github.com/hashicorp/go-version" ) const ( @@ -15,9 +15,10 @@ const ( // PolicySet objects. PolicySets struct is used by PolicyCheck workflow to build // context to enforce policies. type PolicySets struct { - Version *version.Version - Owners PolicyOwners - PolicySets []PolicySet + Version *version.Version + Owners PolicyOwners + ApproveCount int + PolicySets []PolicySet } type PolicyOwners struct { @@ -26,28 +27,36 @@ type PolicyOwners struct { } type PolicySet struct { - Source string - Path string - Name string - Owners PolicyOwners + Source string + Path string + Name string + ApproveCount int + Owners PolicyOwners } func (p *PolicySets) HasPolicies() bool { return len(p.PolicySets) > 0 } +// Check if any level of policy owners includes teams func (p *PolicySets) HasTeamOwners() bool { - return len(p.Owners.Teams) > 0 + hasTeamOwners := len(p.Owners.Teams) > 0 + for _, policySet := range p.PolicySets { + if len(policySet.Owners.Teams) > 0 { + hasTeamOwners = true + } + } + return hasTeamOwners } -func (p *PolicySets) IsOwner(username string, userTeams []string) bool { - for _, uname := range p.Owners.Users { +func (o *PolicyOwners) IsOwner(username string, userTeams []string) bool { + for _, uname := range o.Users { if strings.EqualFold(uname, username) { return true } } - for _, orgTeamName := range p.Owners.Teams { + for _, orgTeamName := range o.Teams { for _, userTeamName := range userTeams { if strings.EqualFold(orgTeamName, userTeamName) { return true diff --git a/server/core/config/valid/policies_test.go b/server/core/config/valid/policies_test.go new file mode 100644 index 0000000000..c575a4585a --- /dev/null +++ b/server/core/config/valid/policies_test.go @@ -0,0 +1,122 @@ +package valid_test + +import ( + "testing" + + "github.com/runatlantis/atlantis/server/core/config/valid" + . "github.com/runatlantis/atlantis/testing" +) + +func TestPoliciesConfig_HasTeamOwners(t *testing.T) { + cases := []struct { + description string + input valid.PolicySets + expResult bool + }{ + { + description: "no team owners", + input: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + }, + }, + }, + expResult: false, + }, + { + description: "has top-level team owner", + input: valid.PolicySets{ + Owners: valid.PolicyOwners{ + Teams: []string{ + "someteam", + }, + }, + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + }, + }, + }, + expResult: true, + }, + { + description: "has policy-level team owner", + input: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + Owners: valid.PolicyOwners{ + Teams: []string{ + "someteam", + }, + }, + }, + }, + }, + expResult: true, + }, + } + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + result := c.input.HasTeamOwners() + Equals(t, c.expResult, result) + }) + } +} + +func TestPoliciesConfig_IsOwners(t *testing.T) { + user := "testuser" + userTeams := []string{"testuserteam"} + + cases := []struct { + description string + input valid.PolicyOwners + expResult bool + }{ + { + description: "user is not owner", + input: valid.PolicyOwners{ + Users: []string{ + "someotheruser", + }, + Teams: []string{ + "someotherteam", + }, + }, + expResult: false, + }, + { + description: "user is owner", + input: valid.PolicyOwners{ + Users: []string{ + "testuser", + "someotheruser", + }, + Teams: []string{ + "someotherteam", + }, + }, + expResult: true, + }, + { + description: "user is owner via team membership", + input: valid.PolicyOwners{ + Users: []string{ + "someotheruser", + }, + Teams: []string{ + "someotherteam", + "testuserteam", + }, + }, + expResult: true, + }, + } + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + result := c.input.IsOwner(user, userTeams) + Equals(t, c.expResult, result) + }) + } +} diff --git a/server/core/db/boltdb.go b/server/core/db/boltdb.go index 14641bf0af..79e0798c29 100644 --- a/server/core/db/boltdb.go +++ b/server/core/db/boltdb.go @@ -357,6 +357,20 @@ func (b *BoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []com res.ProjectName == proj.ProjectName { proj.Status = res.PlanStatus() + + // Updating only policy sets which are included in results; keeping the rest. + if len(proj.PolicyStatus) > 0 { + for i, oldPolicySet := range proj.PolicyStatus { + for _, newPolicySet := range res.PolicyStatus() { + if oldPolicySet.PolicySetName == newPolicySet.PolicySetName { + proj.PolicyStatus[i] = newPolicySet + } + } + } + } else { + proj.PolicyStatus = res.PolicyStatus() + } + updatedExisting = true break } @@ -483,9 +497,10 @@ func (b *BoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models. func (b *BoltDB) projectResultToProject(p command.ProjectResult) models.ProjectStatus { return models.ProjectStatus{ - Workspace: p.Workspace, - RepoRelDir: p.RepoRelDir, - ProjectName: p.ProjectName, - Status: p.PlanStatus(), + Workspace: p.Workspace, + RepoRelDir: p.RepoRelDir, + ProjectName: p.ProjectName, + PolicyStatus: p.PolicyStatus(), + Status: p.PlanStatus(), } } diff --git a/server/core/db/boltdb_test.go b/server/core/db/boltdb_test.go index 67f365d144..e16a10d086 100644 --- a/server/core/db/boltdb_test.go +++ b/server/core/db/boltdb_test.go @@ -649,9 +649,9 @@ func TestPullStatus_UpdateNewCommit(t *testing.T) { }, maybeStatus.Projects) } -// Test that if we update an existing pull status and our new status is for a +// Test that if we update an existing pull status via Apply and our new status is for a // the same commit, that we merge the statuses. -func TestPullStatus_UpdateMerge(t *testing.T) { +func TestPullStatus_UpdateMerge_Apply(t *testing.T) { b := newTestDB2(t) pull := models.PullRequest{ @@ -761,6 +761,120 @@ func TestPullStatus_UpdateMerge(t *testing.T) { } } +// Test that if we update one existing policy status via approve_policies and our new status is for a +// the same commit, that we merge the statuses. +func TestPullStatus_UpdateMerge_ApprovePolicies(t *testing.T) { + b := newTestDB2(t) + + pull := models.PullRequest{ + Num: 1, + HeadCommit: "sha", + URL: "url", + HeadBranch: "head", + BaseBranch: "base", + Author: "lkysow", + State: models.OpenPullState, + BaseRepo: models.Repo{ + FullName: "runatlantis/atlantis", + Owner: "runatlantis", + Name: "atlantis", + CloneURL: "clone-url", + SanitizedCloneURL: "clone-url", + VCSHost: models.VCSHost{ + Hostname: "github.com", + Type: models.Github, + }, + }, + } + _, err := b.UpdatePullWithResults( + pull, + []command.ProjectResult{ + { + Command: command.PolicyCheck, + RepoRelDir: "mergeme", + Workspace: "default", + Failure: "policy failure", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + }, + }, + }, + }, + { + Command: command.PolicyCheck, + RepoRelDir: "projectname", + Workspace: "default", + ProjectName: "projectname", + Failure: "policy failure", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + }, + }, + }, + }, + }) + Ok(t, err) + + updateStatus, err := b.UpdatePullWithResults(pull, + []command.ProjectResult{ + { + Command: command.ApprovePolicies, + RepoRelDir: "mergeme", + Workspace: "default", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + CurApprovals: 1, + }, + }, + }, + }, + }) + Ok(t, err) + + getStatus, err := b.GetPullStatus(pull) + Ok(t, err) + + // Test both the pull state returned from the update call *and* the getCommandLock + // call. + for _, s := range []models.PullStatus{updateStatus, *getStatus} { + Equals(t, pull, s.Pull) + Equals(t, []models.ProjectStatus{ + { + RepoRelDir: "mergeme", + Workspace: "default", + Status: models.PassedPolicyCheckStatus, + PolicyStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Approvals: 1, + }, + }, + }, + { + RepoRelDir: "projectname", + Workspace: "default", + ProjectName: "projectname", + Status: models.ErroredPolicyCheckStatus, + PolicyStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Approvals: 0, + }, + }, + }, + }, updateStatus.Projects) + } +} + // newTestDB returns a TestDB using a temporary path. func newTestDB() (*bolt.DB, *db.BoltDB) { // Retrieve a temporary path. diff --git a/server/core/redis/redis.go b/server/core/redis/redis.go index f9539a1d6d..6f7ee36a2a 100644 --- a/server/core/redis/redis.go +++ b/server/core/redis/redis.go @@ -330,6 +330,20 @@ func (r *RedisDB) UpdatePullWithResults(pull models.PullRequest, newResults []co res.ProjectName == proj.ProjectName { proj.Status = res.PlanStatus() + + // Updating only policy sets which are included in results; keeping the rest. + if len(proj.PolicyStatus) > 0 { + for i, oldPolicySet := range proj.PolicyStatus { + for _, newPolicySet := range res.PolicyStatus() { + if oldPolicySet.PolicySetName == newPolicySet.PolicySetName { + proj.PolicyStatus[i] = newPolicySet + } + } + } + } else { + proj.PolicyStatus = res.PolicyStatus() + } + updatedExisting = true break } @@ -399,9 +413,10 @@ func (r *RedisDB) pullKey(pull models.PullRequest) (string, error) { func (r *RedisDB) projectResultToProject(p command.ProjectResult) models.ProjectStatus { return models.ProjectStatus{ - Workspace: p.Workspace, - RepoRelDir: p.RepoRelDir, - ProjectName: p.ProjectName, - Status: p.PlanStatus(), + Workspace: p.Workspace, + RepoRelDir: p.RepoRelDir, + ProjectName: p.ProjectName, + PolicyStatus: p.PolicyStatus(), + Status: p.PlanStatus(), } } diff --git a/server/core/redis/redis_test.go b/server/core/redis/redis_test.go index 223a2c0d36..63bd1a0873 100644 --- a/server/core/redis/redis_test.go +++ b/server/core/redis/redis_test.go @@ -689,9 +689,9 @@ func TestPullStatus_UpdateNewCommit(t *testing.T) { }, maybeStatus.Projects) } -// Test that if we update an existing pull status and our new status is for a +// Test that if we update an existing pull status via Apply and our new status is for a // the same commit, that we merge the statuses. -func TestPullStatus_UpdateMerge(t *testing.T) { +func TestPullStatus_UpdateMerge_Apply(t *testing.T) { s := miniredis.RunT(t) rdb := newTestRedis(s) @@ -802,6 +802,121 @@ func TestPullStatus_UpdateMerge(t *testing.T) { } } +// Test that if we update one existing policy status via approve_policies and our new status is for a +// the same commit, that we merge the statuses. +func TestPullStatus_UpdateMerge_ApprovePolicies(t *testing.T) { + s := miniredis.RunT(t) + rdb := newTestRedis(s) + + pull := models.PullRequest{ + Num: 1, + HeadCommit: "sha", + URL: "url", + HeadBranch: "head", + BaseBranch: "base", + Author: "lkysow", + State: models.OpenPullState, + BaseRepo: models.Repo{ + FullName: "runatlantis/atlantis", + Owner: "runatlantis", + Name: "atlantis", + CloneURL: "clone-url", + SanitizedCloneURL: "clone-url", + VCSHost: models.VCSHost{ + Hostname: "github.com", + Type: models.Github, + }, + }, + } + _, err := rdb.UpdatePullWithResults( + pull, + []command.ProjectResult{ + { + Command: command.PolicyCheck, + RepoRelDir: "mergeme", + Workspace: "default", + Failure: "policy failure", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + }, + }, + }, + }, + { + Command: command.PolicyCheck, + RepoRelDir: "projectname", + Workspace: "default", + ProjectName: "projectname", + Failure: "policy failure", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + }, + }, + }, + }, + }) + Ok(t, err) + + updateStatus, err := rdb.UpdatePullWithResults(pull, + []command.ProjectResult{ + { + Command: command.ApprovePolicies, + RepoRelDir: "mergeme", + Workspace: "default", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + CurApprovals: 1, + }, + }, + }, + }, + }) + Ok(t, err) + + getStatus, err := rdb.GetPullStatus(pull) + Ok(t, err) + + // Test both the pull state returned from the update call *and* the getCommandLock + // call. + for _, s := range []models.PullStatus{updateStatus, *getStatus} { + Equals(t, pull, s.Pull) + Equals(t, []models.ProjectStatus{ + { + RepoRelDir: "mergeme", + Workspace: "default", + Status: models.PassedPolicyCheckStatus, + PolicyStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Approvals: 1, + }, + }, + }, + { + RepoRelDir: "projectname", + Workspace: "default", + ProjectName: "projectname", + Status: models.ErroredPolicyCheckStatus, + PolicyStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Approvals: 0, + }, + }, + }, + }, updateStatus.Projects) + } +} + func newTestRedis(mr *miniredis.Miniredis) *redis.RedisDB { r, err := redis.New(mr.Host(), mr.Server().Addr().Port, "", false, false, 0) if err != nil { diff --git a/server/core/runtime/policy/conftest_client.go b/server/core/runtime/policy/conftest_client.go index 2c21226c2c..ef7ee656a5 100644 --- a/server/core/runtime/policy/conftest_client.go +++ b/server/core/runtime/policy/conftest_client.go @@ -7,6 +7,10 @@ import ( "runtime" "strings" + "encoding/json" + "regexp" + + "github.com/hashicorp/go-multierror" version "github.com/hashicorp/go-version" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/core/config/valid" @@ -14,6 +18,7 @@ import ( runtime_models "github.com/runatlantis/atlantis/server/core/runtime/models" "github.com/runatlantis/atlantis/server/core/terraform" "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/logging" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -163,46 +168,80 @@ func NewConfTestExecutorWorkflow(log logging.SimpleLogging, versionRootDir strin } func (c *ConfTestExecutorWorkflow) Run(ctx command.ProjectContext, executablePath string, envs map[string]string, workdir string, extraArgs []string) (string, error) { - policyArgs := []Arg{} - policySetNames := []string{} ctx.Log.Debug("policy sets, %s ", ctx.PolicySets) + + inputFile := filepath.Join(workdir, ctx.GetShowResultFileName()) + var policySetResults []models.PolicySetResult + var combinedErr error + for _, policySet := range ctx.PolicySets.PolicySets { - path, err := c.SourceResolver.Resolve(policySet) + path, resolveErr := c.SourceResolver.Resolve(policySet) // Let's not fail the whole step because of a single failure. Log and fail silently - if err != nil { - ctx.Log.Err("Error resolving policyset %s. err: %s", policySet.Name, err.Error()) + if resolveErr != nil { + ctx.Log.Err("Error resolving policyset %s. err: %s", policySet.Name, resolveErr.Error()) continue } - policyArg := NewPolicyArg(path) - policyArgs = append(policyArgs, policyArg) + args := ConftestTestCommandArgs{ + PolicyArgs: []Arg{NewPolicyArg(path)}, + ExtraArgs: extraArgs, + InputFile: inputFile, + Command: executablePath, + } - policySetNames = append(policySetNames, policySet.Name) - } + serializedArgs, _ := args.build() + cmdOutput, cmdErr := c.Exec.CombinedOutput(serializedArgs, envs, workdir) - inputFile := filepath.Join(workdir, ctx.GetShowResultFileName()) + if cmdErr != nil { + // Since we're running conftest for each policyset, individual command errors should be concatenated. + if isValidConftestOutput(cmdOutput) { + combinedErr = multierror.Append(combinedErr, fmt.Errorf("policy_set: %s: conftest: some policies failed", policySet.Name)) + } else { + combinedErr = multierror.Append(combinedErr, fmt.Errorf("policy_set: %s: conftest: %s", policySet.Name, cmdOutput)) + } + } - args := ConftestTestCommandArgs{ - PolicyArgs: policyArgs, - ExtraArgs: extraArgs, - InputFile: inputFile, - Command: executablePath, - } + passed := true + if hasFailures(cmdOutput) { + passed = false + } - serializedArgs, err := args.build() + policySetResults = append(policySetResults, models.PolicySetResult{ + PolicySetName: policySet.Name, + ConftestOutput: cmdOutput, + Passed: passed, + ReqApprovals: policySet.ApproveCount, + }) + } - if err != nil { - ctx.Log.Warn("No policies have been configured") + if policySetResults == nil { + ctx.Log.Warn("no policies have been configured.") return "", nil // TODO: enable when we can pass policies in otherwise e2e tests with policy checks fail // return "", errors.Wrap(err, "building args") } - initialOutput := fmt.Sprintf("Checking plan against the following policies: \n %s\n", strings.Join(policySetNames, "\n ")) - cmdOutput, err := c.Exec.CombinedOutput(serializedArgs, envs, workdir) + marshaledStatus, err := json.Marshal(policySetResults) + if err != nil { + return "", errors.New("cannot marshal data into []PolicySetResult. data") + } + + // Write policy check results to a file which can be used by custom workflow run steps for metrics, notifications, etc. + policyCheckResultFile := filepath.Join(workdir, ctx.GetPolicyCheckResultFileName()) + err = os.WriteFile(policyCheckResultFile, marshaledStatus, 0600) - return c.sanitizeOutput(inputFile, initialOutput+cmdOutput), err + combinedErr = multierror.Append(combinedErr, err) + + // Multierror will wrap combined errors in a way that the upstream functions won't be able to read it as nil. + // Let's pass nil back if there are no wrapped errors. + if errors.Unwrap(combinedErr) == nil { + combinedErr = nil + } + + output := string(marshaledStatus) + + return c.sanitizeOutput(inputFile, output), combinedErr } @@ -255,3 +294,22 @@ func getDefaultVersion() (*version.Version, error) { } return wrappedVersion, nil } + +// Checks if output from conftest is a valid output. +func isValidConftestOutput(output string) bool { + + r := regexp.MustCompile(`^(WARN|FAIL|\[)`) + if match := r.FindString(output); match != "" { + return true + } + return false +} + +// hasFailures checks whether any conftest policies have failed +func hasFailures(output string) bool { + r := regexp.MustCompile(`([1-9]([0-9]?)* failure|failures": \[)`) + if match := r.FindString(output); match != "" { + return true + } + return false +} diff --git a/server/core/runtime/policy/conftest_client_test.go b/server/core/runtime/policy/conftest_client_test.go index 4483786823..22ffb5c1f9 100644 --- a/server/core/runtime/policy/conftest_client_test.go +++ b/server/core/runtime/policy/conftest_client_test.go @@ -161,7 +161,7 @@ func TestRun(t *testing.T) { envs := map[string]string{ "key": "val", } - workdir := "/some_workdir" + workdir := t.TempDir() policySet1 := valid.PolicySet{ Source: valid.LocalPolicySet, @@ -191,19 +191,22 @@ func TestRun(t *testing.T) { var extraArgs []string expectedOutput := "Success" - expectedResult := "Checking plan against the following policies: \n policy1\n policy2\nSuccess" - expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "-p", localPolicySetPath2, "/some_workdir/testproj-default.json", "--no-color"} + expectedResult := `[{"PolicySetName":"policy1","ConftestOutput":"Success","Passed":true,"ReqApprovals":0,"CurApprovals":0},{"PolicySetName":"policy2","ConftestOutput":"Success","Passed":true,"ReqApprovals":0,"CurApprovals":0}]` + + expectedArgsPolicy1 := []string{executablePath, "test", "-p", localPolicySetPath1, filepath.Join(workdir, "testproj-default.json"), "--no-color"} + expectedArgsPolicy2 := []string{executablePath, "test", "-p", localPolicySetPath2, filepath.Join(workdir, "testproj-default.json"), "--no-color"} When(mockResolver.Resolve(policySet1)).ThenReturn(localPolicySetPath1, nil) When(mockResolver.Resolve(policySet2)).ThenReturn(localPolicySetPath2, nil) - When(mockExec.CombinedOutput(expectedArgs, envs, workdir)).ThenReturn(expectedOutput, nil) + When(mockExec.CombinedOutput(expectedArgsPolicy1, envs, workdir)).ThenReturn(expectedOutput, nil) + When(mockExec.CombinedOutput(expectedArgsPolicy2, envs, workdir)).ThenReturn(expectedOutput, nil) result, err := subject.Run(ctx, executablePath, envs, workdir, extraArgs) fmt.Println(result) - Ok(t, err) + Ok(t, errors.Unwrap(err)) Assert(t, result == expectedResult, "result is expected") @@ -213,19 +216,22 @@ func TestRun(t *testing.T) { extraArgs := []string{"--all-namespaces"} expectedOutput := "Success" - expectedResult := "Checking plan against the following policies: \n policy1\n policy2\nSuccess" - expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "-p", localPolicySetPath2, "/some_workdir/testproj-default.json", "--no-color", "--all-namespaces"} + expectedResult := `[{"PolicySetName":"policy1","ConftestOutput":"","Passed":true,"ReqApprovals":0,"CurApprovals":0},{"PolicySetName":"policy2","ConftestOutput":"","Passed":true,"ReqApprovals":0,"CurApprovals":0}]` + + expectedArgsPolicy1 := []string{executablePath, "test", "-p", localPolicySetPath1, filepath.Join(workdir, "testproj-default.json"), "--no-color"} + expectedArgsPolicy2 := []string{executablePath, "test", "-p", localPolicySetPath2, filepath.Join(workdir, "testproj-default.json"), "--no-color"} When(mockResolver.Resolve(policySet1)).ThenReturn(localPolicySetPath1, nil) When(mockResolver.Resolve(policySet2)).ThenReturn(localPolicySetPath2, nil) - When(mockExec.CombinedOutput(expectedArgs, envs, workdir)).ThenReturn(expectedOutput, nil) + When(mockExec.CombinedOutput(expectedArgsPolicy1, envs, workdir)).ThenReturn(expectedOutput, nil) + When(mockExec.CombinedOutput(expectedArgsPolicy2, envs, workdir)).ThenReturn(expectedOutput, nil) result, err := subject.Run(ctx, executablePath, envs, workdir, extraArgs) fmt.Println(result) - Ok(t, err) + Ok(t, errors.Unwrap(err)) Assert(t, result == expectedResult, "result is expected") @@ -235,17 +241,20 @@ func TestRun(t *testing.T) { var extraArgs []string expectedOutput := "Success" - expectedResult := "Checking plan against the following policies: \n policy1\nSuccess" - expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "/some_workdir/testproj-default.json", "--no-color"} + expectedResult := `[{"PolicySetName":"policy1","ConftestOutput":"Success","Passed":true,"ReqApprovals":0,"CurApprovals":0}]` + + expectedArgsPolicy1 := []string{executablePath, "test", "-p", localPolicySetPath1, filepath.Join(workdir, "testproj-default.json"), "--no-color"} + expectedArgsPolicy2 := []string{executablePath, "test", "-p", localPolicySetPath2, filepath.Join(workdir, "testproj-default.json"), "--no-color"} When(mockResolver.Resolve(policySet1)).ThenReturn(localPolicySetPath1, nil) When(mockResolver.Resolve(policySet2)).ThenReturn("", errors.New("err")) - When(mockExec.CombinedOutput(expectedArgs, envs, workdir)).ThenReturn(expectedOutput, nil) + When(mockExec.CombinedOutput(expectedArgsPolicy1, envs, workdir)).ThenReturn(expectedOutput, nil) + When(mockExec.CombinedOutput(expectedArgsPolicy2, envs, workdir)).ThenReturn(expectedOutput, nil) result, err := subject.Run(ctx, executablePath, envs, workdir, extraArgs) - Ok(t, err) + Ok(t, errors.Unwrap(err)) Assert(t, result == expectedResult, "result is expected") @@ -254,13 +263,13 @@ func TestRun(t *testing.T) { t.Run("error resolving both policy sources", func(t *testing.T) { var extraArgs []string - expectedResult := "Success" - expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "/some_workdir/testproj-default.json", "--no-color"} + expectedResult := "" + expectedArgsPolicy1 := []string{executablePath, "test", "-p", localPolicySetPath1, filepath.Join(workdir, "testproj-default.json"), "--no-color"} When(mockResolver.Resolve(policySet1)).ThenReturn("", errors.New("err")) When(mockResolver.Resolve(policySet2)).ThenReturn("", errors.New("err")) - When(mockExec.CombinedOutput(expectedArgs, envs, workdir)).ThenReturn(expectedResult, nil) + When(mockExec.CombinedOutput(expectedArgsPolicy1, envs, workdir)).ThenReturn(expectedResult, nil) result, err := subject.Run(ctx, executablePath, envs, workdir, extraArgs) @@ -270,21 +279,47 @@ func TestRun(t *testing.T) { }) - t.Run("error running cmd", func(t *testing.T) { + t.Run("error running one cmd", func(t *testing.T) { var extraArgs []string - expectedOutput := "FAIL - /some_workdir/testproj-default.json - failure" - expectedResult := "Checking plan against the following policies: \n policy1\n policy2\nFAIL - - failure" - expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "-p", localPolicySetPath2, "/some_workdir/testproj-default.json", "--no-color"} + expectedOutputPolicy1 := fmt.Sprintf("FAIL - %s - failure\n1 tests, 0 passed, 0 warnings, 1 failure, 0 exceptions", filepath.Join(workdir, "testproj-default.json")) + expectedOutputPolicy2 := "Success" + expectedResult := `[{"PolicySetName":"policy1","ConftestOutput":"FAIL - - failure\n1 tests, 0 passed, 0 warnings, 1 failure, 0 exceptions","Passed":false,"ReqApprovals":0,"CurApprovals":0},{"PolicySetName":"policy2","ConftestOutput":"Success","Passed":true,"ReqApprovals":0,"CurApprovals":0}]` + + expectedArgsPolicy1 := []string{executablePath, "test", "-p", localPolicySetPath1, filepath.Join(workdir, "testproj-default.json"), "--no-color"} + expectedArgsPolicy2 := []string{executablePath, "test", "-p", localPolicySetPath2, filepath.Join(workdir, "testproj-default.json"), "--no-color"} + + When(mockResolver.Resolve(policySet1)).ThenReturn(localPolicySetPath1, nil) + When(mockResolver.Resolve(policySet2)).ThenReturn(localPolicySetPath2, nil) + + When(mockExec.CombinedOutput(expectedArgsPolicy1, envs, workdir)).ThenReturn(expectedOutputPolicy1, errors.New("exit status code 1")) + When(mockExec.CombinedOutput(expectedArgsPolicy2, envs, workdir)).ThenReturn(expectedOutputPolicy2, nil) + + result, err := subject.Run(ctx, executablePath, envs, workdir, extraArgs) + + Equals(t, result, expectedResult) + Assert(t, err != nil, "error is expected") + + }) + + t.Run("error running both cmds", func(t *testing.T) { + var extraArgs []string + + expectedOutput := fmt.Sprintf("FAIL - %s - failure\n1 tests, 0 passed, 0 warnings, 1 failure, 0 exceptions", filepath.Join(workdir, "testproj-default.json")) + expectedResult := `[{"PolicySetName":"policy1","ConftestOutput":"FAIL - - failure\n1 tests, 0 passed, 0 warnings, 1 failure, 0 exceptions","Passed":false,"ReqApprovals":0,"CurApprovals":0},{"PolicySetName":"policy2","ConftestOutput":"FAIL - - failure\n1 tests, 0 passed, 0 warnings, 1 failure, 0 exceptions","Passed":false,"ReqApprovals":0,"CurApprovals":0}]` + + expectedArgsPolicy1 := []string{executablePath, "test", "-p", localPolicySetPath1, filepath.Join(workdir, "testproj-default.json"), "--no-color"} + expectedArgsPolicy2 := []string{executablePath, "test", "-p", localPolicySetPath2, filepath.Join(workdir, "testproj-default.json"), "--no-color"} When(mockResolver.Resolve(policySet1)).ThenReturn(localPolicySetPath1, nil) When(mockResolver.Resolve(policySet2)).ThenReturn(localPolicySetPath2, nil) - When(mockExec.CombinedOutput(expectedArgs, envs, workdir)).ThenReturn(expectedOutput, errors.New("exit status code 1")) + When(mockExec.CombinedOutput(expectedArgsPolicy1, envs, workdir)).ThenReturn(expectedOutput, errors.New("exit status code 1")) + When(mockExec.CombinedOutput(expectedArgsPolicy2, envs, workdir)).ThenReturn(expectedOutput, errors.New("exit status code 1")) result, err := subject.Run(ctx, executablePath, envs, workdir, extraArgs) - Assert(t, result == expectedResult, "rseult is expected") + Equals(t, result, expectedResult) Assert(t, err != nil, "error is expected") }) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index c0a4d53efc..b98f38fc34 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -49,6 +49,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str "PATH": fmt.Sprintf("%s:%s", os.Getenv("PATH"), r.TerraformBinDir), "PLANFILE": filepath.Join(path, GetPlanFilename(ctx.Workspace, ctx.ProjectName)), "SHOWFILE": filepath.Join(path, ctx.GetShowResultFileName()), + "POLICYCHECKFILE": filepath.Join(path, ctx.GetPolicyCheckResultFileName()), "PROJECT_NAME": ctx.ProjectName, "PULL_AUTHOR": ctx.Pull.Author, "PULL_NUM": fmt.Sprintf("%d", ctx.Pull.Num), diff --git a/server/events/approve_policies_command_runner.go b/server/events/approve_policies_command_runner.go index 1d1056681b..6deefd242f 100644 --- a/server/events/approve_policies_command_runner.go +++ b/server/events/approve_policies_command_runner.go @@ -1,8 +1,6 @@ package events import ( - "fmt" - "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/vcs" @@ -74,7 +72,7 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *CommentCom return } - result := a.buildApprovePolicyCommandResults(ctx, projectCmds) + result := runProjectCmds(projectCmds, a.prjCmdRunner.ApprovePolicies) a.pullUpdater.updatePull( ctx, @@ -91,39 +89,6 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *CommentCom a.updateCommitStatus(ctx, pullStatus) } -func (a *ApprovePoliciesCommandRunner) buildApprovePolicyCommandResults(ctx *command.Context, prjCmds []command.ProjectContext) (result command.Result) { - // Check if vcs user is in the owner list of the PolicySets. All projects - // share the same Owners list at this time so no reason to iterate over each - // project. - if len(prjCmds) > 0 { - teams := []string{} - - // Only query the users team membership if any teams have been configured as owners. - if prjCmds[0].PolicySets.HasTeamOwners() { - userTeams, err := a.vcsClient.GetTeamNamesForUser(ctx.Pull.BaseRepo, ctx.User) - if err != nil { - ctx.Log.Err("unable to get team membership for user: %s", err) - return - } - teams = append(teams, userTeams...) - } - - if !prjCmds[0].PolicySets.IsOwner(ctx.User.Username, teams) { - result.Error = fmt.Errorf("contact policy owners to approve failing policies") - return - } - } - - var prjResults []command.ProjectResult - - for _, prjCmd := range prjCmds { - prjResult := a.prjCmdRunner.ApprovePolicies(prjCmd) - prjResults = append(prjResults, prjResult) - } - result.ProjectResults = prjResults - return -} - func (a *ApprovePoliciesCommandRunner) updateCommitStatus(ctx *command.Context, pullStatus models.PullStatus) { var numSuccess int var numErrored int diff --git a/server/events/approve_policies_command_runner_test.go b/server/events/approve_policies_command_runner_test.go deleted file mode 100644 index a5ac56e57a..0000000000 --- a/server/events/approve_policies_command_runner_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package events_test - -import ( - "testing" - - . "github.com/petergtz/pegomock" - "github.com/runatlantis/atlantis/server/core/config/valid" - "github.com/runatlantis/atlantis/server/events" - "github.com/runatlantis/atlantis/server/events/command" - "github.com/runatlantis/atlantis/server/events/mocks/matchers" - "github.com/runatlantis/atlantis/server/events/models" - "github.com/runatlantis/atlantis/server/events/models/testdata" - "github.com/runatlantis/atlantis/server/logging" - "github.com/runatlantis/atlantis/server/metrics" -) - -func TestApproveCommandRunner_IsOwner(t *testing.T) { - logger := logging.NewNoopLogger(t) - RegisterMockTestingT(t) - - cases := []struct { - Description string - OwnerUsers []string - OwnerTeams []string // Teams configured as owners - UserTeams []string // Teams the user is a member of - ExpComment string - }{ - { - Description: "When user is not an owner, approval fails", - OwnerUsers: []string{}, - OwnerTeams: []string{}, - ExpComment: "**Approve Policies Error**\n```\ncontact policy owners to approve failing policies\n```", - }, - { - Description: "When user is an owner, approval succeeds", - OwnerUsers: []string{testdata.User.Username}, - OwnerTeams: []string{}, - ExpComment: "Approved Policies for 1 projects:\n\n1. dir: `` workspace: ``", - }, - { - Description: "When user is an owner via team membership, approval succeeds", - OwnerUsers: []string{}, - OwnerTeams: []string{"SomeTeam"}, - UserTeams: []string{"SomeTeam"}, - ExpComment: "Approved Policies for 1 projects:\n\n1. dir: `` workspace: ``", - }, - { - Description: "When user belongs to a team not configured as a owner, approval fails", - OwnerUsers: []string{}, - OwnerTeams: []string{"SomeTeam"}, - UserTeams: []string{"SomeOtherTeam}"}, - ExpComment: "**Approve Policies Error**\n```\ncontact policy owners to approve failing policies\n```", - }, - { - Description: "When user is an owner but not a team member, approval succeeds", - OwnerUsers: []string{testdata.User.Username}, - OwnerTeams: []string{"SomeTeam"}, - UserTeams: []string{"SomeOtherTeam"}, - ExpComment: "Approved Policies for 1 projects:\n\n1. dir: `` workspace: ``", - }, - } - - for _, c := range cases { - t.Run(c.Description, func(t *testing.T) { - vcsClient := setup(t) - - scopeNull, _, _ := metrics.NewLoggingScope(logger, "atlantis") - - modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} - - ctx := &command.Context{ - User: testdata.User, - Log: logging.NewNoopLogger(t), - Scope: scopeNull, - Pull: modelPull, - HeadRepo: testdata.GithubRepo, - Trigger: command.CommentTrigger, - } - - When(projectCommandBuilder.BuildApprovePoliciesCommands(matchers.AnyPtrToCommandContext(), matchers.AnyPtrToEventsCommentCommand())).ThenReturn([]command.ProjectContext{ - { - CommandName: command.ApprovePolicies, - PolicySets: valid.PolicySets{ - Owners: valid.PolicyOwners{ - Users: c.OwnerUsers, - Teams: c.OwnerTeams, - }, - }, - }, - }, nil) - When(vcsClient.GetTeamNamesForUser(testdata.GithubRepo, testdata.User)).ThenReturn(c.UserTeams, nil) - - approvePoliciesCommandRunner.Run(ctx, &events.CommentCommand{Name: command.ApprovePolicies}) - - vcsClient.VerifyWasCalledOnce().CreateComment(testdata.GithubRepo, modelPull.Num, c.ExpComment, "approve_policies") - }) - } -} diff --git a/server/events/command/context.go b/server/events/command/context.go index 968551b99a..94c84585c9 100644 --- a/server/events/command/context.go +++ b/server/events/command/context.go @@ -36,5 +36,8 @@ type Context struct { PullStatus *models.PullStatus + // PolicySet is the policy set to target (if specified) for the approve_policies command. + PolicySet string + Trigger Trigger } diff --git a/server/events/command/project_context.go b/server/events/command/project_context.go index 508d2b55f8..e6a3562a55 100644 --- a/server/events/command/project_context.go +++ b/server/events/command/project_context.go @@ -23,6 +23,9 @@ type ProjectContext struct { // ApplyCmd is the command that users should run to apply this plan. If // this is an apply then this will be empty. ApplyCmd string + // ApprovePoliciesCmd is the command that users should run to approve policies for this plan. If + // this is an apply then this will be empty. + ApprovePoliciesCmd string // PlanRequirements is the list of requirements that must be satisfied // before we will run the plan stage. PlanRequirements []string @@ -62,6 +65,8 @@ type ProjectContext struct { PullReqStatus models.PullReqStatus // CurrentProjectPlanStatus is the status of the current project prior to this command. ProjectPlanStatus models.ProjectPlanStatus + // ProjectPolicyStatus is the status of policy sets of the current project prior to this command. + ProjectPolicyStatus []models.PolicySetStatus // Pull is the pull request we're responding to. Pull models.PullRequest // ProjectName is the name of the project set in atlantis.yaml. If there was @@ -92,6 +97,8 @@ type ProjectContext struct { // PolicySets represent the policies that are run on the plan as part of the // policy check stage PolicySets valid.PolicySets + // PolicySetTarget describes which policy sets to target on the approve_policies step. + PolicySetTarget string // DeleteSourceBranchOnMerge will attempt to allow a branch to be deleted when merged (AzureDevOps & GitLab Support Only) DeleteSourceBranchOnMerge bool // RepoLocking will get a lock when plan @@ -132,6 +139,15 @@ func (p ProjectContext) GetShowResultFileName() string { return fmt.Sprintf("%s-%s.json", projName, p.Workspace) } +// GetPolicyCheckResultFileName returns the filename (not the path) to store the result from conftest_client. +func (p ProjectContext) GetPolicyCheckResultFileName() string { + if p.ProjectName == "" { + return fmt.Sprintf("%s-policyout.json", p.Workspace) + } + projName := strings.Replace(p.ProjectName, "/", planfileSlashReplace, -1) + return fmt.Sprintf("%s-%s-policyout.json", projName, p.Workspace) +} + // Gets a unique identifier for the current pull request as a single string func (p ProjectContext) PullInfo() string { normalizedOwner := strings.ReplaceAll(p.BaseRepo.Owner, "/", "-") @@ -154,3 +170,21 @@ func getProjectIdentifier(relRepoDir string, projectName string) string { replacer := strings.NewReplacer("/", "-", ".", "_") return replacer.Replace(relRepoDir) } + +// PolicyCleared returns whether all policies are passing or not. +func (p ProjectContext) PolicyCleared() bool { + passing := true + for _, psStatus := range p.ProjectPolicyStatus { + if psStatus.Passed { + continue + } + for _, psCfg := range p.PolicySets.PolicySets { + if psStatus.PolicySetName == psCfg.Name { + if psStatus.Approvals != psCfg.ApproveCount { + passing = false + } + } + } + } + return passing +} diff --git a/server/events/command/project_context_test.go b/server/events/command/project_context_test.go new file mode 100644 index 0000000000..a70d344afc --- /dev/null +++ b/server/events/command/project_context_test.go @@ -0,0 +1,161 @@ +package command_test + +import ( + "testing" + + "github.com/runatlantis/atlantis/server/core/config/valid" + "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/events/models" + . "github.com/runatlantis/atlantis/testing" +) + +// Test PolicyCleared and PolicySummary +func TestPolicyCheckResults_PolicyFuncs(t *testing.T) { + cases := []struct { + description string + policySetsConfig valid.PolicySets + policySetStatus []models.PolicySetStatus + policyClearedExp bool + }{ + { + description: "single policy set, not passed", + policySetsConfig: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 1, + }, + }, + }, + policySetStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Passed: false, + Approvals: 0, + }, + }, + policyClearedExp: false, + }, + { + description: "single policy set, passed", + policySetsConfig: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 1, + }, + }, + }, + policySetStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Passed: true, + Approvals: 0, + }, + }, + policyClearedExp: true, + }, + { + description: "single policy set, fully approved", + policySetsConfig: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 1, + }, + }, + }, + policySetStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Passed: false, + Approvals: 1, + }, + }, + policyClearedExp: true, + }, + { + description: "multiple policy sets, different states.", + policySetsConfig: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 2, + }, + { + Name: "policy2", + ApproveCount: 1, + }, + { + Name: "policy3", + ApproveCount: 1, + }, + }, + }, + policySetStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Passed: false, + Approvals: 0, + }, + { + PolicySetName: "policy2", + Passed: false, + Approvals: 1, + }, + { + PolicySetName: "policy3", + Passed: true, + Approvals: 0, + }, + }, + policyClearedExp: false, + }, + { + description: "multiple policy sets, all cleared.", + policySetsConfig: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 2, + }, + { + Name: "policy2", + ApproveCount: 1, + }, + { + Name: "policy3", + ApproveCount: 1, + }, + }, + }, + policySetStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Passed: false, + Approvals: 2, + }, + { + PolicySetName: "policy2", + Passed: false, + Approvals: 1, + }, + { + PolicySetName: "policy3", + Passed: true, + Approvals: 0, + }, + }, + policyClearedExp: true, + }, + } + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + pcs := command.ProjectContext{ + ProjectPolicyStatus: c.policySetStatus, + PolicySets: c.policySetsConfig, + } + Equals(t, c.policyClearedExp, pcs.PolicyCleared()) + }) + } +} diff --git a/server/events/command/project_result.go b/server/events/command/project_result.go index 9191d86f8a..09a84e0335 100644 --- a/server/events/command/project_result.go +++ b/server/events/command/project_result.go @@ -13,7 +13,7 @@ type ProjectResult struct { Error error Failure string PlanSuccess *models.PlanSuccess - PolicyCheckSuccess *models.PolicyCheckSuccess + PolicyCheckResults *models.PolicyCheckResults ApplySuccess string VersionSuccess string ImportSuccess *models.ImportSuccess @@ -32,6 +32,22 @@ func (p ProjectResult) CommitStatus() models.CommitStatus { return models.SuccessCommitStatus } +// PolicyStatus returns the approval status of policy sets of this project result. +func (p ProjectResult) PolicyStatus() []models.PolicySetStatus { + var policyStatuses []models.PolicySetStatus + if p.PolicyCheckResults != nil { + for _, policySet := range p.PolicyCheckResults.PolicySetResults { + policyStatus := models.PolicySetStatus{ + PolicySetName: policySet.PolicySetName, + Passed: policySet.Passed, + Approvals: policySet.CurApprovals, + } + policyStatuses = append(policyStatuses, policyStatus) + } + } + return policyStatuses +} + // PlanStatus returns the plan status. func (p ProjectResult) PlanStatus() models.ProjectPlanStatus { switch p.Command { @@ -64,5 +80,5 @@ func (p ProjectResult) PlanStatus() models.ProjectPlanStatus { // IsSuccessful returns true if this project result had no errors. func (p ProjectResult) IsSuccessful() bool { - return p.PlanSuccess != nil || p.PolicyCheckSuccess != nil || p.ApplySuccess != "" + return p.PlanSuccess != nil || (p.PolicyCheckResults != nil && p.Error == nil && p.Failure == "") || p.ApplySuccess != "" } diff --git a/server/events/command/project_result_test.go b/server/events/command/project_result_test.go index 0c917b9d30..250dc374b8 100644 --- a/server/events/command/project_result_test.go +++ b/server/events/command/project_result_test.go @@ -22,7 +22,7 @@ func TestProjectResult_IsSuccessful(t *testing.T) { }, "policy_check success": { command.ProjectResult{ - PolicyCheckSuccess: &models.PolicyCheckSuccess{}, + PolicyCheckResults: &models.PolicyCheckResults{}, }, true, }, @@ -103,7 +103,7 @@ func TestProjectResult_PlanStatus(t *testing.T) { { p: command.ProjectResult{ Command: command.PolicyCheck, - PolicyCheckSuccess: &models.PolicyCheckSuccess{}, + PolicyCheckResults: &models.PolicyCheckResults{}, }, expStatus: models.PassedPolicyCheckStatus, }, @@ -117,7 +117,7 @@ func TestProjectResult_PlanStatus(t *testing.T) { { p: command.ProjectResult{ Command: command.ApprovePolicies, - PolicyCheckSuccess: &models.PolicyCheckSuccess{}, + PolicyCheckResults: &models.PolicyCheckResults{}, }, expStatus: models.PassedPolicyCheckStatus, }, diff --git a/server/events/command_requirement_handler.go b/server/events/command_requirement_handler.go index 1745d2709c..2fdfbdcd23 100644 --- a/server/events/command_requirement_handler.go +++ b/server/events/command_requirement_handler.go @@ -4,7 +4,6 @@ import ( "github.com/runatlantis/atlantis/server/core/config/raw" "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/events/command" - "github.com/runatlantis/atlantis/server/events/models" ) //go:generate pegomock generate -m --package mocks -o mocks/mock_command_requirement_handler.go CommandRequirementHandler @@ -48,7 +47,8 @@ func (a *DefaultCommandRequirementHandler) ValidateApplyProject(repoDir string, } // this should come before mergeability check since mergeability is a superset of this check. case valid.PoliciesPassedCommandReq: - if ctx.ProjectPlanStatus == models.ErroredPolicyCheckStatus { + // We should rely on this function instead of plan status, since plan status after a failed apply will not carry the policy error over. + if !ctx.PolicyCleared() { return "All policies must pass for project before running apply.", nil } case raw.MergeableRequirement: diff --git a/server/events/command_requirement_handler_test.go b/server/events/command_requirement_handler_test.go index c7ae1b3f01..4ca782f44e 100644 --- a/server/events/command_requirement_handler_test.go +++ b/server/events/command_requirement_handler_test.go @@ -152,6 +152,21 @@ func TestAggregateApplyRequirements_ValidateApplyProject(t *testing.T) { ctx: command.ProjectContext{ ApplyRequirements: []string{valid.PoliciesPassedCommandReq}, ProjectPlanStatus: models.ErroredPolicyCheckStatus, + ProjectPolicyStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Passed: false, + Approvals: 0, + }, + }, + PolicySets: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 1, + }, + }, + }, }, wantFailure: "All policies must pass for project before running apply.", wantErr: assert.NoError, diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 916c35b58b..98c67b67b1 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -279,6 +279,7 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead HeadRepo: headRepo, Scope: scope, Trigger: command.CommentTrigger, + PolicySet: cmd.PolicySet, } if !c.validateCtxAndComment(ctx, cmd.Name) { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 408b5d70ce..bc1e90e3fc 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -812,7 +812,7 @@ func TestFailedApprovalCreatesFailedStatusUpdate(t *testing.T) { matchers.EqModelsCommitStatus(models.SuccessCommitStatus), matchers.EqCommandName(command.PolicyCheck), EqInt(0), - EqInt(0), + EqInt(2), ) } @@ -855,7 +855,7 @@ func TestApprovedPoliciesUpdateFailedPolicyStatus(t *testing.T) { return ReturnValues{ command.ProjectResult{ Command: command.PolicyCheck, - PolicyCheckSuccess: &models.PolicyCheckSuccess{}, + PolicyCheckResults: &models.PolicyCheckResults{}, }, } }) diff --git a/server/events/comment_parser.go b/server/events/comment_parser.go index a822267955..d752fbc342 100644 --- a/server/events/comment_parser.go +++ b/server/events/comment_parser.go @@ -37,6 +37,8 @@ const ( dirFlagShort = "d" projectFlagLong = "project" projectFlagShort = "p" + policySetFlagLong = "policy-set" + policySetFlagShort = "" autoMergeDisabledFlagLong = "auto-merge-disabled" autoMergeDisabledFlagShort = "" verboseFlagLong = "verbose" @@ -67,6 +69,8 @@ type CommentBuilder interface { BuildPlanComment(repoRelDir string, workspace string, project string, commentArgs []string) string // BuildApplyComment builds an apply comment for the specified args. BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool) string + // BuildApprovePoliciesComment builds an approve_policies comment for the specified args. + BuildApprovePoliciesComment(repoRelDir string, workspace string, project string) string } // CommentParser implements CommentParsing @@ -209,6 +213,7 @@ func (e *CommentParser) Parse(rawComment string, vcsHost models.VCSHostType) Com var workspace string var dir string var project string + var policySet string var verbose, autoMergeDisabled bool var flagSet *pflag.FlagSet var name command.Name @@ -236,6 +241,10 @@ func (e *CommentParser) Parse(rawComment string, vcsHost models.VCSHostType) Com name = command.ApprovePolicies flagSet = pflag.NewFlagSet(command.ApprovePolicies.String(), pflag.ContinueOnError) flagSet.SetOutput(io.Discard) + flagSet.StringVarP(&workspace, workspaceFlagLong, workspaceFlagShort, "", "Approve policies for this Terraform workspace.") + flagSet.StringVarP(&dir, dirFlagLong, dirFlagShort, "", "Approve policies for this directory, relative to root of repo, ex. 'child/dir'.") + flagSet.StringVarP(&project, projectFlagLong, projectFlagShort, "", "Approve policies for this project. Refers to the name of the project configured in a repo config file. Cannot be used at same time as workspace or dir flags.") + flagSet.StringVarP(&policySet, policySetFlagLong, policySetFlagShort, "", "Approve policies for this project. Refers to the name of the project configured in a repo config file. Cannot be used at same time as workspace or dir flags.") flagSet.BoolVarP(&verbose, verboseFlagLong, verboseFlagShort, false, "Append Atlantis log to comment.") case command.Unlock.String(): name = command.Unlock @@ -296,7 +305,7 @@ func (e *CommentParser) Parse(rawComment string, vcsHost models.VCSHostType) Com } return CommentParseResult{ - Command: NewCommentCommand(dir, extraArgs, name, subName, verbose, autoMergeDisabled, workspace, project), + Command: NewCommentCommand(dir, extraArgs, name, subName, verbose, autoMergeDisabled, workspace, project, policySet), } } @@ -385,6 +394,12 @@ func (e *CommentParser) BuildApplyComment(repoRelDir string, workspace string, p return fmt.Sprintf("%s %s%s", e.ExecutableName, command.Apply.String(), flags) } +// BuildApprovePoliciesComment builds an apply comment for the specified args. +func (e *CommentParser) BuildApprovePoliciesComment(repoRelDir string, workspace string, project string) string { + flags := e.buildFlags(repoRelDir, workspace, project, false) + return fmt.Sprintf("%s %s%s", e.ExecutableName, command.ApprovePolicies.String(), flags) +} + func (e *CommentParser) buildFlags(repoRelDir string, workspace string, project string, autoMergeDisabled bool) string { // Add quotes if dir has spaces. if strings.Contains(repoRelDir, " ") { diff --git a/server/events/comment_parser_test.go b/server/events/comment_parser_test.go index 4583920999..d594acd24b 100644 --- a/server/events/comment_parser_test.go +++ b/server/events/comment_parser_test.go @@ -1027,7 +1027,16 @@ var ApplyUsage = `Usage of apply: ` var ApprovePolicyUsage = `Usage of approve_policies: - --verbose Append Atlantis log to comment. + -d, --dir string Approve policies for this directory, relative to root of + repo, ex. 'child/dir'. + --policy-set string Approve policies for this project. Refers to the name of + the project configured in a repo config file. Cannot be + used at same time as workspace or dir flags. + -p, --project string Approve policies for this project. Refers to the name of + the project configured in a repo config file. Cannot be + used at same time as workspace or dir flags. + --verbose Append Atlantis log to comment. + -w, --workspace string Approve policies for this Terraform workspace. ` var UnlockUsage = "`Usage of unlock:`\n\n ```cmake\n" + diff --git a/server/events/event_parser.go b/server/events/event_parser.go index 29b6599514..78eefce15a 100644 --- a/server/events/event_parser.go +++ b/server/events/event_parser.go @@ -117,6 +117,8 @@ type CommentCommand struct { // project specified in an atlantis.yaml file. // If empty then the comment specified no project. ProjectName string + // PolicySet is the name of a policy set to run an approval on. + PolicySet string } // IsForSpecificProject returns true if the command is for a specific dir, workspace @@ -148,11 +150,11 @@ func (c CommentCommand) IsAutoplan() bool { // String returns a string representation of the command. func (c CommentCommand) String() string { - return fmt.Sprintf("command=%q verbose=%t dir=%q workspace=%q project=%q flags=%q", c.Name.String(), c.Verbose, c.RepoRelDir, c.Workspace, c.ProjectName, strings.Join(c.Flags, ",")) + return fmt.Sprintf("command=%q verbose=%t dir=%q workspace=%q project=%q policyset=%q flags=%q", c.Name.String(), c.Verbose, c.RepoRelDir, c.Workspace, c.ProjectName, c.PolicySet, strings.Join(c.Flags, ",")) } // NewCommentCommand constructs a CommentCommand, setting all missing fields to defaults. -func NewCommentCommand(repoRelDir string, flags []string, name command.Name, subName string, verbose, autoMergeDisabled bool, workspace string, project string) *CommentCommand { +func NewCommentCommand(repoRelDir string, flags []string, name command.Name, subName string, verbose, autoMergeDisabled bool, workspace string, project string, policySet string) *CommentCommand { // If repoRelDir was empty we want to keep it that way to indicate that it // wasn't specified in the comment. if repoRelDir != "" { @@ -170,6 +172,7 @@ func NewCommentCommand(repoRelDir string, flags []string, name command.Name, sub Workspace: workspace, AutoMergeDisabled: autoMergeDisabled, ProjectName: project, + PolicySet: policySet, } } diff --git a/server/events/event_parser_test.go b/server/events/event_parser_test.go index d28508cbbd..d51d5067c6 100644 --- a/server/events/event_parser_test.go +++ b/server/events/event_parser_test.go @@ -729,14 +729,14 @@ func TestNewCommand_CleansDir(t *testing.T) { for _, c := range cases { t.Run(c.RepoRelDir, func(t *testing.T) { - cmd := events.NewCommentCommand(c.RepoRelDir, nil, command.Plan, "", false, false, "workspace", "") + cmd := events.NewCommentCommand(c.RepoRelDir, nil, command.Plan, "", false, false, "workspace", "", "") Equals(t, c.ExpDir, cmd.RepoRelDir) }) } } func TestNewCommand_EmptyDirWorkspaceProject(t *testing.T) { - cmd := events.NewCommentCommand("", nil, command.Plan, "", false, false, "", "") + cmd := events.NewCommentCommand("", nil, command.Plan, "", false, false, "", "", "") Equals(t, events.CommentCommand{ RepoRelDir: "", Flags: nil, @@ -748,7 +748,7 @@ func TestNewCommand_EmptyDirWorkspaceProject(t *testing.T) { } func TestNewCommand_AllFieldsSet(t *testing.T) { - cmd := events.NewCommentCommand("dir", []string{"a", "b"}, command.Plan, "", true, false, "workspace", "project") + cmd := events.NewCommentCommand("dir", []string{"a", "b"}, command.Plan, "", true, false, "workspace", "project", "policyset") Equals(t, events.CommentCommand{ Workspace: "workspace", RepoRelDir: "dir", @@ -756,6 +756,7 @@ func TestNewCommand_AllFieldsSet(t *testing.T) { Flags: []string{"a", "b"}, Name: command.Plan, ProjectName: "project", + PolicySet: "policyset", }, *cmd) } @@ -794,7 +795,7 @@ func TestCommentCommand_IsAutoplan(t *testing.T) { } func TestCommentCommand_String(t *testing.T) { - exp := `command="plan" verbose=true dir="mydir" workspace="myworkspace" project="myproject" flags="flag1,flag2"` + exp := `command="plan" verbose=true dir="mydir" workspace="myworkspace" project="myproject" policyset="" flags="flag1,flag2"` Equals(t, exp, (events.CommentCommand{ RepoRelDir: "mydir", Flags: []string{"flag1", "flag2"}, diff --git a/server/events/markdown_renderer.go b/server/events/markdown_renderer.go index 09542297a6..f8b90f8e6f 100644 --- a/server/events/markdown_renderer.go +++ b/server/events/markdown_renderer.go @@ -76,17 +76,18 @@ type commonData struct { // errData is data about an error response. type errData struct { - Error string + Error string + RenderedContext string commonData } // failureData is data about a failure response. type failureData struct { - Failure string + Failure string + RenderedContext string commonData } -// resultData is data about a successful response. type resultData struct { Results []projectResultTmplData commonData @@ -101,9 +102,12 @@ type planSuccessData struct { EnableDiffMarkdownFormat bool } -type policyCheckSuccessData struct { - models.PolicyCheckSuccess - PolicyCheckSummary string +type policyCheckResultsData struct { + models.PolicyCheckResults + PolicyCheckSummary string + PolicyApprovalSummary string + PolicyCleared bool + commonData } type projectResultTmplData struct { @@ -166,10 +170,10 @@ func (m *MarkdownRenderer) Render(res command.Result, cmdName command.Name, subC templates := m.markdownTemplates if res.Error != nil { - return m.renderTemplateTrimSpace(templates.Lookup("unwrappedErrWithLog"), errData{res.Error.Error(), common}) + return m.renderTemplateTrimSpace(templates.Lookup("unwrappedErrWithLog"), errData{res.Error.Error(), "", common}) } if res.Failure != "" { - return m.renderTemplateTrimSpace(templates.Lookup("failureWithLog"), failureData{res.Failure, common}) + return m.renderTemplateTrimSpace(templates.Lookup("failureWithLog"), failureData{res.Failure, "", common}) } return m.renderProjectResults(res.ProjectResults, common, vcsHost) } @@ -178,6 +182,7 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult, var resultsTmplData []projectResultTmplData numPlanSuccesses := 0 numPolicyCheckSuccesses := 0 + numPolicyApprovalSuccesses := 0 numVersionSuccesses := 0 templates := m.markdownTemplates @@ -188,15 +193,7 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult, RepoRelDir: result.RepoRelDir, ProjectName: result.ProjectName, } - if result.Error != nil { - tmpl := templates.Lookup("unwrappedErr") - if m.shouldUseWrappedTmpl(vcsHost, result.Error.Error()) { - tmpl = templates.Lookup("wrappedErr") - } - resultData.Rendered = m.renderTemplateTrimSpace(tmpl, errData{result.Error.Error(), common}) - } else if result.Failure != "" { - resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("failure"), failureData{result.Failure, common}) - } else if result.PlanSuccess != nil { + if result.PlanSuccess != nil { result.PlanSuccess.TerraformOutput = strings.TrimSpace(result.PlanSuccess.TerraformOutput) if m.shouldUseWrappedTmpl(vcsHost, result.PlanSuccess.TerraformOutput) { resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("planSuccessWrapped"), planSuccessData{PlanSuccess: *result.PlanSuccess, PlanSummary: result.PlanSuccess.Summary(), PlanWasDeleted: common.PlansDeleted, DisableApply: common.DisableApply, DisableRepoLocking: common.DisableRepoLocking, EnableDiffMarkdownFormat: common.EnableDiffMarkdownFormat}) @@ -205,14 +202,38 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult, } resultData.NoChanges = result.PlanSuccess.NoChanges() numPlanSuccesses++ - } else if result.PolicyCheckSuccess != nil { - result.PolicyCheckSuccess.PolicyCheckOutput = strings.TrimSpace(result.PolicyCheckSuccess.PolicyCheckOutput) - if m.shouldUseWrappedTmpl(vcsHost, result.PolicyCheckSuccess.PolicyCheckOutput) { - resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckSuccessWrapped"), policyCheckSuccessData{PolicyCheckSuccess: *result.PolicyCheckSuccess, PolicyCheckSummary: result.PolicyCheckSuccess.Summary()}) + } else if result.PolicyCheckResults != nil && common.Command == policyCheckCommandTitle { + policyCheckResults := policyCheckResultsData{ + PolicyCheckResults: *result.PolicyCheckResults, + PolicyCheckSummary: result.PolicyCheckResults.Summary(), + PolicyApprovalSummary: result.PolicyCheckResults.PolicySummary(), + PolicyCleared: result.PolicyCheckResults.PolicyCleared(), + commonData: common, + } + if m.shouldUseWrappedTmpl(vcsHost, result.PolicyCheckResults.CombinedOutput()) { + resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckResultsWrapped"), policyCheckResults) + } else { + resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckResultsUnwrapped"), policyCheckResults) + } + if result.Error == nil && result.Failure == "" { + numPolicyCheckSuccesses++ + } + } else if result.PolicyCheckResults != nil && common.Command == approvePoliciesCommandTitle { + policyCheckResults := policyCheckResultsData{ + PolicyCheckResults: *result.PolicyCheckResults, + PolicyCheckSummary: result.PolicyCheckResults.Summary(), + PolicyApprovalSummary: result.PolicyCheckResults.PolicySummary(), + PolicyCleared: result.PolicyCheckResults.PolicyCleared(), + commonData: common, + } + if m.shouldUseWrappedTmpl(vcsHost, result.PolicyCheckResults.CombinedOutput()) { + resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckResultsWrapped"), policyCheckResults) } else { - resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckSuccessUnwrapped"), policyCheckSuccessData{PolicyCheckSuccess: *result.PolicyCheckSuccess}) + resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckResultsUnwrapped"), policyCheckResults) + } + if result.Error == nil && result.Failure == "" { + numPolicyApprovalSuccesses++ } - numPolicyCheckSuccesses++ } else if result.ApplySuccess != "" { output := strings.TrimSpace(result.ApplySuccess) if m.shouldUseWrappedTmpl(vcsHost, result.ApplySuccess) { @@ -242,9 +263,21 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult, } else { resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("stateRmSuccessUnwrapped"), result.StateRmSuccess) } - } else { + // Error out if no template was found, only if there are no errors or failures. + // This is because some errors and failures rely on additional context rendered by templtes, but not all errors or failures. + } else if !(result.Error != nil || result.Failure != "") { resultData.Rendered = "Found no template. This is a bug!" } + // Render error or failure templates. Done outside of previous block so that other context can be rendered for use here. + if result.Error != nil { + tmpl := templates.Lookup("unwrappedErr") + if m.shouldUseWrappedTmpl(vcsHost, result.Error.Error()) { + tmpl = templates.Lookup("wrappedErr") + } + resultData.Rendered = m.renderTemplateTrimSpace(tmpl, errData{result.Error.Error(), resultData.Rendered, common}) + } else if result.Failure != "" { + resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("failure"), failureData{result.Failure, resultData.Rendered, common}) + } resultsTmplData = append(resultsTmplData, resultData) } @@ -257,7 +290,7 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult, case len(resultsTmplData) == 1 && common.Command == policyCheckCommandTitle && numPolicyCheckSuccesses > 0: tmpl = templates.Lookup("singleProjectPlanSuccess") case len(resultsTmplData) == 1 && common.Command == policyCheckCommandTitle && numPolicyCheckSuccesses == 0: - tmpl = templates.Lookup("singleProjectPlanUnsuccessful") + tmpl = templates.Lookup("singleProjectPolicyUnsuccessful") case len(resultsTmplData) == 1 && common.Command == versionCommandTitle && numVersionSuccesses > 0: tmpl = templates.Lookup("singleProjectVersionSuccess") case len(resultsTmplData) == 1 && common.Command == versionCommandTitle && numVersionSuccesses == 0: @@ -273,11 +306,20 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult, default: return fmt.Sprintf("no template matched–this is a bug: command=%s, subcommand=%s", common.Command, common.SubCommand) } - case common.Command == planCommandTitle, - common.Command == policyCheckCommandTitle: + case common.Command == planCommandTitle: tmpl = templates.Lookup("multiProjectPlan") + case common.Command == policyCheckCommandTitle: + if numPolicyCheckSuccesses == len(results) { + tmpl = templates.Lookup("multiProjectPlan") + } else { + tmpl = templates.Lookup("multiProjectPolicyUnsuccessful") + } case common.Command == approvePoliciesCommandTitle: - tmpl = templates.Lookup("approveAllProjects") + if numPolicyApprovalSuccesses == len(results) { + tmpl = templates.Lookup("approveAllProjects") + } else { + tmpl = templates.Lookup("multiProjectPolicyUnsuccessful") + } case common.Command == applyCommandTitle: tmpl = templates.Lookup("multiProjectApply") case common.Command == versionCommandTitle: diff --git a/server/events/markdown_renderer_test.go b/server/events/markdown_renderer_test.go index 9c0548ff91..33381e8b62 100644 --- a/server/events/markdown_renderer_test.go +++ b/server/events/markdown_renderer_test.go @@ -49,11 +49,8 @@ func TestRenderErr(t *testing.T) { { "policy check error", command.PolicyCheck, - err, - "**Policy Check Error**\n```\nerr\n```" + - "\n* :heavy_check_mark: To **approve** failing policies an authorized approver can comment:\n" + - " * `atlantis approve_policies`\n" + - "* :repeat: Or, address the policy failure by modifying the codebase and re-planning.", + fmt.Errorf("some conftest error"), + "**Policy Check Error**\n```\nsome conftest error\n```", }, } @@ -260,18 +257,91 @@ $$$ `, }, { - "single successful policy check with project name", + "single successful policy check with multiple policy sets and project name", command.PolicyCheck, "", []command.ProjectResult{ { - PolicyCheckSuccess: &models.PolicyCheckSuccess{ - // strings.Repeat require to get wrapped result - PolicyCheckOutput: strings.Repeat("line\n", 13) + `Checking plan against the following policies: - test_policy + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + // strings.Repeat require to get wrapped result + ConftestOutput: `FAIL - - main - WARNING: Null Resource creation is prohibited. + +2 tests, 1 passed, 0 warnings, 1 failure, 0 exceptions`, + Passed: false, + ReqApprovals: 1, + }, + { + PolicySetName: "policy2", + // strings.Repeat require to get wrapped result + ConftestOutput: "2 tests, 2 passed, 0 warnings, 0 failure, 0 exceptions", + Passed: true, + ReqApprovals: 1, + }, + }, + LockURL: "lock-url", + RePlanCmd: "atlantis plan -d path -w workspace", + ApplyCmd: "atlantis apply -d path -w workspace", + }, + Workspace: "workspace", + RepoRelDir: "path", + ProjectName: "projectname", + }, + }, + models.Github, + `Ran Policy Check for project: $projectname$ dir: $path$ workspace: $workspace$ + +#### Policy Set: $policy1$ +$$$diff FAIL - - main - WARNING: Null Resource creation is prohibited. -2 tests, 1 passed, 0 warnings, 0 failure, 0 exceptions`, +2 tests, 1 passed, 0 warnings, 1 failure, 0 exceptions +$$$ + +#### Policy Set: $policy2$ +$$$diff +2 tests, 2 passed, 0 warnings, 0 failure, 0 exceptions +$$$ + + +#### Policy Approval Status: +$$$ +policy set: policy1: requires: 1 approval(s), have: 0. +policy set: policy2: passed. +$$$ +* :heavy_check_mark: To **approve** this project, comment: + * $$ +* :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 path -w workspace$ + +--- +* :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$ +`, + }, + { + "single successful policy check with project name", + command.PolicyCheck, + "", + []command.ProjectResult{ + { + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + // strings.Repeat require to get wrapped result + ConftestOutput: strings.Repeat("line\n", 13) + `FAIL - - main - WARNING: Null Resource creation is prohibited. + +2 tests, 1 passed, 0 warnings, 1 failure, 0 exceptions`, + Passed: false, + ReqApprovals: 1, + }, + }, LockURL: "lock-url", RePlanCmd: "atlantis plan -d path -w workspace", ApplyCmd: "atlantis apply -d path -w workspace", @@ -286,21 +356,42 @@ FAIL - - main - WARNING: Null Resource creation is prohibit
Show Output + +#### Policy Set: $policy1$ $$$diff -` + strings.Repeat("line\n", 13) + `Checking plan against the following policies: - test_policy +line +line +line +line +line +line +line +line +line +line +line +line +line FAIL - - main - WARNING: Null Resource creation is prohibited. -2 tests, 1 passed, 0 warnings, 0 failure, 0 exceptions +2 tests, 1 passed, 0 warnings, 1 failure, 0 exceptions $$$ -* :arrow_forward: To **apply** this plan, comment: - * $atlantis apply -d path -w workspace$ + +#### Policy Approval Status: +$$$ +policy set: policy1: requires: 1 approval(s), have: 0. +$$$ +* :heavy_check_mark: To **approve** this project, comment: + * $$ * :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 path -w workspace$
-2 tests, 1 passed, 0 warnings, 0 failure, 0 exceptions + +$$$ +policy set: policy1: 2 tests, 1 passed, 0 warnings, 1 failure, 0 exceptions +$$$ --- * :fast_forward: To **apply** all unapplied plans from this pull request, comment: @@ -472,22 +563,33 @@ $$$ { Workspace: "workspace", RepoRelDir: "path", - PolicyCheckSuccess: &models.PolicyCheckSuccess{ - PolicyCheckOutput: "4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions", - LockURL: "lock-url", - ApplyCmd: "atlantis apply -d path -w workspace", - RePlanCmd: "atlantis plan -d path -w workspace", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + models.PolicySetResult{ + PolicySetName: "policy1", + ConftestOutput: "4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions", + Passed: true, + }, + }, + LockURL: "lock-url", + ApplyCmd: "atlantis apply -d path -w workspace", + RePlanCmd: "atlantis plan -d path -w workspace", }, }, { Workspace: "workspace", RepoRelDir: "path2", ProjectName: "projectname", - PolicyCheckSuccess: &models.PolicyCheckSuccess{ - PolicyCheckOutput: "4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions", - LockURL: "lock-url2", - ApplyCmd: "atlantis apply -d path2 -w workspace", - RePlanCmd: "atlantis plan -d path2 -w workspace", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + models.PolicySetResult{ + PolicySetName: "policy1", + ConftestOutput: "4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions", + Passed: true, + }, + }, LockURL: "lock-url2", + ApplyCmd: "atlantis apply -d path2 -w workspace", + RePlanCmd: "atlantis plan -d path2 -w workspace", }, }, }, @@ -498,10 +600,12 @@ $$$ 1. project: $projectname$ dir: $path2$ workspace: $workspace$ ### 1. dir: $path$ workspace: $workspace$ +#### Policy Set: $policy1$ $$$diff 4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions $$$ + * :arrow_forward: To **apply** this plan, comment: * $atlantis apply -d path -w workspace$ * :put_litter_in_its_place: To **delete** this plan click [here](lock-url) @@ -510,10 +614,12 @@ $$$ --- ### 2. project: $projectname$ dir: $path2$ workspace: $workspace$ +#### Policy Set: $policy1$ $$$diff 4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions $$$ + * :arrow_forward: To **apply** this plan, comment: * $atlantis apply -d path2 -w workspace$ * :put_litter_in_its_place: To **delete** this plan click [here](lock-url2) @@ -670,17 +776,34 @@ $$$ { Workspace: "workspace", RepoRelDir: "path", - PolicyCheckSuccess: &models.PolicyCheckSuccess{ - PolicyCheckOutput: "4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions", - LockURL: "lock-url", - ApplyCmd: "atlantis apply -d path -w workspace", - RePlanCmd: "atlantis plan -d path -w workspace", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + models.PolicySetResult{ + PolicySetName: "policy1", + ConftestOutput: "4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions", + Passed: true, + }, + }, LockURL: "lock-url", + ApplyCmd: "atlantis apply -d path -w workspace", + RePlanCmd: "atlantis plan -d path -w workspace", }, }, { Workspace: "workspace", RepoRelDir: "path2", Failure: "failure", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + models.PolicySetResult{ + PolicySetName: "policy1", + ConftestOutput: "4 tests, 2 passed, 0 warnings, 2 failures, 0 exceptions", + Passed: false, + ReqApprovals: 1, + }, + }, LockURL: "lock-url", + ApplyCmd: "atlantis apply -d path -w workspace", + RePlanCmd: "atlantis plan -d path -w workspace", + }, }, { Workspace: "workspace", @@ -697,10 +820,12 @@ $$$ 1. project: $projectname$ dir: $path3$ workspace: $workspace$ ### 1. dir: $path$ workspace: $workspace$ +#### Policy Set: $policy1$ $$$diff 4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions $$$ + * :arrow_forward: To **apply** this plan, comment: * $atlantis apply -d path -w workspace$ * :put_litter_in_its_place: To **delete** this plan click [here](lock-url) @@ -710,6 +835,21 @@ $$$ --- ### 2. dir: $path2$ workspace: $workspace$ **Policy Check Failed**: failure +#### Policy Set: $policy1$ +$$$diff +4 tests, 2 passed, 0 warnings, 2 failures, 0 exceptions +$$$ + + +#### Policy Approval Status: +$$$ +policy set: policy1: requires: 1 approval(s), have: 0. +$$$ +* :heavy_check_mark: To **approve** this project, comment: + * $$ +* :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 path -w workspace$ --- ### 3. project: $projectname$ dir: $path3$ workspace: $workspace$ @@ -717,15 +857,14 @@ $$$ $$$ error $$$ -* :heavy_check_mark: To **approve** failing policies an authorized approver can comment: - * $atlantis approve_policies$ -* :repeat: Or, address the policy failure by modifying the codebase and re-planning. --- -* :fast_forward: To **apply** all unapplied plans from this pull request, comment: - * $atlantis apply$ +* :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$ `, }, { @@ -1152,11 +1291,12 @@ $$$ // Run policy check with a custom template to validate custom template rendering. func TestRenderCustomPolicyCheckTemplate_DisableApplyAll(t *testing.T) { + var exp string tmpDir := t.TempDir() filePath := fmt.Sprintf("%s/templates.tmpl", tmpDir) _, err := os.Create(filePath) Ok(t, err) - err = os.WriteFile(filePath, []byte("{{ define \"policyCheckSuccessUnwrapped\" -}}somecustometext{{- end}}\n"), 0600) + err = os.WriteFile(filePath, []byte("{{ define \"PolicyCheckResultsUnwrapped\" -}}somecustometext{{- end}}\n"), 0600) Ok(t, err) r := events.NewMarkdownRenderer( false, // gitlabSupportsCommonMark @@ -1175,17 +1315,36 @@ func TestRenderCustomPolicyCheckTemplate_DisableApplyAll(t *testing.T) { { Workspace: "workspace", RepoRelDir: "path", - PolicyCheckSuccess: &models.PolicyCheckSuccess{ - PolicyCheckOutput: "4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions", - LockURL: "lock-url", - ApplyCmd: "atlantis apply -d path -w workspace", - RePlanCmd: "atlantis plan -d path -w workspace", + PolicyCheckResults: &models.PolicyCheckResults{ + PolicySetResults: []models.PolicySetResult{ + models.PolicySetResult{ + PolicySetName: "policy1", + ConftestOutput: "4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions", + Passed: true, + }, + }, LockURL: "lock-url", + ApplyCmd: "atlantis apply -d path -w workspace", + RePlanCmd: "atlantis plan -d path -w workspace", }, }, }, }, command.PolicyCheck, "", "log", false, models.Github) - exp := "Ran Policy Check for dir: `path` workspace: `workspace`\n\nsomecustometext" - Equals(t, exp, rendered) + exp = `Ran Policy Check for dir: $path$ workspace: $workspace$ + +#### Policy Set: $policy1$ +$$$diff +4 tests, 4 passed, 0 warnings, 0 failures, 0 exceptions +$$$ + + +* :arrow_forward: To **apply** this plan, comment: + * $atlantis apply -d path -w workspace$ +* :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 path -w workspace$` + + expWithBackticks := strings.Replace(exp, "$", "`", -1) + Equals(t, expWithBackticks, rendered) } // Test that if folding is disabled that it's not used. diff --git a/server/events/mocks/mock_comment_building.go b/server/events/mocks/mock_comment_building.go index e8a1dbb338..371daab715 100644 --- a/server/events/mocks/mock_comment_building.go +++ b/server/events/mocks/mock_comment_building.go @@ -39,6 +39,21 @@ func (mock *MockCommentBuilder) BuildApplyComment(_param0 string, _param1 string return ret0 } +func (mock *MockCommentBuilder) BuildApprovePoliciesComment(_param0 string, _param1 string, _param2 string) string { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockCommentBuilder().") + } + params := []pegomock.Param{_param0, _param1, _param2} + result := pegomock.GetGenericMockFrom(mock).Invoke("BuildApprovePoliciesComment", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) + var ret0 string + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(string) + } + } + return ret0 +} + func (mock *MockCommentBuilder) BuildPlanComment(_param0 string, _param1 string, _param2 string, _param3 []string) string { if mock == nil { panic("mock must not be nil. Use myMock := NewMockCommentBuilder().") diff --git a/server/events/models/models.go b/server/events/models/models.go index 66ecb47d0b..3145715e65 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -366,6 +366,21 @@ type PlanSuccess struct { HasDiverged bool } +type PolicySetResult struct { + PolicySetName string + ConftestOutput string + Passed bool + ReqApprovals int + CurApprovals int +} + +// PolicySetApproval tracks the number of approvals a given policy set has. +type PolicySetStatus struct { + PolicySetName string + Passed bool + Approvals int +} + // Summary regexes var ( reChangesOutside = regexp.MustCompile(`Note: Objects have changed outside of Terraform`) @@ -411,16 +426,18 @@ func (p PlanSuccess) DiffMarkdownFormattedTerraformOutput() string { return strings.TrimSpace(formattedTerraformOutput) } -// PolicyCheckSuccess is the result of a successful policy check run. -type PolicyCheckSuccess struct { - // PolicyCheckOutput is the output from policy check binary(conftest|opa) - PolicyCheckOutput string +// PolicyCheckResults is the result of a successful policy check run. +type PolicyCheckResults struct { + // 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. LockURL string // RePlanCmd is the command that users should run to re-plan this project. RePlanCmd string // ApplyCmd is the command that users should run to apply this plan. ApplyCmd string + // ApprovePoliciesCmd is the command that users should run to approve policies for this plan. + ApprovePoliciesCmd string // HasDiverged is true if we're using the checkout merge strategy and the // branch we're merging into has been updated since we cloned and merged // it. @@ -443,15 +460,53 @@ type StateRmSuccess struct { RePlanCmd string } -// Summary extracts one line summary of policy check. -func (p *PolicyCheckSuccess) Summary() string { +func (p *PolicyCheckResults) CombinedOutput() string { + combinedOutput := "" + for _, psResult := range p.PolicySetResults { + // accounting for json output from conftest. + for _, psResultLine := range strings.Split(psResult.ConftestOutput, "\\n") { + combinedOutput = fmt.Sprintf("%s\n%s", combinedOutput, psResultLine) + } + } + return combinedOutput +} + +// Summary extracts one line summary of each policy check. +func (p *PolicyCheckResults) Summary() string { note := "" + for _, policySetResult := range p.PolicySetResults { + r := regexp.MustCompile(`\d+ tests?, \d+ passed, \d+ warnings?, \d+ failures?, \d+ exceptions?(, \d skipped)?`) + if match := r.FindString(policySetResult.ConftestOutput); match != "" { + note = fmt.Sprintf("%s\npolicy set: %s: %s", note, policySetResult.PolicySetName, match) + } + } + return strings.Trim(note, "\n") +} - r := regexp.MustCompile(`\d+ tests?, \d+ passed, \d+ warnings?, \d+ failures?, \d+ exceptions?(, \d skipped)?`) - if match := r.FindString(p.PolicyCheckOutput); match != "" { - return note + match +// PolicyCleared is used to determine if policies have all succeeded or been approved. +func (p *PolicyCheckResults) PolicyCleared() bool { + passing := true + for _, policySetResult := range p.PolicySetResults { + if !policySetResult.Passed && (policySetResult.CurApprovals != policySetResult.ReqApprovals) { + passing = false + } + } + return passing +} + +// PolicySummary returns a summary of the current approval state of policy sets. +func (p *PolicyCheckResults) PolicySummary() string { + var summary []string + for _, policySetResult := range p.PolicySetResults { + if policySetResult.Passed { + summary = append(summary, fmt.Sprintf("policy set: %s: passed.", policySetResult.PolicySetName)) + } else if policySetResult.CurApprovals == policySetResult.ReqApprovals { + summary = append(summary, fmt.Sprintf("policy set: %s: approved.", policySetResult.PolicySetName)) + } else { + summary = append(summary, fmt.Sprintf("policy set: %s: requires: %d approval(s), have: %d.", policySetResult.PolicySetName, policySetResult.ReqApprovals, policySetResult.CurApprovals)) + } } - return note + return strings.Join(summary, "\n") } type VersionSuccess struct { @@ -482,6 +537,8 @@ type ProjectStatus struct { Workspace string RepoRelDir string ProjectName string + // PolicySetApprovals tracks the approval status of every PolicySet for a Project. + PolicyStatus []PolicySetStatus // Status is the status of where this project is at in the planning cycle. Status ProjectPlanStatus } diff --git a/server/events/models/models_test.go b/server/events/models/models_test.go index eb08145fbf..3e13e8a520 100644 --- a/server/events/models/models_test.go +++ b/server/events/models/models_test.go @@ -419,18 +419,160 @@ func TestPlanSuccess_DiffSummary(t *testing.T) { } } -func TestPolicyCheckSuccess_Summary(t *testing.T) { - cases := []string{ - "20 tests, 19 passed, 2 warnings, 0 failures, 0 exceptions", - "3 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 1 skipped", - "1 test, 0 passed, 1 warning, 1 failure, 1 exception", +func TestPolicyCheckResults_Summary(t *testing.T) { + cases := []struct { + description string + policysetResults []models.PolicySetResult + exp string + }{ + { + description: "test single format with single policy set", + policysetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ConftestOutput: "20 tests, 19 passed, 2 warnings, 0 failures, 0 exceptions", + }, + }, + exp: "policy set: policy1: 20 tests, 19 passed, 2 warnings, 0 failures, 0 exceptions", + }, + { + description: "test multiple formats with multiple policy sets", + policysetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ConftestOutput: "20 tests, 19 passed, 2 warnings, 0 failures, 0 exceptions", + }, + { + PolicySetName: "policy2", + ConftestOutput: "3 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 1 skipped", + }, + { + PolicySetName: "policy3", + ConftestOutput: "1 test, 0 passed, 1 warning, 1 failure, 1 exception", + }, + }, + exp: `policy set: policy1: 20 tests, 19 passed, 2 warnings, 0 failures, 0 exceptions +policy set: policy2: 3 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 1 skipped +policy set: policy3: 1 test, 0 passed, 1 warning, 1 failure, 1 exception`, + }, } - for i, summary := range cases { - t.Run(fmt.Sprintf("summary %d", i), func(t *testing.T) { - pcs := models.PolicyCheckSuccess{ - PolicyCheckOutput: `WARN - - main - example main package\n` + summary, + for _, summary := range cases { + t.Run(summary.description, func(t *testing.T) { + pcs := models.PolicyCheckResults{ + PolicySetResults: summary.policysetResults, + } + Equals(t, summary.exp, pcs.Summary()) + }) + } +} + +// Test PolicyCleared and PolicySummary +func TestPolicyCheckResults_PolicyFuncs(t *testing.T) { + cases := []struct { + description string + policysetResults []models.PolicySetResult + policyClearedExp bool + policySummaryExp string + }{ + { + description: "single policy set, not passed", + policysetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + Passed: false, + ReqApprovals: 1, + }, + }, + policyClearedExp: false, + policySummaryExp: "policy set: policy1: requires: 1 approval(s), have: 0.", + }, + { + description: "single policy set, passed", + policysetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + Passed: true, + ReqApprovals: 1, + }, + }, + policyClearedExp: true, + policySummaryExp: "policy set: policy1: passed.", + }, + { + description: "single policy set, fully approved", + policysetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + Passed: false, + ReqApprovals: 1, + CurApprovals: 1, + }, + }, + policyClearedExp: true, + policySummaryExp: "policy set: policy1: approved.", + }, + { + description: "multiple policy sets, different states.", + policysetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + Passed: false, + ReqApprovals: 2, + CurApprovals: 0, + }, + { + PolicySetName: "policy2", + Passed: false, + ReqApprovals: 1, + CurApprovals: 1, + }, + { + PolicySetName: "policy3", + Passed: true, + ReqApprovals: 1, + CurApprovals: 0, + }, + }, + policyClearedExp: false, + policySummaryExp: `policy set: policy1: requires: 2 approval(s), have: 0. +policy set: policy2: approved. +policy set: policy3: passed.`, + }, + { + description: "multiple policy sets, all cleared.", + policysetResults: []models.PolicySetResult{ + { + PolicySetName: "policy1", + Passed: false, + ReqApprovals: 2, + CurApprovals: 2, + }, + { + PolicySetName: "policy2", + Passed: false, + ReqApprovals: 1, + CurApprovals: 1, + }, + { + PolicySetName: "policy3", + Passed: true, + ReqApprovals: 1, + CurApprovals: 0, + }, + }, + policyClearedExp: true, + policySummaryExp: `policy set: policy1: approved. +policy set: policy2: approved. +policy set: policy3: passed.`, + }, + } + for _, summary := range cases { + t.Run(summary.description, func(t *testing.T) { + pcs := models.PolicyCheckResults{ + PolicySetResults: summary.policysetResults, } - Equals(t, summary, pcs.Summary()) + Equals(t, summary.policyClearedExp, pcs.PolicyCleared()) + Equals(t, summary.policySummaryExp, pcs.PolicySummary()) }) } } diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 37fe51301a..bf1508d1c8 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -246,7 +246,11 @@ func (p *DefaultProjectCommandBuilder) BuildApplyCommands(ctx *command.Context, } func (p *DefaultProjectCommandBuilder) BuildApprovePoliciesCommands(ctx *command.Context, cmd *CommentCommand) ([]command.ProjectContext, error) { - return p.buildAllProjectCommandsByPlan(ctx, cmd) + if !cmd.IsForSpecificProject() { + return p.buildAllProjectCommandsByPlan(ctx, cmd) + } + pac, err := p.buildProjectCommand(ctx, cmd) + return pac, err } func (p *DefaultProjectCommandBuilder) BuildVersionCommands(ctx *command.Context, cmd *CommentCommand) ([]command.ProjectContext, error) { diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 675f40de60..23705368f2 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -64,6 +64,7 @@ workflows: repoCfg: "", expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: false, @@ -120,6 +121,7 @@ projects: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: true, @@ -180,6 +182,7 @@ projects: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: true, @@ -248,6 +251,7 @@ projects: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: true, @@ -403,6 +407,7 @@ workflows: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: true, @@ -465,6 +470,7 @@ projects: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: true, @@ -530,6 +536,7 @@ workflows: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: true, @@ -581,6 +588,7 @@ projects: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: false, @@ -793,6 +801,7 @@ projects: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -p myproject_1", + ApprovePoliciesCmd: "atlantis approve_policies -p myproject_1", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: true, @@ -969,6 +978,7 @@ repos: repoCfg: "", expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: false, @@ -1030,6 +1040,7 @@ workflows: `, expCtx: command.ProjectContext{ ApplyCmd: "atlantis apply -d project1 -w myworkspace", + ApprovePoliciesCmd: "atlantis approve_policies -d project1 -w myworkspace", BaseRepo: baseRepo, EscapedCommentArgs: []string{`\f\l\a\g`}, AutomergeEnabled: true, @@ -1054,6 +1065,7 @@ workflows: Workspace: "myworkspace", PolicySets: emptyPolicySets, RepoLocking: true, + PolicySetTarget: "", }, expPolicyCheckSteps: []string{"policy_check"}, }, diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 60a4235e44..bdc861363d 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -129,6 +129,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( ctx, cmdName, cb.CommentBuilder.BuildApplyComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, prjCfg.AutoMergeDisabled), + cb.CommentBuilder.BuildApprovePoliciesComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name), cb.CommentBuilder.BuildPlanComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, commentFlags), prjCfg, steps, @@ -191,6 +192,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( ctx, command.PolicyCheck, cb.CommentBuilder.BuildApplyComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, prjCfg.AutoMergeDisabled), + cb.CommentBuilder.BuildApprovePoliciesComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name), cb.CommentBuilder.BuildPlanComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, commentFlags), prjCfg, steps, @@ -213,6 +215,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( func newProjectCommandContext(ctx *command.Context, cmd command.Name, applyCmd string, + approvePoliciesCmd string, planCmd string, projCfg valid.MergedProjectCfg, steps []valid.Step, @@ -227,6 +230,7 @@ func newProjectCommandContext(ctx *command.Context, ) command.ProjectContext { var projectPlanStatus models.ProjectPlanStatus + var projectPolicyStatus []models.PolicySetStatus if ctx.PullStatus != nil { for _, project := range ctx.PullStatus.Projects { @@ -234,11 +238,13 @@ func newProjectCommandContext(ctx *command.Context, // if name is not used, let's match the directory if projCfg.Name == "" && project.RepoRelDir == projCfg.RepoRelDir { projectPlanStatus = project.Status + projectPolicyStatus = project.PolicyStatus break } if projCfg.Name != "" && project.ProjectName == projCfg.Name { projectPlanStatus = project.Status + projectPolicyStatus = project.PolicyStatus break } } @@ -247,6 +253,7 @@ func newProjectCommandContext(ctx *command.Context, return command.ProjectContext{ CommandName: cmd, ApplyCmd: applyCmd, + ApprovePoliciesCmd: approvePoliciesCmd, BaseRepo: ctx.Pull.BaseRepo, EscapedCommentArgs: escapedCommentArgs, AutomergeEnabled: automergeEnabled, @@ -261,6 +268,7 @@ func newProjectCommandContext(ctx *command.Context, Log: ctx.Log, Scope: scope, ProjectPlanStatus: projectPlanStatus, + ProjectPolicyStatus: projectPolicyStatus, Pull: ctx.Pull, ProjectName: projCfg.Name, PlanRequirements: projCfg.PlanRequirements, @@ -274,6 +282,7 @@ func newProjectCommandContext(ctx *command.Context, Verbose: verbose, Workspace: projCfg.Workspace, PolicySets: policySets, + PolicySetTarget: ctx.PolicySet, PullReqStatus: pullStatus, JobID: uuid.New().String(), ExecutionOrderGroup: projCfg.ExecutionOrderGroup, diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index ef1bd55f6d..084a0613a4 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -19,11 +19,15 @@ import ( "path/filepath" "strings" + "encoding/json" + + "github.com/hashicorp/go-multierror" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/core/runtime" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/events/webhooks" "github.com/runatlantis/atlantis/server/logging" ) @@ -197,6 +201,7 @@ func (p *ProjectOutputWrapper) updateProjectPRStatus(commandName command.Name, c // DefaultProjectCommandRunner implements ProjectCommandRunner. type DefaultProjectCommandRunner struct { + VcsClient vcs.Client Locker ProjectLocker LockURLGenerator LockURLGenerator InitStepRunner StepRunner @@ -236,7 +241,7 @@ func (p *DefaultProjectCommandRunner) PolicyCheck(ctx command.ProjectContext) co policySuccess, failure, err := p.doPolicyCheck(ctx) return command.ProjectResult{ Command: command.PolicyCheck, - PolicyCheckSuccess: policySuccess, + PolicyCheckResults: policySuccess, Error: err, Failure: failure, RepoRelDir: ctx.RepoRelDir, @@ -265,7 +270,7 @@ func (p *DefaultProjectCommandRunner) ApprovePolicies(ctx command.ProjectContext Command: command.PolicyCheck, Failure: failure, Error: err, - PolicyCheckSuccess: approvedOut, + PolicyCheckResults: approvedOut, RepoRelDir: ctx.RepoRelDir, Workspace: ctx.Workspace, ProjectName: ctx.ProjectName, @@ -314,17 +319,93 @@ func (p *DefaultProjectCommandRunner) StateRm(ctx command.ProjectContext) comman } } -func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectContext) (*models.PolicyCheckSuccess, string, error) { +func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectContext) (*models.PolicyCheckResults, string, error) { + // Acquire Atlantis lock for this repo/dir/workspace. + lockAttempt, err := p.Locker.TryLock(ctx.Log, ctx.Pull, ctx.User, ctx.Workspace, models.NewProject(ctx.Pull.BaseRepo.FullName, ctx.RepoRelDir), ctx.RepoLocking) + if err != nil { + return nil, "", errors.Wrap(err, "acquiring lock") + } + if !lockAttempt.LockAcquired { + return nil, lockAttempt.LockFailureReason, nil + } + ctx.Log.Debug("acquired lock for project") + + // Acquire internal lock for the directory we're going to operate in. + unlockFn, err := p.WorkingDirLocker.TryLock(ctx.Pull.BaseRepo.FullName, ctx.Pull.Num, ctx.Workspace, ctx.RepoRelDir) + if err != nil { + return nil, "", err + } + defer unlockFn() - // TODO: Make this a bit smarter - // without checking some sort of state that the policy check has indeed passed this is likely to cause issues + teams := []string{} - return &models.PolicyCheckSuccess{ - PolicyCheckOutput: "Policies approved", - }, "", nil + policySetCfg := ctx.PolicySets + + // Only query the users team membership if any teams have been configured as owners on any policy set(s). + if policySetCfg.HasTeamOwners() { + // A convenient way to access vcsClient. Not sure if best way. + userTeams, err := p.VcsClient.GetTeamNamesForUser(ctx.Pull.BaseRepo, ctx.User) + if err != nil { + ctx.Log.Err("unable to get team membership for user: %s", err) + return nil, "", err + } + teams = append(teams, userTeams...) + } + isAdmin := policySetCfg.Owners.IsOwner(ctx.User.Username, teams) + + var failure string + + // Run over each policy set for the project and perform appropriate approval. + var prjPolicySetResults []models.PolicySetResult + var prjErr error + allPassed := true + for _, policySet := range policySetCfg.PolicySets { + isOwner := policySet.Owners.IsOwner(ctx.User.Username, teams) || isAdmin + prjPolicyStatus := ctx.ProjectPolicyStatus + for i, policyStatus := range prjPolicyStatus { + ignorePolicy := false + if policySet.Name == policyStatus.PolicySetName { + // Policy set either passed or has sufficient approvals. Move on. + if policyStatus.Passed || (policyStatus.Approvals == policySet.ApproveCount) { + ignorePolicy = true + } + // Set ignore flag if targeted policy does not match. + if ctx.PolicySetTarget != "" && (ctx.PolicySetTarget != policySet.Name) { + ignorePolicy = true + } + // Increment approval if user is owner. + if isOwner && !ignorePolicy { + prjPolicyStatus[i].Approvals = policyStatus.Approvals + 1 + // User is not authorized to approve policy set. + } else if !ignorePolicy { + prjErr = multierror.Append(prjErr, fmt.Errorf("policy set: %s user %s is not a policy owner - please contact policy owners to approve failing policies", policySet.Name, ctx.User.Username)) + } + // Still bubble up this failure, even if policy set is not targeted. + if !policyStatus.Passed && (prjPolicyStatus[i].Approvals != policySet.ApproveCount) { + allPassed = false + } + prjPolicySetResults = append(prjPolicySetResults, models.PolicySetResult{ + PolicySetName: policySet.Name, + Passed: policyStatus.Passed, + CurApprovals: prjPolicyStatus[i].Approvals, + ReqApprovals: policySet.ApproveCount, + }) + } + } + } + if !allPassed { + failure = `One or more policy sets require additional approval.` + } + return &models.PolicyCheckResults{ + LockURL: p.LockURLGenerator.GenerateLockURL(lockAttempt.LockKey), + PolicySetResults: prjPolicySetResults, + ApplyCmd: ctx.ApplyCmd, + RePlanCmd: ctx.RePlanCmd, + ApprovePoliciesCmd: ctx.ApprovePoliciesCmd, + }, failure, prjErr } -func (p *DefaultProjectCommandRunner) doPolicyCheck(ctx command.ProjectContext) (*models.PolicyCheckSuccess, string, error) { +func (p *DefaultProjectCommandRunner) doPolicyCheck(ctx command.ProjectContext) (*models.PolicyCheckResults, string, error) { // Acquire Atlantis lock for this repo/dir/workspace. // This should already be acquired from the prior plan operation. // if for some reason an unlock happens between the plan and policy check step @@ -376,23 +457,51 @@ func (p *DefaultProjectCommandRunner) doPolicyCheck(ctx command.ProjectContext) return nil, "", DirNotExistErr{RepoRelDir: ctx.RepoRelDir} } + var failure string outputs, err := p.runSteps(ctx.Steps, ctx, absPath) + var errs error if err != nil { - // Note: we are explicitly not unlocking the pr here since a failing policy check will require - // approval - return nil, "", fmt.Errorf("%s\n%s", err, strings.Join(outputs, "\n")) + for { + err = errors.Unwrap(err) + if err == nil { + break + } + // Exclude errors for failed policies + if !strings.Contains(err.Error(), "some policies failed") { + errs = multierror.Append(errs, err) + } + } + + if errs != nil { + // Note: we are explicitly not unlocking the pr here since a failing policy check will require + // approval + return nil, "", errs + } } - return &models.PolicyCheckSuccess{ - LockURL: p.LockURLGenerator.GenerateLockURL(lockAttempt.LockKey), - PolicyCheckOutput: strings.Join(outputs, "\n"), - RePlanCmd: ctx.RePlanCmd, - ApplyCmd: ctx.ApplyCmd, + var policySetResults []models.PolicySetResult + err = json.Unmarshal([]byte(strings.Join(outputs, "\n")), &policySetResults) + if err != nil { + return nil, "", err + } - // set this to false right now because we don't have this information - // TODO: refactor the templates in a sane way so we don't need this - HasDiverged: false, - }, "", nil + result := &models.PolicyCheckResults{ + LockURL: p.LockURLGenerator.GenerateLockURL(lockAttempt.LockKey), + PolicySetResults: policySetResults, + RePlanCmd: ctx.RePlanCmd, + ApplyCmd: ctx.ApplyCmd, + ApprovePoliciesCmd: ctx.ApprovePoliciesCmd, + } + + // Using this function instead of catching failed policy runs with errors, for cases when '--no-fail' is passed to conftest. + // One reason to pass such an arg to conftest would be to prevent workflow termination so custom run scripts + // can be run after the conftest step. + ctx.Log.Err(strings.Join(outputs, "\n")) + if !result.PolicyCleared() { + failure = "Some policy sets did not pass." + } + + return result, failure, nil } func (p *DefaultProjectCommandRunner) doPlan(ctx command.ProjectContext) (*models.PlanSuccess, string, error) { diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index 7762010753..92361efee4 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -29,6 +29,8 @@ import ( "github.com/runatlantis/atlantis/server/events/mocks" "github.com/runatlantis/atlantis/server/events/mocks/matchers" "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/models/testdata" + vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" jobmocks "github.com/runatlantis/atlantis/server/jobs/mocks" "github.com/runatlantis/atlantis/server/logging" . "github.com/runatlantis/atlantis/testing" @@ -746,3 +748,364 @@ type mockURLGenerator struct{} func (m mockURLGenerator) GenerateLockURL(lockID string) string { return "https://" + lockID } + +// Test approve policies logic. +func TestDefaultProjectCommandRunner_ApprovePolicies(t *testing.T) { + cases := []struct { + description string + + policySetCfg valid.PolicySets + policySetStatus []models.PolicySetStatus + userTeams []string // Teams the user is a member of + targetedPolicy string // Policy to target when running approvals + + expOut []models.PolicySetResult + expFailure string + hasErr bool + }{ + { + description: "When user is not an owner at any level, approve policy fails.", + hasErr: true, + policySetCfg: valid.PolicySets{ + Owners: valid.PolicyOwners{ + Users: []string{"someotheruser1"}, + }, + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 1, + Owners: valid.PolicyOwners{ + Teams: []string{"someotherteam"}, + }, + }, + }, + }, + expOut: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + }, + }, + expFailure: "One or more policy sets require additional approval.", + }, + { + description: "When user is a top-level owner, increment approval count on all policies.", + hasErr: false, + policySetCfg: valid.PolicySets{ + Owners: valid.PolicyOwners{ + Users: []string{testdata.User.Username}, + }, + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 1, + }, + { + Name: "policy2", + ApproveCount: 2, + }, + }, + }, + expOut: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + CurApprovals: 1, + }, + { + PolicySetName: "policy2", + ReqApprovals: 2, + CurApprovals: 1, + }, + }, + expFailure: "One or more policy sets require additional approval.", + }, + { + description: "When user is not a top-level owner, but an owner of a policy set, increment approval count only the policy set they are an owner of.", + hasErr: true, + policySetCfg: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Owners: valid.PolicyOwners{ + Users: []string{testdata.User.Username}, + }, + Name: "policy1", + ApproveCount: 1, + }, + { + Name: "policy2", + ApproveCount: 2, + }, + }, + }, + expOut: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + CurApprovals: 1, + }, + { + PolicySetName: "policy2", + ReqApprovals: 2, + CurApprovals: 0, + }, + }, + expFailure: "One or more policy sets require additional approval.", + }, + { + description: "When user is a top-level ownner through membership, increment approval on all policies.", + userTeams: []string{"someuserteam"}, + policySetCfg: valid.PolicySets{ + Owners: valid.PolicyOwners{ + Teams: []string{"someuserteam"}, + }, + PolicySets: []valid.PolicySet{ + { + Name: "policy1", + ApproveCount: 1, + }, + { + Name: "policy2", + ApproveCount: 1, + }, + }, + }, + expOut: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + CurApprovals: 1, + }, + { + PolicySetName: "policy2", + ReqApprovals: 1, + CurApprovals: 1, + }, + }, + expFailure: "", + }, + { + description: "When user is not a top-level owner, but is an owner of one policy set through nembership, increment approval only the policy to which they are an owner.", + hasErr: true, + userTeams: []string{"someuserteam"}, + policySetCfg: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Owners: valid.PolicyOwners{ + Teams: []string{"someuserteam"}, + }, + Name: "policy1", + ApproveCount: 1, + }, + { + Name: "policy2", + ApproveCount: 1, + }, + }, + }, + expOut: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + CurApprovals: 1, + }, + { + PolicySetName: "policy2", + ReqApprovals: 1, + CurApprovals: 0, + }, + }, + expFailure: "One or more policy sets require additional approval.", + }, + { + description: "Do not increment or error on passing or fully-approved policy sets.", + userTeams: []string{"someuserteam"}, + policySetCfg: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Owners: valid.PolicyOwners{ + Teams: []string{"someuserteam"}, + }, + Name: "policy1", + ApproveCount: 2, + }, + }, + }, + policySetStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Approvals: 2, + }, + }, + expOut: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 2, + CurApprovals: 2, + }, + }, + expFailure: ``, + hasErr: false, + }, + { + description: "Policies should not fail if they pass.", + userTeams: []string{"someuserteam"}, + policySetCfg: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Owners: valid.PolicyOwners{ + Teams: []string{"someuserteam"}, + }, + Name: "policy1", + ApproveCount: 2, + }, + }, + }, + policySetStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Passed: true, + Approvals: 0, + }, + }, + expOut: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 2, + CurApprovals: 0, + Passed: true, + }, + }, + expFailure: ``, + hasErr: false, + }, + { + description: "Non-targeted failing policies should still trigger failure when a targeted policy is cleared.", + userTeams: []string{"someuserteam"}, + targetedPolicy: "policy1", + policySetCfg: valid.PolicySets{ + PolicySets: []valid.PolicySet{ + { + Owners: valid.PolicyOwners{ + Teams: []string{"someuserteam"}, + }, + Name: "policy1", + ApproveCount: 1, + }, + { + Owners: valid.PolicyOwners{ + Teams: []string{"someuserteam"}, + }, + Name: "policy2", + ApproveCount: 1, + }, + }, + }, + policySetStatus: []models.PolicySetStatus{ + { + PolicySetName: "policy1", + Approvals: 0, + Passed: false, + }, + { + PolicySetName: "policy2", + Approvals: 0, + Passed: false, + }, + }, + expOut: []models.PolicySetResult{ + { + PolicySetName: "policy1", + ReqApprovals: 1, + CurApprovals: 1, + }, + { + PolicySetName: "policy2", + ReqApprovals: 1, + CurApprovals: 0, + }, + }, + expFailure: `One or more policy sets require additional approval.`, + hasErr: false, + }, + } + + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + RegisterMockTestingT(t) + mockVcsClient := vcsmocks.NewMockClient() + mockInit := mocks.NewMockStepRunner() + mockPlan := mocks.NewMockStepRunner() + mockApply := mocks.NewMockStepRunner() + mockRun := mocks.NewMockCustomStepRunner() + mockEnv := mocks.NewMockEnvStepRunner() + mockWorkingDir := mocks.NewMockWorkingDir() + mockLocker := mocks.NewMockProjectLocker() + mockSender := mocks.NewMockWebhooksSender() + + runner := events.DefaultProjectCommandRunner{ + Locker: mockLocker, + VcsClient: mockVcsClient, + LockURLGenerator: mockURLGenerator{}, + InitStepRunner: mockInit, + PlanStepRunner: mockPlan, + ApplyStepRunner: mockApply, + RunStepRunner: mockRun, + EnvStepRunner: mockEnv, + WorkingDir: mockWorkingDir, + Webhooks: mockSender, + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + } + repoDir := t.TempDir() + When(mockWorkingDir.GetWorkingDir( + matchers.AnyModelsRepo(), + matchers.AnyModelsPullRequest(), + AnyString(), + )).ThenReturn(repoDir, nil) + When(mockLocker.TryLock( + matchers.AnyLoggingSimpleLogging(), + matchers.AnyModelsPullRequest(), + matchers.AnyModelsUser(), + AnyString(), + matchers.AnyModelsProject(), + AnyBool(), + )).ThenReturn(&events.TryLockResponse{ + LockAcquired: true, + LockKey: "lock-key", + }, nil) + + var projPolicyStatus []models.PolicySetStatus + if c.policySetStatus == nil { + for _, p := range c.policySetCfg.PolicySets { + projPolicyStatus = append(projPolicyStatus, models.PolicySetStatus{ + PolicySetName: p.Name, + }) + } + } else { + projPolicyStatus = c.policySetStatus + } + + modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + When(runner.VcsClient.GetTeamNamesForUser(testdata.GithubRepo, testdata.User)).ThenReturn(c.userTeams, nil) + ctx := command.ProjectContext{ + User: testdata.User, + Log: logging.NewNoopLogger(t), + Workspace: "default", + RepoRelDir: ".", + PolicySets: c.policySetCfg, + ProjectPolicyStatus: projPolicyStatus, + Pull: modelPull, + PolicySetTarget: c.targetedPolicy, + } + + res := runner.ApprovePolicies(ctx) + Equals(t, c.expOut, res.PolicyCheckResults.PolicySetResults) + Equals(t, c.expFailure, res.Failure) + if c.hasErr == true { + Assert(t, res.Error != nil, "expecting error.") + } else { + Assert(t, res.Error == nil, "not expecting error.") + } + }) + } +} diff --git a/server/events/templates/failure.tmpl b/server/events/templates/failure.tmpl index 0bfd7403b5..712912a56f 100644 --- a/server/events/templates/failure.tmpl +++ b/server/events/templates/failure.tmpl @@ -1,3 +1,6 @@ {{ define "failure" -}} **{{ .Command }} Failed**: {{ .Failure }} +{{- if ne .RenderedContext ""}} +{{ .RenderedContext }} +{{- end }} {{ end -}} diff --git a/server/events/templates/multi_project_policy_unsuccessful.tmpl b/server/events/templates/multi_project_policy_unsuccessful.tmpl new file mode 100644 index 0000000000..2e8aafd95d --- /dev/null +++ b/server/events/templates/multi_project_policy_unsuccessful.tmpl @@ -0,0 +1,23 @@ +{{ define "multiProjectPolicyUnsuccessful" -}} +{{ template "multiProjectHeader" . }} +{{ $disableApplyAll := .DisableApplyAll -}} +{{ range $i, $result := .Results -}} +### {{ add $i 1 }}. {{ if $result.ProjectName }}project: `{{ $result.ProjectName }}` {{ end }}dir: `{{ $result.RepoRelDir }}` workspace: `{{ $result.Workspace }}` +{{ $result.Rendered }} + +{{ if ne $disableApplyAll true -}} +--- +{{ end -}} +{{ end -}} +{{ if ne .DisableApplyAll true -}} +{{ if and (gt (len .Results) 0) (not .PlansDeleted) -}} +* :heavy_check_mark: To **approve** all unapplied plans from this pull request, comment: + * `{{ .ExecutableName }} approve_policies` +* :put_litter_in_its_place: To delete all plans and locks for the PR, comment: + * `{{ .ExecutableName }} unlock` +* :repeat: To re-run policies **plan** this project again by commenting: + * `{{ .ExecutableName }} plan` +{{ end -}} +{{ end -}} +{{- template "log" . -}} +{{ end -}} diff --git a/server/events/templates/policy_check.tmpl b/server/events/templates/policy_check.tmpl new file mode 100644 index 0000000000..dcabbea963 --- /dev/null +++ b/server/events/templates/policy_check.tmpl @@ -0,0 +1,9 @@ +{{ define "policyCheck" -}} +{{ $policy_sets := . }} +{{ range $ps, $policy_sets }} +#### Policy Set: `{{ $ps.PolicySetName }}` +```diff +{{ $ps.ConftestOutput }} +``` +{{ end }} +{{ end }} diff --git a/server/events/templates/policy_check_results_unwrapped.tmpl b/server/events/templates/policy_check_results_unwrapped.tmpl new file mode 100644 index 0000000000..5d0a76895f --- /dev/null +++ b/server/events/templates/policy_check_results_unwrapped.tmpl @@ -0,0 +1,19 @@ +{{ define "policyCheckResultsUnwrapped" -}} +{{- if eq .Command "Policy Check" }} +{{ template "policyCheck" .PolicySetResults }} +{{- end }} +{{- if .PolicyCleared }} +* :arrow_forward: To **apply** this plan, comment: + * `{{ .ApplyCmd }}` +{{- else }} +#### Policy Approval Status: +``` +{{ .PolicyApprovalSummary }} +``` +* :heavy_check_mark: To **approve** this project, comment: + * `{{ .ApprovePoliciesCmd }}` +{{- end }} +* :put_litter_in_its_place: To **delete** this plan click [here]({{ .LockURL }}) +* :repeat: To re-run policies **plan** this project again by commenting: + * `{{ .RePlanCmd }}` +{{ end -}} diff --git a/server/events/templates/policy_check_results_wrapped.tmpl b/server/events/templates/policy_check_results_wrapped.tmpl new file mode 100644 index 0000000000..afe0c3fb27 --- /dev/null +++ b/server/events/templates/policy_check_results_wrapped.tmpl @@ -0,0 +1,27 @@ +{{ define "policyCheckResultsWrapped" -}} +
Show Output +{{- if eq .Command "Policy Check" }} +{{ template "policyCheck" .PolicySetResults }} +{{- end }} +{{- if .PolicyCleared }} +* :arrow_forward: To **apply** this plan, comment: + * `{{ .ApplyCmd }}` +{{- else }} +#### Policy Approval Status: +``` +{{ .PolicyApprovalSummary }} +``` +* :heavy_check_mark: To **approve** this project, comment: + * `{{ .ApprovePoliciesCmd }}` +{{- end }} +* :put_litter_in_its_place: To **delete** this plan click [here]({{ .LockURL }}) +* :repeat: To re-run policies **plan** this project again by commenting: + * `{{ .RePlanCmd }}` +
+{{- if eq .Command "Policy Check" }} + +``` +{{ .PolicyCheckSummary }} +``` +{{- end }} +{{ end -}} \ No newline at end of file diff --git a/server/events/templates/policy_check_success_unwrapped.tmpl b/server/events/templates/policy_check_success_unwrapped.tmpl deleted file mode 100644 index 663a00449b..0000000000 --- a/server/events/templates/policy_check_success_unwrapped.tmpl +++ /dev/null @@ -1,12 +0,0 @@ -{{ define "policyCheckSuccessUnwrapped" -}} -```diff -{{ .PolicyCheckOutput }} -``` - -* :arrow_forward: To **apply** this plan, comment: - * `{{ .ApplyCmd }}` -* :put_litter_in_its_place: To **delete** this plan click [here]({{ .LockURL }}) -* :repeat: To re-run policies **plan** this project again by commenting: - * `{{ .RePlanCmd }}` -{{ template "diverged" . }} -{{ end -}} diff --git a/server/events/templates/policy_check_success_wrapped.tmpl b/server/events/templates/policy_check_success_wrapped.tmpl deleted file mode 100644 index ed3ea59117..0000000000 --- a/server/events/templates/policy_check_success_wrapped.tmpl +++ /dev/null @@ -1,15 +0,0 @@ -{{ define "policyCheckSuccessWrapped" -}} -
Show Output - -```diff -{{ .PolicyCheckOutput }} -``` - -* :arrow_forward: To **apply** this plan, comment: - * `{{ .ApplyCmd }}` -* :put_litter_in_its_place: To **delete** this plan click [here]({{ .LockURL }}) -* :repeat: To re-run policies **plan** this project again by commenting: - * `{{ .RePlanCmd }}` -
-{{ .PolicyCheckSummary }}{{ template "diverged" . }} -{{ end -}} diff --git a/server/events/templates/single_project_policy_unsuccessful.tmpl b/server/events/templates/single_project_policy_unsuccessful.tmpl new file mode 100644 index 0000000000..6f40027dfa --- /dev/null +++ b/server/events/templates/single_project_policy_unsuccessful.tmpl @@ -0,0 +1,16 @@ +{{ define "singleProjectPolicyUnsuccessful" -}} +{{ $result := index .Results 0 -}} +Ran {{ .Command }} for {{ if $result.ProjectName }}project: `{{ $result.ProjectName }}` {{ end }}dir: `{{ $result.RepoRelDir }}` workspace: `{{ $result.Workspace }}` + +{{ $result.Rendered }} +{{ if ne .DisableApplyAll true }} +--- +* :heavy_check_mark: To **approve** all unapplied plans from this pull request, comment: + * `{{ .ExecutableName }} approve_policies` +* :put_litter_in_its_place: To delete all plans and locks for the PR, comment: + * `{{ .ExecutableName }} unlock` +* :repeat: To re-run policies **plan** this project again by commenting: + * `{{ .ExecutableName }} plan` +{{ end -}} +{{- template "log" . -}} +{{ end -}} diff --git a/server/events/templates/unwrapped_err.tmpl b/server/events/templates/unwrapped_err.tmpl index 9e8dbaec72..1be0a7ef29 100644 --- a/server/events/templates/unwrapped_err.tmpl +++ b/server/events/templates/unwrapped_err.tmpl @@ -2,9 +2,8 @@ **{{.Command}} Error** ``` {{.Error}} -```{{ if eq .Command "Policy Check" }} -* :heavy_check_mark: To **approve** failing policies an authorized approver can comment: - * `{{ .ExecutableName }} approve_policies` -* :repeat: Or, address the policy failure by modifying the codebase and re-planning. +``` +{{- if ne .RenderedContext ""}} +{{ .RenderedContext }} {{- end }} {{- end }} diff --git a/server/events/templates/wrapped_err.tmpl b/server/events/templates/wrapped_err.tmpl index 4ded250010..625ffdea03 100644 --- a/server/events/templates/wrapped_err.tmpl +++ b/server/events/templates/wrapped_err.tmpl @@ -5,5 +5,8 @@ ``` {{ .Error }} ``` +{{- if ne .RenderedContext "" }} +{{ .RenderedContext }} +{{- end }} {{ end -}} diff --git a/server/server.go b/server/server.go index 9613de5067..1cf323fff1 100644 --- a/server/server.go +++ b/server/server.go @@ -610,6 +610,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } projectCommandRunner := &events.DefaultProjectCommandRunner{ + VcsClient: vcsClient, Locker: projectLocker, LockURLGenerator: router, InitStepRunner: &runtime.InitStepRunner{ @@ -777,7 +778,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } commandRunner := &events.DefaultCommandRunner{ - VCSClient: vcsClient, GithubPullGetter: githubClient, GitlabMergeRequestGetter: gitlabClient, AzureDevopsPullGetter: azuredevopsClient,