-
Notifications
You must be signed in to change notification settings - Fork 903
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
gcpkms: Add support for oauth tokens using env var #1188
base: main
Are you sure you want to change the base?
Conversation
Add support for CLOUDSDK_AUTH_ACCESS_TOKEN environment to pass though an oauth access token directly or via a file.
credentials := []byte(envCredentials) | ||
if _, err := os.Stat(envCredentials); err == nil { | ||
if credentials, err = os.ReadFile(envCredentials); err != nil { | ||
return nil, 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.
Perhaps don't exit here, but instead check SopsGoogleCredentialsOAuthEnv
as well?
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.
@warwick-mitchell1 I believe this is also done by the application default credentials: https://cloud.google.com/docs/authentication/application-default-credentials#search_order
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.
You maybe right, this isn't the part I'm really adding with this PR though, this was already like this. I'm adding the Oauth token below which isn't covered by application-defaultcreds
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.
I understand this. However, with your added functionality I believe it makes sense to also adjust the old behavior. If any fail condition happens with the credentials by file lookup, continue to check whether credentials by token area available before failing completely.
Couldn't find anything in the Google docs, but the Terraform provider documentation provides a few more (and different) environment variables: https://registry.terraform.io/providers/hashicorp/google/latest/docs/guides/provider_reference#authentication-configuration |
Let's do this! |
if envToken, isSet := os.LookupEnv(SopsGoogleCredentialsOAuthEnv); isSet { | ||
token := []byte(envToken) | ||
if _, err := os.Stat(envToken); err == nil { | ||
if token, err = os.ReadFile(envToken); err != nil { | ||
return nil, err | ||
} | ||
} | ||
tokenSource := oauth2.StaticTokenSource( | ||
&oauth2.Token{AccessToken: string(token)}, | ||
) | ||
return option.WithTokenSource(tokenSource), nil | ||
} |
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.
I think we should not check for a file here, but rather assume what's documented in https://cloud.google.com/sdk/docs/authorizing. Which only mentions:
Set the
CLOUDSDK_AUTH_ACCESS_TOKEN
environment variable to the access token value.
are there any blockers for this? I currently also facing issues with SOPS using Googles access token. |
bumping this up, what's blocking it? need support for ODIC token auth |
@warwick-mitchell1 Do you plan on continuing the PR? |
Add support for CLOUDSDK_AUTH_ACCESS_TOKEN environment to pass though an oauth access token directly or via a file.