GCS image uploader: Add tests (#28521)

* GCS uploader: Add tests

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* Use go generate

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/components/imguploader/gcs/gcsuploader.go

@@ -0,0 +1,222 @@
+// Package gcs provides an image uploader for GCS.
+package gcs
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path"
+	"path/filepath"
+	"time"
+
+	"cloud.google.com/go/storage"
+	"github.com/grafana/grafana/pkg/ifaces/gcsifaces"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/util"
+	"golang.org/x/oauth2/google"
+	"golang.org/x/oauth2/jwt"
+	"google.golang.org/api/option"
+)
+
+// NewUploader returns a new Uploader.
+func NewUploader(keyFile, bucket, path string, enableSignedURLs bool, signedURLExpiration time.Duration) (*Uploader, error) {
+	if signedURLExpiration <= 0 {
+		return nil, fmt.Errorf("invalid signed URL expiration: %q", signedURLExpiration)
+	}
+	uploader := &Uploader{
+		KeyFile:             keyFile,
+		Bucket:              bucket,
+		path:                path,
+		log:                 log.New("gcsuploader"),
+		enableSignedURLs:    enableSignedURLs,
+		signedURLExpiration: signedURLExpiration,
+	}
+
+	uploader.log.Debug("Created uploader", "key", keyFile, "bucket", bucket, "path", path, "enableSignedUrls",
+		enableSignedURLs, "signedUrlExpiration", signedURLExpiration.String())
+
+	return uploader, nil
+}
+
+// newClient returns a new GCS client.
+// Stubbable by tests.
+var newClient = func(ctx context.Context, opts ...option.ClientOption) (gcsifaces.StorageClient, error) {
+	client, err := storage.NewClient(ctx, opts...)
+	return clientWrapper{client}, err
+}
+
+// Uploader supports uploading images to GCS.
+type Uploader struct {
+	KeyFile             string
+	Bucket              string
+	path                string
+	log                 log.Logger
+	enableSignedURLs    bool
+	signedURLExpiration time.Duration
+}
+
+// Upload uploads an image to GCS.
+func (u *Uploader) Upload(ctx context.Context, imageDiskPath string) (string, error) {
+	fileName, err := util.GetRandomString(20)
+	if err != nil {
+		return "", err
+	}
+
+	ext := filepath.Ext(imageDiskPath)
+	if ext == "" {
+		ext = ".png"
+	}
+	fileName += ext
+
+	key := path.Join(u.path, fileName)
+
+	var keyData []byte
+	if u.KeyFile != "" {
+		u.log.Debug("Opening key file ", u.KeyFile)
+		keyData, err = ioutil.ReadFile(u.KeyFile)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	const scope = storage.ScopeReadWrite
+
+	var client gcsifaces.StorageClient
+	if u.KeyFile != "" {
+		u.log.Debug("Creating Google credentials from JSON")
+		creds, err := google.CredentialsFromJSON(ctx, keyData, scope)
+		if err != nil {
+			return "", err
+		}
+
+		u.log.Debug("Creating GCS client")
+		client, err = newClient(ctx, option.WithCredentials(creds))
+		if err != nil {
+			return "", err
+		}
+	} else {
+		u.log.Debug("Creating GCS client with default application credentials")
+		client, err = newClient(ctx, option.WithScopes(scope))
+		if err != nil {
+			return "", err
+		}
+	}
+
+	if err := u.uploadFile(ctx, client, imageDiskPath, key); err != nil {
+		return "", err
+	}
+
+	if !u.enableSignedURLs {
+		return fmt.Sprintf("https://storage.googleapis.com/%s/%s", u.Bucket, key), nil
+	}
+
+	u.log.Debug("Signing GCS URL")
+	var jwtData []byte
+	if u.KeyFile != "" {
+		jwtData = keyData
+	} else {
+		creds, err := client.FindDefaultCredentials(ctx, scope)
+		if err != nil {
+			return "", fmt.Errorf("failed to find default Google credentials: %s", err)
+		}
+		jwtData = creds.JSON
+	}
+	conf, err := client.JWTConfigFromJSON(jwtData)
+	if err != nil {
+		return "", err
+	}
+	opts := &storage.SignedURLOptions{
+		Scheme:         storage.SigningSchemeV4,
+		Method:         "GET",
+		GoogleAccessID: conf.Email,
+		PrivateKey:     conf.PrivateKey,
+		Expires:        time.Now().Add(u.signedURLExpiration),
+	}
+	signedURL, err := client.SignedURL(u.Bucket, key, opts)
+	if err != nil {
+		return "", err
+	}
+
+	return signedURL, nil
+}
+
+func (u *Uploader) uploadFile(
+	ctx context.Context,
+	client gcsifaces.StorageClient,
+	imageDiskPath,
+	key string,
+) error {
+	u.log.Debug("Opening image file", "path", imageDiskPath)
+	fileReader, err := os.Open(imageDiskPath)
+	if err != nil {
+		return err
+	}
+	defer fileReader.Close()
+
+	// Set public access if not generating a signed URL
+	pubAcc := !u.enableSignedURLs
+
+	u.log.Debug("Uploading to GCS bucket using SDK", "bucket", u.Bucket, "key", key, "public", pubAcc)
+
+	uri := fmt.Sprintf("gs://%s/%s", u.Bucket, key)
+
+	wc := client.Bucket(u.Bucket).Object(key).NewWriter(ctx)
+	if pubAcc {
+		wc.SetACL("publicRead")
+	}
+	if _, err := io.Copy(wc, fileReader); err != nil {
+		_ = wc.Close()
+		return fmt.Errorf("failed to upload to %s: %s", uri, err)
+	}
+	if err := wc.Close(); err != nil {
+		return fmt.Errorf("failed to upload to %s: %s", uri, err)
+	}
+
+	return nil
+}
+
+type clientWrapper struct {
+	client *storage.Client
+}
+
+func (c clientWrapper) Bucket(key string) gcsifaces.StorageBucket {
+	return bucketWrapper{c.client.Bucket(key)}
+}
+
+func (c clientWrapper) FindDefaultCredentials(ctx context.Context, scope string) (*google.Credentials, error) {
+	return google.FindDefaultCredentials(ctx, scope)
+}
+
+func (c clientWrapper) JWTConfigFromJSON(keyJSON []byte) (*jwt.Config, error) {
+	return google.JWTConfigFromJSON(keyJSON)
+}
+
+func (c clientWrapper) SignedURL(bucket, name string, opts *storage.SignedURLOptions) (string, error) {
+	return storage.SignedURL(bucket, name, opts)
+}
+
+type bucketWrapper struct {
+	bucket *storage.BucketHandle
+}
+
+func (b bucketWrapper) Object(key string) gcsifaces.StorageObject {
+	return objectWrapper{b.bucket.Object(key)}
+}
+
+type objectWrapper struct {
+	object *storage.ObjectHandle
+}
+
+func (o objectWrapper) NewWriter(ctx context.Context) gcsifaces.StorageWriter {
+	return writerWrapper{o.object.NewWriter(ctx)}
+}
+
+type writerWrapper struct {
+	*storage.Writer
+}
+
+func (w writerWrapper) SetACL(acl string) {
+	w.ObjectAttrs.PredefinedACL = acl
+}

pkg/components/imguploader/gcs/gcsuploader_test.go

@@ -0,0 +1,164 @@
+package gcs
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"io/ioutil"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"cloud.google.com/go/storage"
+	"github.com/golang/mock/gomock"
+	"github.com/grafana/grafana/pkg/ifaces/gcsifaces"
+	"github.com/grafana/grafana/pkg/mocks/mock_gcsifaces"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/oauth2/google"
+	"golang.org/x/oauth2/jwt"
+	"google.golang.org/api/option"
+)
+
+const dfltExpiration = 7 * 24 * time.Hour
+
+type testConfig struct {
+	signedURL string
+}
+
+func mockSDK(ctx context.Context, t *testing.T, content []byte, bucket string, signed bool) testConfig {
+	t.Helper()
+
+	var cfg testConfig
+
+	ctrl := gomock.NewController(t)
+	t.Cleanup(func() {
+		ctrl.Finish()
+	})
+
+	wm := mock_gcsifaces.NewMockStorageWriter(ctrl)
+	if !signed {
+		wm.
+			EXPECT().
+			SetACL(gomock.Eq("publicRead")).
+			Return()
+	}
+	wm.EXPECT().
+		Write(gomock.Eq(content)).
+		Return(len(content), nil)
+	wm.EXPECT().
+		Close()
+
+	om := mock_gcsifaces.NewMockStorageObject(ctrl)
+	om.
+		EXPECT().
+		NewWriter(gomock.Eq(ctx)).
+		Return(wm)
+
+	bm := mock_gcsifaces.NewMockStorageBucket(ctrl)
+	bm.
+		EXPECT().
+		Object(gomock.Any()).
+		Return(om)
+
+	cm := mock_gcsifaces.NewMockStorageClient(ctrl)
+	cm.
+		EXPECT().
+		Bucket(gomock.Eq(bucket)).
+		Return(bm)
+
+	if signed {
+		const scope = storage.ScopeReadWrite
+		cfg.signedURL = "https://google.com/signed"
+
+		creds := &google.Credentials{
+			JSON: []byte(`{}`),
+		}
+		conf := &jwt.Config{
+			Email:      "test@grafana.com",
+			PrivateKey: []byte("private"),
+		}
+		suOpts := &storage.SignedURLOptions{
+			Scheme:         storage.SigningSchemeV4,
+			Method:         "GET",
+			GoogleAccessID: conf.Email,
+			PrivateKey:     conf.PrivateKey,
+			Expires:        time.Now().Add(dfltExpiration),
+		}
+		cm.
+			EXPECT().
+			FindDefaultCredentials(gomock.Eq(ctx), gomock.Eq(scope)).
+			Return(creds, nil)
+		cm.
+			EXPECT().
+			JWTConfigFromJSON(gomock.Eq(creds.JSON)).
+			Return(conf, nil)
+		cm.
+			EXPECT().
+			SignedURL(gomock.Eq(bucket), gomock.Any(), signedURLOptsMatcher{suOpts}).
+			Return(cfg.signedURL, nil)
+	}
+
+	origNewClient := newClient
+	t.Cleanup(func() {
+		newClient = origNewClient
+	})
+	newClient = func(ctx context.Context, options ...option.ClientOption) (gcsifaces.StorageClient, error) {
+		return cm, nil
+	}
+
+	return cfg
+}
+
+func TestUploadToGCS_DefaultCredentials(t *testing.T) {
+	const bucket = "test"
+	content := []byte("test\n")
+	tmpDir := t.TempDir()
+	fpath := filepath.Join(tmpDir, "test.png")
+	err := ioutil.WriteFile(fpath, content, 0600)
+	require.NoError(t, err)
+
+	t.Run("Without signed URL", func(t *testing.T) {
+		ctx := context.Background()
+		mockSDK(ctx, t, content, bucket, false)
+
+		uploader, err := NewUploader("", bucket, "", false, dfltExpiration)
+		require.NoError(t, err)
+
+		path, err := uploader.Upload(ctx, fpath)
+		require.NoError(t, err)
+
+		assert.Regexp(t, fmt.Sprintf(`^https://storage.googleapis.com/%s/[^/]+\.png$`, bucket), path)
+	})
+
+	t.Run("With signed URL", func(t *testing.T) {
+		ctx := context.Background()
+		cfg := mockSDK(ctx, t, content, bucket, true)
+
+		uploader, err := NewUploader("", bucket, "", true, dfltExpiration)
+		require.NoError(t, err)
+
+		path, err := uploader.Upload(ctx, fpath)
+		require.NoError(t, err)
+
+		assert.Equal(t, cfg.signedURL, path)
+	})
+}
+
+type signedURLOptsMatcher struct {
+	opts *storage.SignedURLOptions
+}
+
+func (m signedURLOptsMatcher) Matches(x interface{}) bool {
+	suOpts, ok := x.(*storage.SignedURLOptions)
+	if !ok {
+		return false
+	}
+
+	return suOpts.Scheme == m.opts.Scheme && suOpts.Method == m.opts.Method && suOpts.GoogleAccessID ==
+		m.opts.GoogleAccessID && bytes.Equal(suOpts.PrivateKey, m.opts.PrivateKey)
+}
+
+func (m signedURLOptsMatcher) String() string {
+	return "Matches two SignedURLOptions"
+}

pkg/components/imguploader/gcsuploader.go

@@ -1,167 +0,0 @@
-package imguploader
-
-import (
-	"context"
-	"fmt"
-	"io"
-	"io/ioutil"
-	"os"
-	"path"
-	"time"
-
-	"golang.org/x/oauth2/jwt"
-
-	"cloud.google.com/go/storage"
-	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/util"
-	"golang.org/x/oauth2/google"
-	"google.golang.org/api/option"
-)
-
-type GCSUploader struct {
-	keyFile             string
-	bucket              string
-	path                string
-	log                 log.Logger
-	enableSignedURLs    bool
-	signedURLExpiration time.Duration
-}
-
-func NewGCSUploader(keyFile, bucket, path string, enableSignedURLs bool, signedURLExpiration string) (*GCSUploader, error) {
-	expiration, err := time.ParseDuration(signedURLExpiration)
-	if err != nil {
-		return nil, err
-	}
-	if expiration <= 0 {
-		return nil, fmt.Errorf("invalid signed url expiration: %q", expiration)
-	}
-	uploader := &GCSUploader{
-		keyFile:             keyFile,
-		bucket:              bucket,
-		path:                path,
-		log:                 log.New("gcsuploader"),
-		enableSignedURLs:    enableSignedURLs,
-		signedURLExpiration: expiration,
-	}
-
-	uploader.log.Debug("Created GCSUploader", "key", keyFile, "bucket", bucket, "path", path, "enableSignedUrls",
-		enableSignedURLs, "signedUrlExpiration", expiration.String())
-
-	return uploader, nil
-}
-
-func (u *GCSUploader) Upload(ctx context.Context, imageDiskPath string) (string, error) {
-	fileName, err := util.GetRandomString(20)
-	if err != nil {
-		return "", err
-	}
-
-	fileName += pngExt
-	key := path.Join(u.path, fileName)
-
-	var keyData []byte
-	if u.keyFile != "" {
-		u.log.Debug("Opening key file ", u.keyFile)
-		keyData, err = ioutil.ReadFile(u.keyFile)
-		if err != nil {
-			return "", err
-		}
-	}
-
-	const scope = storage.ScopeReadWrite
-
-	var client *storage.Client
-	if u.keyFile != "" {
-		u.log.Debug("Creating Google credentials from JSON")
-		creds, err := google.CredentialsFromJSON(ctx, keyData, scope)
-		if err != nil {
-			return "", err
-		}
-
-		u.log.Debug("Creating GCS client")
-		client, err = storage.NewClient(ctx, option.WithCredentials(creds))
-		if err != nil {
-			return "", err
-		}
-	} else {
-		u.log.Debug("Creating GCS client with default application credentials")
-		client, err = storage.NewClient(ctx, option.WithScopes(scope))
-		if err != nil {
-			return "", err
-		}
-	}
-
-	if err := u.uploadFile(ctx, client, imageDiskPath, key); err != nil {
-		return "", err
-	}
-
-	if !u.enableSignedURLs {
-		return fmt.Sprintf("https://storage.googleapis.com/%s/%s", u.bucket, key), nil
-	}
-
-	u.log.Debug("Signing GCS URL")
-	var conf *jwt.Config
-	if u.keyFile != "" {
-		conf, err = google.JWTConfigFromJSON(keyData)
-		if err != nil {
-			return "", err
-		}
-	} else {
-		creds, err := google.FindDefaultCredentials(ctx, scope)
-		if err != nil {
-			return "", fmt.Errorf("failed to find default Google credentials: %s", err)
-		}
-		conf, err = google.JWTConfigFromJSON(creds.JSON)
-		if err != nil {
-			return "", err
-		}
-	}
-	opts := &storage.SignedURLOptions{
-		Scheme:         storage.SigningSchemeV4,
-		Method:         "GET",
-		GoogleAccessID: conf.Email,
-		PrivateKey:     conf.PrivateKey,
-		Expires:        time.Now().Add(u.signedURLExpiration),
-	}
-	signedURL, err := storage.SignedURL(u.bucket, key, opts)
-	if err != nil {
-		return "", err
-	}
-
-	return signedURL, nil
-}
-
-func (u *GCSUploader) uploadFile(
-	ctx context.Context,
-	client *storage.Client,
-	imageDiskPath,
-	key string,
-) error {
-	u.log.Debug("Opening image file", "path", imageDiskPath)
-	fileReader, err := os.Open(imageDiskPath)
-	if err != nil {
-		return err
-	}
-	defer fileReader.Close()
-
-	// Set public access if not generating a signed URL
-	pubAcc := !u.enableSignedURLs
-
-	u.log.Debug("Uploading to GCS bucket using SDK", "bucket", u.bucket, "key", key, "public", pubAcc)
-
-	uri := fmt.Sprintf("gs://%s/%s", u.bucket, key)
-
-	wc := client.Bucket(u.bucket).Object(key).NewWriter(ctx)
-	if pubAcc {
-		wc.ObjectAttrs.PredefinedACL = "publicRead"
-	}
-	if _, err := io.Copy(wc, fileReader); err != nil {
-		_ = wc.Close()
-		return fmt.Errorf("failed to upload to %s: %s", uri, err)
-	}
-	if err := wc.Close(); err != nil {
-		return fmt.Errorf("failed to upload to %s: %s", uri, err)
-	}
-
-	return nil
-}

