Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: selfapprove flag for approving policies #4794

Merged
merged 5 commits into from
Aug 8, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions runatlantis.io/docs/policy-checking.md
Original file line number Diff line number Diff line change
@@ -71,6 +71,7 @@ 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.
- `prevent_self_approve` - Defines whether the PR author can approve policies

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`](custom-workflows.md#adding-extra-arguments-to-terraform-commands) via a custom workflow as shown in the below example.

11 changes: 6 additions & 5 deletions runatlantis.io/docs/server-side-repo-config.md
Original file line number Diff line number Diff line change
@@ -607,11 +607,12 @@ mode: on_apply

### PolicySet

| Key | Type | Default | Required | Description |
| ------ | ------ | ------- | -------- | -------------------------------------- |
| name | string | none | yes | unique name for the policy set |
| path | string | none | yes | path to the rego policies directory |
| source | string | none | yes | only `local` is supported at this time |
| Key | Type | Default | Required | Description |
| ------ | ------ | ------- | -------- | --------------------------------------------------------------------------------------------------------------|
| name | string | none | yes | unique name for the policy set |
| path | string | none | yes | path to the rego policies directory |
| source | string | none | yes | only `local` is supported at this time |
| prevent_self_approve | bool | false | no | Whether or not the author of PR can approve policies. Defaults to `false` (the author must also be in owners) |

### Metrics

12 changes: 7 additions & 5 deletions server/core/config/raw/policies.go
Original file line number Diff line number Diff line change
@@ -70,11 +70,12 @@ 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"`
ApproveCount int `yaml:"approve_count,omitempty" json:"approve_count,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"`
PreventSelfApprove bool `yaml:"self_approve,omitempty" json:"prevent_self_approve,omitempty"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PreventSelfApprove bool         `yaml:"self_approve,omitempty" json:"prevent_self_approve,omitempty"`

Shouldn't both yaml and json defs here have prevent_self_approve?

}

func (p PolicySet) Validate() error {
@@ -94,6 +95,7 @@ func (p PolicySet) ToValid() valid.PolicySet {
policySet.Path = p.Path
policySet.Source = p.Source
policySet.ApproveCount = p.ApproveCount
policySet.PreventSelfApprove = p.PreventSelfApprove
policySet.Owners = p.Owners.ToValid()

return policySet
11 changes: 6 additions & 5 deletions server/core/config/valid/policies.go
Original file line number Diff line number Diff line change
@@ -27,11 +27,12 @@ type PolicyOwners struct {
}

type PolicySet struct {
Source string
Path string
Name string
ApproveCount int
Owners PolicyOwners
Source string
Path string
Name string
ApproveCount int
Owners PolicyOwners
PreventSelfApprove bool
}

func (p *PolicySets) HasPolicies() bool {
3 changes: 2 additions & 1 deletion server/events/project_command_runner.go
Original file line number Diff line number Diff line change
@@ -377,7 +377,7 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte
ignorePolicy = true
}
// Increment approval if user is owner.
if isOwner && !ignorePolicy {
if isOwner && !ignorePolicy && (ctx.User.Username != ctx.Pull.Author || !policySet.PreventSelfApprove) {
if !ctx.ClearPolicyApproval {
prjPolicyStatus[i].Approvals = policyStatus.Approvals + 1
} else {
@@ -391,6 +391,7 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte
if !policyStatus.Passed && (prjPolicyStatus[i].Approvals != policySet.ApproveCount) {
allPassed = false
}

prjPolicySetResults = append(prjPolicySetResults, models.PolicySetResult{
PolicySetName: policySet.Name,
Passed: policyStatus.Passed,
52 changes: 51 additions & 1 deletion server/events/project_command_runner_test.go
Original file line number Diff line number Diff line change
@@ -1168,6 +1168,56 @@ func TestDefaultProjectCommandRunner_ApprovePolicies(t *testing.T) {
expFailure: `One or more policy sets require additional approval.`,
hasErr: false,
},
{
description: "Policy Approval should not be the Author of the PR",
userTeams: []string{"someuserteam"},
clearPolicyApproval: false,
policySetCfg: valid.PolicySets{
PolicySets: []valid.PolicySet{
{
Owners: valid.PolicyOwners{
Users: []string{"lkysow"},
},
Name: "policy1",
ApproveCount: 1,
},
{
Owners: valid.PolicyOwners{
Users: []string{"lkysow"},
},
Name: "policy2",
ApproveCount: 1,
PreventSelfApprove: true,
},
},
},
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: true,
},
}

for _, c := range cases {
@@ -1225,7 +1275,7 @@ func TestDefaultProjectCommandRunner_ApprovePolicies(t *testing.T) {
projPolicyStatus = c.policySetStatus
}

modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num}
modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num, Author: testdata.User.Username}
When(runner.VcsClient.GetTeamNamesForUser(testdata.GithubRepo, testdata.User)).ThenReturn(c.userTeams, nil)
ctx := command.ProjectContext{
User: testdata.User,