-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 branch protection: Automerge cannot work when allowed users or teams who can push to master has been enabled #846
Comments
yep, I was thinking about that. current workaround, for example if some is using pullapprove, to add the bot as allowed contributor. |
yeah, the work around would be to stop using GitHub's direct controls on who can commit and instead use a third party status check like that. Hopefully they fix soon though! |
I updated Renovate's detection of GitHub branch protections yesterday, it now accurate determines if any of these settings are enabled and blocking automerge:
The second one is what the topic of this issue is about. This is where an organization has configured allowed users or teams who can push to The ultimate solution is for GitHub to update the UI and backend so allow you to specify Apps in that field and not just users and teams. But they've known about this for nearly a year and still not fixed it, which is not a good sign. I am considering an option for the app which would allow organisations to add the "real" GitHub user @renovate-bot with master push rights in case you want to have (a) branch push protections, and (b) Renovate still able to do its job. That way the app would use its app permissions for everything else like it does today, but fall back to using @renovate-bot's permissions whenever it needs to merge. |
Is this an issue where the branch is simply a "protected branch"? Or that and when reviews are required and/or "Restrict who can push to this branch" is checked? |
There are a few ways you can protect a branch and each has a different effect. When you limit who can merge to master then that’s a showstopper for Renovate and any other app because GitHub provide no way for you to add a “bot” to that list. The rest (eg required status checks, review approvals, etc) are not showstoppers but may hold up automerge until they pass. |
So if "Require status checks to pass before merging" is the only one checked, once all statuses pass it should automerge? Are there logs that show why auto merge may not be working? |
Yes it should automerge, but right now it’s not triggered by webhooks (ie won’t happen immediately after it goes green), so it should get picked up and automerged on the next hourly cycle. Log in to https://renovatebot.com/dashboard to check the logs
…________________________________
From: Steven Zeck <[email protected]>
Sent: Tuesday, July 10, 2018 7:18 PM
To: renovatebot/renovate
Cc: Rhys Arkins; Author
Subject: Re: [renovatebot/renovate] Automerge not working for GitHub organizations with branch protection (Pushing to branch is restricted) (#846)
So if "Require status checks to pass before merging" is the only one checked, once all statuses pass it should automerge? Are there logs that show why auto merge may not be working?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#846 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AGBPaOPiimAS_r3h5yUPxkdeDMvn_PyFks5uFOH2gaJpZM4Pm0Hj>.
|
Thanks @rarkins! |
To confirm, we use renovate with private repos with protected branches. Specifically, our branches require reviews and all status checks to pass. As @rarkins stated, automerge only happens after these checks pass and on the next renovate check-in, but it does work. I'll approve PRs during the day, and they'll merge themselves during the night. 👍 |
Just curious if this work happened or not so that renovate-bot, if provided write access to a branch, would actually write to it. If so, I'm assuming the procedure in get this enabled if so would be either
Or if in a organization setup
|
@k2snowman69 I have not added this logic yet because I was really hoping GitHub would fix this feature to allow bots to also merge. FYI, |
Hey @rarkins, have you heard more from Github re treating bots like normal users, so we can resolve that issue with automerge and protected banches? .. thinking if there's any third-party solution around that would pull repos + prs, and automerge based on some rules. If not, I might have to craft one while GH is fixing the thing. |
@runk unfortunately not. For some reason they seem to have put it in the “too hard” basket and with no communication updates (the latter is typical of GitHub though, they tend not to forecast product updates big or small). I think the only solutions are:
Renovate already has compatibility with https://github.com/bors-ng/bors-ng if you’re interested in the first option. In that case Renovate would add a special merge comment/command instead of actually automerging. The bors bot - running as a real user - would then do the actual merging to master. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Could Renovate approve the PR through the API? We recently enabled this and I immediately felt the pain that automerge doesn't work anymore :/ |
@tunnckoCore do you know a free alternative to pullapprove? |
@felixfbecker on GitHub, neither a user nor app can approves its own PRs to get around the limitation, but I do also run Renovate Approve and it might do what you want? |
FYI it will only approve PRs that are created by Renovate and which have automerge enabled. |
awesome, exactly what I need. |
Sweeeet, that new bot sounds fantastic. |
👋 If you have push restrictions enabled on your protected branch you can now allow installed apps to push from the branch protection settings page: https://developer.github.com/changes/2019-09-05-apps-protected-branches-api/ |
@feelepxyz that's great news - extra nice of you to even take a custom screenshot 😆 After ~2 years, this issue can now be closed! |
@rarkins yeah it's been a long time coming! 🙌😁 |
I include Renovate app in the settings shown above but still the automerge could not work. I enabled required reviews (which renovate-approve already handles pretty good) and required status check (it passed). So I don't really know where it could go wrong. @feelepxyz can yours work after this configuration? Could you please share your other branch settings to help me identify the issue? Thanks! |
@robertying ah it's probably going to be that the merge is attempted before the approval happens which will block the merge even if the app is added to the list of allowed users (setting both push restrictions and required reviewers will cause this issue). Not sure if it's possible but I think there needs to be a second merge attempt from the |
@feelepxyz I checked the logs from renovate dashboard and there was an attempt after the pull request approval. I guess I'll have to open another issue to better address my problem. Thanks for responding! |
@robertying do you see the error from the GitHub API in the logs? Also, try disabling |
Hmmm. Why I don't see that option in the settings of a repository? 🤔 |
Organizations that have added branch protections are unable to utilise Renovate's automerge feature, because GitHub is (incorrectly) blocking the renovate app account from pushing to master, including via Pull Request. The GitHub UI does not provide a way for org admins to add apps/bots as "users" with permissions, meanwhile GitHub's API ignores that the app has already been granted full "write" permissions.
Waiting for an update from GitHub here: https://platform.github.community/t/repositories-which-have-protected-branches-with-push-restrictions-have-no-ability-to-grant-push-rights-to-integrations/1376/11
The text was updated successfully, but these errors were encountered: