diff --git a/client/client.go b/client/client.go index f23616b8..930a1471 100644 --- a/client/client.go +++ b/client/client.go @@ -34,10 +34,9 @@ var ErrInvalidROWriteOperation = errors.New("the client operates in read only mo // GCSBlobstore encapsulates interaction with the GCS blobstore type GCSBlobstore struct { - // gcsClient is a pre-configured storage.Client. - client *storage.Client - // gcscliConfig is the configuration for interactions with the blobstore - config *config.GCSCli + gcs *storage.Client + config *config.GCSCli + readOnly bool } // validateRemoteConfig determines if the configuration of the client matches @@ -46,11 +45,11 @@ type GCSBlobstore struct { // If operating in read-only mode, no mutations can be performed // so the remote bucket location is always compatible. func (client *GCSBlobstore) validateRemoteConfig() error { - if client.config.IsReadOnly() { + if client.readOnly { return nil } - bucket := client.client.Bucket(client.config.BucketName) + bucket := client.gcs.Bucket(client.config.BucketName) attrs, err := bucket.Attrs(context.Background()) if err != nil { return err @@ -60,33 +59,33 @@ func (client *GCSBlobstore) validateRemoteConfig() error { // getObjectHandle returns a handle to an object at src. func (client GCSBlobstore) getObjectHandle(src string) *storage.ObjectHandle { - handle := client.client.Bucket(client.config.BucketName).Object(src) + handle := client.gcs.Bucket(client.config.BucketName).Object(src) if client.config.EncryptionKey != nil { handle = handle.Key(client.config.EncryptionKey) } return handle } -// New returns a BlobstoreClient configured to operate using the given config -// and client. +// New returns a GCSBlobstore configured to operate using the given config // -// non-nil error is returned on invalid client or config. If the configuration +// non-nil error is returned on invalid Client or config. If the configuration // is incompatible with the GCS bucket, a non-nil error is also returned. -func New(ctx context.Context, gcsClient *storage.Client, - gcscliConfig *config.GCSCli) (GCSBlobstore, error) { - if gcsClient == nil { - return GCSBlobstore{}, errors.New("nil client causes invalid blobstore") +func New(ctx context.Context, cfg *config.GCSCli) (*GCSBlobstore, error) { + if cfg == nil { + return nil, errors.New("expected non-nill config object") } - if gcscliConfig == nil { - return GCSBlobstore{}, errors.New("nil config causes invalid blobstore") + + storageClient, readOnly, err := newStorageClient(ctx, cfg) + if err != nil { + return nil, fmt.Errorf("creating storage client: %v", err) } - return GCSBlobstore{gcsClient, gcscliConfig}, nil + return &GCSBlobstore{gcs: storageClient, config: cfg, readOnly: readOnly}, nil } // Get fetches a blob from the GCS blobstore. // Destination will be overwritten if it already exists. -func (client GCSBlobstore) Get(src string, dest io.Writer) error { +func (client *GCSBlobstore) Get(src string, dest io.Writer) error { remoteReader, err := client.getObjectHandle(src).NewReader(context.Background()) if err != nil { return err @@ -101,8 +100,8 @@ func (client GCSBlobstore) Get(src string, dest io.Writer) error { // Put does not retry if upload fails. This is a change from s3cli/client // which does retry an upload multiple times. // TODO: implement retry -func (client GCSBlobstore) Put(src io.ReadSeeker, dest string) error { - if client.config.IsReadOnly() { +func (client *GCSBlobstore) Put(src io.ReadSeeker, dest string) error { + if client.readOnly { return ErrInvalidROWriteOperation } @@ -122,8 +121,8 @@ func (client GCSBlobstore) Put(src io.ReadSeeker, dest string) error { // Delete removes a blob from from the GCS blobstore. // // If the object does not exist, Delete returns a nil error. -func (client GCSBlobstore) Delete(dest string) error { - if client.config.IsReadOnly() { +func (client *GCSBlobstore) Delete(dest string) error { + if client.readOnly { return ErrInvalidROWriteOperation } @@ -135,7 +134,7 @@ func (client GCSBlobstore) Delete(dest string) error { } // Exists checks if a blob exists in the GCS blobstore. -func (client GCSBlobstore) Exists(dest string) (bool, error) { +func (client *GCSBlobstore) Exists(dest string) (bool, error) { _, err := client.getObjectHandle(dest).Attrs(context.Background()) if err == nil { log.Printf("File '%s' exists in bucket '%s'\n", diff --git a/client/sdk.go b/client/sdk.go index 77cd1a00..68c0b1c7 100644 --- a/client/sdk.go +++ b/client/sdk.go @@ -20,7 +20,6 @@ import ( "context" "errors" - "golang.org/x/oauth2" "golang.org/x/oauth2/google" "google.golang.org/api/option" @@ -29,39 +28,34 @@ import ( "cloud.google.com/go/storage" "github.com/cloudfoundry/bosh-gcscli/config" - "golang.org/x/oauth2/jwt" ) const uaString = "bosh-gcscli" -// NewSDK builds the GCS SDK client from a valid config.GCSCli -func NewSDK(ctx context.Context, c config.GCSCli) (*storage.Client, error) { - var client *storage.Client - var err error - ua := option.WithUserAgent(uaString) - var opt option.ClientOption - switch c.CredentialsSource { - case config.ApplicationDefaultCredentialsSource: - var tokenSource oauth2.TokenSource - tokenSource, err = google.DefaultTokenSource(ctx, storage.ScopeFullControl) - if err == nil { +func newStorageClient(ctx context.Context, cfg *config.GCSCli) (*storage.Client, bool, error) { + // default to a read-only client + readOnly := true + opt := option.WithHTTPClient(http.DefaultClient) + + switch cfg.CredentialsSource { + case config.NoneCredentialsSource: + // no-op + case config.DefaultCredentialsSource: + // attempt to load the application default credentials + if tokenSource, err := google.DefaultTokenSource(ctx, storage.ScopeFullControl); err == nil { opt = option.WithTokenSource(tokenSource) + readOnly = false } - case config.NoneCredentialsSource: - opt = option.WithHTTPClient(http.DefaultClient) case config.ServiceAccountFileCredentialsSource: - var token *jwt.Config - token, err = google.JWTConfigFromJSON([]byte(c.ServiceAccountFile), storage.ScopeFullControl) - if err == nil { - tokenSource := token.TokenSource(ctx) - opt = option.WithTokenSource(tokenSource) + if token, err := google.JWTConfigFromJSON([]byte(cfg.ServiceAccountFile), storage.ScopeFullControl); err == nil { + opt = option.WithTokenSource(token.TokenSource(ctx)) + readOnly = false } default: - err = errors.New("unknown credentials_source in configuration") - } - if err != nil { - return client, err + return nil, false, errors.New("unknown credentials_source in configuration") } - return storage.NewClient(ctx, ua, opt) + gcs, err := storage.NewClient(ctx, option.WithUserAgent(uaString), opt) + + return gcs, readOnly, err } diff --git a/config/config.go b/config/config.go index d8a41ad5..de85b51b 100644 --- a/config/config.go +++ b/config/config.go @@ -27,7 +27,7 @@ type GCSCli struct { // BucketName is the GCS bucket operations will use. BucketName string `json:"bucket_name"` // CredentialsSource is the location of a Service Account File. - // If left empty, Application Default Credentials will be used. + // If left empty, Application Default Credentials will be used if available. // If equal to 'none', read-only scope will be used. // If equal to 'static', json_key will be used. CredentialsSource string `json:"credentials_source"` @@ -52,9 +52,10 @@ const ( defaultMultiRegionalStorageClass = "MULTI_REGIONAL" ) -// ApplicationDefaultCredentialsSource specifies that -// Application Default Credentials should be used for authentication. -const ApplicationDefaultCredentialsSource = "" +// DefaultCredentialsSource specifies that credentials should be detected. +// Application Default Credentials will be used if avaliable. +// A read-only client will be used otherwise. +const DefaultCredentialsSource = "" // NoneCredentialsSource specifies that credentials are explicitly empty // and that the client should be restricted to a read-only scope. @@ -143,8 +144,3 @@ func (c *GCSCli) FitCompatibleLocation(loc string) error { return validLocationStorageClass(loc, c.StorageClass) } - -// IsReadOnly returns if the configuration is limited to only read operations. -func (c *GCSCli) IsReadOnly() bool { - return c.CredentialsSource == NoneCredentialsSource -} diff --git a/config/config_test.go b/config/config_test.go index 00ce490e..9c97370f 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -160,17 +160,6 @@ var _ = Describe("BlobstoreClient configuration", func() { }) }) - Describe("when credentials_source is 'none'", func() { - dummyJSONBytes := []byte(`{"credentials_source": "none", "bucket_name": "some-bucket"}`) - dummyJSONReader := bytes.NewReader(dummyJSONBytes) - - It("is ReadOnly", func() { - c, err := NewFromReader(dummyJSONReader) - Expect(err).ToNot(HaveOccurred()) - Expect(c.IsReadOnly()).To(BeTrue()) - }) - }) - Describe("when credentials_source is not specified", func() { dummyJSONBytes := []byte(`{"credentials_source": "", "bucket_name": "some-bucket"}`) dummyJSONReader := bytes.NewReader(dummyJSONBytes) diff --git a/integration/assertcontext.go b/integration/assertcontext.go index 893f71a7..bbe0ddd6 100644 --- a/integration/assertcontext.go +++ b/integration/assertcontext.go @@ -151,7 +151,7 @@ func AsDefaultCredentials(ctx *AssertContext) { ctx.serviceAccountPath = tempFile.Name() os.Setenv(GoogleAppCredentialsEnv, ctx.serviceAccountPath) - conf.CredentialsSource = config.ApplicationDefaultCredentialsSource + conf.CredentialsSource = config.DefaultCredentialsSource } // Clone returns a new AssertContext configured using the provided options. diff --git a/integration/configurations.go b/integration/configurations.go index 52d568e3..b573ee60 100644 --- a/integration/configurations.go +++ b/integration/configurations.go @@ -17,12 +17,21 @@ package integration import ( + "context" + "errors" "fmt" + "net/http" "os" + "cloud.google.com/go/storage" + "github.com/cloudfoundry/bosh-gcscli/config" . "github.com/onsi/ginkgo/extensions/table" + "golang.org/x/oauth2" + "golang.org/x/oauth2/google" + "golang.org/x/oauth2/jwt" + "google.golang.org/api/option" ) const regionalBucketEnv = "REGIONAL_BUCKET_NAME" @@ -98,3 +107,35 @@ func getInvalidStorageClassConfigs() []TableEntry { Entry("Regional bucket, Multi-Region StorageClass", multiRegion), } } + +// newSDK builds the GCS SDK Client from a valid config.GCSCli +// TODO: Simplify and remove this. Tests should expect a single config and use it. +func newSDK(ctx context.Context, c config.GCSCli) (*storage.Client, error) { + var client *storage.Client + var err error + var opt option.ClientOption + switch c.CredentialsSource { + case config.DefaultCredentialsSource: + var tokenSource oauth2.TokenSource + tokenSource, err = google.DefaultTokenSource(ctx, storage.ScopeFullControl) + if err == nil { + opt = option.WithTokenSource(tokenSource) + } + case config.NoneCredentialsSource: + opt = option.WithHTTPClient(http.DefaultClient) + case config.ServiceAccountFileCredentialsSource: + var token *jwt.Config + token, err = google.JWTConfigFromJSON([]byte(c.ServiceAccountFile), storage.ScopeFullControl) + if err == nil { + tokenSource := token.TokenSource(ctx) + opt = option.WithTokenSource(tokenSource) + } + default: + err = errors.New("unknown credentials_source in configuration") + } + if err != nil { + return client, err + } + + return storage.NewClient(ctx, opt) +} diff --git a/integration/gcs_encryption_test.go b/integration/gcs_encryption_test.go index eafe8551..d29d17b0 100644 --- a/integration/gcs_encryption_test.go +++ b/integration/gcs_encryption_test.go @@ -67,9 +67,7 @@ var _ = Describe("Integration", func() { Expect(err).ToNot(HaveOccurred()) Expect(session.ExitCode()).To(BeZero()) - gcsClient, err := client.NewSDK(env.ctx, *env.Config) - Expect(err).ToNot(HaveOccurred()) - blobstoreClient, err := client.New(env.ctx, gcsClient, env.Config) + blobstoreClient, err := client.New(env.ctx, env.Config) Expect(err).ToNot(HaveOccurred()) env.Config.EncryptionKey[0]++ @@ -93,9 +91,7 @@ var _ = Describe("Integration", func() { Expect(err).ToNot(HaveOccurred()) Expect(session.ExitCode()).To(BeZero()) - gcsClient, err := client.NewSDK(env.ctx, *env.Config) - Expect(err).ToNot(HaveOccurred()) - blobstoreClient, err := client.New(env.ctx, gcsClient, env.Config) + blobstoreClient, err := client.New(env.ctx, env.Config) Expect(err).ToNot(HaveOccurred()) env.Config.EncryptionKey = nil diff --git a/integration/gcs_general_test.go b/integration/gcs_general_test.go index c21dd000..10e839bd 100644 --- a/integration/gcs_general_test.go +++ b/integration/gcs_general_test.go @@ -106,9 +106,7 @@ var _ = Describe("Integration", func() { const twoGB = 1024 * 1024 * 1024 * 2 limited := newrandReadSeeker(twoGB) - gcsClient, err := client.NewSDK(env.ctx, *env.Config) - Expect(err).ToNot(HaveOccurred()) - blobstoreClient, err := client.New(env.ctx, gcsClient, env.Config) + blobstoreClient, err := client.New(env.ctx, env.Config) Expect(err).ToNot(HaveOccurred()) err = blobstoreClient.Put(&limited, env.GCSFileName) @@ -123,9 +121,7 @@ var _ = Describe("Integration", func() { func(config *config.GCSCli) { env.AddConfig(config) - gcsClient, err := client.NewSDK(env.ctx, *env.Config) - Expect(err).ToNot(HaveOccurred()) - blobstoreClient, err := client.New(env.ctx, gcsClient, env.Config) + blobstoreClient, err := client.New(env.ctx, env.Config) Expect(err).ToNot(HaveOccurred()) err = blobstoreClient.Put(&badReadSeeker{}, env.GCSFileName) diff --git a/integration/gcs_public_test.go b/integration/gcs_public_test.go index 1b587efb..4d8446e9 100644 --- a/integration/gcs_public_test.go +++ b/integration/gcs_public_test.go @@ -57,7 +57,7 @@ var _ = Describe("GCS Public Bucket", func() { RunGCSCLI(gcsCLIPath, setupEnv.ConfigPath, "put", setupEnv.ContentFile, setupEnv.GCSFileName) // Make the file public - rwClient, err := client.NewSDK(setupEnv.ctx, *setupEnv.Config) + rwClient, err := newSDK(setupEnv.ctx, *setupEnv.Config) Expect(err).ToNot(HaveOccurred()) bucket := rwClient.Bucket(setupEnv.Config.BucketName) obj := bucket.Object(setupEnv.GCSFileName) diff --git a/main.go b/main.go index 21d2afc8..0daaab5e 100644 --- a/main.go +++ b/main.go @@ -114,12 +114,7 @@ func main() { } ctx := context.Background() - gcsClient, err := client.NewSDK(ctx, gcsConfig) - if err != nil { - log.Fatalf("creating gcs sdk: %v\n", err) - } - - blobstoreClient, err := client.New(ctx, gcsClient, &gcsConfig) + blobstoreClient, err := client.New(ctx, &gcsConfig) if err != nil { log.Fatalf("creating gcs client: %v\n", err) }