Fix all possible setting error related storages and added some tests (#23911)

Follow up #22405

Fix #20703 

This PR rewrites storage configuration read sequences with some breaks
and tests. It becomes more strict than before and also fixed some
inherit problems.

- Move storage's MinioConfig struct into setting, so after the
configuration loading, the values will be stored into the struct but not
still on some section.
- All storages configurations should be stored on one section,
configuration items cannot be overrided by multiple sections. The
prioioty of configuration is `[attachment]` > `[storage.attachments]` |
`[storage.customized]` > `[storage]` > `default`
- For extra override configuration items, currently are `SERVE_DIRECT`,
`MINIO_BASE_PATH`, `MINIO_BUCKET`, which could be configured in another
section. The prioioty of the override configuration is `[attachment]` >
`[storage.attachments]` > `default`.
- Add more tests for storages configurations.
- Update the storage documentations.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

7 files changed, 38 insertions, 121 deletions

diff --git a/modules/storage/helper.go b/modules/storage/helper.go
index d1959830b9..f8dff9e64d 100644
--- a/modules/storage/helper.go
+++ b/modules/storage/helper.go
@@ -8,64 +8,8 @@ import (
 	"io"
 	"net/url"
 	"os"
-	"reflect"
-
-	"code.gitea.io/gitea/modules/json"
 )
 
-// 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
-}
-
 var uninitializedStorage = discardStorage("uninitialized storage")
 
 type discardStorage string
diff --git a/modules/storage/local.go b/modules/storage/local.go
index 73ef306979..9bb532f1df 100644
--- a/modules/storage/local.go
+++ b/modules/storage/local.go
@@ -12,20 +12,12 @@ import (
 	"path/filepath"
 
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 )
 
 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"`
