github-action-merge-dependabot icon indicating copy to clipboard operation
github-action-merge-dependabot copied to clipboard

action fails when repo protection rule specifies 2 reviewers required

Open wenqiglantz opened this issue 2 years ago • 4 comments

UPDATE: see implementation proposal in this comment

Prerequisites

  • [X] I have written a descriptive issue title
  • [X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

3.2.0

Plugin version

No response

Node.js version

16.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

github actions runner

Description

Hi,

We are running into the following error after configuring github branch protection rule to mandate 2 code reviewers for pull requests. See attached the screenshot for our branch protection rule in github, as well as the error thrown when executing your action in our github actions workflow. It seems your action defaults to 1 approver, so if github branch protection rule overwrites that default 1 approver, we are seeing this error. Is there any way for your action to work around the github branch protection rule?

Error: At least 2 approving reviews are required by reviewers with write access.

image

image

Steps to Reproduce

Enable your github repo's branch protection rule to have 2 approvers, then run github actions workflow to auto merge dependabot PR, you will see this error:

At least 2 approving reviews are required by reviewers with write access.

Expected Behavior

Your action should allow input of the number of approvers and handle accordingly.

wenqiglantz avatar Aug 11 '22 19:08 wenqiglantz

Your action should allow input of the number of approvers and handle accordingly.

This is imho out of scope of this action.

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-when-a-pull-request-is-approved

You can add a workflow which listens on the event of review submitted, and with octokit action you can make a request to /repos/:owner/:repo/pulls/:pull_number/reviews and get the reviews (be careful of pagination) and count the reviews where the state is set to "APPROVED", and if it is bigger than your treshold, you can trigger the github-action-merge-dependabot

Or add a PAT of an admin as secret and use it for merging. Or set up "Allow specified actors to bypass required pull requests" accordingly

Uzlopak avatar Aug 11 '22 21:08 Uzlopak

@wenqiglantz that's an interesting scenario, thanks for raising it. The examples you find in the README show how to use this action as part of a hypothetical CI process so that, once CI is completed, automerge is done.

Your is a valid scenario too, which I believe simply requires to change how the action is invoked. If you need at least two reviewers, then what you could do is execute the action after an approving review comes in, as described for instance in the docs here.

Let us know if it's a viable solution.

simoneb avatar Aug 12 '22 08:08 simoneb

@wenqiglantz just checking if you had a chance to look into this? If the solution I suggest works, we'd appreciate a PR to update the docs for this usage scenario

simoneb avatar Aug 19 '22 16:08 simoneb

Ok so, I've had a closer look at this issue and I believe a possible support for this could work like this:

  • in the action, only merge if the PR is mergeable. mergeable is a pull request field returned by the gql API (maybe accessible via other means too, e.g. octokit)
  • this wouldn't completely solve the problem because the recommended use of this action is as a job in a CI workflow. A CI workflow is usually triggered by push/pull_request events, meaning that it wouldn't re-run when an approval is submitted
  • hence, a viable approach might be to
    1. change the action as above, to only merge if the PR is mergeable
    2. suggest a different/complementary usage of the action, which supports both repo which require 0/1 approvals (as is now) or >1. This may involve a new workflow, thereby duplicating usages of the action, or simply suggest that the action is to be used in a standalone workflow, which is triggered on all possible events that could lead to a PR being merged, e.g. status check completion / PR approval

simoneb avatar Oct 20 '22 14:10 simoneb

@wenqiglantz this use case is now supported by enabling the automerge option on the action. This will allow the action to add 1 approve and the PR will be merged automatically after one of the maintainers add the second approval.

Do you think that solves your issue?

guilhermelimak avatar Nov 14 '22 14:11 guilhermelimak

closing as this is a good solution and doesn't require any further changes to the code. plus the OP never responded so no point in waiting for further feedback

simoneb avatar Nov 14 '22 17:11 simoneb