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

Action fails to upload when using pull_request_target on GitHub #155

Closed
kennethtran93 opened this issue Oct 22, 2020 · 18 comments · Fixed by #264
Closed

Action fails to upload when using pull_request_target on GitHub #155

kennethtran93 opened this issue Oct 22, 2020 · 18 comments · Fixed by #264
Assignees

Comments

@kennethtran93
Copy link

kennethtran93 commented Oct 22, 2020

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
Copy link
Contributor

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

@kennethtran93
Copy link
Author

kennethtran93 commented Oct 23, 2020

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
Copy link
Author

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
Copy link
Author

kennethtran93 commented Oct 23, 2020

@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
Copy link
Contributor

@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
Copy link
Author

kennethtran93 commented Nov 5, 2020

@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.

@thomasrockhu
Copy link
Contributor

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

@bfjelds
Copy link

bfjelds commented Nov 20, 2020

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
Copy link
Contributor

@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
Copy link

bfjelds commented Nov 23, 2020

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.

JosephDuffy added a commit to JosephDuffy/Persist that referenced this issue Nov 24, 2020
Codecov fails with pull_request_target. See codecov/codecov-action#155
JosephDuffy added a commit to JosephDuffy/Partial that referenced this issue Nov 24, 2020
Codecov fails with pull_request_target. See codecov/codecov-action#155
@itavero
Copy link

itavero commented Feb 26, 2021

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
Copy link

petrsvihlik commented Feb 28, 2021

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
Copy link
Contributor

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

@petrsvihlik
Copy link

thx, will give it a go :)

petrsvihlik added a commit to petrsvihlik/WopiHost that referenced this issue Mar 5, 2021
@petrsvihlik
Copy link

petrsvihlik commented Mar 5, 2021

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
Copy link

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
Copy link

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
Copy link

RohanNagar commented Mar 28, 2021

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 }}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants