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>
This commit is contained in:
2023-06-14 11:42:38 +08:00
committed by GitHub
parent dc0a7168f1
commit d6dd6d641b
41 changed files with 1152 additions and 452 deletions

View File

@ -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

View File

@ -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)
}

View File

@ -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})
}

View File

@ -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)
}

View File

@ -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",
},
})
}

View File

@ -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
}

View File

@ -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)