Repository: runatlantis/atlantis (language: go)

Issue:
Automerge fails on github free accounts in 0.23.3 due to branch protection api error
<!--- Please keep this note for the community --->

### Community Note

* Please vote on this issue by adding a 👍 [reaction](https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
* Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
* If you are interested in working on this issue or have submitted a pull request, please leave a comment.

<!--- Thank you for keeping this note for the community --->

---

<!---
When filing a bug, please include the following headings if possible.
Any example text in this template can be deleted.
--->

### Overview of the Issue

<!---
Please describe the issue you are having and how you encountered the problem.
--->

After upgrading from 0.23.2 to 0.23.3, Atlantis is no longer able to automerge PRs.  It will perform the apply and even post a comment saying `Automatically merging because all plans have been successfully applied.`, but the PR doesn't get merged.


### Reproduction Steps

1. Run atlantis on a private repo on a free github account
2. Submit a pull request
3. `atlantis apply`

<!--- 
In order to effectively and quickly resolve the issue, please provide exact steps that allow us the reproduce the problem. If no steps are provided, then it will likely take longer to get the issue resolved.
--->


### Logs


<details>
  <summary>Logs</summary>

```
{"level":"info","ts":"2023-03-26T22:04:23.058Z","caller":"events/automerger.go:32","msg":"automerging pull request","json":{"repo":"jescholl/<REDACTED_PRIVATE_REPO>","pull":"162"}}
{"level":"error","ts":"2023-03-26T22:04:23.350Z","caller":"vcs/instrumented_client.go:231","msg":"Unable to merge pull, error: getting branch protection rules: GET https://api.github.com/repos/jescholl/<REDACTED_PRIVATE_REPO>/branches/master/protection: 403 Upgrade to GitHub Pro or make this repository public to enable this feature. []","json":{"pull-num":162},"stacktrace":"...trace below..."}
```

trace

```
stacktrace:github.com/runatlantis/atlantis/server/events/vcs.(*InstrumentedClient).MergePull
	github.com/runatlantis/atlantis/server/events/vcs/instrumented_client.go:231
github.com/runatlantis/atlantis/server/events/vcs.(*ClientProxy).MergePull
	github.com/runatlantis/atlantis/server/events/vcs/proxy.go:84
github.com/runatlantis/atlantis/server/events.(*AutoMerger).automerge
	github.com/runatlantis/atlantis/server/events/automerger.go:35
github.com/runatlantis/atlantis/server/events.(*ApplyCommandRunner).Run
	github.com/runatlantis/atlantis/server/events/apply_command_runner.go:183
github.com/runatlantis/atlantis/server/events.(*DefaultCommandRunner).RunCommentCommand
	github.com/runatlantis/atlantis/server/events/command_runner.go:296
```

</details>


### Environment details

- Atlantis version: 0.23.3

Atlantis server-side config file:

```yaml
# config file
repos:
- id: "/.*/"
  apply_requirements: []
  plan_requirements: []
```

Repo `atlantis.yaml` file:

```yaml
# config file
```


### Additional Context
I thought I may have been able to work around this issue by eliminating the `apply_requirements` (see above config), but I still get the same behavior and same errors in the logs.




Discussion / hints:
The error seems to be

> Unable to merge pull, error: getting branch protection rules: GET https://api.github.com/repos/jescholl/<REDACTED_PRIVATE_REPO>/branches/master/protection: 403 Upgrade to GitHub Pro or make this repository public to enable this feature. []

Not sure if this is atlantis related.

If you downgrade back to 0.23.2, does it work as expected?
This also fails on enterprise accounts, this time with a 404 error, I think because an unexpected payload, if `"json":{"pull-num":5264}` means what I think it means in the error 
I started to get following error since 0.23.3, we're on gh enterprise:
```
{"level":"error","ts":"2023-03-27T07:29:33.761Z","caller":"vcs/instrumented_client.go:231","msg":"Unable to merge pull, error: getting branch protection rules: GET https://api.github.com/repos/<REPO>/branches/master/protection: 403 Resource not accessible by integration []","json":{"pull-num":327},"stacktrace":"github.com/runatlantis/atlantis/server/events/vcs.(*InstrumentedClient).MergePull\n\tgithub.com/runatlantis/atlantis/server/events/vcs/instrumented_client.go:231\ngithub.com/runatlantis/atlantis/server/events/vcs.(*ClientProxy).MergePull\n\tgithub.com/runatlantis/atlantis/server/events/vcs/proxy.go:84\ngithub.com/runatlantis/atlantis/server/events.(*AutoMerger).automerge\n\tgithub.com/runatlantis/atlantis/server/events/automerger.go:35\ngithub.com/runatlantis/atlantis/server/events.(*ApplyCommandRunner).Run\n\tgithub.com/runatlantis/atlantis/server/events/apply_command_runner.go:183\ngithub.com/runatlantis/atlantis/server/events.(*DefaultCommandRunner).RunCommentCommand\n\tgithub.com/runatlantis/atlantis/server/events/command_runner.go:296"}
```
Hi,

We are on an enterprise account (not self hosted however) and are also seeing this error as a 404 with the latest upgrade.

```
"Unable to merge pull, error: getting branch protection rules ... GET [https:// ...](https://api.github.com/repos/X/Y/branches/master/protection) 404 Not Found"
```
> The error seems to be
> 
> > Unable to merge pull, error: getting branch protection rules: GET https://api.github.com/repos/jescholl/<REDACTED_PRIVATE_REPO>/branches/master/protection: 403 Upgrade to GitHub Pro or make this repository public to enable this feature. []
> 
> Not sure if this is atlantis related.
> 
> If you downgrade back to 0.23.2, does it work as expected?

Yes, downgrading to 0.23.2 resolves the problem.
This looks like it came from this PR https://github.com/runatlantis/atlantis/pull/3211 cc: @tpolekhin

See differences between versions https://github.com/runatlantis/atlantis/compare/v0.23.2...v0.23.3

To confirm if the above PR is the culprit, try this version before that PR merge and see if it works

https://github.com/orgs/runatlantis/packages/container/atlantis/78397613?tag=dev-alpine-47f0258
https://github.com/orgs/runatlantis/packages/container/atlantis/78397613?tag=dev-debian-47f0258

Then confirm the version after the PR merge and see if it fails

https://github.com/orgs/runatlantis/packages/container/atlantis/78416368?tag=dev-alpine-7a33828
https://github.com/orgs/runatlantis/packages/container/atlantis/78416368?tag=dev-debian-7a33828
@jescholl That is an edge case that I did not think of
`403 Upgrade to GitHub Pro or make this repository public to enable this feature.`

@adriantr Unfortunately I don't have access to enterprise GitHub to check this, can you please provide a little bit more details on this? Why doesn't it has access to `master` branch? Is this something you intentionally configured?
`403 Resource not accessible by integration []`

@cep21 This is a weird one. Does your `master` have branch protection rules?
Here are the references where we get branch protections

This one existed before the pr and did not cause issues

https://github.com/runatlantis/atlantis/blob/c019d8b07e2326bc8d31eaacd0b275895c9178db/server/events/vcs/github_client.go#L409-L412

This was added in the pr and does cause issues

https://github.com/runatlantis/atlantis/blob/c019d8b07e2326bc8d31eaacd0b275895c9178db/server/events/vcs/github_client.go#L582-L587

Do i have that right?
@nitrocode, I tested as you suggested and it looks like you were right, `dev-debian-47f0258` works as expected but `dev-debian-7a33828` fails to merge after applying.

@nitrocode you are correct. I don't see a flaw in the logic of the #3211 PR. Maybe those issues surfaced because now `GetBranchProtection` is called independent of the Atlantis server configuration and there are some issues with it that didn't occur before?
Before #3211 was merged `GetBranchProtection` was only called if one had [--gh-allow-mergeable-bypass-apply](https://www.runatlantis.io/docs/server-configuration.html#gh-allow-mergeable-bypass-apply) enabled.

@jescholl @cep21 @Heldroe @adriantr did you had this flag enabled and everything was working fine before?

@adriantr According to [GitHub docs](https://docs.github.com/en/rest/overview/permissions-required-for-github-apps?apiVersion=2022-11-28#administration) branch protection API endpoint should be read accessible if you configured [Administration read-only](https://www.runatlantis.io/docs/access-credentials.html#github-app) permissions for the integration.
I did not have that flag enabled
@jescholl your case is pretty straightforward to me, we need to catch this error as we do for `github.ErrBranchNotProtected` and continue as usual. I will work on the fix for that.

Fix patch:
diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go
index 10b31add1e..0dd2989795 100644
--- 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")
 		}
 	}


Test patch:
diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go
index 3da4ad0b7c..ddc658aca8 100644
--- 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":