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

Github Action: pull_request_target not supported #13

Closed
laolubenson opened this issue Sep 12, 2020 · 19 comments
Closed

Github Action: pull_request_target not supported #13

laolubenson opened this issue Sep 12, 2020 · 19 comments
Assignees
Labels
enhancement New feature or request

Comments

@laolubenson
Copy link

I have added this GitHub action to my repo with the event type: pull_request_target instead of pull_request. This seems to run on the base repository as expected but I get the following error message:

2020/09/11 18:18:35 Trigger event: pull_request_target
2020/09/11 18:18:35 Unable to execute action: unknown X-Github-Event in message: pull_request_target

Is there anyway this can be supported?

@srvaroa
Copy link
Owner

srvaroa commented Sep 12, 2020

Hi, thanks for trying it out. It depends on the go-github library supporting that event. On a quick inspection it looks like the latest version doesn't support it yet (https://github.com/google/go-github/blob/159b448168db1fc1bc61a556e6280e12dfc00a6d/github/messages.go#L43). If you can make a feature request there I'm able to integrate it fairly quickly once it's available.

I may be able to implement it myself but can't promise it'll be too fast.

@srvaroa
Copy link
Owner

srvaroa commented Sep 12, 2020

I just noticed there is an open PR for that new event type so it'll hopefully get merged soon.

@srvaroa srvaroa added the enhancement New feature or request label Sep 12, 2020
@srvaroa
Copy link
Owner

srvaroa commented Sep 18, 2020

It's been merged, I'll wait for a release and upgrade.

@kbendick
Copy link

kbendick commented Nov 4, 2020

Hi @srvaroa, has this been fixed? The pull_request_target is important for repos where people fork and submit PRs but don't necessarily have write access. For example, many of the Apache repos. The example github labeler action doesn't support the features that we need for the Apache Spark repo.

If this has been merged, can you cut a release or let me know which release version needs to be used so that pull_request_target is supported?

@srvaroa
Copy link
Owner

srvaroa commented Nov 4, 2020

Hi @kbendick I haven't seen a release including that PR yet. What we can do is try to bump the go-github version and use the current master so we can see that event before they publish a final release. I can try this this week, but do feel free to send a PR!

@laolubenson
Copy link
Author

laolubenson commented Jan 14, 2021

@srvaroa Think this might be out in a release now

@srvaroa
Copy link
Owner

srvaroa commented Jan 15, 2021

Thanks for the heads up @akinsolb I'll take a look

@jhlegarreta
Copy link

@srvaroa folks at goolge said that the feature in the PR mentioned in #13 (comment) will cut into a release within the next few hours/maybe a day or two.

Now, inspecting the webhook event mapping you pointed, that PR does not include the pull_request_target event. So I'm wondering whether the mentioned PR does in reality contain what is necessary to support pull_request_target events in your PR labeler action or else the mapping would still be missing. Looking forward to the feature. Thanks.

@gmlewis
Copy link

gmlewis commented Mar 26, 2021

Thank you, @jhlegarreta - it looks like we haven't implemented support for that webhook event. I've opened a new issue to support it: google/go-github#1834

@jhlegarreta
Copy link

go-github now provides support for pull_request_targert events 🎉.

@srvaroa provided that they tag a new version (they do that on demand), do you think that is enough to support them in the labeler actions/would have the time to implement it? Maybe an attempt on the go-github might cast light on whether something is still missing on that end? Thanks.

@srvaroa
Copy link
Owner

srvaroa commented Apr 8, 2021

Hi, sorry I missed replying here. A release from their side would help to try and implement this case here. I can probably put a bit of time to try and get it working (of course, very happy if someone else wants to take a stab at it).

@gmlewis
Copy link

gmlewis commented Apr 8, 2021

So a tagged release of go-github would be useful here? If so, I can start working on one.

@srvaroa
Copy link
Owner

srvaroa commented Apr 8, 2021

yup, thanks!

@jhlegarreta
Copy link

Wonderful both @srvaroa and @gmlewis 💯 !!

@gmlewis
Copy link

gmlewis commented Apr 14, 2021

I finally got an LGTM, so https://github.com/google/go-github/releases/tag/v35.0.0 has now been released.

@jhlegarreta
Copy link

Wonderful @gmlewis. Thanks !

@srvaroa any progress on this would be much appreciated !

@srvaroa
Copy link
Owner

srvaroa commented Apr 24, 2021

Hi, thanks @gmlewis and @jhlegarreta, I've just submitted a PR that should make this work. I can't test it more thoroughly now but if you can try it out (using srvaroa/labeller@g.dev) it'd be appreciated. A sample payload that I can add to a test would also help.

PR: #27

Cheers,

@srvaroa srvaroa self-assigned this Apr 24, 2021
@jhlegarreta
Copy link

Thanks for the effort @srvaroa 👍.

Unfortunately, it looks like the first attempt was unsuccessful here:
https://github.com/InsightSoftwareConsortium/ITK/runs/2427975641?check_suite_focus=true#step:3:78

But that's because the workflow is still attempting to use the master branch:
https://github.com/InsightSoftwareConsortium/ITK/runs/2427975641?check_suite_focus=true#step:3:1

And I fear that unless I merge the PR, we will not know if the changes in #27 work for the target use case. I'd like to avoid doing various merge and revert cycles or tests on that repository for many reasons 😔.

Maybe your or I can open a test repository that includes an action that uses your labeler, and the other one can try submitting a PR to see if pull_request_target-like events are working off the branch in #27?

@jhlegarreta
Copy link

For the records, following the above comment we tested the branch in another repository and the action labeled the issue 🚀 !

Besides, the action creates the corresponding label if it does not exist when a match is found in the configuration file.

Thnx @srvaroa 💯 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants