fix(github): branch protection not supported (#3276)

* fix: github branch protection not supported * fix: typo protectionAvailable
runatlantis · Mar 30, 2023 · ebc06c1 · ebc06c1
1 parent 05e9c2f
commit ebc06c1
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 3 deletions.
diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go
@@ -34,6 +34,10 @@ import (
 // by GitHub.
 const maxCommentLength = 65536
 
+// Error message GitHub API returns if branch protection is not available
+// in current repository
+const githubBranchProtectionNotAvailable string = "Upgrade to GitHub Pro or make this repository public to enable this feature."
+
 var (
 	clientMutationID            = githubv4.NewString("atlantis")
 	pullRequestDismissalMessage = *githubv4.NewString("Dismissing reviews because of plan changes")
@@ -570,6 +574,11 @@ func (g *GithubClient) UpdateStatus(repo models.Repo, pull models.PullRequest, s
 	return err
 }
 
+func isBranchProtectionNotAvailable(err error) bool {
+	errorResponse, ok := err.(*github.ErrorResponse)
+	return ok && errorResponse.Message == githubBranchProtectionNotAvailable
+}
+
 // MergePull merges the pull request.
 func (g *GithubClient) MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error {
 	// Users can set their repo to disallow certain types of merging.
@@ -581,7 +590,7 @@ func (g *GithubClient) MergePull(pull models.PullRequest, pullOptions models.Pul
 	}
 	protection, _, err := g.client.Repositories.GetBranchProtection(context.Background(), repo.Owner.GetLogin(), *repo.Name, pull.BaseBranch)
 	if err != nil {
-		if !errors.Is(err, github.ErrBranchNotProtected) {
+		if !errors.Is(err, github.ErrBranchNotProtected) && !isBranchProtectionNotAvailable(err) {
 			return errors.Wrap(err, "getting branch protection rules")
 		}
 	}

diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go
@@ -841,6 +841,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 		allowSquash          bool
 		requireLinearHistory bool
 		protectedBranch      bool
+		protectionAvailable  bool
 		expMethod            string
 	}{
 		"all true": {
@@ -849,6 +850,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: false,
 			protectedBranch:      false,
+			protectionAvailable:  true,
 			expMethod:            "merge",
 		},
 		"all false (edge case)": {
@@ -857,6 +859,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          false,
 			requireLinearHistory: false,
 			protectedBranch:      false,
+			protectionAvailable:  true,
 			expMethod:            "merge",
 		},
 		"merge: false rebase: true squash: true": {
@@ -865,6 +868,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: false,
 			protectedBranch:      false,
+			protectionAvailable:  true,
 			expMethod:            "rebase",
 		},
 		"merge: false rebase: false squash: true": {
@@ -873,6 +877,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: false,
 			protectedBranch:      false,
+			protectionAvailable:  true,
 			expMethod:            "squash",
 		},
 		"merge: false rebase: true squash: false": {
@@ -881,6 +886,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          false,
 			requireLinearHistory: false,
 			protectedBranch:      false,
+			protectionAvailable:  true,
 			expMethod:            "rebase",
 		},
 		"protected, all true, rlh: false": {
@@ -889,6 +895,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: false,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "merge",
 		},
 		"protected, all false (edge case), rlh: false": {
@@ -897,6 +904,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          false,
 			requireLinearHistory: false,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "merge",
 		},
 		"protected, merge: false rebase: true squash: true, rlh: false": {
@@ -905,6 +913,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: false,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "rebase",
 		},
 		"protected, merge: false rebase: false squash: true, rlh: false": {
@@ -913,6 +922,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: false,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "squash",
 		},
 		"protected, merge: false rebase: true squash: false, rlh: false": {
@@ -921,6 +931,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          false,
 			requireLinearHistory: false,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "rebase",
 		},
 		"protected, all true, rlh: true": {
@@ -929,6 +940,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: true,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "rebase",
 		},
 		"protected, all false (edge case), rlh: true": {
@@ -937,6 +949,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          false,
 			requireLinearHistory: true,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "merge",
 		},
 		"protected, merge: false rebase: true squash: true, rlh: true": {
@@ -945,6 +958,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: true,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "rebase",
 		},
 		"protected, merge: false rebase: false squash: true, rlh: true": {
@@ -953,6 +967,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          true,
 			requireLinearHistory: true,
 			protectedBranch:      true,
+			protectionAvailable:  true,
 			expMethod:            "squash",
 		},
 		"protected, merge: false rebase: true squash: false, rlh: true": {
@@ -961,6 +976,43 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 			allowSquash:          false,
 			requireLinearHistory: true,
 			protectedBranch:      true,
+			protectionAvailable:  true,
+			expMethod:            "rebase",
+		},
+		"protection not supported, all true": {
+			allowMerge:           true,
+			allowRebase:          true,
+			allowSquash:          true,
+			requireLinearHistory: false,
+			protectedBranch:      false,
+			protectionAvailable:  false,
+			expMethod:            "merge",
+		},
+		"protection not supported, merge: false, rebase: true, squash: true": {
+			allowMerge:           false,
+			allowRebase:          true,
+			allowSquash:          true,
+			requireLinearHistory: false,
+			protectedBranch:      false,
+			protectionAvailable:  false,
+			expMethod:            "rebase",
+		},
+		"protection not supported, merge: false, rebase: false, squash: true": {
+			allowMerge:           false,
+			allowRebase:          false,
+			allowSquash:          true,
+			requireLinearHistory: false,
+			protectedBranch:      false,
+			protectionAvailable:  false,
+			expMethod:            "squash",
+		},
+		"protection not supported, merge: false, rebase: true, squash: false": {
+			allowMerge:           false,
+			allowRebase:          true,
+			allowSquash:          false,
+			requireLinearHistory: false,
+			protectedBranch:      false,
+			protectionAvailable:  false,
 			expMethod:            "rebase",
 		},
 	}
@@ -999,11 +1051,14 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 						w.Write([]byte(resp)) // nolint: errcheck
 						return
 					case "/api/v3/repos/runatlantis/atlantis/branches/master/protection":
-						if c.protectedBranch {
+						if c.protectedBranch && c.protectionAvailable {
 							w.Write([]byte(protected)) // nolint: errcheck
-						} else {
+						} else if !c.protectedBranch && c.protectionAvailable {
 							w.WriteHeader(404)
 							w.Write([]byte("{\"message\":\"Branch not protected\"}")) // nolint: errcheck
+						} else if !c.protectionAvailable {
+							w.WriteHeader(403)
+							w.Write([]byte("{\"message\":\"Upgrade to GitHub Pro or make this repository public to enable this feature.\"}")) // nolint: errcheck
 						}
 						return
 					case "/api/v3/repos/runatlantis/atlantis/pulls/1/merge":