From 5da8a84328a4d45c124d28c50dd71af111a7ca8c Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 26 Oct 2020 01:40:46 +0000 Subject: [PATCH] Fix Storage mapping (#13297) (#13307) * Fix Storage mapping (#13297) Backport #13297 This PR fixes several bugs in setting storage * The default STORAGE_TYPE should be the provided type. * The Storage config should be passed in to NewStorage as a pointer - otherwise the Mappable interface function MapTo will not be found * There was a bug in the MapTo function. Fix #13286 Signed-off-by: Andrew Thornton * add missing changes from backport #13164 Signed-off-by: Andrew Thornton Co-authored-by: Lunny Xiao --- modules/setting/storage.go | 4 ++-- modules/storage/local.go | 3 ++- modules/storage/minio.go | 26 ++++++++++++++++---------- modules/storage/storage.go | 13 +++++++++---- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index ab0598ccf8..e743d6c20c 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -21,7 +21,7 @@ type Storage struct { // MapTo implements the Mappable interface func (s *Storage) MapTo(v interface{}) error { - pathValue := reflect.ValueOf(v).FieldByName("Path") + pathValue := reflect.ValueOf(v).Elem().FieldByName("Path") if pathValue.IsValid() && pathValue.Kind() == reflect.String { pathValue.SetString(s.Path) } @@ -46,7 +46,7 @@ func getStorage(name, typ string, overrides ...*ini.Section) Storage { var storage Storage - storage.Type = sec.Key("STORAGE_TYPE").MustString("") + storage.Type = sec.Key("STORAGE_TYPE").MustString(typ) storage.ServeDirect = sec.Key("SERVE_DIRECT").MustBool(false) // Global Defaults diff --git a/modules/storage/local.go b/modules/storage/local.go index e270a40b77..f7ffb2ddc1 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" ) @@ -39,7 +40,7 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error return nil, err } config := configInterface.(LocalStorageConfig) - + log.Info("Creating new Local Storage at %s", config.Path) if err := os.MkdirAll(config.Path, os.ModePerm); err != nil { return nil, err } diff --git a/modules/storage/minio.go b/modules/storage/minio.go index b65af8ddbd..092b3c93c3 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -13,6 +13,7 @@ import ( "strings" "time" + "code.gitea.io/gitea/modules/log" "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" ) @@ -82,16 +83,17 @@ func convertMinioErr(err error) error { func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) { configInterface, err := toConfig(MinioStorageConfig{}, cfg) if err != nil { - return nil, err + return nil, convertMinioErr(err) } config := configInterface.(MinioStorageConfig) + log.Info("Creating Minio storage at %s:%s with base path %s", config.Endpoint, config.Bucket, config.BasePath) minioClient, err := minio.New(config.Endpoint, &minio.Options{ Creds: credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, ""), Secure: config.UseSSL, }) if err != nil { - return nil, err + return nil, convertMinioErr(err) } if err := minioClient.MakeBucket(ctx, config.Bucket, minio.MakeBucketOptions{ @@ -100,7 +102,7 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error // Check to see if we already own this bucket (which happens if you run this twice) exists, errBucketExists := minioClient.BucketExists(ctx, config.Bucket) if !exists || errBucketExists != nil { - return nil, err + return nil, convertMinioErr(err) } } @@ -121,7 +123,7 @@ func (m *MinioStorage) Open(path string) (Object, error) { var opts = minio.GetObjectOptions{} object, err := m.client.GetObject(m.ctx, m.bucket, m.buildMinioPath(path), opts) if err != nil { - return nil, err + return nil, convertMinioErr(err) } return &minioObject{object}, nil } @@ -137,7 +139,7 @@ func (m *MinioStorage) Save(path string, r io.Reader) (int64, error) { minio.PutObjectOptions{ContentType: "application/octet-stream"}, ) if err != nil { - return 0, err + return 0, convertMinioErr(err) } return uploadInfo.Size, nil } @@ -184,14 +186,17 @@ func (m *MinioStorage) Stat(path string) (os.FileInfo, error) { return nil, os.ErrNotExist } } - return nil, err + return nil, convertMinioErr(err) } return &minioFileInfo{info}, nil } // Delete delete a file func (m *MinioStorage) Delete(path string) error { - return m.client.RemoveObject(m.ctx, m.bucket, m.buildMinioPath(path), minio.RemoveObjectOptions{}) + if err := m.client.RemoveObject(m.ctx, m.bucket, m.buildMinioPath(path), minio.RemoveObjectOptions{}); err != nil { + return convertMinioErr(err) + } + return nil } // URL gets the redirect URL to a file. The presigned link is valid for 5 minutes. @@ -199,7 +204,8 @@ func (m *MinioStorage) URL(path, name string) (*url.URL, error) { reqParams := make(url.Values) // TODO it may be good to embed images with 'inline' like ServeData does, but we don't want to have to read the file, do we? reqParams.Set("response-content-disposition", "attachment; filename=\""+quoteEscaper.Replace(name)+"\"") - return m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams) + u, err := m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams) + return u, convertMinioErr(err) } // IterateObjects iterates across the objects in the miniostorage @@ -213,13 +219,13 @@ func (m *MinioStorage) IterateObjects(fn func(path string, obj Object) error) er }) { object, err := m.client.GetObject(lobjectCtx, m.bucket, mObjInfo.Key, opts) if err != nil { - return err + return convertMinioErr(err) } if err := func(object *minio.Object, fn func(path string, obj Object) error) error { defer object.Close() return fn(strings.TrimPrefix(m.basePath, mObjInfo.Key), &minioObject{object}) }(object, fn); err != nil { - return err + return convertMinioErr(err) } } return nil diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 1fa04119c7..ec3a1c14a1 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -12,6 +12,7 @@ import ( "net/url" "os" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" ) @@ -141,21 +142,25 @@ func NewStorage(typStr string, cfg interface{}) (ObjectStorage, error) { } func initAvatars() (err error) { - Avatars, err = NewStorage(setting.Avatar.Storage.Type, setting.Avatar.Storage) + log.Info("Initialising Avatar storage with type: %s", setting.Avatar.Storage.Type) + Avatars, err = NewStorage(setting.Avatar.Storage.Type, &setting.Avatar.Storage) return } func initAttachments() (err error) { - Attachments, err = NewStorage(setting.Attachment.Storage.Type, setting.Attachment.Storage) + log.Info("Initialising Attachment storage with type: %s", setting.Attachment.Storage.Type) + Attachments, err = NewStorage(setting.Attachment.Storage.Type, &setting.Attachment.Storage) return } func initLFS() (err error) { - LFS, err = NewStorage(setting.LFS.Storage.Type, setting.LFS.Storage) + log.Info("Initialising LFS storage with type: %s", setting.LFS.Storage.Type) + LFS, err = NewStorage(setting.LFS.Storage.Type, &setting.LFS.Storage) return } func initRepoAvatars() (err error) { - RepoAvatars, err = NewStorage(setting.RepoAvatar.Storage.Type, setting.RepoAvatar.Storage) + log.Info("Initialising Repository Avatar storage with type: %s", setting.RepoAvatar.Storage.Type) + RepoAvatars, err = NewStorage(setting.RepoAvatar.Storage.Type, &setting.RepoAvatar.Storage) return }