From 76696e29f37350cf9dea891019c2f055adf64134 Mon Sep 17 00:00:00 2001 From: Kenneth Halim Date: Mon, 25 Sep 2023 16:14:04 +0700 Subject: [PATCH 1/2] Add use default credentials toggle in google cloud storage client options --- pkg/storage/gcs/hystrix.go | 23 ++++++++++++++++------- pkg/storage/gcs/hystrix_test.go | 10 ++++++++-- pkg/storage/gcs/options.go | 2 ++ pkg/storage/gcs/storage.go | 2 +- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/pkg/storage/gcs/hystrix.go b/pkg/storage/gcs/hystrix.go index c8d5636..1c34cf6 100644 --- a/pkg/storage/gcs/hystrix.go +++ b/pkg/storage/gcs/hystrix.go @@ -2,6 +2,7 @@ package gcs import ( "context" + "golang.org/x/oauth2/google" "net/http" "cloud.google.com/go/storage" @@ -14,8 +15,8 @@ import ( const userAgent = "gcloud-golang-storage/20151204" -func newHeimdallHTTPClient(ctx context.Context, hc heimdall.Client, credentialsJSON []byte) (*http.Client, error) { - t, err := newTransport(ctx, hc, credentialsJSON) +func newHeimdallHTTPClient(ctx context.Context, opts *Options) (*http.Client, error) { + t, err := newTransport(ctx, opts) if err != nil { return nil, err } @@ -24,16 +25,24 @@ func newHeimdallHTTPClient(ctx context.Context, hc heimdall.Client, credentialsJ }, nil } -func newTransport(ctx context.Context, hc heimdall.Client, credentialsJSON []byte) (http.RoundTripper, error) { +func newTransport(ctx context.Context, opts *Options) (http.RoundTripper, error) { o := option.WithoutAuthentication() - if len(credentialsJSON) > 0 { - o = option.WithCredentialsJSON(credentialsJSON) + if opts.UseDefaultCredential { + credential, err := google.FindDefaultCredentials(ctx) + if err != nil { + return nil, err + } + o = option.WithCredentials(credential) + } else if len(opts.CredentialsJSON) > 0 { + o = option.WithCredentialsJSON(opts.CredentialsJSON) } return gcloud.NewTransport(ctx, - &hystrixTransport{client: hc}, + &hystrixTransport{client: opts.Client}, option.WithUserAgent(userAgent), option.WithScopes(storage.ScopeReadOnly), - o) + o, + ) + } type hystrixTransport struct { diff --git a/pkg/storage/gcs/hystrix_test.go b/pkg/storage/gcs/hystrix_test.go index 1276a25..3392832 100644 --- a/pkg/storage/gcs/hystrix_test.go +++ b/pkg/storage/gcs/hystrix_test.go @@ -11,14 +11,20 @@ import ( func TestNewHeimdallHTTPClientWithInvalidCredentials(t *testing.T) { hc := hystrix.NewClient() - hhc, err := newHeimdallHTTPClient(context.TODO(), hc, []byte("random")) + hhc, err := newHeimdallHTTPClient(context.TODO(), &Options{ + CredentialsJSON: []byte("random"), + Client: hc, + }) assert.Nil(t, hhc) assert.Error(t, err) } func TestNewHeimdallHTTPClientWithNoCredentials(t *testing.T) { hc := hystrix.NewClient() - hhc, err := newHeimdallHTTPClient(context.TODO(), hc, []byte("")) + hhc, err := newHeimdallHTTPClient(context.TODO(), &Options{ + CredentialsJSON: []byte(""), + Client: hc, + }) assert.NotNil(t, hhc) assert.NoError(t, err) req, _ := http.NewRequest(http.MethodGet, "", nil) diff --git a/pkg/storage/gcs/options.go b/pkg/storage/gcs/options.go index 44743fa..1babdaa 100644 --- a/pkg/storage/gcs/options.go +++ b/pkg/storage/gcs/options.go @@ -8,6 +8,8 @@ type Options struct { BucketName string // CredentialsJSON holds the json data for credentials of a service account CredentialsJSON []byte + // UseDefaultCredential toggle the usage of google application default credential to authenticate with cloud storage + UseDefaultCredential bool // Client can be used to specify a heimdall.Client with hystrix like circuit breaker Client heimdall.Client } diff --git a/pkg/storage/gcs/storage.go b/pkg/storage/gcs/storage.go index ba61384..828ba22 100644 --- a/pkg/storage/gcs/storage.go +++ b/pkg/storage/gcs/storage.go @@ -32,7 +32,7 @@ type Storage struct { // NewStorage returns a new gcs.Storage instance func NewStorage(opts Options) (*Storage, error) { ctx := context.TODO() - client, err := newHeimdallHTTPClient(ctx, opts.Client, opts.CredentialsJSON) + client, err := newHeimdallHTTPClient(ctx, &opts) if err != nil { return nil, err } From 41e4341b261f8ecb1acb251851948ada73fd3118 Mon Sep 17 00:00:00 2001 From: Kenneth Halim Date: Mon, 2 Oct 2023 12:03:26 +0700 Subject: [PATCH 2/2] fix: Refactor opts to receive Credentials --- pkg/storage/gcs/hystrix.go | 24 +++++++++++------------- pkg/storage/gcs/hystrix_test.go | 15 +++++++++++++++ pkg/storage/gcs/options.go | 9 ++++++--- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/pkg/storage/gcs/hystrix.go b/pkg/storage/gcs/hystrix.go index 1c34cf6..3c1abf0 100644 --- a/pkg/storage/gcs/hystrix.go +++ b/pkg/storage/gcs/hystrix.go @@ -2,7 +2,6 @@ package gcs import ( "context" - "golang.org/x/oauth2/google" "net/http" "cloud.google.com/go/storage" @@ -25,24 +24,23 @@ func newHeimdallHTTPClient(ctx context.Context, opts *Options) (*http.Client, er }, nil } -func newTransport(ctx context.Context, opts *Options) (http.RoundTripper, error) { - o := option.WithoutAuthentication() - if opts.UseDefaultCredential { - credential, err := google.FindDefaultCredentials(ctx) - if err != nil { - return nil, err - } - o = option.WithCredentials(credential) - } else if len(opts.CredentialsJSON) > 0 { - o = option.WithCredentialsJSON(opts.CredentialsJSON) +func getCredentialsOption(opts *Options) option.ClientOption { + if opts.Credentials != nil { + return option.WithCredentials(opts.Credentials) + } + if len(opts.CredentialsJSON) > 0 { + return option.WithCredentialsJSON(opts.CredentialsJSON) } + return option.WithoutAuthentication() +} + +func newTransport(ctx context.Context, opts *Options) (http.RoundTripper, error) { return gcloud.NewTransport(ctx, &hystrixTransport{client: opts.Client}, option.WithUserAgent(userAgent), option.WithScopes(storage.ScopeReadOnly), - o, + getCredentialsOption(opts), ) - } type hystrixTransport struct { diff --git a/pkg/storage/gcs/hystrix_test.go b/pkg/storage/gcs/hystrix_test.go index 3392832..7f7893d 100644 --- a/pkg/storage/gcs/hystrix_test.go +++ b/pkg/storage/gcs/hystrix_test.go @@ -2,6 +2,7 @@ package gcs import ( "context" + "golang.org/x/oauth2/google" "net/http" "testing" @@ -31,3 +32,17 @@ func TestNewHeimdallHTTPClientWithNoCredentials(t *testing.T) { _, err = hhc.Do(req) assert.Error(t, err, "expecting unsupported protocol error") } + +func TestNewHeimdallHTTPClientWithCustomCredentials(t *testing.T) { + hc := hystrix.NewClient() + hhc, err := newHeimdallHTTPClient(context.TODO(), &Options{ + CredentialsJSON: nil, + Credentials: &google.Credentials{ProjectID: "sample"}, + Client: hc, + }) + assert.NotNil(t, hhc) + assert.NoError(t, err) + req, _ := http.NewRequest(http.MethodGet, "", nil) + _, err = hhc.Do(req) + assert.Error(t, err, "expecting unsupported protocol error") +} diff --git a/pkg/storage/gcs/options.go b/pkg/storage/gcs/options.go index 1babdaa..111823d 100644 --- a/pkg/storage/gcs/options.go +++ b/pkg/storage/gcs/options.go @@ -1,6 +1,9 @@ package gcs -import "github.com/gojektech/heimdall" +import ( + "github.com/gojektech/heimdall" + "golang.org/x/oauth2/google" +) // Options represents the Google Cloud Storage storage options type Options struct { @@ -8,8 +11,8 @@ type Options struct { BucketName string // CredentialsJSON holds the json data for credentials of a service account CredentialsJSON []byte - // UseDefaultCredential toggle the usage of google application default credential to authenticate with cloud storage - UseDefaultCredential bool + // Credentials represents google credentials, including Application Default Credentials + Credentials *google.Credentials // Client can be used to specify a heimdall.Client with hystrix like circuit breaker Client heimdall.Client }