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

Make required atlantis/apply status check work with mergeable using --gh-allow-mergeable-bypass-apply #2436

Merged
merged 8 commits into from
Aug 18, 2022
148 changes: 148 additions & 0 deletions server/events/vcs/fixtures/github-commit-status-full.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
{
"state": "blocked",
"statuses": [
{
"url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e",
"avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4",
"id": 16230299674,
"node_id": "SC_kwDOFRFvL88AAAADx2a4Gg",
"state": "success",
"description": "Plan succeeded.",
"target_url": "https://localhost/jobs/octocat/Hello-World/1/project1",
"context": "atlantis/plan: project1",
"created_at": "2022-02-10T15:26:01Z",
"updated_at": "2022-02-10T15:26:01Z"
},
{
"url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e",
"avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4",
"id": 16230303174,
"node_id": "SC_kwDOFRFvL88AAAADx2bFxg",
"state": "success",
"description": "Plan succeeded.",
"target_url": "https://localhost/jobs/octocat/Hello-World/1/project2",
"context": "atlantis/plan: project2",
"created_at": "2022-02-10T15:26:12Z",
"updated_at": "2022-02-10T15:26:12Z"
},
{
"url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e",
"avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4",
"id": 16230303679,
"node_id": "SC_kwDOFRFvL88AAAADx2bHvw",
"state": "success",
"description": "2/2 projects planned successfully.",
"target_url": "",
"context": "atlantis/plan",
"created_at": "2022-02-10T15:26:13Z",
"updated_at": "2022-02-10T15:26:13Z"
},
{
"url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e",
"avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4",
"id": 16230307923,
"node_id": "SC_kwDOFRFvL88AAAADx2bYUw",
"state": "failure",
"description": "Apply failed.",
"target_url": "https://localhost/jobs/octocat/Hello-World/1/project1",
"context": "atlantis/apply: project1",
"created_at": "2022-02-10T15:26:27Z",
"updated_at": "2022-02-10T15:26:27Z"
},
{
"url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e",
"avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4",
"id": 16230308153,
"node_id": "SC_kwDOFRFvL88AAAADx2bZOQ",
"state": "failure",
"description": "Apply failed.",
"target_url": "https://localhost/jobs/octocat/Hello-World/1/project2",
"context": "atlantis/apply: project2",
"created_at": "2022-02-10T15:26:27Z",
"updated_at": "2022-02-10T15:26:27Z"
},
{
"url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e",
"avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4",
"id": 16230308528,
"node_id": "SC_kwDOFRFvL88AAAADx2basA",
"state": "failure",
"description": "0/2 projects applied successfully.",
"target_url": "",
"context": "atlantis/apply",
"created_at": "2022-02-10T15:26:28Z",
"updated_at": "2022-02-10T15:26:28Z"
}
],
"sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
"total_count": 0,
"repository": {
"id": 1296269,
"node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5",
"name": "Hello-World",
"full_name": "octocat/Hello-World",
"private": false,
"owner": {
"login": "octocat",
"id": 583231,
"node_id": "MDQ6VXNlcjU4MzIzMQ==",
"avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
},
"html_url": "https://github.com/octocat/Hello-World",
"description": "My first repository on GitHub!",
"fork": false,
"url": "https://api.github.com/repos/octocat/Hello-World",
"forks_url": "https://api.github.com/repos/octocat/Hello-World/forks",
"keys_url": "https://api.github.com/repos/octocat/Hello-World/keys{/key_id}",
"collaborators_url": "https://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}",
"teams_url": "https://api.github.com/repos/octocat/Hello-World/teams",
"hooks_url": "https://api.github.com/repos/octocat/Hello-World/hooks",
"issue_events_url": "https://api.github.com/repos/octocat/Hello-World/issues/events{/number}",
"events_url": "https://api.github.com/repos/octocat/Hello-World/events",
"assignees_url": "https://api.github.com/repos/octocat/Hello-World/assignees{/user}",
"branches_url": "https://api.github.com/repos/octocat/Hello-World/branches{/branch}",
"tags_url": "https://api.github.com/repos/octocat/Hello-World/tags",
"blobs_url": "https://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}",
"git_tags_url": "https://api.github.com/repos/octocat/Hello-World/git/tags{/sha}",
"git_refs_url": "https://api.github.com/repos/octocat/Hello-World/git/refs{/sha}",
"trees_url": "https://api.github.com/repos/octocat/Hello-World/git/trees{/sha}",
"statuses_url": "https://api.github.com/repos/octocat/Hello-World/statuses/{sha}",
"languages_url": "https://api.github.com/repos/octocat/Hello-World/languages",
"stargazers_url": "https://api.github.com/repos/octocat/Hello-World/stargazers",
"contributors_url": "https://api.github.com/repos/octocat/Hello-World/contributors",
"subscribers_url": "https://api.github.com/repos/octocat/Hello-World/subscribers",
"subscription_url": "https://api.github.com/repos/octocat/Hello-World/subscription",
"commits_url": "https://api.github.com/repos/octocat/Hello-World/commits{/sha}",
"git_commits_url": "https://api.github.com/repos/octocat/Hello-World/git/commits{/sha}",
"comments_url": "https://api.github.com/repos/octocat/Hello-World/comments{/number}",
"issue_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments{/number}",
"contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/{+path}",
"compare_url": "https://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}",
"merges_url": "https://api.github.com/repos/octocat/Hello-World/merges",
"archive_url": "https://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}",
"downloads_url": "https://api.github.com/repos/octocat/Hello-World/downloads",
"issues_url": "https://api.github.com/repos/octocat/Hello-World/issues{/number}",
"pulls_url": "https://api.github.com/repos/octocat/Hello-World/pulls{/number}",
"milestones_url": "https://api.github.com/repos/octocat/Hello-World/milestones{/number}",
"notifications_url": "https://api.github.com/repos/octocat/Hello-World/notifications{?since,all,participating}",
"labels_url": "https://api.github.com/repos/octocat/Hello-World/labels{/name}",
"releases_url": "https://api.github.com/repos/octocat/Hello-World/releases{/id}",
"deployments_url": "https://api.github.com/repos/octocat/Hello-World/deployments"
},
"commit_url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e",
"url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e/status"
}
70 changes: 70 additions & 0 deletions server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/base64"
"fmt"
"net/http"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -296,12 +297,62 @@ func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest)
return approvalStatus, nil
}

