gazebo-tooling / action-gz-ci

MIT License
5 stars 4 forks source link

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

Closed j-rivero closed 3 years ago

j-rivero commented 3 years ago

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: https://github.com/j-rivero/ign-math/pull/1

chapulina commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

chapulina commented 3 years ago

I noticed that the latest run of https://github.com/ignitionrobotics/ign-math/pull/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

chapulina commented 3 years ago

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

j-rivero commented 3 years ago

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.