Skip to content

Commit

Permalink
Revert "[fix] Ignore commit checks for atlantis apply on Github (runa…
Browse files Browse the repository at this point in the history
…tlantis#2311)" (runatlantis#2388)

* Revert "[fix] Ignore commit checks for atlantis apply on Github (runatlantis#2311)"

This reverts commit a3db712.
  • Loading branch information
lilincmu authored and krrrr38 committed Dec 16, 2022
1 parent bba040d commit 6d3b383
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 101 deletions.
43 changes: 0 additions & 43 deletions server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/google/go-github/v31/github"
"github.com/pkg/errors"
"github.com/runatlantis/atlantis/server/core/config"
"github.com/runatlantis/atlantis/server/events/command"
"github.com/runatlantis/atlantis/server/events/models"
"github.com/runatlantis/atlantis/server/events/vcs/common"
"github.com/runatlantis/atlantis/server/logging"
Expand Down Expand Up @@ -304,7 +303,6 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
return false, errors.Wrap(err, "getting pull request")
}
state := githubPR.GetMergeableState()
g.logger.Debug("PR mergeable state is %v", state)
// We map our mergeable check to when the GitHub merge button is clickable.
// This corresponds to the following states:
// clean: No conflicts, all requirements satisfied.
Expand All @@ -313,48 +311,7 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
// status checks. Merging is allowed (yellow box).
// has_hooks: GitHub Enterprise only, if a repo has custom pre-receive
// hooks. Merging is allowed (green box).
// blocked: Blocked by a failing/missing required status check.
// See: https://github.com/octokit/octokit.net/issues/1763
if state == "blocked" {
var allStatuses []*github.RepoStatus
nextPage := 0
for {
g.logger.Debug("GET /repos/%v/%v/commits/%d/status", repo.Owner, repo.Name, pull.HeadBranch)
combinedStatus, resp, err := g.client.Repositories.GetCombinedStatus(g.ctx, repo.Owner, repo.Name, pull.HeadBranch, &github.ListOptions{
Page: nextPage,
})
if err != nil {
return false, errors.Wrap(err, "fetching PR statuses")
}
allStatuses = append(allStatuses, combinedStatus.Statuses...)
if resp.NextPage == 0 {
break
}
nextPage = resp.NextPage
}
g.logger.Debug("GET /repos/%v/%v/branches/%d/protection/required_status_checks", repo.Owner, repo.Name, pull.BaseBranch)
requiredChecks, _, err := g.client.Repositories.GetRequiredStatusChecks(g.ctx, repo.Owner, repo.Name, pull.BaseBranch)
if err != nil {
return false, errors.Wrap(err, "fetching PR required checks")
}
for _, status := range allStatuses {
for _, requiredCheck := range requiredChecks.Contexts {
// Ignore any commit statuses with 'atlantis/apply' as prefix
if strings.HasPrefix(status.GetContext(), fmt.Sprintf("%s/%s", vcsstatusname, command.Apply.String())) {
continue
}
if status.GetContext() == requiredCheck {
if status.GetState() == "failure" || status.GetState() == "pending" {
g.logger.Debug("Failed check %v", requiredCheck)
return false, nil
}
}
}
}
g.logger.Debug("Blocked only by atlantis/apply")
return true, nil

}
if state != "clean" && state != "unstable" && state != "has_hooks" {
return false, nil
}
Expand Down
65 changes: 7 additions & 58 deletions server/events/vcs/github_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,77 +477,45 @@ func TestGithubClient_PullIsApproved(t *testing.T) {
func TestGithubClient_PullIsMergeable(t *testing.T) {
vcsStatusName := "atlantis-test"
cases := []struct {
state string
requiredCheckName string
requiredCheckStatus string
expMergeable bool
state string
expMergeable bool
}{
{
"dirty",
"",
"",
false,
},
{
"unknown",
"",
"",
false,
},
{
"blocked",
false,
},
{
"behind",
"",
"",
false,
},
{
"random",
"",
"",
false,
},
{
"unstable",
"",
"",
true,
},
{
"has_hooks",
"",
"",
true,
},
{
"clean",
"",
"",
true,
},
{
"",
"",
"",
false,
},
{
"blocked",
fmt.Sprintf("%s/apply", vcsStatusName),
"failure",
true,
},
{
"blocked",
fmt.Sprintf("%s/apply", vcsStatusName),
"pending",
true,
},
{
"blocked",
"required_check",
"failure",
false,
},
}

// Use a real GitHub json response and edit the mergeable_state field.
Expand All @@ -562,30 +530,13 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
fmt.Sprintf(`"mergeable_state": "%s"`, c.state),
1,
)
responseStatus, _ := json.Marshal(map[string][]map[string]string{
"statuses": {{
"context": c.requiredCheckName,
"state": c.requiredCheckStatus,
}},
})
responseRequiredChecks, _ := json.Marshal(map[string][]string{
"contexts": {
c.requiredCheckName,
},
})

testServer := httptest.NewTLSServer(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.RequestURI {
case "/api/v3/repos/owner/repo/pulls/1":
w.Write([]byte(response)) // nolint: errcheck
return
case "/api/v3/repos/owner/repo/commits/headBranch/status":
w.Write(responseStatus) // nolint: errcheck
return
case "/api/v3/repos/owner/repo/branches/baseBranch/protection/required_status_checks":
w.Write(responseRequiredChecks) // nolint: errcheck
return
default:
t.Errorf("got unexpected request at %q", r.RequestURI)
http.Error(w, "not found", http.StatusNotFound)
Expand All @@ -609,9 +560,7 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
Hostname: "github.com",
},
}, models.PullRequest{
Num: 1,
HeadBranch: "headBranch",
BaseBranch: "baseBranch",
Num: 1,
}, vcsStatusName)
Ok(t, err)
Equals(t, c.expMergeable, actMergeable)
Expand Down

0 comments on commit 6d3b383

Please sign in to comment.