feat: selfapprove flag for approving policies (runatlantis#4794)

Co-authored-by: PePe Amengual <[email protected]>
terakoya76 · Dec 31, 2024 · 26b299e · 26b299e
1 parent c618ba1
commit 26b299e
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 17 deletions.
diff --git a/runatlantis.io/docs/policy-checking.md b/runatlantis.io/docs/policy-checking.md
@@ -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.
 

diff --git a/runatlantis.io/docs/server-side-repo-config.md b/runatlantis.io/docs/server-side-repo-config.md
@@ -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
 

diff --git a/server/core/config/raw/policies.go b/server/core/config/raw/policies.go
@@ -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"`
 }
 
 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

diff --git a/server/core/config/valid/policies.go b/server/core/config/valid/policies.go
@@ -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 {

diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go
@@ -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,

diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go
@@ -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,