Use secondary mechanism to approve PRs that are approved by `Tribler/core`

[qstokkink]

This issue is a different approach to core problem of #1091.

To reiterate: GitHub only sees PR approvals from users with write access as "real" approvals and the security model of this repository seeks to use as little write access as possible. This leads to the situation where nobody can offer formal PR approval, even though members of Tribler/core should be able to offer "real" approvals.

My new proposal is to use a secondary mechanism that:

1. Listens for new approvals on a PR.
2. Checks if the approving user is a member of Tribler/core and separately approves the PR as well.

To make this secondary approval valid/formal/"real" we can use the @tribler-ci account, the only account that will always have write access.

These are potential GitHub actions we could use for this: https://github.com/marketplace/actions/auto-approve, https://github.com/marketplace/actions/approve-pull-request

[drew2a]

GitHub allows PR reviews from anybody with read access to the repo:

After a pull request is opened, anyone with read access can review and comment on the changes it proposes. You can also suggest specific changes to lines of code, which the author can apply directly from the pull request. For more information, see "Reviewing proposed changes in a pull request." By default, in public repositories, any user can submit reviews that approve or request changes to a pull request.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews

But as far as I understood, the problem is not in the technical possibility of doing this, but of doing this in the current workflow that includes branch protection rules.

If I'm correct, now we have branch protection rule that protect master branch from merges. In this protection rule one of the steps is "require 1 approval". And this approval could be given only from a person with write access to the repo.

If your repository requires reviews, pull requests must have a specific number of approving reviews from people with write or admin permissions in the repository before they can be merged.

https://stackoverflow.com/questions/62318630/require-pr-review-in-branch-protection-rules-without-write-access

[qstokkink]

@drew2a you're indeed correct. The IPv8 repository settings are configured as shown in the stackoverflow answer you linked. The problem lies in the fact that GitHub does not allow you to self-approve. This means that the one person (maintainer) that approves PRs can never make PRs that are approved, because this would be self-approval. This is what we do now.

For the exceptional case of the maintainer creating a PR, I want to use the (non-human) @tribler-ci account, which also has write access. Following the two rules of OP, I want to create a Jenkins job or GitHub Action that uses @tribler-ci to automatically approve PRs if a Tribler/core team member approved. The flow would be something like this:

0. Precondition: a PR is done and ready to be approved.
1. A member of Tribler/core approves the PR. The user does not have write access and the approval is greyed out.
2. ??? (Jenkins/GitHub Action magic here that detects step 1)
3. The @tribler-ci account approves the PR. @tribler-ci does have write acccess and the approval is recognized by GitHub.
4. The maintainer merges the PR.

Step 2 and step 3 would need a new implementation.

[drew2a]

@qstokkink thank you for the clarification! The problem and the desired approach are clear for me now.

As a quick and a raw tip... Maybe we can use this settings to allow the maintainer to bypass a required PR:

[qstokkink]

Something like this perhaps?

on:
  pull_request_review:
    types: [submitted]

jobs:
  build:
  if: github.event.review.state == 'approved'
  runs-on: ubuntu-latest
  permissions:
    pull-requests: write
  steps:
    - uses: tspascoal/get-user-teams-membership@v1
      id: checkUserMember
      with:
         username: ${{ github.event.review.user.login }}
         team: 'Tribler/core'
    - uses: hmarr/auto-approve-action@v2
      if: github.actor != 'github-actions' && ${{ steps.checkUserMember.outputs.isTeamMember == 'true' }}

[drew2a]

@qstokkink looks good to me!

Just one note. That approval will come from the "github-actions" bot user. I'm not sure if it is a problem or not in our case.

By default, this will use the automatic GitHub token that's provided to the workflow. This means the approval will come from the "github-actions" bot user. Make sure you enable the pull-requests: write permission in your workflow.

Anyway, it is possible to approve a PR by any user (eg @tribler-ci):

To approve the pull request as a different user, pass a GitHub Personal Access Token into the github-token input:

name: Auto approve

on: pull_request

jobs:
  auto-approve:
    runs-on: ubuntu-latest
    steps:
      - uses: hmarr/auto-approve-action@v2
        with:
          github-token: ${{ secrets.SOME_USERS_PAT }}

Ref:

• https://github.com/hmarr/auto-approve-action

[qstokkink]

@drew2a thanks for the review!

I'm also unsure if "github-actions" will actually be accepted by GitHub as a valid review. I figured: let's try it and see what happens :-) In the worst case, we can always make the action more complex and use @tribler-ci using the github-token approach you suggested. Initially, I'd like to avoid managing a token for our CI account.

[qstokkink]

@drew2a do you know where the file would have to be located for this to work? Should it be ./.github/actions/something.yml?

[drew2a]

In terms of GitHub what we want to create is called workflow and then it should be located in ./.github/workflows/, eg ./.github/workflows/something.yml

But the workflow can call a custom action. For custom actions, there is a folder ./.github/actions/

Ref:

• https://docs.github.com/en/actions/creating-actions
• https://docs.github.com/en/actions/using-workflows/about-workflows

[qstokkink]

Thanks. To confirm: naming the file ./.github/workflows/autoprreview.yml with the following content should work, right?

name: autoprreview
on:
  pull_request_review:
    types: [submitted]
jobs:
  build:
  if: github.event.review.state == 'approved'
  runs-on: ubuntu-latest
  permissions:
    pull-requests: write
  steps:
    - uses: tspascoal/get-user-teams-membership@v1
      id: checkUserMember
      with:
         username: ${{ github.event.review.user.login }}
         team: 'Tribler/core'
    - uses: hmarr/auto-approve-action@v2
      if: github.actor != 'github-actions' && ${{ steps.checkUserMember.outputs.isTeamMember == 'true' }}

[drew2a]

Yes, this should work.

But to be triggered by pull_request_review this workflow probably must be in the master branch (for forks it is 100% but for branches, I'm not sure)

Ref:

• https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflows-in-forked-repositories-1

[qstokkink]

@drew2a great! IPv8 doesn't allow branches anyway.

[qstokkink]

https://github.com/Tribler/py-ipv8/actions/runs/3377210158 The script appears to be broken. I'll reopen this issue.

[qstokkink]

I'm guessing that the indentation of the if statement is incorrect (based on this documentation)

[qstokkink]

After the failure of #1106, I concluded that Jenkins was the only way forward. However, it may also be possible to create our own GitHub action script. The GitHub API documentation lists single calls for getting a user's team membership and approving a PR (which can be invoked using the "GitHub" GitHub Actions Toolkit).

If we go for our own Jenkins script/GitHub Action script, we can also make this mechanism slightly more fancy and only approve with the @tribler-ci account if a user is part of Tribler/core but has no write permissions.

[qstokkink]

It's probably not worth creating a new script. Whatever we make is going to be non-trivial and I doubt anyone wants to maintain this. We should probably just drop this ambition and remove the autoprreview.yml file.