Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Mar 10, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion runatlantis.io/docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@
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 token is required for this feature.

Check failure on line 490 in runatlantis.io/docs/server-configuration.md

View workflow job for this annotation

GitHub Actions / Website Check

Trailing spaces

runatlantis.io/docs/server-configuration.md:490:160 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md009.md

### `--emoji-reaction`

Expand Down
2 changes: 1 addition & 1 deletion server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion server/events/plan_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/azuredevops_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketcloud/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketserver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/gitea/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions server/events/vcs/gitlab_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,8 +577,16 @@ 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)
_, err := g.Client.MergeRequestApprovals.ResetApprovalsOfMergeRequest(repo.FullName, pull.Num)
if err != nil {
return errors.Wrap(err, "unable to reset approvals")
}

return nil
}

Expand Down
57 changes: 57 additions & 0 deletions server/events/vcs/gitlab_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
2 changes: 1 addition & 1 deletion server/events/vcs/mocks/mock_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion server/events/vcs/not_configured_vcs_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions server/events/vcs/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading