diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index 9e0a907cf..1e658a244 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/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` diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 5927b6d97..7591521a4 100644 --- a/server/events/command_runner_test.go +++ b/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) { diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index 57771c609..5d38a2d4f 100644 --- a/server/events/plan_command_runner.go +++ b/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) } } diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index 01cf1f669..5dbae71cc 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/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 } diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index e56f2eb9f..6e10476bd 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/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 } diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index 47f61a526..61d2db706 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/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 } diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index 3344ffa6e..58feb26fd 100644 --- a/server/events/vcs/client.go +++ b/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) diff --git a/server/events/vcs/gitea/client.go b/server/events/vcs/gitea/client.go index e262d8d82..d1fa00dc8 100644 --- a/server/events/vcs/gitea/client.go +++ b/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 diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 89a79bf98..fc8eb1865 100644 --- a/server/events/vcs/github_client.go +++ b/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 diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index 6d54df4c5..f71a629f3 100644 --- a/server/events/vcs/github_client_test.go +++ b/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 diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 72280b127..30ac4de89 100644 --- a/server/events/vcs/gitlab_client.go +++ b/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 } diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index aaa85b0e5..693183012 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/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) + } + }) + } +} diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index 3b4bd7dbf..265fa4b60 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -41,11 +41,11 @@ func (mock *MockClient) CreateComment(logger logging.SimpleLogging, repo models. return _ret0 } -func (mock *MockClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error { +func (mock *MockClient) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") } - _params := []pegomock.Param{repo, pull} + _params := []pegomock.Param{logger, repo, pull} _result := pegomock.GetGenericMockFrom(mock).Invoke("DiscardReviews", _params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var _ret0 error if len(_result) != 0 { @@ -377,8 +377,8 @@ func (c *MockClient_CreateComment_OngoingVerification) GetAllCapturedArguments() return } -func (verifier *VerifierMockClient) DiscardReviews(repo models.Repo, pull models.PullRequest) *MockClient_DiscardReviews_OngoingVerification { - _params := []pegomock.Param{repo, pull} +func (verifier *VerifierMockClient) DiscardReviews(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) *MockClient_DiscardReviews_OngoingVerification { + _params := []pegomock.Param{logger, repo, pull} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DiscardReviews", _params, verifier.timeout) return &MockClient_DiscardReviews_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index 0a48f210b..62d1b5b88 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_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) { diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index 71d66116b..944fc392b 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.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) {