-	TemporaryPath string `ini:"TEMPORARY_PATH"`
-}
-
 // LocalStorage represents a local files storage
 type LocalStorage struct {
 	ctx    context.Context
@@ -34,13 +26,7 @@ type LocalStorage struct {
 }
 
 // NewLocalStorage returns a local files
-func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) {
-	configInterface, err := toConfig(LocalStorageConfig{}, cfg)
-	if err != nil {
-		return nil, err
-	}
-	config := configInterface.(LocalStorageConfig)
-
+func NewLocalStorage(ctx context.Context, config *setting.Storage) (ObjectStorage, error) {
 	if !filepath.IsAbs(config.Path) {
 		return nil, fmt.Errorf("LocalStorageConfig.Path should have been prepared by setting/storage.go and should be an absolute path, but not: %q", config.Path)
 	}
@@ -164,5 +150,5 @@ func (l *LocalStorage) IterateObjects(dirName string, fn func(path string, obj O
 }
 
 func init() {
-	RegisterStorageType(LocalStorageType, NewLocalStorage)
+	RegisterStorageType(setting.LocalStorageType, NewLocalStorage)
 }
diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go
index 1c4b856ab6..e230323f67 100644
--- a/modules/storage/local_test.go
+++ b/modules/storage/local_test.go
@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -55,5 +57,5 @@ func TestBuildLocalPath(t *testing.T) {
 
 func TestLocalStorageIterator(t *testing.T) {
 	dir := filepath.Join(os.TempDir(), "TestLocalStorageIteratorTestDir")
-	testStorageIterator(t, string(LocalStorageType), LocalStorageConfig{Path: dir})
+	testStorageIterator(t, setting.LocalStorageType, &setting.Storage{Path: dir})
 }
diff --git a/modules/storage/minio.go b/modules/storage/minio.go
index c78f351e9c..81774fb9cf 100644
--- a/modules/storage/minio.go
+++ b/modules/storage/minio.go
@@ -16,6 +16,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/minio/minio-go/v7"
@@ -41,25 +42,9 @@ 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"`
-	InsecureSkipVerify bool   `ini:"MINIO_INSECURE_SKIP_VERIFY"`
-	ChecksumAlgorithm  string `ini:"MINIO_CHECKSUM_ALGORITHM"`
-}
-
 // MinioStorage returns a minio bucket storage
 type MinioStorage struct {
-	cfg      *MinioStorageConfig
+	cfg      *setting.MinioStorageConfig
 	ctx      context.Context
 	client   *minio.Client
 	bucket   string
@@ -87,13 +72,8 @@ func convertMinioErr(err error) error {
 }
 
 // NewMinioStorage returns a minio storage
-func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) {
-	configInterface, err := toConfig(MinioStorageConfig{}, cfg)
-	if err != nil {
-		return nil, convertMinioErr(err)
-	}
-	config := configInterface.(MinioStorageConfig)
-
+func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, error) {
+	config := cfg.MinioConfig
 	if config.ChecksumAlgorithm != "" && config.ChecksumAlgorithm != "default" && config.ChecksumAlgorithm != "md5" {
 		return nil, fmt.Errorf("invalid minio checksum algorithm: %s", config.ChecksumAlgorithm)
 	}
@@ -258,5 +238,5 @@ func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj O
 }
 
 func init() {
-	RegisterStorageType(MinioStorageType, NewMinioStorage)
+	RegisterStorageType(setting.MinioStorageType, NewMinioStorage)
 }
diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go
index bfddf33032..af392b7e22 100644
--- a/modules/storage/minio_test.go
+++ b/modules/storage/minio_test.go
@@ -6,6 +6,8 @@ package storage
 import (
 	"os"
 	"testing"
+
+	"code.gitea.io/gitea/modules/setting"
 )
 
 func TestMinioStorageIterator(t *testing.T) {
@@ -13,11 +15,13 @@ func TestMinioStorageIterator(t *testing.T) {
 		t.Skip("minioStorage not present outside of CI")
 		return
 	}
-	testStorageIterator(t, string(MinioStorageType), MinioStorageConfig{
-		Endpoint:        "127.0.0.1:9000",
-		AccessKeyID:     "123456",
-		SecretAccessKey: "12345678",
-		Bucket:          "gitea",
-		Location:        "us-east-1",
+	testStorageIterator(t, setting.MinioStorageType, &setting.Storage{
+		MinioConfig: setting.MinioStorageConfig{
+			Endpoint:        "127.0.0.1:9000",
+			AccessKeyID:     "123456",
+			SecretAccessKey: "12345678",
+			Bucket:          "gitea",
+			Location:        "us-east-1",
+		},
 	})
 }
diff --git a/modules/storage/storage.go b/modules/storage/storage.go
index 5b6efccb6a..c3396f0c7f 100644
--- a/modules/storage/storage.go
+++ b/modules/storage/storage.go
@@ -37,16 +37,15 @@ func IsErrInvalidConfiguration(err error) bool {
 	return ok
 }
 
-// Type is a type of Storage
-type Type string
+type Type = setting.StorageType
 
 // NewStorageFunc is a function that creates a storage
-type NewStorageFunc func(ctx context.Context, cfg interface{}) (ObjectStorage, error)
+type NewStorageFunc func(ctx context.Context, cfg *setting.Storage) (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)) {
+func RegisterStorageType(typ Type, fn func(ctx context.Context, cfg *setting.Storage) (ObjectStorage, error)) {
 	storageMap[typ] = fn
 }
 
@@ -151,11 +150,11 @@ func Init() error {
 }
 
 // NewStorage takes a storage type and some config and returns an ObjectStorage or an error
-func NewStorage(typStr string, cfg interface{}) (ObjectStorage, error) {
+func NewStorage(typStr Type, cfg *setting.Storage) (ObjectStorage, error) {
 	if len(typStr) == 0 {
-		typStr = string(LocalStorageType)
+		typStr = setting.LocalStorageType
 	}
-	fn, ok := storageMap[Type(typStr)]
+	fn, ok := storageMap[typStr]
 	if !ok {
 		return nil, fmt.Errorf("Unsupported storage type: %s", typStr)
 	}
@@ -165,7 +164,7 @@ func NewStorage(typStr string, cfg interface{}) (ObjectStorage, error) {
 
 func initAvatars() (err error) {
 	log.Info("Initialising Avatar storage with type: %s", setting.Avatar.Storage.Type)
-	Avatars, err = NewStorage(setting.Avatar.Storage.Type, &setting.Avatar.Storage)
+	Avatars, err = NewStorage(setting.Avatar.Storage.Type, setting.Avatar.Storage)
 	return err
 }
 
@@ -175,7 +174,7 @@ func initAttachments() (err error) {
 		return nil
 	}
 	log.Info("Initialising Attachment storage with type: %s", setting.Attachment.Storage.Type)
-	Attachments, err = NewStorage(setting.Attachment.Storage.Type, &setting.Attachment.Storage)
+	Attachments, err = NewStorage(setting.Attachment.Storage.Type, setting.Attachment.Storage)
 	return err
 }
 
@@ -185,19 +184,19 @@ func initLFS() (err error) {
 		return nil
 	}
 	log.Info("Initialising LFS storage with type: %s", setting.LFS.Storage.Type)
-	LFS, err = NewStorage(setting.LFS.Storage.Type, &setting.LFS.Storage)
+	LFS, err = NewStorage(setting.LFS.Storage.Type, setting.LFS.Storage)
 	return err
 }
 
 func initRepoAvatars() (err error) {
 	log.Info("Initialising Repository Avatar storage with type: %s", setting.RepoAvatar.Storage.Type)
-	RepoAvatars, err = NewStorage(setting.RepoAvatar.Storage.Type, &setting.RepoAvatar.Storage)
+	RepoAvatars, err = NewStorage(setting.RepoAvatar.Storage.Type, setting.RepoAvatar.Storage)
 	return err
 }
 
 func initRepoArchives() (err error) {
 	log.Info("Initialising Repository Archive storage with type: %s", setting.RepoArchive.Storage.Type)
-	RepoArchives, err = NewStorage(setting.RepoArchive.Storage.Type, &setting.RepoArchive.Storage)
+	RepoArchives, err = NewStorage(setting.RepoArchive.Storage.Type, setting.RepoArchive.Storage)
 	return err
 }
 
@@ -207,7 +206,7 @@ func initPackages() (err error) {
 		return nil
 	}
 	log.Info("Initialising Packages storage with type: %s", setting.Packages.Storage.Type)
-	Packages, err = NewStorage(setting.Packages.Storage.Type, &setting.Packages.Storage)
+	Packages, err = NewStorage(setting.Packages.Storage.Type, setting.Packages.Storage)
 	return err
 }
 
@@ -218,10 +217,10 @@ func initActions() (err error) {
 		return nil
 	}
 	log.Info("Initialising Actions storage with type: %s", setting.Actions.LogStorage.Type)
-	if Actions, err = NewStorage(setting.Actions.LogStorage.Type, &setting.Actions.LogStorage); err != nil {
+	if Actions, err = NewStorage(setting.Actions.LogStorage.Type, setting.Actions.LogStorage); err != nil {
 		return err
 	}
 	log.Info("Initialising ActionsArtifacts storage with type: %s", setting.Actions.ArtifactStorage.Type)
-	ActionsArtifacts, err = NewStorage(setting.Actions.ArtifactStorage.Type, &setting.Actions.ArtifactStorage)
+	ActionsArtifacts, err = NewStorage(setting.Actions.ArtifactStorage.Type, setting.Actions.ArtifactStorage)
 	return err
 }
diff --git a/modules/storage/storage_test.go b/modules/storage/storage_test.go
index b517a9e71a..5e3e9c7dba 100644
--- a/modules/storage/storage_test.go
+++ b/modules/storage/storage_test.go
@@ -7,10 +7,12 @@ import (
 	"bytes"
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/stretchr/testify/assert"
 )
 
-func testStorageIterator(t *testing.T, typStr string, cfg interface{}) {
+func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) {
 	l, err := NewStorage(typStr, cfg)
 	assert.NoError(t, err)