diff --git a/pkg/infra/filestorage/api.go b/pkg/infra/filestorage/api.go index b89f42c46bb..4ae5e67bf9e 100644 --- a/pkg/infra/filestorage/api.go +++ b/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 { - return strings.HasPrefix(path, Delimiter+string(storageName)) + // makes the API more forgiving for clients without compromising safety + return multipleDelimiters.ReplaceAllString(joinedPath, Delimiter) } type File struct { diff --git a/pkg/infra/filestorage/api_test.go b/pkg/infra/filestorage/api_test.go index a15d1f5edfe..ca557e3a140 100644 --- a/pkg/infra/filestorage/api_test.go +++ b/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)) - }) - } -} diff --git a/pkg/infra/filestorage/cdk_blob_filestorage.go b/pkg/infra/filestorage/cdk_blob_filestorage.go index 61691e7ea48..6619d1c00aa 100644 --- a/pkg/infra/filestorage/cdk_blob_filestorage.go +++ b/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) { - attributes, err := c.bucket.Attributes(ctx, obj.Key) - if err != nil { - c.log.Error("Failed while retrieving attributes", "path", obj.Key, "err", err) - return nil, err + if options.isAllowed(obj.Key) { + if dirPath == "" { + dirPath = getParentFolderPath(obj.Key) } - if attributes.Metadata != nil { - if path, ok := attributes.Metadata[originalPathAttributeKey]; ok { - currentDirPath = getParentFolderPath(path) + if dirMarkerPath == "" && !obj.IsDir { + attributes, err := c.bucket.Attributes(ctx, obj.Key) + 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 != "" { - foundPaths = append(foundPaths, fixPath(currentDirPath)) + if dirMarkerPath != "" { + foundPaths = append(foundPaths, fixPath(dirMarkerPath)) + } else if dirPath != "" { + // TODO replicate the changes in `createFolder` + foundPaths = append(foundPaths, fixPath(dirPath)) } return foundPaths, nil } diff --git a/pkg/infra/filestorage/config.go b/pkg/infra/filestorage/config.go new file mode 100644 index 00000000000..ee75a1e70c6 --- /dev/null +++ b/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}, + }, + }, + } +} diff --git a/pkg/infra/filestorage/filestorage.go b/pkg/infra/filestorage/filestorage.go index 5c9e1ae952f..ed1a70049cb 100644 --- a/pkg/infra/filestorage/filestorage.go +++ b/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") - - path := fmt.Sprintf("file://%s", cfg.StaticRootPath) - grafanaDsStorageLogger.Info("Initializing grafana ds storage", "path", path) - bucket, err := blob.OpenBucket(context.Background(), path) - if err != nil { - currentDir, _ := os.Getwd() - grafanaDsStorageLogger.Error("Failed to initialize grafana ds storage", "path", path, "error", err, "cwd", currentDir) - return nil, err + logger := log.New("fileStorageLogger") + + backendByName := make(map[string]FileStorage) + dummyBackend := &wrapper{ + log: logger, + wrapped: &dummyFileStorage{}, + pathFilters: &PathFilters{allowedPrefixes: []string{}}, + supportedOperations: []Operation{}, } - prefixes := []string{ - "testdata/", - "img/icons/", - "img/bg/", - "gazetteer/", - "maps/", - "upload/", - } - - 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{} + if !features.IsEnabled(featuremgmt.FlagFileStoreApi) { + logger.Info("Filestorage API disabled") + return &service{ + backendByName: backendByName, + dummyBackend: dummyBackend, + log: logger, + }, nil } - return &service{ - grafanaDsStorage: grafanaDsStorage, - log: log.New("fileStorageService"), - }, nil -} - -type service struct { - log log.Logger - grafanaDsStorage FileStorage -} + fsConfig := newConfig(cfg.StaticRootPath) -func (b service) Get(ctx context.Context, path string) (*File, error) { - var filestorage FileStorage - if belongsToStorage(path, StorageNamePublic) { - filestorage = b.grafanaDsStorage - path = removeStoragePrefix(path) + s := &service{ + backendByName: backendByName, + dummyBackend: dummyBackend, + log: logger, } - if err := validatePath(path); err != nil { - return nil, err + for _, backend := range fsConfig.Backends { + 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 - if belongsToStorage(path, StorageNamePublic) { - filestorage = b.grafanaDsStorage - path = removeStoragePrefix(path) - } else { - return nil, errors.New("not implemented") - } - - if err := validatePath(path); err != nil { + backend, backendSpecificPath, backendName, err := b.getBackend(path) + if 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 - if belongsToStorage(path, StorageNamePublic) { - filestorage = b.grafanaDsStorage - path = removeStoragePrefix(path) - } else { - return nil, errors.New("not implemented") + backend, backendSpecificPath, backendName, err := b.getBackend(path) + if err != nil { + return nil, err } - if err := validatePath(path); err != nil { - return nil, err + folders, err := backend.ListFolders(ctx, backendSpecificPath, options) + 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 true, errors.New("not implemented") + return backend.DeleteFolder(ctx, backendSpecificPath) } func (b service) close() error { - return b.grafanaDsStorage.close() + var lastError error + for _, backend := range b.backendByName { + lastError = backend.close() + } + + return lastError } diff --git a/pkg/infra/filestorage/filestorage_test.go b/pkg/infra/filestorage/filestorage_test.go index f432c849195..32ab2fb3c27 100644 --- a/pkg/infra/filestorage/filestorage_test.go +++ b/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)) }) } } diff --git a/pkg/infra/filestorage/fs_integration_test.go b/pkg/infra/filestorage/fs_integration_test.go index 2440ad34463..fc88bb49098 100644 --- a/pkg/infra/filestorage/fs_integration_test.go +++ b/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 { diff --git a/pkg/infra/filestorage/wrapper.go b/pkg/infra/filestorage/wrapper.go index 10e769bc95a..69f45adf116 100644 --- a/pkg/infra/filestorage/wrapper.go +++ b/pkg/infra/filestorage/wrapper.go @@ -19,9 +19,10 @@ var ( ) type wrapper struct { - log log.Logger - wrapped FileStorage - pathFilters *PathFilters + log log.Logger + wrapped FileStorage + 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 }