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

Rework the codecov support: deprecate codecov-token add hash checks for uploader #32

Closed
wants to merge 4 commits into from

Conversation

j-rivero
Copy link
Contributor

The PR deprecates the use of codecov-token input since I think that we have wrongly used the token for public repositories when there was no need for that. The code using it should keep working the same.

Added a couple of new inputs: codecov-enabled to enable the codecov runs and codecov-token-private-repos to make explicit the exact use of the token.

I've also added the hash check for codecov bash uploader. I've tested this branch here:
j-rivero/ign-math#1

@chapulina
Copy link
Contributor

I see this error on the ign-math PR:

{'detail': ErrorDetail(string='Could not find a repository, try using upload token', code='not_found')}

we have wrongly used the token for public repositories when there was no need for that.

I'm not sure I understand. If there's no token to protect the upload of new reports, anyone would be able to upload reports and mess up with them. It's like on GitHub, being public doesn't mean the whole world has write access, just read access.

@nuclearsandwich
Copy link

On some providers Codecov has some provider level authenticity mechanism. Here is the relevant section of the codecov doc regarding the bash uploader

If you have a public project on TravisCI, CircleCI, AppVeyor, Azure Pipelines, or GitHub Actions an upload token is not required.

It could be that there are further caveats there that are not fully elaborated. If we continue to have trouble it makes sense to contact Codecov's support so that we can either manage the token safely if required or properly invoke codecov without it. If we can avoid use of the token as a secret it will be easier for community contributions to receive coverage data because pull requests from forks do not have access to secrets.

@j-rivero
Copy link
Contributor Author

I see this error on the ign-math PR:

{'detail': ErrorDetail(string='Could not find a repository, try using upload token', code='not_found')}

I've removed the CODECOV_TOKEN from Ignition repositories, it could be possible that we are running with the configuration expecting the token while the token is gone. Please point me to the failures so I can give them a look to verify this hypothesis.

@chapulina
Copy link
Contributor

Please point me to the failures so I can give them a look to verify this hypothesis.

That's from the PR that you linked on the description, see the Ubuntu Bionic CI check: https://github.com/j-rivero/ign-math/pull/1/checks?check_run_id=2420711201

@chapulina
Copy link
Contributor

I'd also expect CODECOV to comment back to that PR with the results, which didn't happen.

@chapulina
Copy link
Contributor

I noticed that the latest run of gazebosim/gz-math#206 didn't upload to codecov because the token is empty. We should get this in to re-enable codecov on repositories that had their tokens removed.

We should try this branch on a real Ignition repository just to make sure it works, in case the issue above is because it's using @j-rivero 's fork

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works!

@chapulina
Copy link
Contributor

By the way, this should be propagated to the bionic and focal branches, master is not used anymore.

@j-rivero
Copy link
Contributor Author

It works!

"You of little faith, why are you so afraid?" Matthew 8:26

By the way, this should be propagated to the bionic and focal branches, master is not used anymore.

I go for it.

@j-rivero j-rivero closed this Apr 28, 2021
j-rivero added a commit to gazebosim/gz-cmake that referenced this pull request May 6, 2021
j-rivero added a commit to gazebosim/gz-fuel-tools that referenced this pull request May 6, 2021
j-rivero added a commit to gazebosim/gz-gui that referenced this pull request May 6, 2021
j-rivero added a commit to gazebosim/gz-transport that referenced this pull request May 6, 2021
j-rivero added a commit to gazebosim/gz-sensors that referenced this pull request May 6, 2021
j-rivero added a commit to gazebosim/gz-physics that referenced this pull request May 6, 2021
mjcarroll pushed a commit to gazebosim/gz-common that referenced this pull request May 6, 2021
@chapulina chapulina deleted the codecov_token_refactor branch May 6, 2021 15:58
chapulina pushed a commit to gazebosim/gz-fuel-tools that referenced this pull request May 12, 2021
chapulina pushed a commit to gazebosim/gz-gui that referenced this pull request May 12, 2021
chapulina pushed a commit to gazebosim/gz-transport that referenced this pull request May 12, 2021
chapulina pushed a commit to gazebosim/gz-launch that referenced this pull request May 12, 2021
chapulina pushed a commit to gazebosim/gz-utils that referenced this pull request May 12, 2021
chapulina pushed a commit to gazebosim/gz-rendering that referenced this pull request May 12, 2021
chapulina pushed a commit to gazebosim/gz-sim that referenced this pull request May 12, 2021
chapulina pushed a commit to gazebosim/gz-plugin that referenced this pull request May 12, 2021
adlarkin pushed a commit to gazebosim/gz-cmake that referenced this pull request May 14, 2021
* Port codecov to new configuration

See gazebo-tooling/action-gz-ci#32

Signed-off-by: Jose Luis Rivero <[email protected]>

* no codecov support in this repo
j-rivero added a commit to gazebosim/gz-cmake that referenced this pull request May 18, 2021
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.

3 participants