From a3c570d26414b3cba2fa12e3fc4457babb847d70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20L=C3=B3pez?= <1953782+julio-lopez@users.noreply.github.com> Date: Sun, 27 Oct 2024 18:39:45 -0700 Subject: [PATCH] refactor(providers): simplify GCS client construction (#4120) Changes: * simplify GCS client creation; * early parameter validation in `gcs.New()`; * remove GCS internal tests; * remove no-longer used GCS creds helpers. --- repo/blob/gcs/gcs_internal_test.go | 80 ------------------------------ repo/blob/gcs/gcs_storage.go | 52 ++++--------------- 2 files changed, 10 insertions(+), 122 deletions(-) delete mode 100644 repo/blob/gcs/gcs_internal_test.go diff --git a/repo/blob/gcs/gcs_internal_test.go b/repo/blob/gcs/gcs_internal_test.go deleted file mode 100644 index 7de0993d3..000000000 --- a/repo/blob/gcs/gcs_internal_test.go +++ /dev/null @@ -1,80 +0,0 @@ -// Package gcs implements Storage based on Google Cloud Storage bucket. -package gcs - -import ( - "context" - "io/fs" - "os" - "testing" - - gcsclient "cloud.google.com/go/storage" - "github.com/stretchr/testify/require" -) - -func TestGCSStorageCredentialsHelpers(t *testing.T) { - ctx := context.Background() - scope := gcsclient.ScopeReadOnly - - var fileMode fs.FileMode = 0o644 - - // Service Account key - gsaKeyServiceAccount := `{ - "type": "service_account", - "project_id": "kopia-test-project", - "private_key_id": "kopia-test", - "private_key": "some-private-key", - "client_email": "kopia-test@developer.gserviceaccount.com", - "client_id": "kopia-test.apps.googleusercontent.com", - "auth_uri": "https://accounts.google.com/o/oauth2/auth", - "token_uri": "http://localhost:8080/token" - }` - gsaKeyServiceAccountFileName := "service-account.json" - errWriteFile := os.WriteFile(gsaKeyServiceAccountFileName, []byte(gsaKeyServiceAccount), fileMode) - require.NoError(t, errWriteFile) - t.Cleanup(func() { - os.Remove(gsaKeyServiceAccountFileName) - }) - - t.Run("tokenSourceFromCredentialsJSON with service account key", func(t *testing.T) { - ts, err := tokenSourceFromCredentialsJSON(ctx, []byte(gsaKeyServiceAccount), scope) - require.NoError(t, err) - require.NotNil(t, ts) - }) - t.Run("tokenSourceFromCredentialsFile with service account key file", func(t *testing.T) { - ts, err := tokenSourceFromCredentialsFile(ctx, gsaKeyServiceAccountFileName, scope) - require.NoError(t, err) - require.NotNil(t, ts) - }) - - // External Account key - gsaKeyExternalAccount := `{ - "type": "external_account", - "audience": "some-audience", - "subject_token_type": "urn:ietf:params:oauth:token-type:jwt", - "token_url": "https://sts.googleapis.com/v1/token", - "service_account_impersonation_url": "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/kopia-test@kopia-test-project.iam.gserviceaccount.com:generateAccessToken", - "credential_source": { - "file": "/var/run/secrets/serviceaccount/token", - "format": { - "type": "text" - } - } - }` - gsaKeyExternalAccountFileName := "external-account.json" - errWriteFile = os.WriteFile(gsaKeyExternalAccountFileName, []byte(gsaKeyExternalAccount), fileMode) - require.NoError(t, errWriteFile) - t.Cleanup(func() { - os.Remove(gsaKeyExternalAccountFileName) - }) - - t.Run("tokenSourceFromCredentialsJSON with external account key", func(t *testing.T) { - ts, err := tokenSourceFromCredentialsJSON(ctx, []byte(gsaKeyExternalAccount), scope) - require.NoError(t, err) - require.NotNil(t, ts) - }) - t.Run("tokenSourceFromCredentialsFile with external account key file", func(t *testing.T) { - ts, err := tokenSourceFromCredentialsFile(ctx, gsaKeyExternalAccountFileName, scope) - require.NoError(t, err) - require.NotNil(t, ts) - }) -} diff --git a/repo/blob/gcs/gcs_storage.go b/repo/blob/gcs/gcs_storage.go index fdc8e744d..6b8ed3b3d 100644 --- a/repo/blob/gcs/gcs_storage.go +++ b/repo/blob/gcs/gcs_storage.go @@ -3,17 +3,13 @@ package gcs import ( "context" - "encoding/json" "fmt" "net/http" - "os" "strconv" "time" gcsclient "cloud.google.com/go/storage" "github.com/pkg/errors" - "golang.org/x/oauth2" - "golang.org/x/oauth2/google" "google.golang.org/api/googleapi" "google.golang.org/api/iterator" "google.golang.org/api/option" @@ -246,24 +242,6 @@ func (gcs *gcsStorage) toBlobID(blobName string) blob.ID { return blob.ID(blobName[len(gcs.Prefix):]) } -func tokenSourceFromCredentialsFile(ctx context.Context, fn string, scopes ...string) (oauth2.TokenSource, error) { - data, err := os.ReadFile(fn) //nolint:gosec - if err != nil { - return nil, errors.Wrap(err, "error reading credentials file") - } - - return tokenSourceFromCredentialsJSON(ctx, data, scopes...) -} - -func tokenSourceFromCredentialsJSON(ctx context.Context, data json.RawMessage, scopes ...string) (oauth2.TokenSource, error) { - creds, err := google.CredentialsFromJSON(ctx, data, scopes...) - if err != nil { - return nil, errors.Wrap(err, "google.CredentialsFromJSON") - } - - return creds.TokenSource, nil -} - // New creates new Google Cloud Storage-backed storage with specified options: // // - the 'BucketName' field is required and all other parameters are optional. @@ -273,38 +251,28 @@ func tokenSourceFromCredentialsJSON(ctx context.Context, data json.RawMessage, s func New(ctx context.Context, opt *Options, isCreate bool) (blob.Storage, error) { _ = isCreate - var ts oauth2.TokenSource - - var err error + if opt.BucketName == "" { + return nil, errors.New("bucket name must be specified") + } scope := gcsclient.ScopeFullControl if opt.ReadOnly { scope = gcsclient.ScopeReadOnly } - if sa := opt.ServiceAccountCredentialJSON; len(sa) > 0 { - ts, err = tokenSourceFromCredentialsJSON(ctx, sa, scope) - } else if sa := opt.ServiceAccountCredentialsFile; sa != "" { - ts, err = tokenSourceFromCredentialsFile(ctx, sa, scope) - } else { - ts, err = google.DefaultTokenSource(ctx, scope) + clientOptions := []option.ClientOption{option.WithScopes(scope)} + + if j := opt.ServiceAccountCredentialJSON; len(j) > 0 { + clientOptions = append(clientOptions, option.WithCredentialsJSON(j)) + } else if fn := opt.ServiceAccountCredentialsFile; fn != "" { + clientOptions = append(clientOptions, option.WithCredentialsFile(fn)) } - if err != nil { - return nil, errors.Wrap(err, "unable to initialize token source") - } - - hc := oauth2.NewClient(ctx, ts) - - cli, err := gcsclient.NewClient(ctx, option.WithHTTPClient(hc)) + cli, err := gcsclient.NewClient(ctx, clientOptions...) if err != nil { return nil, errors.Wrap(err, "unable to create GCS client") } - if opt.BucketName == "" { - return nil, errors.New("bucket name must be specified") - } - st := &gcsStorage{ Options: *opt, storageClient: cli,