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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ package gcs

import (
"context"

"google.golang.org/api/iterator"

ps "github.com/beyondstorage/go-storage/v4/pairs"
typ "github.com/beyondstorage/go-storage/v4/types"
"google.golang.org/api/iterator"
)

func (s *Service) create(ctx context.Context, name string, opt pairServiceCreate) (store typ.Storager, err error) {
Expand Down Expand Up @@ -47,7 +45,6 @@ func (s *Service) list(ctx context.Context, opt pairServiceList) (it *typ.Storag

func (s *Service) nextStoragePage(ctx context.Context, page *typ.StoragerPage) error {
it := s.service.Buckets(ctx, s.projectID)

for {
bucket, err := it.Next()
if err == iterator.Done {
Expand All @@ -56,12 +53,10 @@ func (s *Service) nextStoragePage(ctx context.Context, page *typ.StoragerPage) e
if err != nil {
return err
}

store, err := s.newStorage(ps.WithName(bucket.Name))
if err != nil {
return err
}

page.Data = append(page.Data, store)
}
}
43 changes: 1 addition & 42 deletions storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,15 @@ import (
"io"

gs "cloud.google.com/go/storage"
"google.golang.org/api/iterator"

ps "github.com/beyondstorage/go-storage/v4/pairs"
"github.com/beyondstorage/go-storage/v4/pkg/iowrap"
"github.com/beyondstorage/go-storage/v4/services"
. "github.com/beyondstorage/go-storage/v4/types"
"google.golang.org/api/iterator"
)

func (s *Storage) create(path string, opt pairStorageCreate) (o *Object) {
rp := s.getAbsPath(path)

if opt.HasObjectMode && opt.ObjectMode.IsDir() {
if !s.features.VirtualDir {
return
Expand All @@ -29,7 +27,6 @@ func (s *Storage) create(path string, opt pairStorageCreate) (o *Object) {
o = s.newObject(false)
o.Mode = ModeRead
}

o.ID = rp
o.Path = path
return o
Expand All @@ -40,25 +37,20 @@ func (s *Storage) createDir(ctx context.Context, path string, opt pairStorageCre
err = NewOperationNotImplementedError("create_dir")
return
}

rp := s.getAbsPath(path)

// Add `/` at the end of `path` to simulate a directory.
// ref: https://cloud.google.com/storage/docs/naming-objects
rp += "/"

object := s.bucket.Object(rp)
w := object.NewWriter(ctx)
w.Size = 0
if opt.HasStorageClass {
w.StorageClass = opt.StorageClass
}

cerr := w.Close()
if cerr != nil {
err = cerr
}

o = s.newObject(true)
o.ID = rp
o.Path = path
Expand All @@ -68,16 +60,13 @@ func (s *Storage) createDir(ctx context.Context, path string, opt pairStorageCre

func (s *Storage) delete(ctx context.Context, path string, opt pairStorageDelete) (err error) {
rp := s.getAbsPath(path)

if opt.HasObjectMode && opt.ObjectMode.IsDir() {
if !s.features.VirtualDir {
err = services.PairUnsupportedError{Pair: ps.WithObjectMode(opt.ObjectMode)}
return
}

rp += "/"
}

err = s.bucket.Object(rp).Delete(ctx)
if err != nil && errors.Is(err, gs.ErrObjectNotExist) {
// Omit `ErrObjectNotExist` error here.
Expand All @@ -94,15 +83,12 @@ func (s *Storage) list(ctx context.Context, path string, opt pairStorageList) (o
input := &objectPageStatus{
prefix: s.getAbsPath(path),
}

if !opt.HasListMode {
// Support `ListModePrefix` as the default `ListMode`.
// ref: [GSP-654](https://github.com/beyondstorage/go-storage/blob/master/docs/rfcs/654-unify-list-behavior.md)
opt.ListMode = ListModePrefix
}

var nextFn NextObjectFunc

switch {
case opt.ListMode.IsDir():
input.delimiter = "/"
Expand All @@ -112,7 +98,6 @@ func (s *Storage) list(ctx context.Context, path string, opt pairStorageList) (o
default:
return nil, services.ListModeInvalidError{Actual: opt.ListMode}
}

return NewObjectIterator(ctx, nextFn, input), nil
}

Expand All @@ -125,12 +110,10 @@ func (s *Storage) metadata(opt pairStorageMetadata) (meta *StorageMeta) {

func (s *Storage) nextObjectPageByDir(ctx context.Context, page *ObjectPage) error {
input := page.Status.(*objectPageStatus)

it := s.bucket.Objects(ctx, &gs.Query{
Prefix: input.prefix,
Delimiter: input.delimiter,
})

remaining := 200
for remaining > 0 {
object, err := it.Next()
Expand All @@ -140,7 +123,6 @@ func (s *Storage) nextObjectPageByDir(ctx context.Context, page *ObjectPage) err
if err != nil {
return err
}

// Prefix is set only for ObjectAttrs which represent synthetic "directory
// entries" when iterating over buckets using Query.Delimiter. See
// ObjectIterator.Next. When set, no other fields in ObjectAttrs will be
Expand All @@ -150,32 +132,25 @@ func (s *Storage) nextObjectPageByDir(ctx context.Context, page *ObjectPage) err
o.ID = object.Prefix
o.Path = s.getRelPath(object.Prefix)
o.Mode |= ModeDir

page.Data = append(page.Data, o)

remaining -= 1
continue
}

o, err := s.formatFileObject(object)
if err != nil {
return err
}

page.Data = append(page.Data, o)
remaining -= 1
}

return nil
}

func (s *Storage) nextObjectPageByPrefix(ctx context.Context, page *ObjectPage) error {
input := page.Status.(*objectPageStatus)

it := s.bucket.Objects(ctx, &gs.Query{
Prefix: input.prefix,
})

remaining := 200
for remaining > 0 {
object, err := it.Next()
Expand All @@ -185,24 +160,19 @@ func (s *Storage) nextObjectPageByPrefix(ctx context.Context, page *ObjectPage)
if err != nil {
return err
}

o, err := s.formatFileObject(object)
if err != nil {
return err
}

page.Data = append(page.Data, o)
remaining -= 1
}

return nil
}

func (s *Storage) read(ctx context.Context, path string, w io.Writer, opt pairStorageRead) (n int64, err error) {
rp := s.getAbsPath(path)

var rc io.ReadCloser

object := s.bucket.Object(rp)
if opt.HasEncryptionKey {
object = object.Key(opt.EncryptionKey)
Expand All @@ -217,47 +187,38 @@ func (s *Storage) read(ctx context.Context, path string, w io.Writer, opt pairSt
err = cerr
}
}()

if opt.HasIoCallback {
rc = iowrap.CallbackReadCloser(rc, opt.IoCallback)
}

return io.Copy(w, rc)
}

func (s *Storage) stat(ctx context.Context, path string, opt pairStorageStat) (o *Object, err error) {
rp := s.getAbsPath(path)

if opt.HasObjectMode && opt.ObjectMode.IsDir() {
if !s.features.VirtualDir {
err = services.PairUnsupportedError{Pair: ps.WithObjectMode(opt.ObjectMode)}
return
}

rp += "/"
}

attr, err := s.bucket.Object(rp).Attrs(ctx)
if err != nil {
return nil, err
}

o, err = s.formatFileObject(attr)
if err != nil {
return nil, err
}

if opt.HasObjectMode && opt.ObjectMode.IsDir() {
o.Path = path
o.Mode.Add(ModeDir)
}

return o, nil
}

func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int64, opt pairStorageWrite) (n int64, err error) {
rp := s.getAbsPath(path)

object := s.bucket.Object(rp)
if opt.HasEncryptionKey {
object = object.Key(opt.EncryptionKey)
Expand All @@ -269,7 +230,6 @@ func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int6
err = cerr
}
}()

w.Size = size
if opt.HasContentMd5 {
// FIXME: we need to check value's encoding type.
Expand All @@ -284,6 +244,5 @@ func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int6
if opt.HasIoCallback {
r = iowrap.CallbackReader(r, opt.IoCallback)
}

return io.Copy(w, r)
}
30 changes: 20 additions & 10 deletions utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,15 @@ import (
"strings"

gs "cloud.google.com/go/storage"
"golang.org/x/oauth2"
"golang.org/x/oauth2/google"

"google.golang.org/api/googleapi"
"google.golang.org/api/option"

ps "github.com/beyondstorage/go-storage/v4/pairs"
"github.com/beyondstorage/go-storage/v4/pkg/credential"
"github.com/beyondstorage/go-storage/v4/pkg/httpclient"
"github.com/beyondstorage/go-storage/v4/services"
typ "github.com/beyondstorage/go-storage/v4/types"
"golang.org/x/oauth2"
"golang.org/x/oauth2/google"
"google.golang.org/api/googleapi"
"google.golang.org/api/option"
)

// Service is the gcs config.
Expand Down Expand Up @@ -111,15 +109,27 @@ 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

// Let the GCS client fetch the creds from the env
default:
return nil, services.PairUnsupportedError{Pair: ps.WithCredential(opt.Credential)}
}

// Loading token source from binary data.
creds, err := google.CredentialsFromJSON(ctx, credJSON, gs.ScopeFullControl)
if err != nil {
return nil, err
var creds *google.Credentials
if len(credJSON) > 0 {
// Loading token source from binary data.
creds, err = google.CredentialsFromJSON(ctx, credJSON, gs.ScopeFullControl)
if err != nil {
return nil, err
}
} else {
// Loading token source from environment.
creds, err = google.FindDefaultCredentials(ctx, gs.ScopeFullControl)
if err != nil {
return nil, err
}
}

ot := &oauth2.Transport{
Source: creds.TokenSource,
Base: hc.Transport,
Expand Down