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

Replace usage of jsonwebtoken for GCS #578

Merged
merged 1 commit into from
Nov 11, 2019
Merged

Replace usage of jsonwebtoken for GCS #578

merged 1 commit into from
Nov 11, 2019

Conversation

Jake-Shadle
Copy link
Contributor

@Jake-Shadle Jake-Shadle commented Oct 31, 2019

This removes the use of the patched jsonwebtoken

jsonwebtoken = { git = "https://github.com/Jake-Shadle/jsonwebtoken.git", rev = "2f469a61" }

which is still needed due to no release from jsonwebtoken with the relevant changes (last release was May). Using patches are supposed to be temporary, because what ends up happening is that people that do cargo install sccache --features=gcs end up with a broken sccache because the patched version of jsonwebtoken that "works" is not used when installing from crates.io, and for some reason, the prebuilt binaries (at least the musl one) in github releases are also incorrect.

Rather than wait for a release, this change removes the need for jsonwebtoken altogether for gcs caching, by just doing the relevant pieces directly using base64/ring.

  • Serializes (JSON + base64) the JWT header (signing algorithm)
  • Serializes (JSON + base64) the JWT claim (scope, timestamps, uri etc)
  • Signs the JWT payload with the private key from the service account
  • Concatenates the JWT payload and the base64 encoded signature

jsonwebtoken is still used in the dist stuff, only required 2 changes due to a field being removed from JWT.

Resolves: #489

froydnj added a commit to froydnj/sccache that referenced this pull request Nov 4, 2019
This change should help mozilla#578 not cause errors on the linux64 musl
target...though why that target isn't failing *now*, with the older
version of `jobserver` that we have, is something I don't understand
chmanchester pushed a commit that referenced this pull request Nov 6, 2019
This change should help #578 not cause errors on the linux64 musl
target...though why that target isn't failing *now*, with the older
version of `jobserver` that we have, is something I don't understand
@froydnj
Copy link
Contributor

froydnj commented Nov 11, 2019

Can you rebase this now that we're resolved the weird failures with jobserver? Thanks!

@froydnj
Copy link
Contributor

froydnj commented Nov 11, 2019

Hm, it looks like development is happening on jsonwebtoken...but it doesn't seem to be happening fast enough. We can merge this for now, but I'd love it if/when jsonwebtoken gets updated if this change were reverted.

Can you force-push to squash your commits and preserve bisectability, please?

@Jake-Shadle
Copy link
Contributor Author

Done!

@froydnj froydnj merged commit 9163240 into mozilla:master Nov 11, 2019
@froydnj
Copy link
Contributor

froydnj commented Nov 11, 2019

Thank you!

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.

GCS storage failing with "invalid RSA key"
2 participants