sgibson91 / test-this-pr-action Goto Github PK

Move a PR from a forked repo into the parent repo so that workflows that require secrets can be run

License: MIT License

Python 90.44% Dockerfile 9.56%

test-this-pr-action's Introduction

Test this PR! - Docker-based Action

The fork --> develop --> open pull request workflow is a popular one across large software projects. However if you have tests that require secrets, such as deploying to staging environments before production, this workflow can be a hindrance when managing that workflow through GitHub Actions, since the runner doesn't automatically grant access to repository secrets.

This repository is a Docker-based GitHub Action that will push the changes of a pull request opened from a fork into a new branch in the parent repo so test workflows that require secrets can be executed.

You will need to create a GitHub access token with enough permissions to write to the parent repo and save this as a repository secret. If the parent repo is public, public_repo should be enough.

Inputs

Input variable	Description	Required?	Default value
`access_token`	A GitHub token with read/write access to the parent repository	Yes
`repository`	The name of the parent repository in the form `owner/project`	No	`${{ github.repository }}`
`pr_number`	The number of the Pull Request to be tested	No	`${{ github.event.issue.number }}`

Example Usage

The below example demonstrates how to trigger the GitHub Action by leaving a comment containing /test-this-pr on a pull request, providing the comment author has appropriate permissions on the parent repository.

NOTE: These permissions are provided as an example only and users should carefully read the GitHub documentation before deciding which roles to use.

name: Move forked-PR into parent repo for testing

on:
  issue_comment:
    types: [created]

jobs:
  test-this-pr:
    runs-on: ubuntu-latest
    if: |
      # Check this issue is a pull request
      (github.event.issue.pull_request != null) &&
      # Check the comment contains the trigger string
      contains(github.event.comment.body, '/test-this-pr') &&
      # Check the comment author has appropriate permissions
      contains(
        github.event.comment.author_association,
        ['OWNER', 'COLLABORATOR', 'MEMBER']
      )

    steps:
      - uses: sgibson91/test-this-pr-action@main
        with:
          access_token: ${{ secrets.ACCESS_TOKEN }}

test-this-pr-action's People

Stargazers

Watchers

Forkers

minrk manics

test-this-pr-action's Issues

Provide a link to GitHub Actions instead of branch

The comment at the end provides a link directly to the Actions page for that branch, e.g. https://github.com/binderhub-test-org/pr-test/actions?query=branch%3Amore-tests

Initial review

This is great!

Some notes from looking at the action:

#17 the branch is pushed to test-this-pr/$NUMBER. If a PR is force-updated and then the test comment is issued again, I think this push will fail unless it's done with --force
#18 I believe there is already a merge ref available so you can avoid fetching both full repos and doing the merge yourself: git fetch origin pull/$NUMBER/merge:test-this-pr/$NUMBER will fetch the existing merge ref and create the local branch test-this-pr/$NUMBER.
#19 The comment at the end provides a link to the tree view of the new branch. Since this is for testing, it might make sense to link directly to the Actions page for that branch, e.g. https://github.com/binderhub-test-org/pr-test/actions?query=branch%3Amore-tests
Should the test-this-pr branch be deleted automatically on any particular event? E.g. merge/close of the parent PR?

I still don't know if it will work best to 'just' test the branch, or open local-branch PRs. WDYT so far after trying it out?

My initial thoughts:

pro PR: you get all the nice checks UI, easier view of success/failure
pro just branch: two PRs for the same change might get confusing. Which one do we merge? What happens when they update? If they are in-sync, merging either one should close both, but it's not easy to see if/when they are out of sync.
pro just branch: It seems like the checks/test conditions to avoid too many redundant tests might be easier for this case? I'm not sure.

Use force-push so repeated action triggers don't fail

the branch is pushed to test-this-pr/$NUMBER. If a PR is force-updated and then the test comment is issued again, I think this push will fail unless it's done with --force

[BUG] `git push` step is failing inside Docker container

Describe the bug

When running the Python script directly, everything works as expected.

However, building and running the Docker image fails at the git push step with the following message

fatal: Authentication failed for 'https://***:[email protected]/{REPOSITORY}.git/'

As a consequence of this, the action does not work in GitHub Actions runners

Expected behaviour

Changes to the git repository are able to be pushed back to GitHub successfully from within the Docker container

Steps to reproduce

Clone the repo

git clone https://github.com/sgibson91/test-this-pr-action.git
cd test-this-pr-action

Build the Docker image

docker build -t sgibson91/test-this-pr:dev .

Export secrets

export INPUT_ACCESS_TOKEN=<PASTE GITHUB TOKEN HERE>
export INPUT_REPOSITORY=binderhub-test-org/pr-test
export INPUT_PR_NUMBER=1

Run Docker image

docker run -it sgibson91/test-this-pr:dev

See error message

Environment

No response

Anything else?

No response

[BUG] Using `permissions` key to set GITHUB_TOKEN when running the action

Summary

At the minute, I pass a PAT with public_repo scope to test-this-pr-action which allows me to push a new branch to a repo I have write access to and leave a comment on the open PR.

I was playing around with setting the permissions of the GITHUB_TOKEN to see if I could grant it just enough to do the above instead of my account. The following permissions can create the branch and leave the comment:

permissions:
  contents: write
  pulls-requests: write

But seemingly with these permissions to push to a branch, does not seem to trigger the a separate action that is set to trigger on pushes to main and test-this-pr/** branches. Seems counter-intuitive to me!

I have been testing the action in this repo: https://github.com/binderhub-test-org/pr-test

Refs:

https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idpermissions

Use the merge ref instead of adding the forked repo as a remote

there is already a merge ref available so you can avoid fetching both full repos and doing the merge yourself: git fetch origin pull/$NUMBER/merge:test-this-pr/$NUMBER will fetch the existing merge ref and create the local branch test-this-pr/$NUMBER.