Skip to content
This repository has been archived by the owner on Oct 19, 2021. It is now read-only.

Allow GCS client to use credentials from the environment if needed #58

Merged
merged 5 commits into from
Aug 18, 2021

Conversation

cmaster11
Copy link
Contributor

When running in GCP environment, the credentials can be fetched automatically by the GCP SDK, and there is no need to specify any additional JSON/env variables (https://cloud.google.com/docs/authentication/production#automatically).

When running in GCP environment, the credentials can be fetched automatically by the GCP SDK, and there is no need to specify any additional JSON/env variables (https://cloud.google.com/docs/authentication/production#automatically).
@Xuanwo
Copy link
Contributor

Xuanwo commented Aug 17, 2021

diff check failed, use make build can resolve this problem.

@@ -111,20 +109,25 @@ func newServicer(pairs ...typ.Pair) (srv *Service, err error) {
if err != nil {
return nil, err
}
case credential.ProtocolEnv:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to support both env and other ways.

How about using google.FindDefaultCredentials instead?

ref: https://pkg.go.dev/golang.org/x/oauth2/google#FindDefaultCredentials

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FindDefaultCredentials gets called automatically by the GCS client if no credentials are provided (e.g. if you don't use the WithCredentialsJSON).

So, if you pass env as credential type, the client will just fetch them all at runtime.

This happens here: https://github.com/googleapis/google-api-go-client/blob/master/internal/creds.go#L50

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gets called automatically by the GCS client if no credentials are provided

Yep, I know about this logic here.

But I don't want to depend on this behavior. I prefer to control them by ourselves.

  • If the user uses base64 or file, it's OK, read them and use WithCredentialsJSON to construct the client.
  • If the user uses env, we use FindDefaultCredentials to read default credentials from the env.

Everything is clear, no need to read the code provided by deps.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely got your point :) I cleaned up the code

@cmaster11
Copy link
Contributor Author

diff check failed, use make build can resolve this problem.

Ran and diff is fine now :)

@Xuanwo
Copy link
Contributor

Xuanwo commented Aug 18, 2021

Thanks for your contributions!

@Xuanwo Xuanwo merged commit fdeda31 into beyondstorage:master Aug 18, 2021
@cmaster11 cmaster11 deleted the credentialsenv branch August 18, 2021 03:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants