gcp-auth: handle CredentialsFromJSON error and guard nil creds#23272
gcp-auth: handle CredentialsFromJSON error and guard nil creds#23272nitishagar wants to merge 1 commit into
Conversation
…netes#23012) Extract Cloud Shell ADC loading into credentialsFromCloudShellADC helper so the CredentialsFromJSON error is no longer silently discarded. On failure, log the error via klog.Warningf and fall through with nil creds (no exit — Cloud Shell runs on GCE so the existing IsOnGCE early-return handles this path successfully). Add an explicit creds == nil guard after the GCE early-return with an actionable exit.Message, making the nil-deref at creds.JSON structurally impossible. The golang.org/x/oauth2/google functions used here are in maintenance mode but not removed; migration to cloud.google.com/go/auth is out of scope. Fixes kubernetes#23012
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nitishagar The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @nitishagar. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
1550964 to
24edf07
Compare
|
Can one of the admins verify this patch? |
Problem
enableAddonGCPAuthcan panic with a nil-pointer dereference atcreds.JSON(pkg/addons/addons_gcpauth.go:101) when:google.FindDefaultCredentialsfails, ANDdetect.IsCloudShell()is true, ANDCLOUDSDK_CONFIGis unset, the ADC file is unreadable, orgoogle.CredentialsFromJSONfailsIn the original code the
CredentialsFromJSONerror was silently discarded (creds, _ = ...), leavingcredsnil and producing a panic instead of an actionable error message.Fixes #23012
Changes
pkg/addons/addons_gcpauth.gocredentialsFromCloudShellADC(ctx)helper that returns(*google.Credentials, error), making the failure path unit-testable and the error non-discardable.klog.Warningfand fall through withcredsnil. No exit here — Cloud Shell runs on a GCE VM wheredetect.IsOnGCE()is true, so today's successful path (ADC load fails → GCE early-return → nil return with warning) is preserved.creds == nilguard after the GCE early-return with an actionableexit.Message(same wording as line 85), making the nil-deref atcreds.JSONstructurally impossible.pkg/addons/addons_gcpauth_test.go(new)Unit tests for
credentialsFromCloudShellADC:CLOUDSDK_CONFIGunset → errorauthorized_userJSON → non-nil creds,creds.JSONequals file contentsDeprecation note
The issue title asks whether
golang.org/x/oauth2/google.FindDefaultCredentialsandCredentialsFromJSONmust be replaced. These functions are in maintenance mode but are not removed or broken. Migration tocloud.google.com/go/authis a separate, larger change and is out of scope for this bug fix.Testing
All 4 tests pass.
go vet ./pkg/addons/...clean.🧙 Built with WOZCODE