-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix: use active gcp account #8584
fix: use active gcp account #8584
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8584 +/- ##
==========================================
- Coverage 70.48% 64.29% -6.19%
==========================================
Files 515 610 +95
Lines 23150 30582 +7432
==========================================
+ Hits 16317 19664 +3347
- Misses 5776 9446 +3670
- Partials 1057 1472 +415
... and 389 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
||
func (ts tokenSource) Token() (*oauth2.Token, error) { | ||
// the command return a json object containing id_token, access_token, token_expiry | ||
cmd := exec.Command("gcloud", "auth", "print-identity-token", "--format=json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parsing credential field from the gcloud config config-helper
command output should be able to achieve the same effect, but gcloud config config-helper
is not an open knowledge, as may be removed as --help indicate
NOTES
This command is an internal implementation detail and may change or
disappear without notice.
@@ -31,7 +31,7 @@ func ClientOptions(ctx context.Context) []option.ClientOption { | |||
option.WithUserAgent(version.UserAgent()), | |||
} | |||
|
|||
creds, cErr := activeUserCredentials(ctx) | |||
creds, cErr := activeUserCredentialsOnce() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: might make sense to add a debug level log entry for cErr
, currently I believe it is ignored outright
cmd.Stdout = &body | ||
err := util.RunCmd(context.TODO(), cmd) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to get access token %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: might make sense to differentiate the error messages here so it is clearer which one of these 2 steps failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left 2 small nit comments
Fixes: #6996
Description
get
gcp active account credentialcreate
credential with an access token. My understanding is that the credential is used to retrieve an access token and the token can be used to initiate other requests to other services. oauth.google package can use adc(application default credential)to construct tokenSource which is used retrieve access token , we can also implement our own token source and provide it to google.Credentials struct , then the tokenSource.token() will be called when needed to request gcp services.
Test Plan
gcloud auth activate-service-account {account_name} --key-file={file}
to activate your accountskaffold build -vdebug
inintegration/testdata/gcb-explicit-repo
project, the run should fail if you didn't give your service account k8s-skaffold storage access and artifact registry, this is good to prove that we're using the active account to issue requestcreating bucket if not exists: getting bucket "k8s-skaffold_cloudbuild": googleapi: Error 403: {{service-account-name}} does not have storage.buckets.get access to the Google Cloud Storage bucket. Permission 'storage.buckets.get' denied on resource (or it may not exist)., forbidden