diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 3257a4eb61..10b31add1e 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -579,12 +579,14 @@ func (g *GithubClient) MergePull(pull models.PullRequest, pullOptions models.Pul if err != nil { return errors.Wrap(err, "fetching repo info") } - protection, resp, err := g.client.Repositories.GetBranchProtection(context.Background(), *repo.Owner.Name, *repo.Name, pull.BaseBranch) + protection, _, err := g.client.Repositories.GetBranchProtection(context.Background(), repo.Owner.GetLogin(), *repo.Name, pull.BaseBranch) if err != nil { - return errors.Wrap(err, "getting branch protection rules") + if !errors.Is(err, github.ErrBranchNotProtected) { + return errors.Wrap(err, "getting branch protection rules") + } } requireLinearHistory := false - if resp.Response.StatusCode == 200 { + if protection != nil { requireLinearHistory = protection.GetRequireLinearHistory().Enabled } const ( diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index c1c30f03d4..8f50ff3646 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -787,6 +787,10 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) { defer r.Body.Close() // nolint: errcheck w.WriteHeader(c.code) w.Write([]byte(resp)) // nolint: errcheck + case "/api/v3/repos/runatlantis/atlantis/branches/master/protection": + w.WriteHeader(404) + w.Write([]byte("{\"message\":\"Branch not protected\"}")) + return default: t.Errorf("got unexpected request at %q", r.RequestURI) http.Error(w, "not found", http.StatusNotFound) @@ -813,7 +817,8 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) { Hostname: "github.com", }, }, - Num: 1, + Num: 1, + BaseBranch: "master", }, models.PullRequestOptions{ DeleteSourceBranchOnMerge: false, }) @@ -831,40 +836,132 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) { // use that method func TestGithubClient_MergePullCorrectMethod(t *testing.T) { cases := map[string]struct { - allowMerge bool - allowRebase bool - allowSquash bool - expMethod string + allowMerge bool + allowRebase bool + allowSquash bool + requireLinearHistory bool + protectedBranch bool + expMethod string }{ "all true": { - allowMerge: true, - allowRebase: true, - allowSquash: true, - expMethod: "merge", + allowMerge: true, + allowRebase: true, + allowSquash: true, + requireLinearHistory: false, + protectedBranch: false, + expMethod: "merge", }, "all false (edge case)": { - allowMerge: false, - allowRebase: false, - allowSquash: false, - expMethod: "merge", + allowMerge: false, + allowRebase: false, + allowSquash: false, + requireLinearHistory: false, + protectedBranch: false, + expMethod: "merge", }, "merge: false rebase: true squash: true": { - allowMerge: false, - allowRebase: true, - allowSquash: true, - expMethod: "rebase", + allowMerge: false, + allowRebase: true, + allowSquash: true, + requireLinearHistory: false, + protectedBranch: false, + expMethod: "rebase", }, "merge: false rebase: false squash: true": { - allowMerge: false, - allowRebase: false, - allowSquash: true, - expMethod: "squash", + allowMerge: false, + allowRebase: false, + allowSquash: true, + requireLinearHistory: false, + protectedBranch: false, + expMethod: "squash", }, "merge: false rebase: true squash: false": { - allowMerge: false, - allowRebase: true, - allowSquash: false, - expMethod: "rebase", + allowMerge: false, + allowRebase: true, + allowSquash: false, + requireLinearHistory: false, + protectedBranch: false, + expMethod: "rebase", + }, + "protected, all true, rlh: false": { + allowMerge: true, + allowRebase: true, + allowSquash: true, + requireLinearHistory: false, + protectedBranch: true, + expMethod: "merge", + }, + "protected, all false (edge case), rlh: false": { + allowMerge: false, + allowRebase: false, + allowSquash: false, + requireLinearHistory: false, + protectedBranch: true, + expMethod: "merge", + }, + "protected, merge: false rebase: true squash: true, rlh: false": { + allowMerge: false, + allowRebase: true, + allowSquash: true, + requireLinearHistory: false, + protectedBranch: true, + expMethod: "rebase", + }, + "protected, merge: false rebase: false squash: true, rlh: false": { + allowMerge: false, + allowRebase: false, + allowSquash: true, + requireLinearHistory: false, + protectedBranch: true, + expMethod: "squash", + }, + "protected, merge: false rebase: true squash: false, rlh: false": { + allowMerge: false, + allowRebase: true, + allowSquash: false, + requireLinearHistory: false, + protectedBranch: true, + expMethod: "rebase", + }, + "protected, all true, rlh: true": { + allowMerge: true, + allowRebase: true, + allowSquash: true, + requireLinearHistory: true, + protectedBranch: true, + expMethod: "rebase", + }, + "protected, all false (edge case), rlh: true": { + allowMerge: false, + allowRebase: false, + allowSquash: false, + requireLinearHistory: true, + protectedBranch: true, + expMethod: "merge", + }, + "protected, merge: false rebase: true squash: true, rlh: true": { + allowMerge: false, + allowRebase: true, + allowSquash: true, + requireLinearHistory: true, + protectedBranch: true, + expMethod: "rebase", + }, + "protected, merge: false rebase: false squash: true, rlh: true": { + allowMerge: false, + allowRebase: false, + allowSquash: true, + requireLinearHistory: true, + protectedBranch: true, + expMethod: "squash", + }, + "protected, merge: false rebase: true squash: false, rlh: true": { + allowMerge: false, + allowRebase: true, + allowSquash: false, + requireLinearHistory: true, + protectedBranch: true, + expMethod: "rebase", }, } @@ -874,7 +971,10 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) { // Modify response. jsBytes, err := os.ReadFile("testdata/github-repo.json") Ok(t, err) + rlhBytes, err := os.ReadFile("testdata/github-branch-protection-require-linear-history.json") + Ok(t, err) resp := string(jsBytes) + protected := string(rlhBytes) resp = strings.Replace(resp, `"allow_squash_merge": true`, fmt.Sprintf(`"allow_squash_merge": %t`, c.allowSquash), @@ -887,6 +987,10 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) { `"allow_rebase_merge": true`, fmt.Sprintf(`"allow_rebase_merge": %t`, c.allowRebase), -1) + protected = strings.Replace(protected, + `"enabled": true`, + fmt.Sprintf(`"enabled": %t`, c.requireLinearHistory), + -1) testServer := httptest.NewTLSServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -894,6 +998,14 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) { case "/api/v3/repos/runatlantis/atlantis": w.Write([]byte(resp)) // nolint: errcheck return + case "/api/v3/repos/runatlantis/atlantis/branches/master/protection": + if c.protectedBranch { + w.Write([]byte(protected)) + } else { + w.WriteHeader(404) + w.Write([]byte("{\"message\":\"Branch not protected\"}")) + } + return case "/api/v3/repos/runatlantis/atlantis/pulls/1/merge": body, err := io.ReadAll(r.Body) Ok(t, err) @@ -936,7 +1048,8 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) { Hostname: "github.com", }, }, - Num: 1, + Num: 1, + BaseBranch: "master", }, models.PullRequestOptions{ DeleteSourceBranchOnMerge: false, }) diff --git a/server/events/vcs/testdata/github-branch-protection-require-linear-history.json b/server/events/vcs/testdata/github-branch-protection-require-linear-history.json new file mode 100644 index 0000000000..c4d4de34e4 --- /dev/null +++ b/server/events/vcs/testdata/github-branch-protection-require-linear-history.json @@ -0,0 +1,6 @@ +{ + "url": "https://api.github.com/repos/octocat/Hello-World/branches/master/protection", + "required_linear_history": { + "enabled": true + } +} \ No newline at end of file