fix(api/auth/gcp): check metadata response status when fetching GCE JWT - #32071
Open
alekc wants to merge 2 commits into
Open
fix(api/auth/gcp): check metadata response status when fetching GCE JWT#32071alekc wants to merge 2 commits into
alekc wants to merge 2 commits into
Conversation
Signed-off-by: Alexander Chernov <alexander@chernov.it>
|
Deployment failed with the following error: Learn More: https://vercel.com/docs/concepts/projects/project-configuration |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Signed-off-by: Alexander Chernov <alexander@chernov.it>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #32068. Hands the metadata call to
metadata.GetWithContextinstead of the hand-rolled request, which covers all three defects at once: the missing status check, theio.ReadAllerror ordering, and the ignored context. That package is already a direct dependency, sogo.modandgo.sumare unchanged.Behaviour that shifts because the library differs, none of it required by the issue:
169.254.169.254rather thanhttp://metadata/, which relies on a.google.internalsearch suffix that netgo-without-cgo builds lack.GCE_METADATA_HOSTnow applies to the fetch, not just theOnGCEgate.HTTP_PROXY, so theNO_PROXYworkaround stops mattering.The gate deliberately stays on
metadata.OnGCE():OnGCEWithContextmemoizes in async.Once, so one caller whose context expires mid-probe would cache "not on GCE" and break every later login until the process restarts.Out of scope:
signJWTstill usescontext.Background(), and a 200 with an empty body still passes through as an empty token. Either can go in this PR if you prefer.command/agentproxyshared/auth/gcp/gcp.gohas the same missing status check, which I can file separately.8 new tests, the module had none. They aim the client at an httptest server via
GCE_METADATA_HOST, so nothing in production code exists only for tests and the suite needs no network. I checked they fail against the unfixed code. CI likely runs none of them:API_PACKAGESin the Makefile iscd api && go list ./..., which skipsapi/auth/gcpas a nested module.A
changelog/<PR>.txtentry follows in a second commit.PCI review checklist