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

CI: Fix how we supply the codecov token #1324

Closed
wants to merge 2 commits into from

Conversation

pitdicker
Copy link
Collaborator

Codecov is not happy the past few days. Maybe this helps?

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (be8f2fd) 91.25% compared to head (11776f0) 91.50%.
Report is 63 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1324      +/-   ##
==========================================
+ Coverage   91.25%   91.50%   +0.25%     
==========================================
  Files          38       38              
  Lines       17114    17314     +200     
==========================================
+ Hits        15617    15844     +227     
+ Misses       1497     1470      -27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pitdicker
Copy link
Collaborator Author

Apparently not...

[2023-09-26T14:55:26.118Z] ['info'] 
     _____          _
    / ____|        | |
   | |     ___   __| | ___  ___ _____   __
   | |    / _ \ / _` |/ _ \/ __/ _ \ \ / /
   | |___| (_) | (_| |  __/ (_| (_) \ V /
    \_____\___/ \__,_|\___|\___\___/ \_/

  Codecov report uploader 0.6.2
[2023-09-26T14:55:26.126Z] ['info'] => Project root located at: /home/runner/work/chrono/chrono
[2023-09-26T14:55:26.130Z] ['info'] -> No token specified or token is empty
[2023-09-26T14:55:26.140Z] ['info'] Searching for coverage files...
[2023-09-26T14:55:26.273Z] ['info'] => Found 1 possible coverage files:
  lcov.info
[2023-09-26T14:55:26.273Z] ['info'] Processing /home/runner/work/chrono/chrono/lcov.info...

'No token specified or token is empty'
@djc Can you please take a second look at the secrets some time? I don't know.

@djc
Copy link
Member

djc commented Sep 26, 2023

The secret turned out to be empty, indeed. I've updated it so should be better now. Want to amend and force push the branch to see if it's better now?

@pitdicker
Copy link
Collaborator Author

I checked the results in this PR and another, but no change yet...

@djc
Copy link
Member

djc commented Sep 27, 2023

I figure this is probably an issue in the upstream action?

@pitdicker
Copy link
Collaborator Author

We don't seem to have a lot of problems with codecov lately? I'll close this.

@pitdicker pitdicker closed this Jan 30, 2024
@pitdicker pitdicker deleted the codecov_token branch January 30, 2024 14:30
@pitdicker pitdicker restored the codecov_token branch January 30, 2024 16:54
@pitdicker pitdicker reopened this Jan 30, 2024
@pitdicker
Copy link
Collaborator Author

Spoke to soon, got a failure 2½ hours after closing this PR 🤣. Okay, I'll investigate some more.

@pitdicker pitdicker marked this pull request as draft January 30, 2024 16:58
@pitdicker
Copy link
Collaborator Author

pitdicker commented Jan 30, 2024

Aha. For security reasons it seems GitHub secrets are not available on pull request with the branch originating from a forked repository (basically all of our PRs).

@djc Can you look into the permissions of our codecov token? https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token

@pitdicker
Copy link
Collaborator Author

pitdicker commented Feb 2, 2024

I wonder if we can place the token in the GitHub environment variables as CODECOV_TOKEN. It is not the biggest secret in the world, and it would still be usually invisible unless someone crafts a PR to echo it.

@djc
Copy link
Member

djc commented Feb 2, 2024

Well, maybe you could at least just work from feature branches in this repo instead of in your fork? Should be fine.

@pitdicker
Copy link
Collaborator Author

Final idea before I admit defeat:

We can just supply the token via codecov.yaml. What can happen if it is no longer secret?

Hard coding the token will allow anyone to upload reports as if they were your project. It will not allow anyone to make changes to your account, or change any settings.

If that is your only option, it’s probably fine.

(https://community.codecov.com/t/security-issues-regarding-upload-token/429/8)

@djc any objections?

@djc
Copy link
Member

djc commented Feb 7, 2024

Yeah, I don't think I want to put the token in plaintext in the repo. I would still suggest you at least make your PRs from branches within this repo.

@pitdicker
Copy link
Collaborator Author

It seems to work at the moment, I'll close this PR.

@pitdicker pitdicker closed this Feb 7, 2024
@pitdicker pitdicker deleted the codecov_token branch February 7, 2024 14:31
@pitdicker
Copy link
Collaborator Author

I would still suggest you at least make your PRs from branches within this repo.

What is your reason? It feels messy to have many branches in the main repo.

@djc
Copy link
Member

djc commented Feb 7, 2024

Well, if it fixes the coverage issue that's worth something? We should enable the thing that deletes branches after merge if we haven't yet...

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

Successfully merging this pull request may close these issues.

2 participants