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

Drop tuf client dependency on GCS client library #1967

Merged
merged 7 commits into from
Jun 7, 2022

Conversation

imjasonh
Copy link
Member

@imjasonh imjasonh commented Jun 7, 2022

Signed-off-by: Jason Hall jason@chainguard.dev

cc @haydentherapper @asraa

Requests to get TUF roots from Google Cloud Storage are made using HTTP directly, without using the GCS client library.

Signed-off-by: Jason Hall <jason@chainguard.dev>
pkg/cosign/tuf/store.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2022

Codecov Report

Merging #1967 (65641cc) into main (75b2bd1) will decrease coverage by 0.14%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main    #1967      +/-   ##
==========================================
- Coverage   34.80%   34.66%   -0.15%     
==========================================
  Files         153      152       -1     
  Lines       10119    10089      -30     
==========================================
- Hits         3522     3497      -25     
+ Misses       6226     6222       -4     
+ Partials      371      370       -1     
Impacted Files Coverage Δ
pkg/cosign/tuf/client.go 63.99% <85.71%> (-0.73%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75b2bd1...65641cc. Read the comment docs.

Signed-off-by: Jason Hall <jason@chainguard.dev>
@haydentherapper
Copy link
Contributor

cc @asraa

Signed-off-by: Jason Hall <jason@chainguard.dev>
asraa
asraa previously approved these changes Jun 7, 2022
Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Can we just use an HTTP remote store?

https://github.com/theupdateframework/go-tuf/blob/ad96eca0239ec2cc9b6e408fbe42b2f9e9d6b1dd/client/remote_store.go#L31

If it's a little hard to understand, I can take a closer look too

@asraa
Copy link
Contributor

asraa commented Jun 7, 2022

Oops, did not mean to approve, just meant to comment

Signed-off-by: Jason Hall <jason@chainguard.dev>
@imjasonh
Copy link
Member Author

imjasonh commented Jun 7, 2022

Can we just use an HTTP remote store?

https://github.com/theupdateframework/go-tuf/blob/ad96eca0239ec2cc9b6e408fbe42b2f9e9d6b1dd/client/remote_store.go#L31

If it's a little hard to understand, I can take a closer look too

That seems like a good idea. I believe the previous behavior was just to interpret a mirror name like "sigstore-tuf-root" to mean a GCS bucket, and in that case, we can just upgrade that string to a URL like "https://sigstore-tuf-root.storage.googleapis.com" and use the HTTP client. More red lines! 🎉

@asraa
Copy link
Contributor

asraa commented Jun 7, 2022

I believe the previous behavior was just to interpret a mirror name like "sigstore-tuf-root" to mean a GCS bucket, and in that case, we can just upgrade that string to a URL like "https://sigstore-tuf-root.storage.googleapis.com" and use the HTTP client. More red lines!

Exactly!! Let's do it! The less code the less... bugs? (I hope!) Thank you :D

@imjasonh
Copy link
Member Author

imjasonh commented Jun 7, 2022

This change cuts 2MB off the size of cosign:

(drop-gcs)$ go build ./cmd/cosign/ && ls -lh cosign && rm cosign
-rwxr-xr-x  1 jason  staff    85M Jun  7 11:42 cosign

(main)$ go build ./cmd/cosign/ && ls -lh cosign && rm cosign
-rwxr-xr-x  1 jason  staff    87M Jun  7 11:42 cosign

@dlorenc
Copy link
Member

dlorenc commented Jun 7, 2022

This change cuts 2MB off the size of cosign:

NICE

Signed-off-by: Jason Hall <jason@chainguard.dev>
haydentherapper
haydentherapper previously approved these changes Jun 7, 2022
pkg/cosign/tuf/client.go Show resolved Hide resolved
Signed-off-by: Jason Hall <jason@chainguard.dev>
haydentherapper
haydentherapper previously approved these changes Jun 7, 2022
@dlorenc
Copy link
Member

dlorenc commented Jun 7, 2022

Nooo, one last docgen update

Signed-off-by: Jason Hall <jason@chainguard.dev>
@dlorenc dlorenc merged commit 2ef684f into sigstore:main Jun 7, 2022
@github-actions github-actions bot added this to the v1.10.0 milestone Jun 7, 2022
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.

5 participants