Provide self-registering storage system (#12978)

* Provide self-registering storage system

Signed-off-by: Andrew Thornton <art27@cantab.net>

* More simplification

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Remove old strings from setting

Signed-off-by: Andrew Thornton <art27@cantab.net>

* oops attachments not attachment

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

cmd/migrate_storage.go

@@ -32,8 +32,8 @@ var CmdMigrateStorage = cli.Command{
 		},
 		cli.StringFlag{
 			Name:  "storage, s",
-			Value: setting.LocalStorageType,
+			Value: "",
-			Usage: "New storage type, local or minio",
+			Usage: "New storage type: local (default) or minio",
 		},
 		cli.StringFlag{
 			Name:  "path, p",
@@ -107,6 +107,8 @@ func runMigrateStorage(ctx *cli.Context) error {
 		return err
 	}
 
+	goCtx := context.Background()
+
 	if err := storage.Init(); err != nil {
 		return err
 	}
@@ -114,24 +116,31 @@ func runMigrateStorage(ctx *cli.Context) error {
 	var dstStorage storage.ObjectStorage
 	var err error
 	switch strings.ToLower(ctx.String("storage")) {
-	case setting.LocalStorageType:
+	case "":
+		fallthrough
+	case string(storage.LocalStorageType):
 		p := ctx.String("path")
 		if p == "" {
 			log.Fatal("Path must be given when storage is loal")
 			return nil
 		}
-		dstStorage, err = storage.NewLocalStorage(p)
+		dstStorage, err = storage.NewLocalStorage(
-	case setting.MinioStorageType:
+			goCtx,
+			storage.LocalStorageConfig{
+				Path: p,
+			})
+	case string(storage.MinioStorageType):
 		dstStorage, err = storage.NewMinioStorage(
-			context.Background(),
+			goCtx,
-			ctx.String("minio-endpoint"),
+			storage.MinioStorageConfig{
-			ctx.String("minio-access-key-id"),
+				Endpoint:        ctx.String("minio-endpoint"),
-			ctx.String("minio-secret-access-key"),
+				AccessKeyID:     ctx.String("minio-access-key-id"),
-			ctx.String("minio-bucket"),
+				SecretAccessKey: ctx.String("minio-secret-access-key"),
-			ctx.String("minio-location"),
+				Bucket:          ctx.String("minio-bucket"),
-			ctx.String("minio-base-path"),
+				Location:        ctx.String("minio-location"),
-			ctx.Bool("minio-use-ssl"),
+				BasePath:        ctx.String("minio-base-path"),
-		)
+				UseSSL:          ctx.Bool("minio-use-ssl"),
+			})
 	default:
 		return fmt.Errorf("Unsupported attachments storage type: %s", ctx.String("storage"))
 	}

models/unit_tests.go

@@ -67,10 +67,8 @@ func MainTest(m *testing.M, pathToGiteaRoot string) {
 	if err != nil {
 		fatalTestError("url.Parse: %v\n", err)
 	}
-	setting.Attachment.Storage.Type = setting.LocalStorageType
 	setting.Attachment.Storage.Path = filepath.Join(setting.AppDataPath, "attachments")
 
-	setting.LFS.Storage.Type = setting.LocalStorageType
 	setting.LFS.Storage.Path = filepath.Join(setting.AppDataPath, "lfs")
 	if err = storage.Init(); err != nil {
 		fatalTestError("storage.Init: %v\n", err)

modules/setting/attachment.go

@@ -4,12 +4,6 @@
 
 package setting
 
-import (
-	"path/filepath"
-
-	"code.gitea.io/gitea/modules/log"
-)
-
 var (
 	// Attachment settings
 	Attachment = struct {
@@ -20,7 +14,6 @@ var (
 		Enabled      bool
 	}{
 		Storage: Storage{
-			Type:        LocalStorageType,
 			ServeDirect: false,
 		},
 		AllowedTypes: "image/jpeg,image/png,application/zip,application/gzip",
@@ -32,37 +25,9 @@ var (
 
 func newAttachmentService() {
 	sec := Cfg.Section("attachment")
-	Attachment.Storage.Type = sec.Key("STORAGE_TYPE").MustString("")
+	storageType := sec.Key("STORAGE_TYPE").MustString("")
-	if Attachment.Storage.Type == "" {
-		Attachment.Storage.Type = "default"
-	}
 
-	if Attachment.Storage.Type != LocalStorageType && Attachment.Storage.Type != MinioStorageType {
+	Attachment.Storage = getStorage("attachments", storageType, sec)
-		storage, ok := storages[Attachment.Storage.Type]
-		if !ok {
-			log.Fatal("Failed to get attachment storage type: %s", Attachment.Storage.Type)
-		}
-		Attachment.Storage = storage
-	}
-
-	// Override
-	Attachment.ServeDirect = sec.Key("SERVE_DIRECT").MustBool(Attachment.ServeDirect)
-
-	switch Attachment.Storage.Type {
-	case LocalStorageType:
-		Attachment.Path = sec.Key("PATH").MustString(filepath.Join(AppDataPath, "attachments"))
-		if !filepath.IsAbs(Attachment.Path) {
-			Attachment.Path = filepath.Join(AppWorkPath, Attachment.Path)
-		}
-	case MinioStorageType:
-		Attachment.Minio.Endpoint = sec.Key("MINIO_ENDPOINT").MustString(Attachment.Minio.Endpoint)
-		Attachment.Minio.AccessKeyID = sec.Key("MINIO_ACCESS_KEY_ID").MustString(Attachment.Minio.AccessKeyID)
-		Attachment.Minio.SecretAccessKey = sec.Key("MINIO_SECRET_ACCESS_KEY").MustString(Attachment.Minio.SecretAccessKey)
-		Attachment.Minio.Bucket = sec.Key("MINIO_BUCKET").MustString(Attachment.Minio.Bucket)
-		Attachment.Minio.Location = sec.Key("MINIO_LOCATION").MustString(Attachment.Minio.Location)
-		Attachment.Minio.UseSSL = sec.Key("MINIO_USE_SSL").MustBool(Attachment.Minio.UseSSL)
-		Attachment.Minio.BasePath = sec.Key("MINIO_BASE_PATH").MustString("attachments/")
-	}
 
 	Attachment.AllowedTypes = sec.Key("ALLOWED_TYPES").MustString(".docx,.gif,.gz,.jpeg,.jpg,.log,.pdf,.png,.pptx,.txt,.xlsx,.zip")
 	Attachment.MaxSize = sec.Key("MAX_SIZE").MustInt64(4)

modules/setting/lfs.go

@@ -37,40 +37,15 @@ func newLFSService() {
 	}
 
 	lfsSec := Cfg.Section("lfs")
-	LFS.Storage.Type = lfsSec.Key("STORAGE_TYPE").MustString("")
+	storageType := lfsSec.Key("STORAGE_TYPE").MustString("")
-	if LFS.Storage.Type == "" {
-		LFS.Storage.Type = "default"
-	}
 
-	if LFS.Storage.Type != LocalStorageType && LFS.Storage.Type != MinioStorageType {
+	// Specifically default PATH to LFS_CONTENT_PATH
-		storage, ok := storages[LFS.Storage.Type]
+	lfsSec.Key("PATH").MustString(
-		if !ok {
+		sec.Key("LFS_CONTENT_PATH").String())
-			log.Fatal("Failed to get lfs storage type: %s", LFS.Storage.Type)
-		}
-		LFS.Storage = storage
-	}
 
-	// Override
+	LFS.Storage = getStorage("lfs", storageType, lfsSec)
-	LFS.ServeDirect = lfsSec.Key("SERVE_DIRECT").MustBool(LFS.ServeDirect)
-	switch LFS.Storage.Type {
-	case LocalStorageType:
-		// keep compatible
-		LFS.Path = sec.Key("LFS_CONTENT_PATH").MustString(filepath.Join(AppDataPath, "lfs"))
-		LFS.Path = lfsSec.Key("PATH").MustString(LFS.Path)
-		if !filepath.IsAbs(LFS.Path) {
-			LFS.Path = filepath.Join(AppWorkPath, LFS.Path)
-		}
-
-	case MinioStorageType:
-		LFS.Minio.Endpoint = lfsSec.Key("MINIO_ENDPOINT").MustString(LFS.Minio.Endpoint)
-		LFS.Minio.AccessKeyID = lfsSec.Key("MINIO_ACCESS_KEY_ID").MustString(LFS.Minio.AccessKeyID)
-		LFS.Minio.SecretAccessKey = lfsSec.Key("MINIO_SECRET_ACCESS_KEY").MustString(LFS.Minio.SecretAccessKey)
-		LFS.Minio.Bucket = lfsSec.Key("MINIO_BUCKET").MustString(LFS.Minio.Bucket)
-		LFS.Minio.Location = lfsSec.Key("MINIO_LOCATION").MustString(LFS.Minio.Location)
-		LFS.Minio.UseSSL = lfsSec.Key("MINIO_USE_SSL").MustBool(LFS.Minio.UseSSL)
-		LFS.Minio.BasePath = lfsSec.Key("MINIO_BASE_PATH").MustString("lfs/")
-	}
 
+	// Rest of LFS service settings
 	if LFS.LocksPagingNum == 0 {
 		LFS.LocksPagingNum = 50
 	}

modules/setting/setting.go

@@ -804,7 +804,6 @@ func NewContext() {
 		}
 	}
 
-	newStorageService()
 	newAttachmentService()
 	newLFSService()
 

modules/setting/storage.go

@@ -5,65 +5,77 @@
 package setting
 
 import (
-	"strings"
+	"path/filepath"
+	"reflect"
 
-	"code.gitea.io/gitea/modules/log"
 	ini "gopkg.in/ini.v1"
 )
 
-// enumerate all storage types
-const (
-	LocalStorageType = "local"
-	MinioStorageType = "minio"
-)
-
 // Storage represents configuration of storages
 type Storage struct {
 	Type        string
 	Path        string
+	Section     *ini.Section
 	ServeDirect bool
-	Minio       struct {
+}
-		Endpoint        string
+
-		AccessKeyID     string
+// MapTo implements the Mappable interface
-		SecretAccessKey string
+func (s *Storage) MapTo(v interface{}) error {
-		UseSSL          bool
+	pathValue := reflect.ValueOf(v).FieldByName("Path")
-		Bucket          string
+	if pathValue.IsValid() && pathValue.Kind() == reflect.String {
-		Location        string
+		pathValue.SetString(s.Path)
-		BasePath        string
+	}
+	if s.Section != nil {
+		return s.Section.MapTo(v)
+	}
+	return nil
+}
+
+func getStorage(name, typ string, overrides ...*ini.Section) Storage {
+	sectionName := "storage"
+	if len(name) > 0 {
+		sectionName = sectionName + "." + typ
+	}
+	sec := Cfg.Section(sectionName)
+
+	if len(overrides) == 0 {
+		overrides = []*ini.Section{
+			Cfg.Section(sectionName + "." + name),
 		}
 	}
 
-var (
-	storages = make(map[string]Storage)
-)
-
-func getStorage(sec *ini.Section) Storage {
 	var storage Storage
-	storage.Type = sec.Key("STORAGE_TYPE").MustString(LocalStorageType)
+
+	storage.Type = sec.Key("STORAGE_TYPE").MustString("")
 	storage.ServeDirect = sec.Key("SERVE_DIRECT").MustBool(false)
-	switch storage.Type {
+
-	case LocalStorageType:
+	// Global Defaults
-	case MinioStorageType:
+	sec.Key("MINIO_ENDPOINT").MustString("localhost:9000")
-		storage.Minio.Endpoint = sec.Key("MINIO_ENDPOINT").MustString("localhost:9000")
+	sec.Key("MINIO_ACCESS_KEY_ID").MustString("")
-		storage.Minio.AccessKeyID = sec.Key("MINIO_ACCESS_KEY_ID").MustString("")
+	sec.Key("MINIO_SECRET_ACCESS_KEY").MustString("")
-		storage.Minio.SecretAccessKey = sec.Key("MINIO_SECRET_ACCESS_KEY").MustString("")
+	sec.Key("MINIO_BUCKET").MustString("gitea")
-		storage.Minio.Bucket = sec.Key("MINIO_BUCKET").MustString("gitea")
+	sec.Key("MINIO_LOCATION").MustString("us-east-1")
-		storage.Minio.Location = sec.Key("MINIO_LOCATION").MustString("us-east-1")
+	sec.Key("MINIO_USE_SSL").MustBool(false)
-		storage.Minio.UseSSL = sec.Key("MINIO_USE_SSL").MustBool(false)
+
+	storage.Section = sec
+
+	for _, override := range overrides {
+		for _, key := range storage.Section.Keys() {
+			if !override.HasKey(key.Name()) {
+				_, _ = override.NewKey(key.Name(), key.Value())
 			}
+		}
+		storage.ServeDirect = override.Key("SERVE_DIRECT").MustBool(false)
+		storage.Section = override
+	}
+
+	// Specific defaults
+	storage.Path = storage.Section.Key("PATH").MustString(filepath.Join(AppDataPath, name))
+	if !filepath.IsAbs(storage.Path) {
+		storage.Path = filepath.Join(AppWorkPath, storage.Path)
+		storage.Section.Key("PATH").SetValue(storage.Path)
+	}
+	storage.Section.Key("MINIO_BASE_PATH").MustString(name + "/")
+
 	return storage
 }
-
-func newStorageService() {
-	sec := Cfg.Section("storage")
-	storages["default"] = getStorage(sec)
-
-	for _, sec := range Cfg.Section("storage").ChildSections() {
-		name := strings.TrimPrefix(sec.Name(), "storage.")
-		if name == "default" || name == LocalStorageType || name == MinioStorageType {
-			log.Error("storage name %s is system reserved!", name)
-			continue
-		}
-		storages[name] = getStorage(sec)
-	}
-}

modules/storage/helper.go

@@ -0,0 +1,65 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package storage
+
+import (
+	"encoding/json"
+	"reflect"
+)
+
+// Mappable represents an interface that can MapTo another interface
+type Mappable interface {
+	MapTo(v interface{}) error
+}
+
+// toConfig will attempt to convert a given configuration cfg into the provided exemplar type.
+//
+// It will tolerate the cfg being passed as a []byte or string of a json representation of the
+// exemplar or the correct type of the exemplar itself
+func toConfig(exemplar, cfg interface{}) (interface{}, error) {
+
+	// First of all check if we've got the same type as the exemplar - if so it's all fine.
+	if reflect.TypeOf(cfg).AssignableTo(reflect.TypeOf(exemplar)) {
+		return cfg, nil
+	}
+
+	// Now if not - does it provide a MapTo function we can try?
+	if mappable, ok := cfg.(Mappable); ok {
+		newVal := reflect.New(reflect.TypeOf(exemplar))
+		if err := mappable.MapTo(newVal.Interface()); err == nil {
+			return newVal.Elem().Interface(), nil
+		}
+		// MapTo has failed us ... let's try the json route ...
+	}
+
+	// OK we've been passed a byte array right?
+	configBytes, ok := cfg.([]byte)
+	if !ok {
+		// oh ... it's a string then?
+		var configStr string
+
+		configStr, ok = cfg.(string)
+		configBytes = []byte(configStr)
+	}
+	if !ok {
+		// hmm ... can we marshal it to json?
+		var err error
+
+		configBytes, err = json.Marshal(cfg)
+		ok = (err == nil)
+	}
+	if !ok {
+		// no ... we've tried hard enough at this point - throw an error!
+		return nil, ErrInvalidConfiguration{cfg: cfg}
+	}
+
+	// OK unmarshal the byte array into a new copy of the exemplar
+	newVal := reflect.New(reflect.TypeOf(exemplar))
+	if err := json.Unmarshal(configBytes, newVal.Interface()); err != nil {
+		// If we can't unmarshal it then return an error!
+		return nil, ErrInvalidConfiguration{cfg: cfg, err: err}
+	}
+	return newVal.Elem().Interface(), nil
+}

modules/storage/local.go

@@ -5,6 +5,7 @@
 package storage
 
 import (
+	"context"
 	"io"
 	"net/url"
 	"os"
@@ -17,19 +18,35 @@ var (
 	_ ObjectStorage = &LocalStorage{}
 )
 
+// LocalStorageType is the type descriptor for local storage
+const LocalStorageType Type = "local"
+
+// LocalStorageConfig represents the configuration for a local storage
+type LocalStorageConfig struct {
+	Path string `ini:"PATH"`
+}
+
 // LocalStorage represents a local files storage
 type LocalStorage struct {
+	ctx context.Context
 	dir string
 }
 
 // NewLocalStorage returns a local files
-func NewLocalStorage(bucket string) (*LocalStorage, error) {
+func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) {
-	if err := os.MkdirAll(bucket, os.ModePerm); err != nil {
+	configInterface, err := toConfig(LocalStorageConfig{}, cfg)
+	if err != nil {
+		return nil, err
+	}
+	config := configInterface.(LocalStorageConfig)
+
+	if err := os.MkdirAll(config.Path, os.ModePerm); err != nil {
 		return nil, err
 	}
 
 	return &LocalStorage{
-		dir: bucket,
+		ctx: ctx,
+		dir: config.Path,
 	}, nil
 }
 
@@ -80,6 +97,11 @@ func (l *LocalStorage) IterateObjects(fn func(path string, obj Object) error) er
 		if err != nil {
 			return err
 		}
+		select {
+		case <-l.ctx.Done():
+			return l.ctx.Err()
+		default:
+		}
 		if path == l.dir {
 			return nil
 		}
@@ -98,3 +120,7 @@ func (l *LocalStorage) IterateObjects(fn func(path string, obj Object) error) er
 		return fn(relPath, obj)
 	})
 }
+
+func init() {
+	RegisterStorageType(LocalStorageType, NewLocalStorage)
+}

modules/storage/minio.go

@@ -19,6 +19,7 @@ import (
 
 var (
 	_ ObjectStorage = &MinioStorage{}
+
 	quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"")
 )
 
@@ -35,6 +36,20 @@ func (m *minioObject) Stat() (os.FileInfo, error) {
 	return &minioFileInfo{oi}, nil
 }
 
+// MinioStorageType is the type descriptor for minio storage
+const MinioStorageType Type = "minio"
+
+// MinioStorageConfig represents the configuration for a minio storage
+type MinioStorageConfig struct {
+	Endpoint        string `ini:"MINIO_ENDPOINT"`
+	AccessKeyID     string `ini:"MINIO_ACCESS_KEY_ID"`
+	SecretAccessKey string `ini:"MINIO_SECRET_ACCESS_KEY"`
+	Bucket          string `ini:"MINIO_BUCKET"`
+	Location        string `ini:"MINIO_LOCATION"`
+	BasePath        string `ini:"MINIO_BASE_PATH"`
+	UseSSL          bool   `ini:"MINIO_USE_SSL"`
+}
+
 // MinioStorage returns a minio bucket storage
 type MinioStorage struct {
 	ctx      context.Context
@@ -44,20 +59,26 @@ type MinioStorage struct {
 }
 
 // NewMinioStorage returns a minio storage
-func NewMinioStorage(ctx context.Context, endpoint, accessKeyID, secretAccessKey, bucket, location, basePath string, useSSL bool) (*MinioStorage, error) {
+func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) {
-	minioClient, err := minio.New(endpoint, &minio.Options{
+	configInterface, err := toConfig(MinioStorageConfig{}, cfg)
-		Creds:  credentials.NewStaticV4(accessKeyID, secretAccessKey, ""),
+	if err != nil {
-		Secure: useSSL,
+		return nil, err
+	}
+	config := configInterface.(MinioStorageConfig)
+
+	minioClient, err := minio.New(config.Endpoint, &minio.Options{
+		Creds:  credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, ""),
+		Secure: config.UseSSL,
 	})
 	if err != nil {
 		return nil, err
 	}
 
-	if err := minioClient.MakeBucket(ctx, bucket, minio.MakeBucketOptions{
+	if err := minioClient.MakeBucket(ctx, config.Bucket, minio.MakeBucketOptions{
-		Region: location,
+		Region: config.Location,
 	}); err != nil {
 		// Check to see if we already own this bucket (which happens if you run this twice)
-		exists, errBucketExists := minioClient.BucketExists(ctx, bucket)
+		exists, errBucketExists := minioClient.BucketExists(ctx, config.Bucket)
 		if !exists || errBucketExists != nil {
 			return nil, err
 		}
@@ -66,8 +87,8 @@ func NewMinioStorage(ctx context.Context, endpoint, accessKeyID, secretAccessKey
 	return &MinioStorage{
 		ctx:      ctx,
 		client:   minioClient,
-		bucket:   bucket,
+		bucket:   config.Bucket,
-		basePath: basePath,
+		basePath: config.BasePath,
 	}, nil
 }
 
@@ -183,3 +204,7 @@ func (m *MinioStorage) IterateObjects(fn func(path string, obj Object) error) er
 	}
 	return nil
 }
+
+func init() {
+	RegisterStorageType(MinioStorageType, NewMinioStorage)
+}

modules/storage/storage.go

@@ -22,6 +22,38 @@ var (
 	ErrIterateObjectsNotSupported = errors.New("iterateObjects method not supported")
 )
 
+// ErrInvalidConfiguration is called when there is invalid configuration for a storage
+type ErrInvalidConfiguration struct {
+	cfg interface{}
+	err error
+}
+
+func (err ErrInvalidConfiguration) Error() string {
+	if err.err != nil {
+		return fmt.Sprintf("Invalid Configuration Argument: %v: Error: %v", err.cfg, err.err)
+	}
+	return fmt.Sprintf("Invalid Configuration Argument: %v", err.cfg)
+}
+
+// IsErrInvalidConfiguration checks if an error is an ErrInvalidConfiguration
+func IsErrInvalidConfiguration(err error) bool {
+	_, ok := err.(ErrInvalidConfiguration)
+	return ok
+}
+
+// Type is a type of Storage
+type Type string
+
+// NewStorageFunc is a function that creates a storage
+type NewStorageFunc func(ctx context.Context, cfg interface{}) (ObjectStorage, error)
+
+var storageMap = map[Type]NewStorageFunc{}
+
+// RegisterStorageType registers a provided storage type with a function to create it
+func RegisterStorageType(typ Type, fn func(ctx context.Context, cfg interface{}) (ObjectStorage, error)) {
+	storageMap[typ] = fn
+}
+
 // Object represents the object on the storage
 type Object interface {
 	io.ReadCloser
@@ -67,41 +99,25 @@ func Init() error {
 	return initLFS()
 }
 
-func initStorage(storageCfg setting.Storage) (ObjectStorage, error) {
+// NewStorage takes a storage type and some config and returns an ObjectStorage or an error
-	var err error
+func NewStorage(typStr string, cfg interface{}) (ObjectStorage, error) {
-	var s ObjectStorage
+	if len(typStr) == 0 {
-	switch storageCfg.Type {
+		typStr = string(LocalStorageType)
-	case setting.LocalStorageType:
+	}
-		s, err = NewLocalStorage(storageCfg.Path)
+	fn, ok := storageMap[Type(typStr)]
-	case setting.MinioStorageType:
+	if !ok {
-		minio := storageCfg.Minio
+		return nil, fmt.Errorf("Unsupported storage type: %s", typStr)
-		s, err = NewMinioStorage(
-			context.Background(),
-			minio.Endpoint,
-			minio.AccessKeyID,
-			minio.SecretAccessKey,
-			minio.Bucket,
-			minio.Location,
-			minio.BasePath,
-			minio.UseSSL,
-		)
-	default:
-		return nil, fmt.Errorf("Unsupported attachment store type: %s", storageCfg.Type)
 	}
 
-	if err != nil {
+	return fn(context.Background(), cfg)
-		return nil, err
-	}
-
-	return s, nil
 }
 
 func initAttachments() (err error) {
-	Attachments, err = initStorage(setting.Attachment.Storage)
+	Attachments, err = NewStorage(setting.Attachment.Storage.Type, setting.Attachment.Storage)
 	return
 }
 
 func initLFS() (err error) {
-	LFS, err = initStorage(setting.LFS.Storage)
+	LFS, err = NewStorage(setting.LFS.Storage.Type, setting.LFS.Storage)
 	return
 }