Skip to content

Commit edfd096

Browse files
lee2shjamengual
authored andcommitted
feat: selfapprove flag for approving policies (runatlantis#4794)
Co-authored-by: PePe Amengual <[email protected]> Signed-off-by: kvanzuijlen <[email protected]>
1 parent 4dec726 commit edfd096

File tree

6 files changed

+73
-17
lines changed

6 files changed

+73
-17
lines changed

runatlantis.io/docs/policy-checking.md

+1
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ policies:
7171
- `source` - Tells atlantis where to fetch the policies from. Currently you can only host policies locally by using `local`.
7272
- `owners` - Defines the users/teams which are able to approve a specific policy set.
7373
- `approve_count` - Defines the number of approvals needed to bypass policy checks. Defaults to the top-level policies configuration, if not specified.
74+
- `prevent_self_approve` - Defines whether the PR author can approve policies
7475

7576
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.
7677

runatlantis.io/docs/server-side-repo-config.md

+6-5
Original file line numberDiff line numberDiff line change
@@ -607,11 +607,12 @@ mode: on_apply
607607

608608
### PolicySet
609609

610-
| Key | Type | Default | Required | Description |
611-
| ------ | ------ | ------- | -------- | -------------------------------------- |
612-
| name | string | none | yes | unique name for the policy set |
613-
| path | string | none | yes | path to the rego policies directory |
614-
| source | string | none | yes | only `local` is supported at this time |
610+
| Key | Type | Default | Required | Description |
611+
| ------ | ------ | ------- | -------- | --------------------------------------------------------------------------------------------------------------|
612+
| name | string | none | yes | unique name for the policy set |
613+
| path | string | none | yes | path to the rego policies directory |
614+
| source | string | none | yes | only `local` is supported at this time |
615+
| 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) |
615616

616617
### Metrics
617618

server/core/config/raw/policies.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,12 @@ func (o PolicyOwners) ToValid() valid.PolicyOwners {
7070
}
7171

7272
type PolicySet struct {
73-
Path string `yaml:"path" json:"path"`
74-
Source string `yaml:"source" json:"source"`
75-
Name string `yaml:"name" json:"name"`
76-
Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"`
77-
ApproveCount int `yaml:"approve_count,omitempty" json:"approve_count,omitempty"`
73+
Path string `yaml:"path" json:"path"`
74+
Source string `yaml:"source" json:"source"`
75+
Name string `yaml:"name" json:"name"`
76+
Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"`
77+
ApproveCount int `yaml:"approve_count,omitempty" json:"approve_count,omitempty"`
78+
PreventSelfApprove bool `yaml:"self_approve,omitempty" json:"prevent_self_approve,omitempty"`
7879
}
7980

8081
func (p PolicySet) Validate() error {
@@ -94,6 +95,7 @@ func (p PolicySet) ToValid() valid.PolicySet {
9495
policySet.Path = p.Path
9596
policySet.Source = p.Source
9697
policySet.ApproveCount = p.ApproveCount
98+
policySet.PreventSelfApprove = p.PreventSelfApprove
9799
policySet.Owners = p.Owners.ToValid()
98100

99101
return policySet

server/core/config/valid/policies.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,12 @@ type PolicyOwners struct {
2727
}
2828

2929
type PolicySet struct {
30-
Source string
31-
Path string
32-
Name string
33-
ApproveCount int
34-
Owners PolicyOwners
30+
Source string
31+
Path string
32+
Name string
33+
ApproveCount int
34+
Owners PolicyOwners
35+
PreventSelfApprove bool
3536
}
3637

3738
func (p *PolicySets) HasPolicies() bool {

server/events/project_command_runner.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte
377377
ignorePolicy = true
378378
}
379379
// Increment approval if user is owner.
380-
if isOwner && !ignorePolicy {
380+
if isOwner && !ignorePolicy && (ctx.User.Username != ctx.Pull.Author || !policySet.PreventSelfApprove) {
381381
if !ctx.ClearPolicyApproval {
382382
prjPolicyStatus[i].Approvals = policyStatus.Approvals + 1
383383
} else {
@@ -391,6 +391,7 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte
391391
if !policyStatus.Passed && (prjPolicyStatus[i].Approvals != policySet.ApproveCount) {
392392
allPassed = false
393393
}
394+
394395
prjPolicySetResults = append(prjPolicySetResults, models.PolicySetResult{
395396
PolicySetName: policySet.Name,
396397
Passed: policyStatus.Passed,

server/events/project_command_runner_test.go

+51-1
Original file line numberDiff line numberDiff line change
@@ -1168,6 +1168,56 @@ func TestDefaultProjectCommandRunner_ApprovePolicies(t *testing.T) {
11681168
expFailure: `One or more policy sets require additional approval.`,
11691169
hasErr: false,
11701170
},
1171+
{
1172+
description: "Policy Approval should not be the Author of the PR",
1173+
userTeams: []string{"someuserteam"},
1174+
clearPolicyApproval: false,
1175+
policySetCfg: valid.PolicySets{
1176+
PolicySets: []valid.PolicySet{
1177+
{
1178+
Owners: valid.PolicyOwners{
1179+
Users: []string{"lkysow"},
1180+
},
1181+
Name: "policy1",
1182+
ApproveCount: 1,
1183+
},
1184+
{
1185+
Owners: valid.PolicyOwners{
1186+
Users: []string{"lkysow"},
1187+
},
1188+
Name: "policy2",
1189+
ApproveCount: 1,
1190+
PreventSelfApprove: true,
1191+
},
1192+
},
1193+
},
1194+
policySetStatus: []models.PolicySetStatus{
1195+
{
1196+
PolicySetName: "policy1",
1197+
Approvals: 0,
1198+
Passed: false,
1199+
},
1200+
{
1201+
PolicySetName: "policy2",
1202+
Approvals: 0,
1203+
Passed: false,
1204+
},
1205+
},
1206+
expOut: []models.PolicySetResult{
1207+
{
1208+
PolicySetName: "policy1",
1209+
ReqApprovals: 1,
1210+
CurApprovals: 1,
1211+
},
1212+
{
1213+
PolicySetName: "policy2",
1214+
ReqApprovals: 1,
1215+
CurApprovals: 0,
1216+
},
1217+
},
1218+
expFailure: `One or more policy sets require additional approval.`,
1219+
hasErr: true,
1220+
},
11711221
}
11721222

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

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

0 commit comments

Comments
 (0)