#45498: externalize fs backend config

pkg/infra/filestorage/api.go

@@ -3,14 +3,21 @@ package filestorage
 import (
 	"context"
 	"errors"
+	"regexp"
 	"strings"
 	"time"
 )
 
-type StorageName string
+type Operation string
 
 const (
-	StorageNamePublic StorageName = "public"
+	OperationGet          Operation = "get"
+	OperationDelete       Operation = "delete"
+	OperationUpsert       Operation = "upsert"
+	OperationListFiles    Operation = "listFiles"
+	OperationListFolders  Operation = "listFolders"
+	OperationCreateFolder Operation = "createFolder"
+	OperationDeleteFolder Operation = "deleteFolder"
 )
 
 var (
@@ -19,15 +26,16 @@ var (
 	ErrPathTooLong           = errors.New("path is too long")
 	ErrPathInvalid           = errors.New("path is invalid")
 	ErrPathEndsWithDelimiter = errors.New("path can not end with delimiter")
+	ErrOperationNotSupported = errors.New("operation not supported")
 	Delimiter                = "/"
+	multipleDelimiters       = regexp.MustCompile(`/+`)
 )
 
 func Join(parts ...string) string {
-	return Delimiter + strings.Join(parts, Delimiter)
+	joinedPath := Delimiter + strings.Join(parts, Delimiter)
-}
 
-func belongsToStorage(path string, storageName StorageName) bool {
+	// makes the API more forgiving for clients without compromising safety
-	return strings.HasPrefix(path, Delimiter+string(storageName))
+	return multipleDelimiters.ReplaceAllString(joinedPath, Delimiter)
 }
 
 type File struct {

pkg/infra/filestorage/api_test.go

@@ -34,42 +34,3 @@ func TestFilestorageApi_Join(t *testing.T) {
 		})
 	}
 }
-
-func TestFilestorageApi_belongToStorage(t *testing.T) {
-	var tests = []struct {
-		name     string
-		path     string
-		storage  StorageName
-		expected bool
-	}{
-		{
-			name:     "should return true if path is prefixed with delimiter and the storage name",
-			path:     "/public/abc/d",
-			storage:  StorageNamePublic,
-			expected: true,
-		},
-		{
-			name:     "should return true if path consists just of the delimiter and the storage name",
-			path:     "/public",
-			storage:  StorageNamePublic,
-			expected: true,
-		},
-		{
-			name:     "should return false if path is not prefixed with delimiter",
-			path:     "public/abc/d",
-			storage:  StorageNamePublic,
-			expected: false,
-		},
-		{
-			name:     "should return false if storage name does not match",
-			path:     "/notpublic/abc/d",
-			storage:  StorageNamePublic,
-			expected: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			require.Equal(t, tt.expected, belongsToStorage(tt.path, tt.storage))
-		})
-	}
-}

pkg/infra/filestorage/cdk_blob_filestorage.go

@@ -22,7 +22,7 @@ type cdkBlobStorage struct {
 	rootFolder string
 }
 
-func NewCdkBlobStorage(log log.Logger, bucket *blob.Bucket, rootFolder string, pathFilters *PathFilters) FileStorage {
+func NewCdkBlobStorage(log log.Logger, bucket *blob.Bucket, rootFolder string, pathFilters *PathFilters, supportedOperations []Operation) FileStorage {
 	return &wrapper{
 		log: log,
 		wrapped: &cdkBlobStorage{
@@ -30,7 +30,8 @@ func NewCdkBlobStorage(log log.Logger, bucket *blob.Bucket, rootFolder string, p
 			bucket:     bucket,
 			rootFolder: rootFolder,
 		},
-		pathFilters: pathFilters,
+		pathFilters:         pathFilters,
+		supportedOperations: supportedOperations,
 	}
 }
 
@@ -313,7 +314,8 @@ func (c cdkBlobStorage) listFolderPaths(ctx context.Context, parentFolderPath st
 
 	recursive := options.Recursive
 
-	currentDirPath := ""
+	dirPath := ""
+	dirMarkerPath := ""
 	foundPaths := make([]string, 0)
 	for {
 		obj, err := iterator.Next(ctx)
@@ -326,16 +328,22 @@ func (c cdkBlobStorage) listFolderPaths(ctx context.Context, parentFolderPath st
 			return nil, err
 		}
 
-		if currentDirPath == "" && !obj.IsDir && options.isAllowed(obj.Key) {
+		if options.isAllowed(obj.Key) {
-			attributes, err := c.bucket.Attributes(ctx, obj.Key)
+			if dirPath == "" {
-			if err != nil {
+				dirPath = getParentFolderPath(obj.Key)
-				c.log.Error("Failed while retrieving attributes", "path", obj.Key, "err", err)
-				return nil, err
 			}
 
-			if attributes.Metadata != nil {
+			if dirMarkerPath == "" && !obj.IsDir {
-				if path, ok := attributes.Metadata[originalPathAttributeKey]; ok {
+				attributes, err := c.bucket.Attributes(ctx, obj.Key)
-					currentDirPath = getParentFolderPath(path)
+				if err != nil {
+					c.log.Error("Failed while retrieving attributes", "path", obj.Key, "err", err)
+					return nil, err
+				}
+
+				if attributes.Metadata != nil {
+					if path, ok := attributes.Metadata[originalPathAttributeKey]; ok {
+						dirMarkerPath = getParentFolderPath(path)
+					}
 				}
 			}
 		}
@@ -354,8 +362,11 @@ func (c cdkBlobStorage) listFolderPaths(ctx context.Context, parentFolderPath st
 		}
 	}
 
-	if currentDirPath != "" {
+	if dirMarkerPath != "" {
-		foundPaths = append(foundPaths, fixPath(currentDirPath))
+		foundPaths = append(foundPaths, fixPath(dirMarkerPath))
+	} else if dirPath != "" {
+		// TODO replicate the changes in `createFolder`
+		foundPaths = append(foundPaths, fixPath(dirPath))
 	}
 	return foundPaths, nil
 }

pkg/infra/filestorage/config.go

@@ -0,0 +1,49 @@
+package filestorage
+
+type BackendType string
+
+const (
+	BackendTypeFS BackendType = "fs"
+	BackendTypeDB BackendType = "db"
+)
+
+type fsBackendConfig struct {
+	RootPath string `json:"path"`
+}
+
+type backendConfig struct {
+	Type                BackendType `json:"type"`
+	Name                string      `json:"name"`
+	AllowedPrefixes     []string    `json:"allowedPrefixes"`     // null -> all paths are allowed
+	SupportedOperations []Operation `json:"supportedOperations"` // null -> all operations are supported
+
+	FSBackendConfig *fsBackendConfig `json:"fsBackendConfig"`
+	// DBBackendConfig *dbBackendConfig
+}
+
+type filestorageConfig struct {
+	Backends []backendConfig `json:"backends"`
+}
+
+func newConfig(staticRootPath string) filestorageConfig {
+	return filestorageConfig{
+		Backends: []backendConfig{
+			{
+				Type: BackendTypeFS,
+				Name: "public",
+				AllowedPrefixes: []string{
+					"testdata/",
+					"img/icons/",
+					"img/bg/",
+					"gazetteer/",
+					"maps/",
+					"upload/",
+				},
+				SupportedOperations: []Operation{
+					OperationListFiles, OperationListFolders,
+				},
+				FSBackendConfig: &fsBackendConfig{RootPath: staticRootPath},
+			},
+		},
+	}
+}

pkg/infra/filestorage/filestorage.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"strings"
 
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -21,67 +20,49 @@ const (
 )
 
 func ProvideService(features featuremgmt.FeatureToggles, cfg *setting.Cfg) (FileStorage, error) {
-	grafanaDsStorageLogger := log.New("grafanaDsStorage")
+	logger := log.New("fileStorageLogger")
-
+
-	path := fmt.Sprintf("file://%s", cfg.StaticRootPath)
+	backendByName := make(map[string]FileStorage)
-	grafanaDsStorageLogger.Info("Initializing grafana ds storage", "path", path)
+	dummyBackend := &wrapper{
-	bucket, err := blob.OpenBucket(context.Background(), path)
+		log:                 logger,
-	if err != nil {
+		wrapped:             &dummyFileStorage{},
-		currentDir, _ := os.Getwd()
+		pathFilters:         &PathFilters{allowedPrefixes: []string{}},
-		grafanaDsStorageLogger.Error("Failed to initialize grafana ds storage", "path", path, "error", err, "cwd", currentDir)
+		supportedOperations: []Operation{},
-		return nil, err
 	}
 
-	prefixes := []string{
+	if !features.IsEnabled(featuremgmt.FlagFileStoreApi) {
-		"testdata/",
+		logger.Info("Filestorage API disabled")
-		"img/icons/",
+		return &service{
-		"img/bg/",
+			backendByName: backendByName,
-		"gazetteer/",
+			dummyBackend:  dummyBackend,
-		"maps/",
+			log:           logger,
-		"upload/",
+		}, nil
-	}
-
-	var grafanaDsStorage FileStorage
-	if features.IsEnabled(featuremgmt.FlagFileStoreApi) {
-		grafanaDsStorage = &wrapper{
-			log: grafanaDsStorageLogger,
-			wrapped: cdkBlobStorage{
-				log:        grafanaDsStorageLogger,
-				bucket:     bucket,
-				rootFolder: "",
-			},
-			pathFilters: &PathFilters{allowedPrefixes: prefixes},
-		}
-	} else {
-		grafanaDsStorage = &dummyFileStorage{}
 	}
 
-	return &service{
+	fsConfig := newConfig(cfg.StaticRootPath)
-		grafanaDsStorage: grafanaDsStorage,
-		log:              log.New("fileStorageService"),
-	}, nil
-}
-
-type service struct {
-	log              log.Logger
-	grafanaDsStorage FileStorage
-}
 
-func (b service) Get(ctx context.Context, path string) (*File, error) {
+	s := &service{
-	var filestorage FileStorage
+		backendByName: backendByName,
-	if belongsToStorage(path, StorageNamePublic) {
+		dummyBackend:  dummyBackend,
-		filestorage = b.grafanaDsStorage
+		log:           logger,
-		path = removeStoragePrefix(path)
 	}
 
-	if err := validatePath(path); err != nil {
+	for _, backend := range fsConfig.Backends {
-		return nil, err
+		if err := s.addBackend(backend); err != nil {
+			return nil, err
+		}
 	}
 
-	return filestorage.Get(ctx, path)
+	return s, nil
 }
 
-func removeStoragePrefix(path string) string {
+type service struct {
+	log           log.Logger
+	dummyBackend  FileStorage
+	backendByName map[string]FileStorage
+}
+
+func removeBackendNamePrefix(path string) string {
 	path = strings.TrimPrefix(path, Delimiter)
 	if path == Delimiter || path == "" {
 		return Delimiter
@@ -103,58 +84,143 @@ func removeStoragePrefix(path string) string {
 	return strings.Join(split, Delimiter)
 }
 
+func (b service) addBackend(backend backendConfig) error {
+	if backend.Type != BackendTypeFS {
+		// TODO add support for DB
+		return nil
+	}
+
+	if backend.FSBackendConfig == nil {
+		return errors.New("Invalid backend configuration " + backend.Name)
+	}
+
+	fsBackendLogger := log.New("fileStorage-" + backend.Name)
+	path := fmt.Sprintf("file://%s", backend.FSBackendConfig.RootPath)
+	bucket, err := blob.OpenBucket(context.Background(), path)
+	if err != nil {
+		return err
+	}
+
+	// TODO mutex
+	if _, ok := b.backendByName[backend.Name]; ok {
+		return errors.New("Duplicate backend name " + backend.Name)
+	}
+
+	pathFilters := &PathFilters{allowedPrefixes: backend.AllowedPrefixes}
+	b.backendByName[backend.Name] = NewCdkBlobStorage(fsBackendLogger, bucket, "", pathFilters, backend.SupportedOperations)
+	return nil
+}
+
+func (b service) validatePath(path string) error {
+	if err := validatePath(path); err != nil {
+		b.log.Error("Path failed validation", "path", path, "error", err)
+		return err
+	}
+	return nil
+}
+
+func (b service) getBackend(path string) (FileStorage, string, string, error) {
+	for backendName, backend := range b.backendByName {
+		if strings.HasPrefix(path, Delimiter+backendName) || backendName == path {
+			backendSpecificPath := removeBackendNamePrefix(path)
+			if err := b.validatePath(backendSpecificPath); err != nil {
+				return nil, "", "", err
+			}
+			return backend, backendSpecificPath, backendName, nil
+		}
+	}
+
+	if err := b.validatePath(path); err != nil {
+		return nil, "", "", err
+	}
+	b.log.Warn("Backend not found", "path", path)
+	return b.dummyBackend, path, "", nil
+}
+
+func (b service) Get(ctx context.Context, path string) (*File, error) {
+	backend, backendSpecificPath, backendName, err := b.getBackend(path)
+	if err != nil {
+		return nil, err
+	}
+
+	file, err := backend.Get(ctx, backendSpecificPath)
+	if file != nil {
+		file.FullPath = Join(backendName, file.FullPath)
+	}
+	return file, err
+}
+
 func (b service) Delete(ctx context.Context, path string) error {
-	return errors.New("not implemented")
+	backend, backendSpecificPath, _, err := b.getBackend(path)
+	if err != nil {
+		return err
+	}
+
+	return backend.Delete(ctx, backendSpecificPath)
 }
 
 func (b service) Upsert(ctx context.Context, file *UpsertFileCommand) error {
-	return errors.New("not implemented")
+	backend, backendSpecificPath, _, err := b.getBackend(file.Path)
+	if err != nil {
+		return err
+	}
+
+	file.Path = backendSpecificPath
+	return backend.Upsert(ctx, file)
 }
 
 func (b service) ListFiles(ctx context.Context, path string, cursor *Paging, options *ListOptions) (*ListFilesResponse, error) {
-	var filestorage FileStorage
+	backend, backendSpecificPath, backendName, err := b.getBackend(path)
-	if belongsToStorage(path, StorageNamePublic) {
+	if err != nil {
-		filestorage = b.grafanaDsStorage
-		path = removeStoragePrefix(path)
-	} else {
-		return nil, errors.New("not implemented")
-	}
-
-	if err := validatePath(path); err != nil {
 		return nil, err
 	}
 
-	return filestorage.ListFiles(ctx, path, cursor, options)
+	resp, err := backend.ListFiles(ctx, backendSpecificPath, cursor, options)
+	if resp != nil && resp.Files != nil {
+		for i := range resp.Files {
+			resp.Files[i].FullPath = Join(backendName, resp.Files[i].FullPath)
+		}
+	}
+	return resp, err
 }
 
 func (b service) ListFolders(ctx context.Context, path string, options *ListOptions) ([]FileMetadata, error) {
-	var filestorage FileStorage
+	backend, backendSpecificPath, backendName, err := b.getBackend(path)
-	if belongsToStorage(path, StorageNamePublic) {
+	if err != nil {
-		filestorage = b.grafanaDsStorage
+		return nil, err
-		path = removeStoragePrefix(path)
-	} else {
-		return nil, errors.New("not implemented")
 	}
 
-	if err := validatePath(path); err != nil {
+	folders, err := backend.ListFolders(ctx, backendSpecificPath, options)
-		return nil, err
+	for i := range folders {
+		folders[i].FullPath = Join(backendName, folders[i].FullPath)
 	}
 
-	return filestorage.ListFolders(ctx, path, options)
+	return folders, err
 }
 
 func (b service) CreateFolder(ctx context.Context, path string) error {
-	return errors.New("not implemented")
+	backend, backendSpecificPath, _, err := b.getBackend(path)
+	if err != nil {
+		return err
+	}
+
+	return backend.CreateFolder(ctx, backendSpecificPath)
 }
 
 func (b service) DeleteFolder(ctx context.Context, path string) error {
-	return errors.New("not implemented")
+	backend, backendSpecificPath, _, err := b.getBackend(path)
-}
+	if err != nil {
+		return err
+	}
 
-func (b service) IsFolderEmpty(ctx context.Context, path string) (bool, error) {
+	return backend.DeleteFolder(ctx, backendSpecificPath)
-	return true, errors.New("not implemented")
 }
 
 func (b service) close() error {
-	return b.grafanaDsStorage.close()
+	var lastError error
+	for _, backend := range b.backendByName {
+		lastError = backend.close()
+	}
+
+	return lastError
 }

pkg/infra/filestorage/filestorage_test.go

@@ -36,11 +36,45 @@ func TestFilestorage_removeStoragePrefix(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(fmt.Sprintf("%s%s", "absolute: ", tt.name), func(t *testing.T) {
-			require.Equal(t, tt.expected, removeStoragePrefix(Delimiter+tt.path))
+			require.Equal(t, tt.expected, removeBackendNamePrefix(Delimiter+tt.path))
 		})
 
 		t.Run(fmt.Sprintf("%s%s", "relative: ", tt.name), func(t *testing.T) {
-			require.Equal(t, tt.expected, removeStoragePrefix(tt.path))
+			require.Equal(t, tt.expected, removeBackendNamePrefix(tt.path))
+		})
+	}
+}
+
+func TestFilestorage_getParentFolderPath(t *testing.T) {
+	var tests = []struct {
+		name     string
+		path     string
+		expected string
+	}{
+		{
+			name:     "should return root if path has a single part - relative, suffix",
+			path:     "ab/",
+			expected: Delimiter,
+		},
+		{
+			name:     "should return root if path has a single part - relative, no suffix",
+			path:     "ab",
+			expected: Delimiter,
+		},
+		{
+			name:     "should return root if path has a single part - abs, no suffix",
+			path:     "/public/",
+			expected: Delimiter,
+		},
+		{
+			name:     "should return root if path has a single part - abs, suffix",
+			path:     "/public/",
+			expected: Delimiter,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf(tt.name), func(t *testing.T) {
+			require.Equal(t, tt.expected, getParentFolderPath(tt.path))
 		})
 	}
 }

pkg/infra/filestorage/fs_integration_test.go

@@ -61,7 +61,7 @@ func runTests(createCases func() []fsTestCase, t *testing.T) {
 	setupInMemFS := func() {
 		commonSetup()
 		bucket, _ := blob.OpenBucket(context.Background(), "mem://")
-		filestorage = NewCdkBlobStorage(testLogger, bucket, Delimiter, nil)
+		filestorage = NewCdkBlobStorage(testLogger, bucket, Delimiter, nil, nil)
 	}
 
 	//setupSqlFS := func() {
@@ -82,7 +82,7 @@ func runTests(createCases func() []fsTestCase, t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		filestorage = NewCdkBlobStorage(testLogger, bucket, "", nil)
+		filestorage = NewCdkBlobStorage(testLogger, bucket, "", nil, nil)
 	}
 
 	backends := []struct {

pkg/infra/filestorage/wrapper.go

@@ -19,9 +19,10 @@ var (
 )
 
 type wrapper struct {
-	log         log.Logger
+	log                 log.Logger
-	wrapped     FileStorage
+	wrapped             FileStorage
-	pathFilters *PathFilters
+	pathFilters         *PathFilters
+	supportedOperations []Operation
 }
 
 var (
@@ -33,6 +34,8 @@ func getParentFolderPath(path string) string {
 		return Delimiter
 	}
 
+	path = strings.TrimSuffix(path, Delimiter)
+
 	if !strings.Contains(path, Delimiter) {
 		return Delimiter
 	}
@@ -91,7 +94,24 @@ func (b wrapper) validatePath(path string) error {
 	return nil
 }
 
+func (b wrapper) assureOperationIsAllowed(operation Operation) error {
+	if b.supportedOperations == nil {
+		return nil
+	}
+
+	for _, allowedOperation := range b.supportedOperations {
+		if allowedOperation == operation {
+			return nil
+		}
+	}
+	return ErrOperationNotSupported
+}
+
 func (b wrapper) Get(ctx context.Context, path string) (*File, error) {
+	if err := b.assureOperationIsAllowed(OperationGet); err != nil {
+		return nil, err
+	}
+
 	if err := b.validatePath(path); err != nil {
 		return nil, err
 	}
@@ -103,6 +123,10 @@ func (b wrapper) Get(ctx context.Context, path string) (*File, error) {
 	return b.wrapped.Get(ctx, path)
 }
 func (b wrapper) Delete(ctx context.Context, path string) error {
+	if err := b.assureOperationIsAllowed(OperationDelete); err != nil {
+		return err
+	}
+
 	if err := b.validatePath(path); err != nil {
 		return err
 	}
@@ -126,6 +150,10 @@ func detectContentType(path string, originalGuess string) string {
 }
 
 func (b wrapper) Upsert(ctx context.Context, file *UpsertFileCommand) error {
+	if err := b.assureOperationIsAllowed(OperationUpsert); err != nil {
+		return err
+	}
+
 	if err := b.validatePath(file.Path); err != nil {
 		return err
 	}
@@ -172,6 +200,10 @@ func (b wrapper) withDefaults(options *ListOptions, folderQuery bool) *ListOptio
 }
 
 func (b wrapper) ListFiles(ctx context.Context, path string, paging *Paging, options *ListOptions) (*ListFilesResponse, error) {
+	if err := b.assureOperationIsAllowed(OperationListFiles); err != nil {
+		return nil, err
+	}
+
 	if err := b.validatePath(path); err != nil {
 		return nil, err
 	}
@@ -188,6 +220,10 @@ func (b wrapper) ListFiles(ctx context.Context, path string, paging *Paging, opt
 }
 
 func (b wrapper) ListFolders(ctx context.Context, path string, options *ListOptions) ([]FileMetadata, error) {
+	if err := b.assureOperationIsAllowed(OperationListFolders); err != nil {
+		return nil, err
+	}
+
 	if err := b.validatePath(path); err != nil {
 		return nil, err
 	}
@@ -196,6 +232,10 @@ func (b wrapper) ListFolders(ctx context.Context, path string, options *ListOpti
 }
 
 func (b wrapper) CreateFolder(ctx context.Context, path string) error {
+	if err := b.assureOperationIsAllowed(OperationCreateFolder); err != nil {
+		return err
+	}
+
 	if err := b.validatePath(path); err != nil {
 		return err
 	}
@@ -208,6 +248,10 @@ func (b wrapper) CreateFolder(ctx context.Context, path string) error {
 }
 
 func (b wrapper) DeleteFolder(ctx context.Context, path string) error {
+	if err := b.assureOperationIsAllowed(OperationDeleteFolder); err != nil {
+		return err
+	}
+
 	if err := b.validatePath(path); err != nil {
 		return err
 	}