diff --git a/cmd/server.go b/cmd/server.go index 98d7a0257b..99eba5b19c 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -63,6 +63,7 @@ const ( DisableAutoplanFlag = "disable-autoplan" DisableMarkdownFoldingFlag = "disable-markdown-folding" DisableRepoLockingFlag = "disable-repo-locking" + DiscardApprovalOnPlanFlag = "discard-approval-on-plan" EnablePolicyChecksFlag = "enable-policy-checks" EnableRegExpCmdFlag = "enable-regexp-cmd" EnableDiffMarkdownFormat = "enable-diff-markdown-format" @@ -407,6 +408,10 @@ var boolFlags = map[string]boolFlag{ DisableRepoLockingFlag: { description: "Disable atlantis locking repos", }, + DiscardApprovalOnPlanFlag: { + description: "Enables the discarding of approval if a new plan has been executed. Currently only Github is supported", + defaultValue: false, + }, EnablePolicyChecksFlag: { description: "Enable atlantis to run user defined policy checks. This is explicitly disabled for TFE/TFC backends since plan files are inaccessible.", defaultValue: false, diff --git a/cmd/server_test.go b/cmd/server_test.go index e2741fc50a..c8113a0b15 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -72,6 +72,7 @@ var testFlags = map[string]interface{}{ DisableApplyFlag: true, DisableMarkdownFoldingFlag: true, DisableRepoLockingFlag: true, + DiscardApprovalOnPlanFlag: true, GHHostnameFlag: "ghhostname", GHTokenFlag: "token", GHUserFlag: "user", diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index de720fec1b..0344ffc366 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -163,6 +163,10 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { baseRepo := ctx.Pull.BaseRepo pull := ctx.Pull + if err = p.pullUpdater.VCSClient.DiscardReviews(baseRepo, pull); err != nil { + ctx.Log.Err("failed to remove approvals: %s", err) + } + if err = p.commitStatusUpdater.UpdateCombined(baseRepo, pull, models.PendingCommitStatus, command.Plan); err != nil { ctx.Log.Warn("unable to update commit status: %s", err) } diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index 72aaf0ccdb..dbb50b8f13 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/server/events/vcs/azuredevops_client.go @@ -166,6 +166,11 @@ func (g *AzureDevopsClient) PullIsApproved(repo models.Repo, pull models.PullReq return approvalStatus, nil } +func (g *AzureDevopsClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error { + // TODO implement + return nil +} + // PullIsMergeable returns true if the merge request can be merged. func (g *AzureDevopsClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) { owner, project, repoName := SplitAzureDevopsRepoFullName(repo.FullName) diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index 4bdad53128..c526fdb018 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -228,6 +228,11 @@ func (b *Client) prepRequest(method string, path string, body io.Reader) (*http. return req, nil } +func (b *Client) DiscardReviews(repo models.Repo, pull models.PullRequest) error { + // TODO implement + return nil +} + func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]byte, error) { req, err := b.prepRequest(method, path, reqBody) if err != nil { diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index 88ba703272..495d190668 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -192,6 +192,11 @@ func (b *Client) PullIsApproved(repo models.Repo, pull models.PullRequest) (appr return approvalStatus, nil } +func (b *Client) DiscardReviews(repo models.Repo, pull models.PullRequest) error { + // TODO implement + return nil +} + // PullIsMergeable returns true if the merge request has no conflicts and can be merged. func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) { projectKey, err := b.GetProjectKey(repo.Name, repo.SanitizedCloneURL) diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index 170ea1ad59..1d83726472 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -36,6 +36,7 @@ type Client interface { // url is an optional link that users should click on for more information // about this status. UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error + DiscardReviews(repo models.Repo, pull models.PullRequest) error MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error MarkdownPullLink(pull models.PullRequest) (string, error) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 1517dc9239..24ba5705db 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -35,6 +35,11 @@ import ( // by GitHub. const maxCommentLength = 65536 +var ( + clientMutationID = githubv4.NewString("atlantis") + pullRequestDismissalMessage = *githubv4.NewString("Dismissing reviews because of plan changes") +) + // GithubClient is used to perform GitHub actions. type GithubClient struct { user string @@ -59,6 +64,19 @@ type GithubAppTemporarySecrets struct { URL string } +type GithubReview struct { + ID githubv4.ID + SubmittedAt githubv4.DateTime + Author struct { + Login githubv4.String + } +} + +type GithubPRReviewSummary struct { + ReviewDecision githubv4.String + Reviews []GithubReview +} + // NewGithubClient returns a valid GitHub client. func NewGithubClient(hostname string, credentials GithubCredentials, config GithubConfig, logger logging.SimpleLogging) (*GithubClient, error) { transport, err := credentials.Client() @@ -241,6 +259,58 @@ func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, co return nil } +// getPRReviews Retrieves PR reviews for a pull request on a specific repository. +// The reviews are being retrieved using pages with the size of 10 reviews. +func (g *GithubClient) getPRReviews(repo models.Repo, pull models.PullRequest) (GithubPRReviewSummary, error) { + var query struct { + Repository struct { + PullRequest struct { + ReviewDecision githubv4.String + Reviews struct { + Nodes []GithubReview + // contains pagination information + PageInfo struct { + EndCursor githubv4.String + HasNextPage githubv4.Boolean + } + } `graphql:"reviews(first: $entries, after: $reviewCursor, states: $reviewState)"` + } `graphql:"pullRequest(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.Owner), + "name": githubv4.String(repo.Name), + "number": githubv4.Int(pull.Num), + "entries": githubv4.Int(10), + "reviewState": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStateApproved}, + "reviewCursor": (*githubv4.String)(nil), // initialize the reviewCursor with null + } + + var allReviews []GithubReview + for { + err := g.v4Client.Query(g.ctx, &query, variables) + if err != nil { + return GithubPRReviewSummary{ + query.Repository.PullRequest.ReviewDecision, + allReviews, + }, errors.Wrap(err, "getting reviewDecision") + } + + allReviews = append(allReviews, query.Repository.PullRequest.Reviews.Nodes...) + // if we don't have a NextPage pointer, we have requested all pages + if !query.Repository.PullRequest.Reviews.PageInfo.HasNextPage { + break + } + // set the end cursor, so the next batch of reviews is going to be requested and not the same again + variables["reviewCursor"] = githubv4.NewString(query.Repository.PullRequest.Reviews.PageInfo.EndCursor) + } + return GithubPRReviewSummary{ + query.Repository.PullRequest.ReviewDecision, + allReviews, + }, nil +} + // PullIsApproved returns true if the pull request was approved. func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (approvalStatus models.ApprovalStatus, err error) { nextPage := 0 @@ -273,6 +343,39 @@ func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest) return approvalStatus, nil } +// DiscardReviews dismisses all reviews on a pull request +func (g *GithubClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error { + reviewStatus, err := g.getPRReviews(repo, pull) + if err != nil { + return err + } + + // https://docs.github.com/en/graphql/reference/input-objects#dismisspullrequestreviewinput + var mutation struct { + DismissPullRequestReview struct { + PullRequestReview struct { + ID githubv4.ID + } + } `graphql:"dismissPullRequestReview(input: $input)"` + } + + // dismiss every review one by one. + // currently there is no way to dismiss them in one mutation. + for _, review := range reviewStatus.Reviews { + input := githubv4.DismissPullRequestReviewInput{ + PullRequestReviewID: review.ID, + Message: pullRequestDismissalMessage, + ClientMutationID: clientMutationID, + } + mutationResult := &mutation + err := g.v4Client.Mutate(g.ctx, mutationResult, input, nil) + if err != nil { + return errors.Wrap(err, "dismissing reviewDecision") + } + } + return nil +} + // isRequiredCheck is a helper function to determine if a check is required or not func isRequiredCheck(check string, required []string) bool { //in go1.18 can prob replace this with slices.Contains diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index 9d938ca42c..331abeac46 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -1170,3 +1170,199 @@ func TestGithubClient_GetTeamNamesForUser(t *testing.T) { Ok(t, err) Equals(t, []string{"Frontend Developers", "frontend-developers", "Employees", "employees"}, teams) } + +func TestGithubClient_DiscardReviews(t *testing.T) { + type ResponseDef struct { + httpCode int + body string + } + type fields struct { + responses []ResponseDef + } + type args struct { + repo models.Repo + pull models.PullRequest + } + + queryResponseSingleReview := `{ + "data": { + "repository": { + "pullRequest": { + "reviewDecision": "APPROVED", + "reviews": { + "nodes": [ + { + "id": "PRR_kwDOFxULt85HBb7A", + "submittedAt": "2022-11-23T12:28:30Z", + "author": { + "login": "atlantis-test" + } + } + ] + } + } + } + } +}` + queryResponseMultipleReviews := `{ + "data": { + "repository": { + "pullRequest": { + "reviewDecision": "APPROVED", + "reviews": { + "nodes": [ + { + "id": "PRR_kwDOFxULt85HBb7A", + "submittedAt": "2022-11-23T12:28:30Z", + "author": { + "login": "atlantis-test" + } + }, + { + "id": "PRR_kwDOFxULt85HBb7B", + "submittedAt": "2022-11-23T14:28:30Z", + "author": { + "login": "atlantis-test2" + } + } + ] + } + } + } + } +}` + mutationResponseSingleReviewDismissal := `{ + "data": { + "dismissPullRequestReview": { + "pullRequestReview": { + "id": "PRR_kwDOFxULt85HBb7A" + } + } + } +}` + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "return no error if dismissing a single approval", + fields: fields{ + responses: []ResponseDef{ + { + httpCode: 200, + body: queryResponseSingleReview, + }, + { + httpCode: 200, + body: mutationResponseSingleReviewDismissal, + }, + }, + }, + args: args{}, + wantErr: false, + }, + { + name: "return no error if dismissing multiple reviews", + fields: fields{ + responses: []ResponseDef{ + { + httpCode: 200, + body: queryResponseMultipleReviews, + }, + { + httpCode: 200, + body: mutationResponseSingleReviewDismissal, + }, + { + httpCode: 200, + body: mutationResponseSingleReviewDismissal, + }, + }, + }, + args: args{}, + wantErr: false, + }, + { + name: "return error if query fails", + fields: fields{ + responses: []ResponseDef{ + { + httpCode: 500, + body: "", + }, + }, + }, + args: args{}, + wantErr: true, + }, + { + name: "return error if mutating fails", + fields: fields{ + responses: []ResponseDef{ + { + httpCode: 200, + body: queryResponseSingleReview, + }, + { + httpCode: 500, + body: "", + }, + }, + }, + args: args{}, + wantErr: true, + }, + { + name: "return error if dismissing fails after already dismissing one", + fields: fields{ + responses: []ResponseDef{ + { + httpCode: 200, + body: queryResponseMultipleReviews, + }, + { + httpCode: 200, + body: mutationResponseSingleReviewDismissal, + }, + { + httpCode: 500, + body: "", + }, + }, + }, + args: args{}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Mocked GraphQL response for two teams + responseIndex := 0 + responseLength := len(tt.fields.responses) + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.RequestURI != "/api/graphql" { + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + Assert(t, responseIndex < responseLength, "requesting more responses than are defined") + response := tt.fields.responses[responseIndex] + responseIndex++ + w.WriteHeader(response.httpCode) + w.Write([]byte(response.body)) // nolint: errcheck + })) + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) + Ok(t, err) + defer disableSSLVerification()() + if err := client.DiscardReviews(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 26eb93f223..e7e4aa5ebf 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -315,6 +315,11 @@ func (g *GitlabClient) MarkdownPullLink(pull models.PullRequest) (string, error) return fmt.Sprintf("!%d", pull.Num), nil } +func (g *GitlabClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error { + // TODO implement + return nil +} + // GetVersion returns the version of the Gitlab server this client is using. func (g *GitlabClient) GetVersion() (*version.Version, error) { req, err := g.Client.NewRequest("GET", "/version", nil, nil) diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index 3522cc8e0f..6d581214a4 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -15,6 +15,21 @@ type MockClient struct { fail func(message string, callerSkip ...int) } +func (mock *MockClient) DiscardReviews(_param0 models.Repo, _param1 models.PullRequest) error { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockClient().") + } + params := []pegomock.Param{_param0, _param1} + result := pegomock.GetGenericMockFrom(mock).Invoke("DiscardReviews", params, []reflect.Type{reflect.TypeOf((*[]string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(error) + } + } + return ret0 +} + func NewMockClient(options ...pegomock.Option) *MockClient { mock := &MockClient{} for _, option := range options { diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index c470ac7ef6..e98772974f 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -38,6 +38,9 @@ func (a *NotConfiguredVCSClient) HidePrevCommandComments(repo models.Repo, pullN func (a *NotConfiguredVCSClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) { return models.ApprovalStatus{}, a.err() } +func (a *NotConfiguredVCSClient) DiscardReviews(repo models.Repo, pull models.PullRequest) error { + return nil +} func (a *NotConfiguredVCSClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) { return false, a.err() } diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index ddc3e35f28..9703d11372 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -68,6 +68,10 @@ func (d *ClientProxy) PullIsApproved(repo models.Repo, pull models.PullRequest) return d.clients[repo.VCSHost.Type].PullIsApproved(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) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) { return d.clients[repo.VCSHost.Type].PullIsMergeable(repo, pull, vcsstatusname) } diff --git a/server/user_config.go b/server/user_config.go index a635bd0593..f0b493aaf9 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -31,6 +31,7 @@ type UserConfig struct { DisableAutoplan bool `mapstructure:"disable-autoplan"` DisableMarkdownFolding bool `mapstructure:"disable-markdown-folding"` DisableRepoLocking bool `mapstructure:"disable-repo-locking"` + DiscardApprovalOnPlanFlag bool `mapstructure:"discard-approval-on-plan"` EnablePolicyChecksFlag bool `mapstructure:"enable-policy-checks"` EnableRegExpCmd bool `mapstructure:"enable-regexp-cmd"` EnableDiffMarkdownFormat bool `mapstructure:"enable-diff-markdown-format"`