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

clarify github actions for fork PRs #262

Closed
maxheld83 opened this issue Jun 18, 2019 · 20 comments
Closed

clarify github actions for fork PRs #262

maxheld83 opened this issue Jun 18, 2019 · 20 comments
Labels
actions these are, well, actual actions (the building blocks) blocked github Related to / requiring support from github (actions) placeholder this is really waiting on an issue in another repo
Projects

Comments

@maxheld83
Copy link
Owner

it's a bit unclear how users (at tidyverse developer day etc.) can use github actions if they are not on the beta.

@maxheld83 maxheld83 added the actions these are, well, actual actions (the building blocks) label Jun 18, 2019
@maxheld83 maxheld83 added this to the useR 2019 milestone Jun 18, 2019
@maxheld83 maxheld83 added this to To do in Kanban via automation Jun 18, 2019
@maxheld83
Copy link
Owner Author

@VerenaHeld @Testuser13029 could you paste the screenshot in here?

It appears that first of all, non-beta users don't see the "actions" tab in repos with actions.

@VerenaHeld
Copy link
Collaborator

no actions tab: Screenshot 2019-06-18 at 09 49 35

@maxheld83
Copy link
Owner Author

says GitHub support:

This is a mitigation against the possibility that a bad actor could open PRs against your repo and do things like list out secrets or just run up a large bill (once we start charging) on your account. The actions are in fact executed, but it happens against the fork, not against the base repo.

This does require that the forked repo also has actions enabled though. During the beta period that means that the owner of the forked repo must also be in the beta.

@maxheld83
Copy link
Owner Author

maxheld83 commented Jun 18, 2019

