in 0.22.3+, auto-merge only works with admin repo permissions

[bl-robinson]

Community Note

• Please vote on this issue by adding a 👍 reaction 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.

---

Overview of the Issue

Auto-merge fails with user setup with personal access token and repo write permissions. This worked fine previously. (we were upgrading from 0.22.3 but I suspect it is introduced in 0.23.3 due to the conversation on a similar issue. #3268)

With our Atlantis user set up as per

• https://www.runatlantis.io/docs/access-credentials.html#create-an-atlantis-user-optional
• https://www.runatlantis.io/docs/access-credentials.html#github-user

The user has 'write' permissions on the Repository.

• 'write' Does not appear to provide access to the https://api.github.com/repos/OWNER/REPO/branches/BRANCH/protection
  api endpoint.
• Confirmed if we update the users repo permissions to 'admin' (In the "Collaborators and Teams" Section of the Repository permissions.) then this works perfectly fine.

Do the docs just need updating to reflect this? Is it desired that Atlantis must have repo 'admin' access?

Reproduction Steps

• Have a github user with a Personal access token with repo scope.
• Grant the user write permissions on the repo.
• atlantis apply

Logs

{
    "level": "error",
    "ts": "2023-04-11T10:34:54.517Z",
    "caller": "vcs/instrumented_client.go:231",
    "msg": "Unable to merge pull, error: getting branch protection rules: GET https://api.github.com/repos/ORG/REPO/branches/master/protection: 404 Not Found []",
    "json": {
        "pull-num": #######
    },
    "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"
},
{
    "level": "info",
    "ts": "2023-04-11T10:34:54.107Z",
    "caller": "events/automerger.go:32",
    "msg": "automerging pull request",
    "json": {
        "repo": "ORG/REPO",
        "pull": "######"
    }
}

Environment details

• Atlantis version: 0.23.4

Additional Context

A similar issue was handled here #3268 but the fix only covered a specific case.

[nitrocode]

cc @tpolekhin

[GenPage]

@tpolekhin @nitrocode We should probably revert the series of PRs as technically this constitutes a breaking change. I've confirmed that you cannot access the protection API endpoint unless your user has admin permissions and not write

[jamengual]

so should we revert and then maybe add a flag to enabled this?

[tpolekhin]

I have no hard feelings reverting this.
It started as a quick fix for a bug but eventually brought a lot of issues.
The problem though, that we have a call to get protected branches in another function, that is rarely used, but still will cause issues if permissions aren't set correctly.

UPDATE:
I just checked and get required status checks also requires admin permissions, so if --gh-allow-mergeable-bypass-apply flag is set, but admin permissions aren't granted it will fail as well.

[GenPage]

@tpolekhin No worries. Unfortunately since this requires informing users about changing permissions, we have to be more careful about staging this change.

We definitely can re-introduce it again but will need a more coordinated rollout that includes documentation and possibly a flag for backwards compatibility or holding it until a major release with announcing the breaking changes to permissions.

[tpolekhin]

@nitrocode @GenPage @jamengual just wanted to double check if my previous comment update didn't went unnoticed

We still have getBranchProtection call in the code to get required status checks:

atlantis/server/events/vcs/github_client.go

Line 409 in fd1ce43

required, _, err := g.client.Repositories.GetBranchProtection(context.Background(), repo.Owner, repo.Name, *pull.Base.Ref)

Fortunately this is only called if --gh-allow-mergeable-bypass-apply flag is set.

But I did not found any mentions regarding required permissions for this to work.

So probably we need to add this do the documentation https://www.runatlantis.io/docs/server-configuration.html#gh-allow-mergeable-bypass-apply

[bschaatsbergen]

Thanks for the quick revert @GenPage 👍🏼

[GenPage]

@tpolekhin Let's get another issues to track this and the documentation work needed. We should definitely have documentation updated to recommend Admin vs Write or encourage people to use a Github App instead if they need more fine grained permissions.

We could also then clean up/deprecate flags as well to ease the transition for users when we re-introduce your changes.