pkg/components/imguploader/gcsuploader_test.go

@@ -1,26 +0,0 @@
-package imguploader
-
-import (
-	"context"
-	"testing"
-
-	"github.com/grafana/grafana/pkg/setting"
-	. "github.com/smartystreets/goconvey/convey"
-)
-
-func TestUploadToGCS(t *testing.T) {
-	SkipConvey("[Integration test] for external_image_store.gcs", t, func() {
-		cfg := setting.NewCfg()
-		err := cfg.Load(&setting.CommandLineArgs{
-			HomePath: "../../../",
-		})
-		So(err, ShouldBeNil)
-
-		gcsUploader, _ := NewImageUploader()
-
-		path, err := gcsUploader.Upload(context.Background(), "../../../public/img/logo_transparent_400x.png")
-
-		So(err, ShouldBeNil)
-		So(path, ShouldNotEqual, "")
-	})
-}

pkg/components/imguploader/imguploader.go

@@ -6,13 +6,14 @@ import (
 	"regexp"
 	"time"
 
+	"github.com/grafana/grafana/pkg/components/imguploader/gcs"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
 const (
-	pngExt                         = ".png"
-	defaultSGcsSignedUrlExpiration = 7 * 24 * time.Hour // 7 days
+	pngExt                        = ".png"
+	defaultGCSSignedURLExpiration = 7 * 24 * time.Hour // 7 days
 )
 
 type ImageUploader interface {
@@ -87,9 +88,18 @@ func NewImageUploader() (ImageUploader, error) {
 		bucketName := gcssec.Key("bucket").MustString("")
 		path := gcssec.Key("path").MustString("")
 		enableSignedURLs := gcssec.Key("enable_signed_urls").MustBool(false)
-		signedURLExpiration := gcssec.Key("signed_url_expiration").MustString(defaultSGcsSignedUrlExpiration.String())
+		exp := gcssec.Key("signed_url_expiration").MustString("")
+		var suExp time.Duration
+		if exp != "" {
+			suExp, err = time.ParseDuration(exp)
+			if err != nil {
+				return nil, err
+			}
+		} else {
+			suExp = defaultGCSSignedURLExpiration
+		}
 
-		return NewGCSUploader(keyFile, bucketName, path, enableSignedURLs, signedURLExpiration)
+		return gcs.NewUploader(keyFile, bucketName, path, enableSignedURLs, suExp)
 	case "azure_blob":
 		azureBlobSec, err := setting.Raw.GetSection("external_image_storage.azure_blob")
 		if err != nil {

pkg/components/imguploader/imguploader_test.go

@@ -3,6 +3,7 @@ package imguploader
 import (
 	"testing"
 
+	"github.com/grafana/grafana/pkg/components/imguploader/gcs"
 	"github.com/grafana/grafana/pkg/setting"
 
 	. "github.com/smartystreets/goconvey/convey"
@@ -130,10 +131,10 @@ func TestImageUploaderFactory(t *testing.T) {
 			uploader, err := NewImageUploader()
 			So(err, ShouldBeNil)
 
-			original, ok := uploader.(*GCSUploader)
+			original, ok := uploader.(*gcs.Uploader)
 			So(ok, ShouldBeTrue)
-			So(original.keyFile, ShouldEqual, "/etc/secrets/project-79a52befa3f6.json")
-			So(original.bucket, ShouldEqual, "project-grafana-east")
+			So(original.KeyFile, ShouldEqual, "/etc/secrets/project-79a52befa3f6.json")
+			So(original.Bucket, ShouldEqual, "project-grafana-east")
 		})
 
 		Convey("AzureBlobUploader config", func() {