codecov-api
codecov-api copied to clipboard
Tokenless V3
Purpose/Motivation
We don't want to make GH API calls when doing tokenless anymore
Links to relevant tickets
https://github.com/codecov/engineering-team/issues/1574
Related: https://github.com/codecov/codecov-action/pull/1406
What does this PR do?
- Remove GitHub API calls from TokenlessAuthentication
Codecov Report
Attention: Patch coverage is 94.11765%
with 2 lines
in your changes missing coverage. Please review.
Project coverage is 91.45%. Comparing base (
9b8d730
) to head (eca8697
).
:white_check_mark: All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #533 +/- ##
==========================================
- Coverage 91.49% 91.45% -0.04%
==========================================
Files 615 615
Lines 16370 16354 -16
==========================================
- Hits 14977 14957 -20
- Misses 1393 1397 +4
Flag | Coverage Δ | |
---|---|---|
unit | 91.45% <94.11%> (-0.04%) |
:arrow_down: |
unit-latest-uploader | 91.45% <94.11%> (-0.04%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Files | Coverage Δ | |
---|---|---|
upload/views/reports.py | 100.00% <ø> (ø) |
|
codecov_auth/authentication/repo_auth.py | 96.87% <96.42%> (-1.63%) |
:arrow_down: |
upload/views/commits.py | 97.14% <83.33%> (-2.86%) |
:arrow_down: |
:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!
Codecov Report
Attention: Patch coverage is 94.11765%
with 2 lines
in your changes are missing coverage. Please review.
Project coverage is 91.45%. Comparing base (
9b8d730
) to head (eca8697
).
:white_check_mark: All tests successful. No failed tests found :relaxed:
@@ Coverage Diff @@
## main #533 +/- ##
==========================================
- Coverage 91.49% 91.45% -0.04%
==========================================
Files 615 615
Lines 16370 16354 -16
==========================================
- Hits 14977 14957 -20
- Misses 1393 1397 +4
Flag | Coverage Δ | |
---|---|---|
unit | 91.45% <94.11%> (-0.04%) |
:arrow_down: |
unit-latest-uploader | 91.45% <94.11%> (-0.04%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Files | Coverage Δ | |
---|---|---|
upload/views/reports.py | 100.00% <ø> (ø) |
|
codecov_auth/authentication/repo_auth.py | 96.87% <96.42%> (-1.63%) |
:arrow_down: |
upload/views/commits.py | 97.14% <83.33%> (-2.86%) |
:arrow_down: |
Codecov Report
Attention: Patch coverage is 94.11765%
with 2 lines
in your changes missing coverage. Please review.
Project coverage is 95.91%. Comparing base (
9b8d730
) to head (eca8697
).
:white_check_mark: All tests successful. No failed tests found.
Files | Patch % | Lines |
---|---|---|
codecov_auth/authentication/repo_auth.py | 96.42% | 1 Missing :warning: |
upload/views/commits.py | 83.33% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #533 +/- ##
================================================
- Coverage 95.92000 95.91000 -0.01000
================================================
Files 793 793
Lines 17688 17671 -17
================================================
- Hits 16968 16949 -19
- Misses 720 722 +2
Flag | Coverage Δ | |
---|---|---|
unit | 91.45% <94.11%> (-0.04%) |
:arrow_down: |
unit-latest-uploader | 91.45% <94.11%> (-0.04%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
We need to be careful merging these changes to avoid re-introducing the bug of having an upload from a fork branch accidentally overwrite coverage in the upstream branch.
Considering the upload endpoints used by the CLI either we change the CLI to send the branch info every time (maybe through headers) OR we use the info we have to make the validation that the branch name is in the format fork:branch
. If that is not the case, reject the upload.
Currently I believe it should be possible to make this check:
- For the commit creation the branch should be part of the body
- For the subsequent requests (create report and send the upload) we can pull the commit from the database and check its branch information.
Personally I feel that we should be able to accept / reject a request without having to look at the request body. So I'd opt to make the CLI send specific headers with the information we need. (Also by removing the current checks maybe X-Tokenless-PR header is useless?)
So I'd opt to make the CLI send specific headers with the information we need. (Also by removing the current checks maybe X-Tokenless-PR header is useless?)
We could just validate that the x-tokenless-pr header is in the format we expect and has the correct repo name (it matches the one in the url)?
X-Tokenless-PR is a number [1] that we used to get the correct PR from the provider. That does little for us considering the changes in this PR. If you just change the value the header name will be misleading.
X-Tokenless should not match the repo name in the URL if it comes from a fork [1], cause it's supposed to be the fork slug, while the slug in the URL should be the upstream's.
[1] https://github.com/codecov/codecov-cli/blob/7b028499a521f8df3cf3bb6b642246a19e07bee9/codecov_cli/services/report/init.py#L57-L58
Codecov Report
Attention: Patch coverage is 94.11765%
with 2 lines
in your changes missing coverage. Please review.
:white_check_mark: All tests successful. No failed tests found.
Files | Patch % | Lines |
---|---|---|
codecov_auth/authentication/repo_auth.py | 96.42% | 1 Missing :warning: |
upload/views/commits.py | 83.33% | 1 Missing :warning: |
:loudspeaker: Thoughts on this report? Let us know!