feat: Enable discard-approval-on-plan for gitlab (#5388)

Author: moritz31

runatlantis.io/docs/server-configuration.md

@@ -487,7 +487,8 @@ and set `--autoplan-modules` to `false`.
   ATLANTIS_DISCARD_APPROVAL_ON_PLAN=true
   ```
 
-  If set, discard approval if a new plan has been executed. Currently only supported in Github.
+  If set, discard approval if a new plan has been executed. Currently only supported on GitHub and GitLab. For GitLab a bot, group or project token is required for this feature.  
+  Reference: [reset-approvals-of-a-merge-request](https://docs.gitlab.com/api/merge_request_approvals/#reset-approvals-of-a-merge-request)
 
 ### `--emoji-reaction`
 

server/events/command_runner_test.go

@@ -1003,7 +1003,7 @@ func TestRunAutoplanCommandWithError_DeletePlans(t *testing.T) {
 	// gets called twice: the first time before the plan starts, the second time after the plan errors
 	pendingPlanFinder.VerifyWasCalled(Times(2)).DeletePlans(tmp)
 
-	vcsClient.VerifyWasCalled(Times(0)).DiscardReviews(Any[models.Repo](), Any[models.PullRequest]())
+	vcsClient.VerifyWasCalled(Times(0)).DiscardReviews(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())
 }
 
 func TestRunGenericPlanCommand_DiscardApprovals(t *testing.T) {
@@ -1033,7 +1033,7 @@ func TestRunGenericPlanCommand_DiscardApprovals(t *testing.T) {
 	pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
 	lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num)
 
-	vcsClient.VerifyWasCalledOnce().DiscardReviews(Any[models.Repo](), Any[models.PullRequest]())
+	vcsClient.VerifyWasCalledOnce().DiscardReviews(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())
 }
 
 func TestFailedApprovalCreatesFailedStatusUpdate(t *testing.T) {

server/events/plan_command_runner.go

@@ -178,7 +178,7 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) {
 	}
 
 	if p.DiscardApprovalOnPlan {
-		if err = p.pullUpdater.VCSClient.DiscardReviews(baseRepo, pull); err != nil {
+		if err = p.pullUpdater.VCSClient.DiscardReviews(ctx.Log, baseRepo, pull); err != nil {
 			ctx.Log.Err("failed to remove approvals: %s", err)
 		}
 	}

server/events/vcs/azuredevops_client.go

@@ -186,7 +186,7 @@ func (g *AzureDevopsClient) PullIsApproved(logger logging.SimpleLogging, repo mo
 	return approvalStatus, nil
 }
 
-func (g *AzureDevopsClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error { //nolint: revive
+func (g *AzureDevopsClient) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error { //nolint: revive
 	// TODO implement
 	return nil
 }

server/events/vcs/bitbucketcloud/client.go

@@ -322,7 +322,7 @@ func (b *Client) prepRequest(method string, path string, body io.Reader) (*http.
 	return req, nil
 }
 
-func (b *Client) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
+func (b *Client) DiscardReviews(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) error {
 	// TODO implement
 	return nil
 }

server/events/vcs/bitbucketserver/client.go

@@ -197,7 +197,7 @@ func (b *Client) PullIsApproved(logger logging.SimpleLogging, repo models.Repo,
 	return approvalStatus, nil
 }
 
-func (b *Client) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
+func (b *Client) DiscardReviews(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) error {
 	// TODO implement
 	return nil
 }

server/events/vcs/client.go

@@ -39,7 +39,7 @@ type Client interface {
 	// url is an optional link that users should click on for more information
 	// about this status.
 	UpdateStatus(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error
-	DiscardReviews(repo models.Repo, pull models.PullRequest) error
+	DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error
 	MergePull(logger logging.SimpleLogging, pull models.PullRequest, pullOptions models.PullRequestOptions) error
 	MarkdownPullLink(pull models.PullRequest) (string, error)
 	GetTeamNamesForUser(logger logging.SimpleLogging, repo models.Repo, user models.User) ([]string, error)

server/events/vcs/gitea/client.go

@@ -337,7 +337,7 @@ func (c *GiteaClient) UpdateStatus(logger logging.SimpleLogging, repo models.Rep
 }
 
 // DiscardReviews discards / dismisses all pull request reviews
-func (c *GiteaClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error {
+func (c *GiteaClient) DiscardReviews(_ logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error {
 	page := 0
 	nextPage := 1
 

server/events/vcs/github_client.go

@@ -430,7 +430,8 @@ func (g *GithubClient) PullIsApproved(logger logging.SimpleLogging, repo models.
 }
 
 // DiscardReviews dismisses all reviews on a pull request
-func (g *GithubClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error {
+func (g *GithubClient) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error {
+	logger.Debug("Discarding all reviews on GitHub pull request %d", pull.Num)
 	reviewStatus, err := g.getPRReviews(repo, pull)
 	if err != nil {
 		return err

server/events/vcs/github_client_test.go

@@ -1410,6 +1410,7 @@ func TestGithubClient_GetTeamNamesForUser(t *testing.T) {
 }
 
 func TestGithubClient_DiscardReviews(t *testing.T) {
+	logger := logging.NewNoopLogger(t)
 	type ResponseDef struct {
 		httpCode int
 		body     string
@@ -1597,7 +1598,7 @@ func TestGithubClient_DiscardReviews(t *testing.T) {
 			client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
 			Ok(t, err)
 			defer disableSSLVerification()()
-			if err := client.DiscardReviews(tt.args.repo, tt.args.pull); (err != nil) != tt.wantErr {
+			if err := client.DiscardReviews(logger, tt.args.repo, tt.args.pull); (err != nil) != tt.wantErr {
 				t.Errorf("DiscardReviews() error = %v, wantErr %v", err, tt.wantErr)
 			}
 			Equals(t, responseLength, responseIndex) // check if all defined requests have been used

server/events/vcs/gitlab_client.go

@@ -577,8 +577,19 @@ func (g *GitlabClient) MarkdownPullLink(pull models.PullRequest) (string, error)
 	return fmt.Sprintf("!%d", pull.Num), nil
 }
 
-func (g *GitlabClient) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
-	// TODO implement
+// DiscardReviews discards all reviews on a pull request
+// This is only available with a bot token and otherwise will return 401 unauthorized
+// https://docs.gitlab.com/api/merge_request_approvals/#reset-approvals-of-a-merge-request
+func (g *GitlabClient) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error {
+	logger.Debug("Reset approvals for merge request %d", pull.Num)
+	resp, err := g.Client.MergeRequestApprovals.ResetApprovalsOfMergeRequest(repo.FullName, pull.Num)
+	if resp != nil {
+		logger.Debug("PUT /projects/%s/merge_requests/%d/reset_approvals returned: %d", repo.FullName, pull.Num, resp.StatusCode)
+	}
+	if err != nil {
+		return errors.Wrap(err, "unable to reset approvals")
+	}
+
 	return nil
 }
 

server/events/vcs/gitlab_client_test.go

@@ -1200,3 +1200,60 @@ func TestGitlabClient_GetTeamNamesForUser(t *testing.T) {
 		})
 	}
 }
+
+func TestGithubClient_DiscardReviews(t *testing.T) {
+	logger := logging.NewNoopLogger(t)
+	cases := []struct {
+		description   string
+		repoFullName  string
+		pullReqeustId int
+		wantErr       bool
+	}{
+		{
+			"success",
+			"runatlantis/atlantis",
+			42,
+			false,
+		},
+		{
+			"error",
+			"runatlantis/atlantis",
+			32,
+			true,
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.description, func(t *testing.T) {
+			testServer := httptest.NewServer(
+				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					switch r.RequestURI {
+					case "/api/v4/projects/runatlantis%2Fatlantis/merge_requests/42/reset_approvals":
+						w.WriteHeader(http.StatusOK)
+					case "/api/v4/projects/runatlantis%2Fatlantis/merge_requests/32/reset_approvals":
+						http.Error(w, "No bot token", http.StatusUnauthorized)
+					default:
+						t.Errorf("got unexpected request at %q", r.RequestURI)
+						http.Error(w, "not found", http.StatusNotFound)
+					}
+				}))
+			internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL))
+			Ok(t, err)
+			client := &GitlabClient{
+				Client:  internalClient,
+				Version: nil,
+			}
+
+			repo := models.Repo{
+				FullName: c.repoFullName,
+			}
+
+			pr := models.PullRequest{
+				Num: c.pullReqeustId,
+			}
+
+			if err := client.DiscardReviews(logger, repo, pr); (err != nil) != c.wantErr {
+				t.Errorf("DiscardReviews() error = %v", err)
+			}
+		})
+	}
+}

server/events/vcs/mocks/mock_client.go

@@ -42,7 +42,7 @@ func (a *NotConfiguredVCSClient) ReactToComment(logger logging.SimpleLogging, re
 func (a *NotConfiguredVCSClient) PullIsApproved(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) (models.ApprovalStatus, error) {
 	return models.ApprovalStatus{}, a.err()
 }
-func (a *NotConfiguredVCSClient) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
+func (a *NotConfiguredVCSClient) DiscardReviews(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) error {
 	return nil
 }
 func (a *NotConfiguredVCSClient) PullIsMergeable(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest, _ string, _ []string) (bool, error) {

server/events/vcs/not_configured_vcs_client.go

@@ -77,8 +77,8 @@ func (d *ClientProxy) PullIsApproved(logger logging.SimpleLogging, repo models.R
 	return d.clients[repo.VCSHost.Type].PullIsApproved(logger, repo, pull)
 }
 
-func (d *ClientProxy) DiscardReviews(repo models.Repo, pull models.PullRequest) error {
-	return d.clients[repo.VCSHost.Type].DiscardReviews(repo, pull)
+func (d *ClientProxy) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error {
+	return d.clients[repo.VCSHost.Type].DiscardReviews(logger, repo, pull)
 }
 
 func (d *ClientProxy) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string, ignoreVCSStatusNames []string) (bool, error) {