Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub Mergability check should support ruleset bypasser status #4116

Open
1 task done
usmonster opened this issue Jan 3, 2024 · 0 comments
Open
1 task done

GitHub Mergability check should support ruleset bypasser status #4116

usmonster opened this issue Jan 3, 2024 · 0 comments
Labels
feature New functionality/enhancement Stale

Comments

@usmonster
Copy link

usmonster commented Jan 3, 2024

Note

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.

Describe the user story

As GitHub user, I want Atlantis to merge my pull request when rulesets allow it, but Atlantis will not merge if the ruleset doesn't pass, even if it's explicitly allowed to bypass the rule, but instead adds the comment "Apply Failed: Pull request must be mergeable before running apply."

Please note that this issue doesn't manifest on repos using "classic" branch protections, for example Restrict who can push to matching branches branch protection, which works as expected (i.e. merging is possible).

(More context here and here.)

Describe the solution you'd like

If a GitHub pull request targets a branch protected by a ruleset that Atlantis is explicitly permitted to bypass, then the PR should be considered as mergable. Taking my team's real use case as an example: if I activate a Restrict updates ruleset on my repo and add Atlantis (app or user) to the ruleset's bypass list, then Atlantis should still be able to merge pull requests to this branch.

Describe the drawbacks of your solution

Though the "what" is clear, the "how" is tricky or perhaps currently impossible.

The main issue is that GitHub's pull request API currently returns a merge_status of "blocked" on PRs protected by rules that aren't passing, even if the caller has bypass rights. As noted above, this is different behavior than what's seen when using the legacy Restrict who can push to matching branches branch protection with Atlantis on the "People, teams, or apps with push access" list, in which case the API returns "clean" (assuming the PR is otherwise mergeable).

Any solution would have to support both cases, at least until legacy branch protections are completely phased out, which can add complexity that makes Atlantis's GitHub client harder to maintain.

More importantly, any acceptable solution would need a reliable way to detect when Atlantis is allowed to bypass a ruleset that "blocks" PR, and this may not be something GitHub provides today.

Describe alternatives you've considered

  • We could imagine a mechanism similar to the policy check override (e.g. atlantis approve_merge), but it's not scalable if it's required on every pull request.
  • A configuration option to skip the merge check altogether could be too risky and lead to inconsistent Terraform state if an apply is done when an immediate "bypass merge" of the PR isn't possible due to failing non-bypassable rules.
  • An on-demand atlantis apply --skip-merge-check or similar PR command parameter would have similar drawbacks if the human makes a mistake in assessing atlantis's ability to merge.
  • We can try to limit the PR mergeability check by excluding those we know to be bypassable, similarly to what was done for gh-allow-mergeable-bypass-apply (code), but this can become very complex given the fact that any kind of ruleset can be bypassable.
  • One last option would be very specific to my use case (Restrict updates ruleset + bypass list), and could be to not implement anything and just directly use --gh-allow-mergeable-bypass-apply (with a required atlantis/apply check) since it only checks approvals & commit status, but it feels rather hacky, and there may be more cases that need to be addressed in the advent of repository rulesets becoming the new standard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement Stale
Projects
None yet
Development

No branches or pull requests

1 participant