so, to sum up, for now:

  1. non-beta users won't see the action tab anywhere.
  2. pull requests from a fork won't show the check results in the PR view (see Update README.md #261 (non-beta), ship with ghactions, try PR #262 #263 (beta)).
    Presumably, this is because an action on a PR gets run against the fork, not base, for security reasons (bad actors could make main.workflow reveal secrets).
    If the fork creator has the beta, s/he can at least see the actions run there. (Actually, s/he can't because first need to delete/recommit main.workflow as per must delete, recommit main.workflow in forks/moved repos #264).
    If the for creator has is off beta, no such luck.
  3. By extension, programmatic-commit type actions will still run on the forks (such as to fix documentation), but only if the fork creator is on the beta.

So this isn't so great for the tidyverse developer day, unless we want to invite everyone as repo contributors.

This all works fine for PRs within the same repo (which is all I have used so far, never noticed this).

@maxheld83 maxheld83 moved this from To do to In progress in Kanban Jun 18, 2019
@maxheld83
Copy link
Owner Author

also relevant from the docs:

Pull request events for forked repositories

When a pull request is opened from a forked repository, the pull request will be opened on the base repository by default. The pull_request event is sent to the base repository, with no pull request events on the forked repository because a forked repository does not carry over all of the webhook configuration of the original repository. If you intend to use the pull_request event to trigger CI tests, it's recommended to listen to the push event.

Pull request with base and head branches in the same repository

  • The repository receives a pull_request event where the SHA is the latest commit of head branch and the ref is the head branch.

Pull request with base and head branches in different repositories

  • The base repository receives a pull_request event where the SHA is the latest commit of base branch and ref is the base branch.

@maxheld83
Copy link
Owner Author

taken together, this seems like a pretty foundational / wide-reaching limitation, unrelated to the limitations for non-beta users, that I hadn't really grasped before.

  • workflows on = "push" always run against the repo to which the push occurred (makes sense).
  • there is no special exception for pull requests: these also always run against the repo to which the push occurred (the fork!).
  • More fundamentally, GitHub actions doesn't do an automatic merge on every commit to the PR (and why would it?), and check that. (Travis used to do this).

So at the moment, out-of-the-box, this is pretty limiting for CI/CD via actions out of the box for anything involving forks.
(Everything works fine on branches, so I never thought about this).

  • workflows on = "pull_request" do run on base (i.e. the PR-receiving repo), but always against master (?) branch of base.
    (Only for PRs from the same repo, does on = "pull_request" run on the head branch.)

These limitation make sense, unless/until GitHub adds some kind of protection around secret environment variables.
You could then write workflows which only use actions which don't require secrets, much as happens on Travis.

@maxheld83
Copy link
Owner Author

maxheld83 commented Jun 18, 2019

Work around 1 would be to use on = "pull_request" and add a (custom) action which git clones the fork repo, and run later against that, too.
Pretty crufty.

This is what the impressive zeit get-stats action appears to do:

  await checkoutRepo(MAIN_REPO, MAIN_REF, mainDir)
  await buildRepo(mainDir)
  console.log()


  await checkoutRepo(PR_REPO, PR_REF, prDir)

This might work, but would essentially open us up to all the dangers this limitation is supposed to mitigate, so I think that's a bad idea.

@maxheld83
Copy link
Owner Author

maxheld83 commented Jun 18, 2019

Work around 2 might be to use on = "push", and to somehow hard-code the references to "upstream" into the actions, so that even when run on fork, they would "phone home" to the check suite of the pull request in question if there is an outstanding pull request.
(Actually, the Checks API is exclusive to GitHub apps, so that would require yet more work).
So you could just phone home to simple issue comments.

The downstream workflow might then also clone the upstream repo to run tests and comparisons.

This should (?) be safer, because never is code run in the environments where there are secrets (the upstream repo).

But this seems like quite a bit of work and custom stuff for something that really, github actions ought to be doing out of the box.

@maxheld83
Copy link
Owner Author

Option 3 might be to just hum loudly and hope for the problem to disappear (i.e. for github to roll out more features).

Seems like this might be the best use of resources for now.

The programmatic commit actions would be a bit hamstrung for forks, at least as long as not too many people are in the beta and #264 persists.
At least they should run when the fork PRs are merged.

The programmatic code review stuff really would need work around 2 to make much sense.
Otherwise, only branch PRs could be code reviewed, which seems pretty limited.

@maxheld83
Copy link
Owner Author

going to put this on blocked and ignore this for a while.

@maxheld83 maxheld83 changed the title clarify github actions for non-beta users clarify github actions for fork PRs Jun 18, 2019
@maxheld83 maxheld83 added blocked github Related to / requiring support from github (actions) placeholder this is really waiting on an issue in another repo labels Jun 18, 2019
@maxheld83 maxheld83 moved this from In progress to Blocked in Kanban Jun 18, 2019
@maxheld83 maxheld83 removed this from the useR 2019 milestone Jul 17, 2019
@maxheld83
Copy link
Owner Author

obsolete or done in the new yml-based GitHub actions.

Kanban automation moved this from Blocked to Done Aug 14, 2019
@LizhangX
Copy link

Ran into the same issue with fork repos, even in yml based actions. Going to test it with 2 beta accounts and see if that works. If it does, whenever Github releases it to general, everyone should have access to actions and it won't be a problem.

@tomfriedhof
Copy link

I am using forks and have beta access in the github organization as well as my personal account and I can confirm that PR's from my fork to the organization upstream does not trigger a workflow for pull requests.

@maxheld83 maxheld83 reopened this Sep 18, 2019
Kanban automation moved this from Done to In progress Sep 18, 2019
@maxheld83 maxheld83 moved this from In progress to To do in Kanban Sep 19, 2019
@s-i-l-k-e
Copy link

@tomfriedhof are you still experiencing this problem?

@tomfriedhof
Copy link

@s-i-l-k-e just tested this again. Builds still do not trigger from a forked repo when PR'd

@tomfriedhof
Copy link

Also, why is it that when I create a PR from the same fork, that I see my actions being fired off twice?

Screen Shot 2019-10-09 at 4 40 31 PM

@MIreland
Copy link

MIreland commented Oct 9, 2019

I'm pretty sure that that is getting triggered once each from the pull request and from the push.

@tomfriedhof
Copy link

Ah, I need to set push to only run on specific branches like master so that it's not run on PR branches. Thanks @MIreland!

tsvetomir pushed a commit to telerik/kendo-angular-messages that referenced this issue Nov 1, 2019
GitHub Actions are not usable for PR based workflow, see maxheld83/ghactions#262 (comment)

This reverts commit 8d3114f.
kubedan pushed a commit to kubedan/kendo-angular-messages that referenced this issue Dec 5, 2019
GitHub Actions are not usable for PR based workflow, see maxheld83/ghactions#262 (comment)

This reverts commit 8d3114f.
kubedan pushed a commit to kubedan/kendo-angular-messages that referenced this issue Dec 5, 2019
GitHub Actions are not usable for PR based workflow, see maxheld83/ghactions#262 (comment)

This reverts commit 8d3114f.
@mhoeger
Copy link

mhoeger commented Jun 5, 2020

Any updates on this issue? I think a comment trigger would be also really helpful for this scenario: https://docs.microsoft.com/en-us/azure/devops/pipelines/repos/github?view=azure-devops&tabs=yaml#comment-triggers

@maxheld83
Copy link
Owner Author

I think most of the discussion in here was about the old (HCL-based) GitHub Actions.

Just in case people are still wondering, this is how you can explicate when a workflow should run:

on:
  push:
  pull_request:
    types: [assigned, opened, synchronize, reopened]
  release:
    types: [published, created, edited]

More on the official documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actions these are, well, actual actions (the building blocks) blocked github Related to / requiring support from github (actions) placeholder this is really waiting on an issue in another repo
Projects
No open projects
Kanban
  
To do
Development

No branches or pull requests

7 participants