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

build(gha): Use pull_request_target for acceptance workflow #21600

Merged

Conversation

billyvg
Copy link
Member

@billyvg billyvg commented Oct 26, 2020

This changes our visual snapshots/acceptance workflow to use the pull_request_target event instead of pull_request so that we can have Visual Snapshots working on fork PRs. By default, forks do not have write access tokens, but when using pull_request_target, forked PRs will use the base repository workflows as the source. This ensures that secrets/apis do not get exposed from by the fork changing workflows. See https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows#pull_request_target for more information.

Important notes about pull_request_target:

  • Used to allow forks to have write-access tokens + secrets
  • Ensures safety by only running workflow from the main branch
  • You can test workflow changes by making your branch the base branch in a Pull Request
  • Note that the workflow seems to be cached after opening the PR
    • e.g. if you make a pull request against a feature branch, the workflow that will be used is the workflow in the base branch at the point when you create the PR. From there on, you won't be able to change the workflow that is run
  • You must specify the ref + repository when using the checkout action

This changes our visual snapshots/acceptance workflow to use the `pull_request_target` event instead of `pull_request` so that we can have Visual Snapshots working on fork PRs. By default, forks do not have write access tokens, but when using `pull_request_target`, forked PRs will use the base repository workflows as the source. This ensures that secrets/apis do not get exposed from by the fork changing workflows. See https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows\#pull_request_target for more information
This changes our visual snapshots/acceptance workflow to use the `pull_request_target` event instead of `pull_request` so that we can have Visual Snapshots working on fork PRs. By default, forks do not have write access tokens, but when using `pull_request_target`, forked PRs will use the base repository workflows as the source. This ensures that secrets/apis do not get exposed from by the fork changing workflows. See https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows\#pull_request_target for more information
@billyvg
Copy link
Member Author

billyvg commented Oct 28, 2020

Follow-up to #21489 - there are some subtleties that are described in the PR body. (Mainly: when testing and having this branch as the base branch, the workflow is cached when you create the PR, and will not be updated despite updating this branch. Also, it requires you to specify the repo + ref when checking out code).

You can see test PR here: #21492 (Snapshots show the changes)

@billyvg
Copy link
Member Author

billyvg commented Oct 28, 2020

Also this PR won't be able to run acceptance 2.7 because we changed the workflow target from pull_request to pull_request_target

@billyvg
Copy link
Member Author

billyvg commented Oct 28, 2020

Admin merging this as our acceptance.yml workflow is unable to fire because we're changing the workflow target from pull_request to pull_request_target (which means it runs the workflow on main branch (which doesn't exist))

@billyvg billyvg merged commit d7b28d6 into master Oct 28, 2020
@billyvg billyvg deleted the build/gha/change-acceptance-workflow-pull-request-target branch October 28, 2020 20:55
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants