Skip to content

Commit 16ae727

Browse files
moritz31inkel
authored andcommitted
feat: Enable discard-approval-on-plan for gitlab (runatlantis#5388)
1 parent ecd8b36 commit 16ae727

15 files changed

+91
-20
lines changed

runatlantis.io/docs/server-configuration.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,8 @@ and set `--autoplan-modules` to `false`.
487487
ATLANTIS_DISCARD_APPROVAL_ON_PLAN=true
488488
```
489489

490-
If set, discard approval if a new plan has been executed. Currently only supported in Github.
490+
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.
491+
Reference: [reset-approvals-of-a-merge-request](https://docs.gitlab.com/api/merge_request_approvals/#reset-approvals-of-a-merge-request)
491492

492493
### `--emoji-reaction`
493494

server/events/command_runner_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1003,7 +1003,7 @@ func TestRunAutoplanCommandWithError_DeletePlans(t *testing.T) {
10031003
// gets called twice: the first time before the plan starts, the second time after the plan errors
10041004
pendingPlanFinder.VerifyWasCalled(Times(2)).DeletePlans(tmp)
10051005

1006-
vcsClient.VerifyWasCalled(Times(0)).DiscardReviews(Any[models.Repo](), Any[models.PullRequest]())
1006+
vcsClient.VerifyWasCalled(Times(0)).DiscardReviews(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())
10071007
}
10081008

10091009
func TestRunGenericPlanCommand_DiscardApprovals(t *testing.T) {
@@ -1033,7 +1033,7 @@ func TestRunGenericPlanCommand_DiscardApprovals(t *testing.T) {
10331033
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
10341034
lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num)
10351035

1036-
vcsClient.VerifyWasCalledOnce().DiscardReviews(Any[models.Repo](), Any[models.PullRequest]())
1036+
vcsClient.VerifyWasCalledOnce().DiscardReviews(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())
10371037
}
10381038

10391039
func TestFailedApprovalCreatesFailedStatusUpdate(t *testing.T) {

server/events/plan_command_runner.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) {
178178
}
179179

180180
if p.DiscardApprovalOnPlan {
181-
if err = p.pullUpdater.VCSClient.DiscardReviews(baseRepo, pull); err != nil {
181+
if err = p.pullUpdater.VCSClient.DiscardReviews(ctx.Log, baseRepo, pull); err != nil {
182182
ctx.Log.Err("failed to remove approvals: %s", err)
183183
}
184184
}

server/events/vcs/azuredevops_client.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func (g *AzureDevopsClient) PullIsApproved(logger logging.SimpleLogging, repo mo
186186
return approvalStatus, nil
187187
}
188188

189-
func (g *AzureDevopsClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error { //nolint: revive
189+
func (g *AzureDevopsClient) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error { //nolint: revive
190190
// TODO implement
191191
return nil
192192
}

server/events/vcs/bitbucketcloud/client.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ func (b *Client) prepRequest(method string, path string, body io.Reader) (*http.
322322
return req, nil
323323
}
324324

325-
func (b *Client) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
325+
func (b *Client) DiscardReviews(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) error {
326326
// TODO implement
327327
return nil
328328
}

server/events/vcs/bitbucketserver/client.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func (b *Client) PullIsApproved(logger logging.SimpleLogging, repo models.Repo,
197197
return approvalStatus, nil
198198
}
199199

200-
func (b *Client) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
200+
func (b *Client) DiscardReviews(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) error {
201201
// TODO implement
202202
return nil
203203
}

server/events/vcs/client.go

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

server/events/vcs/gitea/client.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ func (c *GiteaClient) UpdateStatus(logger logging.SimpleLogging, repo models.Rep
337337
}
338338

339339
// DiscardReviews discards / dismisses all pull request reviews
340-
func (c *GiteaClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error {
340+
func (c *GiteaClient) DiscardReviews(_ logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error {
341341
page := 0
342342
nextPage := 1
343343

server/events/vcs/github_client.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,8 @@ func (g *GithubClient) PullIsApproved(logger logging.SimpleLogging, repo models.
430430
}
431431

432432
// DiscardReviews dismisses all reviews on a pull request
433-
func (g *GithubClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error {
433+
func (g *GithubClient) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error {
434+
logger.Debug("Discarding all reviews on GitHub pull request %d", pull.Num)
434435
reviewStatus, err := g.getPRReviews(repo, pull)
435436
if err != nil {
436437
return err

server/events/vcs/github_client_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -1410,6 +1410,7 @@ func TestGithubClient_GetTeamNamesForUser(t *testing.T) {
14101410
}
14111411

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

server/events/vcs/gitlab_client.go

+13-2
Original file line numberDiff line numberDiff line change
@@ -577,8 +577,19 @@ func (g *GitlabClient) MarkdownPullLink(pull models.PullRequest) (string, error)
577577
return fmt.Sprintf("!%d", pull.Num), nil
578578
}
579579

580-
func (g *GitlabClient) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
581-
// TODO implement
580+
// DiscardReviews discards all reviews on a pull request
581+
// This is only available with a bot token and otherwise will return 401 unauthorized
582+
// https://docs.gitlab.com/api/merge_request_approvals/#reset-approvals-of-a-merge-request
583+
func (g *GitlabClient) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error {
584+
logger.Debug("Reset approvals for merge request %d", pull.Num)
585+
resp, err := g.Client.MergeRequestApprovals.ResetApprovalsOfMergeRequest(repo.FullName, pull.Num)
586+
if resp != nil {
587+
logger.Debug("PUT /projects/%s/merge_requests/%d/reset_approvals returned: %d", repo.FullName, pull.Num, resp.StatusCode)
588+
}
589+
if err != nil {
590+
return errors.Wrap(err, "unable to reset approvals")
591+
}
592+
582593
return nil
583594
}
584595

server/events/vcs/gitlab_client_test.go

+57
Original file line numberDiff line numberDiff line change
@@ -1200,3 +1200,60 @@ func TestGitlabClient_GetTeamNamesForUser(t *testing.T) {
12001200
})
12011201
}
12021202
}
1203+
1204+
func TestGithubClient_DiscardReviews(t *testing.T) {
1205+
logger := logging.NewNoopLogger(t)
1206+
cases := []struct {
1207+
description string
1208+
repoFullName string
1209+
pullReqeustId int
1210+
wantErr bool
1211+
}{
1212+
{
1213+
"success",
1214+
"runatlantis/atlantis",
1215+
42,
1216+
false,
1217+
},
1218+
{
1219+
"error",
1220+
"runatlantis/atlantis",
1221+
32,
1222+
true,
1223+
},
1224+
}
1225+
for _, c := range cases {
1226+
t.Run(c.description, func(t *testing.T) {
1227+
testServer := httptest.NewServer(
1228+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1229+
switch r.RequestURI {
1230+
case "/api/v4/projects/runatlantis%2Fatlantis/merge_requests/42/reset_approvals":
1231+
w.WriteHeader(http.StatusOK)
1232+
case "/api/v4/projects/runatlantis%2Fatlantis/merge_requests/32/reset_approvals":
1233+
http.Error(w, "No bot token", http.StatusUnauthorized)
1234+
default:
1235+
t.Errorf("got unexpected request at %q", r.RequestURI)
1236+
http.Error(w, "not found", http.StatusNotFound)
1237+
}
1238+
}))
1239+
internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL))
1240+
Ok(t, err)
1241+
client := &GitlabClient{
1242+
Client: internalClient,
1243+
Version: nil,
1244+
}
1245+
1246+
repo := models.Repo{
1247+
FullName: c.repoFullName,
1248+
}
1249+
1250+
pr := models.PullRequest{
1251+
Num: c.pullReqeustId,
1252+
}
1253+
1254+
if err := client.DiscardReviews(logger, repo, pr); (err != nil) != c.wantErr {
1255+
t.Errorf("DiscardReviews() error = %v", err)
1256+
}
1257+
})
1258+
}
1259+
}

server/events/vcs/mocks/mock_client.go

+4-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/events/vcs/not_configured_vcs_client.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (a *NotConfiguredVCSClient) ReactToComment(logger logging.SimpleLogging, re
4242
func (a *NotConfiguredVCSClient) PullIsApproved(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) (models.ApprovalStatus, error) {
4343
return models.ApprovalStatus{}, a.err()
4444
}
45-
func (a *NotConfiguredVCSClient) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
45+
func (a *NotConfiguredVCSClient) DiscardReviews(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) error {
4646
return nil
4747
}
4848
func (a *NotConfiguredVCSClient) PullIsMergeable(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest, _ string, _ []string) (bool, error) {

server/events/vcs/proxy.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ func (d *ClientProxy) PullIsApproved(logger logging.SimpleLogging, repo models.R
7777
return d.clients[repo.VCSHost.Type].PullIsApproved(logger, repo, pull)
7878
}
7979

80-
func (d *ClientProxy) DiscardReviews(repo models.Repo, pull models.PullRequest) error {
81-
return d.clients[repo.VCSHost.Type].DiscardReviews(repo, pull)
80+
func (d *ClientProxy) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error {
81+
return d.clients[repo.VCSHost.Type].DiscardReviews(logger, repo, pull)
8282
}
8383

8484
func (d *ClientProxy) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string, ignoreVCSStatusNames []string) (bool, error) {

0 commit comments

Comments
 (0)