From 6d3b383a731096ff6a35d454d7b94a2c3b29fb88 Mon Sep 17 00:00:00 2001 From: Li Lin Date: Wed, 13 Jul 2022 17:11:03 -0700 Subject: [PATCH] Revert "[fix] Ignore commit checks for atlantis apply on Github (#2311)" (#2388) * Revert "[fix] Ignore commit checks for atlantis apply on Github (#2311)" This reverts commit a3db71292007497d97d4c417589cfe701b560ba8. --- server/events/vcs/github_client.go | 43 ---------------- server/events/vcs/github_client_test.go | 65 +++---------------------- 2 files changed, 7 insertions(+), 101 deletions(-) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 6837115bc2..7dc0ce3b51 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -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" @@ -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. @@ -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 } diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index c2b7b7e816..c705750a47 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -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. @@ -562,17 +530,6 @@ 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) { @@ -580,12 +537,6 @@ 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/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) @@ -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)