Action fails to upload when using pull_request_target on GitHub

kennethtran93 commented 3 years ago

The addition of pull_request_target events allows PRs from forked repos coming into the head repo to utilize some functions that pull_request alone cannot do if PR is coming from a forked repo.

With that in mind the action always fail because GITHUB_REF is the PR Base Branch path (refs/heads/branch name). Is there a way this can be fixed through actions to detect the PR number that actions is running that will also allow usage of pull_request_target?

For the time being I am manually using the bash uploader but manually parsing through the event and extracting the PR number, which I then force the uploader to use it via -P. While this works, there's nothing shown on that Pull Request from CodeCov unfortunately.

thomasrockhu commented 3 years ago

@kennethtran93, can you share the snippet that you are using with the bash uploader and potentially a build URL?

kennethtran93 commented 3 years ago

With my alternate way, I'm using GitHub's Pull Request Object:

      - name: Codecov.io Alternative Coverage Upload
        if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
        env:
          PR_NUM: ${{ github.event.pull_request.number }}
        run: bash ./.github/codecov_alt.sh $GITHUB_EVENT_PATH

The $GITHUB_EVENT_PATH passed in is the event object, though at this time I'm not parsing anything from it so it can be removed. And from within the bash script

if [ -z "$PR_NUM" ]; then
  bash <(curl -s https://codecov.io/bash) -f ./coverage/* -Z
  exit $?
else
  bash <(curl -s https://codecov.io/bash) -f ./coverage/* -P $PR_NUM -Z
  exit $?
fi

As for the build URL...I'm not sure what you need exactly from me. Here's one of the runs that uses this action within: https://github.com/Cookie-AutoDelete/Cookie-AutoDelete/runs/1290859899?check_suite_focus=true

kennethtran93 commented 3 years ago

Maybe as a sidenote since there's override args in the bash script, perhaps add it into the actions input so that it gets passed over as well? Not sure if that's hard to do but looking at the source code here it doesn't look too difficult to implement.

kennethtran93 commented 3 years ago

@thomasrockhu If it helps I tried to implement a passthrough of bash arguments for the bash uploader via #156 . May resolve some older requests for such args within action.

thomasrockhu commented 3 years ago

@kennethtran93, apologies for the wait here and thanks for all of this! By build URL, I'm looking for a GitHub action run that uses pull_request_target that is failing to be on the right branch and PR number.

kennethtran93 commented 3 years ago

@kennethtran93, apologies for the wait here and thanks for all of this! By build URL, I'm looking for a GitHub action run that uses pull_request_target that is failing to be on the right branch and PR number.

In that case I've already edited one of the previous post with the link which uses pull_request_target. I'll expand on that below.

https://github.com/Cookie-AutoDelete/Cookie-AutoDelete/runs/1290859143?check_suite_focus=true
- This runs first, but it is simply a small job to parse and extract certain variables. In here you can check the entire github object as well.
https://github.com/Cookie-AutoDelete/Cookie-AutoDelete/runs/1290859899?check_suite_focus=true
- Somewhere down the middle of the list of steps is the labelled Codecov.io Coverage Upload which runs the action and spits out the error pr must match pattern ^(\d+|false|null|undefined|true)$
- The step right after that is my alternate upload.

thomasrockhu commented 3 years ago

@kennethtran93, this is excellent, I'll work on deploying a fix over the next few days.

bfjelds commented 3 years ago

I've just started looking at this, but for pull_request_target, you may want to specify the commit (-C) as well. In my quick testing, when I've use -P, the pull request number is properly noted, but the codecov bash script seems (from -> View reports at) to pick up the last commit SHA in the merged code (rather than the SHA of the last commit in the PR).

thomasrockhu commented 3 years ago

@bfjelds

the last commit SHA in the merged code (rather than the SHA of the last commit in the PR).

Would you mind spelling this out for me a little more?

bfjelds commented 3 years ago

It is likely that I am misunderstanding, but here is my attempt at explanation :)

I have a GH workflow that uploads some coverage info and is triggered by pull_request_target.

When it executes, it passes the PR# using -P and the codecov bash script (https://github.com/deislabs/akri/blob/b043a7024397f9bb60ccd002f18ef5e1636219ac/.github/workflows/run-tarpaulin.yml#L72)

When I inspect the log of the workflow (https://github.com/deislabs/akri/runs/1428057802?check_suite_focus=true), I see this:

    -> View reports at https://codecov.io/github/deislabs/akri/commit/b043a7024397f9bb60ccd002f18ef5e1636219ac

The commit referenced in the link is the commit that the PR is merged into, a commit from the PR. Perhaps this is intentional, but I expected the referenced commit to be the last commit in the PR.

itavero commented 3 years ago

Took a bit of trial and error, but this step seems to work:

      - name: Publish Code Coverage
        uses: codecov/codecov-action@v1
        if: github.event_name == 'pull_request_target'
        with:
          override_pr: ${{ github.event.number }}
          override_sha: ${{ github.event.pull_request.head.sha }}
          files: ./coverage/clover.xml
          flags: tests
          fail_ci_if_error: true

petrsvihlik commented 3 years ago

Workaround for codecov-action v1.2.1:

    steps:
    - uses: actions/checkout@v2      
      with:
        fetch-depth: 2
    - name: Codecov
      uses: codecov/codecov-action@v1.2.1
      with:
        token: ${{ secrets.CODECOV_TOKEN }}
        override_pr: ${{ github.event.number }}
        override_commit: ${{ github.event.pull_request.head.sha }}

Run: https://github.com/petrsvihlik/WopiHost/runs/1999272805?check_suite_focus=true Workflow: https://github.com/petrsvihlik/WopiHost/actions/runs/608352041/workflow

thomasrockhu commented 3 years ago

@petrsvihlik, it should work if you use fetch-depth: 2 on the actions/checkout step as marked here

petrsvihlik commented 3 years ago

thx, will give it a go :)

petrsvihlik commented 3 years ago

Adding fetch-depth: 2 fixed the warning. However override_pr and override_commit still seem to be required in my scenario.

Run: https://github.com/petrsvihlik/WopiHost/runs/2039630506?check_suite_focus=true#step:7:63

Anyways, I'm glad it works as I expect now. Thanks for the help @thomasrockhu and @itavero.

RohanNagar commented 3 years ago

Does anyone know if these workarounds are still required with version 1.3.1? I am using codecov/codecov-action@v1 in my build (which I believe pulls the latest version), and it's not working for pull_request_target.

petrsvihlik commented 3 years ago

I am also interested. The release v1.2.2 seems to be addressing the issue via #244. @RohanNagar why don't you give it a try? I might do so too later this week.

RohanNagar commented 3 years ago

I think I still need the workaround, I just don't need the fetch-depth. Doing this worked for me:

- name: Upload Codecov report (pull_request)
  if: startsWith(github.event_name, 'pull_request')
  uses: codecov/codecov-action@v1.3.1
  with:
   directory: .
   override_pr: ${{ github.event.number }}
   override_commit: ${{ github.event.pull_request.head.sha }}

codecov / codecov-action

Action fails to upload when using pull_request_target on GitHub #155