// GetCombinedStatusMinusApply checks Statuses for PR, excluding atlantis apply. Returns true if all other statuses are not in failure.
func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *github.PullRequest) (bool, error) {
status, _, err := g.client.Repositories.GetCombinedStatus(g.ctx, repo.Owner, repo.Name, *pull.Head.Ref, nil)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be #2601 - here the repo.Owner and repo.Name come from the PR Destination repository, and the pull.Head.Ref may be the branch name in the Fork (Source repository)

From https://docs.github.com/en/rest/commits/statuses#get-the-combined-status-for-a-specific-reference it's not clear how to do this for commits on a fork.

if err != nil {
return false, errors.Wrap(err, "getting combined status")
}

//compile for performance reasons
matcher, _ := regexp.Compile("atlantis/apply")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to pass vcsstatusname down here since it's actually configurable. See the counterpart of GitLab as an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot about that - will fix.


for _, r := range status.Statuses {
if !matcher.MatchString(*r.Context) {
if *r.State == "failure" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to return false when checks are pending?

return false, nil
}
}
}

return true, nil
}

// GetPullReviewDecision gets the pull review decision, which takes into account CODEOWNERS
func (g *GithubClient) GetPullReviewDecision(repo models.Repo, pull models.PullRequest) (approvalStatus bool, err error) {
var query struct {
Repository struct {
PullRequest struct {
ReviewDecision string
} `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),
}

err = g.v4QueryClient.Query(g.ctx, &query, variables)
if err != nil {
return approvalStatus, errors.Wrap(err, "getting reviewDecision")
}

if query.Repository.PullRequest.ReviewDecision == "APPROVED" {
return true, nil
}

return false, nil
}

// PullIsMergeable returns true if the pull request is mergeable.
func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
githubPR, err := g.GetPullRequest(repo, pull.Num)
if err != nil {
return false, errors.Wrap(err, "getting pull request")
}

state := githubPR.GetMergeableState()
// We map our mergeable check to when the GitHub merge button is clickable.
// This corresponds to the following states:
Expand All @@ -313,6 +364,25 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
// hooks. Merging is allowed (green box).
// See: https://github.com/octokit/octokit.net/issues/1763
if state != "clean" && state != "unstable" && state != "has_hooks" {
if state == "blocked" {
//check status excluding atlantis apply
status, err := g.GetCombinedStatusMinusApply(repo, githubPR)
if err != nil {
return false, errors.Wrap(err, "getting pull request status")
}

//check to see if pr is approved using reviewDecision
approved, err := g.GetPullReviewDecision(repo, pull)
if err != nil {
return false, errors.Wrap(err, "getting pull request reviewDecision")
}

//if all other status checks EXCEPT atlantis/apply are successful, and the PR is approved based on reviewDecision, let it proceed
if status && approved {
return true, nil
}
}

return false, nil
}
return true, nil
Expand Down
23 changes: 23 additions & 0 deletions server/events/vcs/github_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,22 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
Ok(t, err)
prJSON := string(jsBytes)

// Status Check Response
jsBytes, err = os.ReadFile("fixtures/github-commit-status-full.json")
Ok(t, err)
commitJSON := string(jsBytes)

//reviewDecision Response
reviewDecision := `{
"data": {
"repository": {
"pullRequest": {
"reviewDecision": "REVIEW_REQUIRED"
}
}
}
}`

for _, c := range cases {
t.Run(c.state, func(t *testing.T) {
response := strings.Replace(prJSON,
Expand All @@ -537,6 +553,13 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
case "/api/v3/repos/owner/repo/pulls/1":
w.Write([]byte(response)) // nolint: errcheck
return
case "/api/v3/repos/owner/repo/pulls/1/reviews?per_page=300":
w.Write([]byte("[]")) // nolint: errcheck
return
case "/api/v3/repos/owner/repo/commits/new-topic/status":
w.Write([]byte(commitJSON)) // nolint: errcheck
case "/api/graphql":
w.Write([]byte(reviewDecision)) // nolint: errcheck
default:
t.Errorf("got unexpected request at %q", r.RequestURI)
http.Error(w, "not found", http.StatusNotFound)
Expand Down