Reduce binary size#2238
Conversation
The cloud.google.com/go/storage library's transitive dependency tree (envoyproxy, grpc, xds, opentelemetry) is the primary driver of binary size growth. Replace with direct HTTP calls to the GCS JSON API, using the existing HTTP fetcher and oauth2 token source.
Removes cloud.google.com/go/storage and transitive deps including envoyproxy, grpc, cncf/xds, and opentelemetry. Binary size drops from 65 MB to 21 MB.
There was a problem hiding this comment.
Code Review
This pull request replaces the GCS client library with direct HTTP calls to the GCS JSON API, significantly reducing the binary size by removing heavy dependencies like cloud.google.com/go/storage and related libraries. While this is a great optimization, the new implementation introduces critical concurrency issues. Specifically, the lazy initialization of GCSTokenSource in fetchFromGCS is not thread-safe and can cause a data race when resources are fetched concurrently. Additionally, mutating opts.Headers directly poses a risk of concurrent map write panics if the options are shared across goroutines. Addressing these issues by using sync.Once for initialization and cloning the headers map will ensure a safe and robust implementation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Nice find. It got in via #2005. |
|
Code seems OK but I only gave it a quick look. Do we have a test for that use case? It would be good to make sure we have one to ensure we don't regress here. Thanks! |
|
Very nice! I wonder if there's more low-hanging fruit here to tackle. Something an AI agent would probably be good at identifying. |
We have kola tests for both auth'd https://github.com/coreos/fedora-coreos-config/blob/9b14cac40de19089d4362b960cb7d098fd523301/tests/kola/ignition/resource/authenticated-gs/config.bu and anon fetching Also just added some test coverage to verify form of our request ie..
But we will need to still rely on the kola tests for actual fetch verification .. ie..
|
bb3c0d9 to
47a6c8e
Compare
Test URL construction (path escaping, query params) and end-to-end fetch with httptest server verifying request path, auth headers, and response body for anonymous, authenticated, and nested-path cases. Also fix double-encoding of object paths by setting RawPath. Co-Authored-By: Claude <noreply@anthropic.com>
47a6c8e to
5a1f1d8
Compare
Definitely! I have already of a size breakdown of the remaining packages. The next packages I believe would be the AWS SDK (3.48 MB) and Azure SDK (0.51 MB), while I think it would be easy enough to determine what breaks from the CI, I still think it could be better to have them be separate PRs scoping them as separate improvements wdyt? |
|
Out of scope, but I think it would be worth to investigate systemd-imds integration with Ignition to remove those vendors libraries: https://www.freedesktop.org/software/systemd/man/latest/systemd-imdsd@.service.html# (maybe open an issue on the Ignition repo) |
To see the size breakdown between 2.20.0 and 2.26.0 you can run this gist.
Feel free to compare to this branch as well by changing the var to HEAD.
Packages not in v2.20.0
Packages that have changed in size
Size Change: Before and After PR
See: #2045