diff --git a/modules/setting/storage.go b/modules/setting/storage.go index c28f2be4ed..cc3a2899d7 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -91,134 +91,172 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S return nil, errors.New("no name for storage") } - var targetSec ConfigSection - // check typ first - if typ != "" { - var err error - targetSec, err = rootCfg.GetSection(storageSectionName + "." + typ) - if err != nil { - if !IsValidStorageType(StorageType(typ)) { - return nil, fmt.Errorf("get section via storage type %q failed: %v", typ, err) - } - } - if targetSec != nil { - targetType := targetSec.Key("STORAGE_TYPE").String() - if targetType == "" { - if !IsValidStorageType(StorageType(typ)) { - return nil, fmt.Errorf("unknow storage type %q", typ) - } - targetSec.Key("STORAGE_TYPE").SetValue(typ) - } else if !IsValidStorageType(StorageType(targetType)) { - return nil, fmt.Errorf("unknow storage type %q for section storage.%v", targetType, typ) - } - } + targetSec, tp, err := getStorageTargetSection(rootCfg, name, typ, sec) + if err != nil { + return nil, err } - if targetSec == nil && sec != nil { - secTyp := sec.Key("STORAGE_TYPE").String() - if IsValidStorageType(StorageType(secTyp)) { - targetSec = sec - } else if secTyp != "" { - targetSec, _ = rootCfg.GetSection(storageSectionName + "." + secTyp) - } - } + overrideSec := getStorageOverrideSection(rootCfg, targetSec, sec, tp, name) - targetSecIsStoragename := false - storageNameSec, _ := rootCfg.GetSection(storageSectionName + "." + name) - if targetSec == nil { - targetSec = storageNameSec - targetSecIsStoragename = storageNameSec != nil + targetType := targetSec.Key("STORAGE_TYPE").String() + switch targetType { + case string(LocalStorageType): + return getStorageForLocal(targetSec, overrideSec, tp, name) + case string(MinioStorageType): + return getStorageForMinio(targetSec, overrideSec, tp, name) + default: + return nil, fmt.Errorf("unsupported storage type %q", targetType) } +} - if targetSec == nil { - targetSec = getDefaultStorageSection(rootCfg) - } else { - targetType := targetSec.Key("STORAGE_TYPE").String() - switch { - case targetType == "": - if targetSec != storageNameSec && storageNameSec != nil { - targetSec = storageNameSec - targetSecIsStoragename = true - if targetSec.Key("STORAGE_TYPE").String() == "" { - return nil, fmt.Errorf("storage section %s.%s has no STORAGE_TYPE", storageSectionName, name) - } - } else { - if targetSec.Key("PATH").String() == "" { - targetSec = getDefaultStorageSection(rootCfg) - } else { - targetSec.Key("STORAGE_TYPE").SetValue("local") - } - } - default: - newTargetSec, _ := rootCfg.GetSection(storageSectionName + "." + targetType) - if newTargetSec == nil { - if !IsValidStorageType(StorageType(targetType)) { - return nil, fmt.Errorf("invalid storage section %s.%q", storageSectionName, targetType) - } - } else { - targetSec = newTargetSec - if IsValidStorageType(StorageType(targetType)) { - tp := targetSec.Key("STORAGE_TYPE").String() - if tp == "" { - targetSec.Key("STORAGE_TYPE").SetValue(targetType) - } - } - } +type targetSecType int + +const ( + targetSecIsTyp targetSecType = iota // target section is [storage.type] which the type from parameter + targetSecIsStorage // target section is [storage] + targetSecIsDefault // target section is the default value + targetSecIsStorageWithName // target section is [storage.name] + targetSecIsSec // target section is from the name seciont [name] +) + +func getStorageSectionByType(rootCfg ConfigProvider, typ string) (ConfigSection, targetSecType, error) { + targetSec, err := rootCfg.GetSection(storageSectionName + "." + typ) + if err != nil { + if !IsValidStorageType(StorageType(typ)) { + return nil, 0, fmt.Errorf("get section via storage type %q failed: %v", typ, err) } + // if typ is a valid storage type, but there is no [storage.local] or [storage.minio] section + // it's not an error + return nil, 0, nil } targetType := targetSec.Key("STORAGE_TYPE").String() - if !IsValidStorageType(StorageType(targetType)) { - return nil, fmt.Errorf("invalid storage type %q", targetType) + if targetType == "" { + if !IsValidStorageType(StorageType(typ)) { + return nil, 0, fmt.Errorf("unknow storage type %q", typ) + } + targetSec.Key("STORAGE_TYPE").SetValue(typ) + } else if !IsValidStorageType(StorageType(targetType)) { + return nil, 0, fmt.Errorf("unknow storage type %q for section storage.%v", targetType, typ) } - // extra config section will be read SERVE_DIRECT, PATH, MINIO_BASE_PATH, MINIO_BUCKET to override the targetsec when possible - extraConfigSec := sec - if extraConfigSec == nil { - extraConfigSec = storageNameSec + return targetSec, targetSecIsTyp, nil +} + +func getStorageTargetSection(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (ConfigSection, targetSecType, error) { + // check typ first + if typ == "" { + if sec != nil { // check sec's type secondly + typ = sec.Key("STORAGE_TYPE").String() + if IsValidStorageType(StorageType(typ)) { + if targetSec, _ := rootCfg.GetSection(storageSectionName + "." + typ); targetSec == nil { + return sec, targetSecIsSec, nil + } + } + } } - var storage Storage - storage.Type = StorageType(targetType) - - switch targetType { - case string(LocalStorageType): - targetPath := ConfigSectionKeyString(targetSec, "PATH", "") - if targetPath == "" { - targetPath = AppDataPath - } else if !filepath.IsAbs(targetPath) { - targetPath = filepath.Join(AppDataPath, targetPath) + if typ != "" { + targetSec, tp, err := getStorageSectionByType(rootCfg, typ) + if targetSec != nil || err != nil { + return targetSec, tp, err } + } - var fallbackPath string - if targetSecIsStoragename { - fallbackPath = targetPath - } else { - fallbackPath = filepath.Join(targetPath, name) - } + // check stoarge name thirdly + targetSec, _ := rootCfg.GetSection(storageSectionName + "." + name) + if targetSec != nil { + targetType := targetSec.Key("STORAGE_TYPE").String() + switch { + case targetType == "": + if targetSec.Key("PATH").String() == "" { // both storage type and path are empty, use default + return getDefaultStorageSection(rootCfg), targetSecIsDefault, nil + } - if extraConfigSec == nil { - storage.Path = fallbackPath - } else { - storage.Path = ConfigSectionKeyString(extraConfigSec, "PATH", fallbackPath) - if !filepath.IsAbs(storage.Path) { - storage.Path = filepath.Join(targetPath, storage.Path) + targetSec.Key("STORAGE_TYPE").SetValue("local") + default: + targetSec, tp, err := getStorageSectionByType(rootCfg, targetType) + if targetSec != nil || err != nil { + return targetSec, tp, err } } - case string(MinioStorageType): - if err := targetSec.MapTo(&storage.MinioConfig); err != nil { - return nil, fmt.Errorf("map minio config failed: %v", err) + return targetSec, targetSecIsStorageWithName, nil + } + + return getDefaultStorageSection(rootCfg), targetSecIsDefault, nil +} + +// getStorageOverrideSection override section will be read SERVE_DIRECT, PATH, MINIO_BASE_PATH, MINIO_BUCKET to override the targetsec when possible +func getStorageOverrideSection(rootConfig ConfigProvider, targetSec, sec ConfigSection, targetSecType targetSecType, name string) ConfigSection { + if targetSecType == targetSecIsSec { + return nil + } + + if sec != nil { + return sec + } + + if targetSecType != targetSecIsStorageWithName { + nameSec, _ := rootConfig.GetSection(storageSectionName + "." + name) + return nameSec + } + return nil +} + +func getStorageForLocal(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { + storage := Storage{ + Type: StorageType(targetSec.Key("STORAGE_TYPE").String()), + } + + targetPath := ConfigSectionKeyString(targetSec, "PATH", "") + var fallbackPath string + if targetPath == "" { // no path + fallbackPath = filepath.Join(AppDataPath, name) + } else { + if tp == targetSecIsStorage || tp == targetSecIsDefault { + fallbackPath = filepath.Join(targetPath, name) + } else { + fallbackPath = targetPath } + if !filepath.IsAbs(fallbackPath) { + fallbackPath = filepath.Join(AppDataPath, fallbackPath) + } + } - storage.MinioConfig.BasePath = name + "/" - - if extraConfigSec != nil { - storage.MinioConfig.ServeDirect = ConfigSectionKeyBool(extraConfigSec, "SERVE_DIRECT", storage.MinioConfig.ServeDirect) - storage.MinioConfig.BasePath = ConfigSectionKeyString(extraConfigSec, "MINIO_BASE_PATH", storage.MinioConfig.BasePath) - storage.MinioConfig.Bucket = ConfigSectionKeyString(extraConfigSec, "MINIO_BUCKET", storage.MinioConfig.Bucket) + if overrideSec == nil { // no override section + storage.Path = fallbackPath + } else { + storage.Path = ConfigSectionKeyString(overrideSec, "PATH", "") + if storage.Path == "" { // overrideSec has no path + storage.Path = fallbackPath + } else if !filepath.IsAbs(storage.Path) { + if targetPath == "" { + storage.Path = filepath.Join(AppDataPath, storage.Path) + } else { + storage.Path = filepath.Join(targetPath, storage.Path) + } } } return &storage, nil } + +func getStorageForMinio(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) { + var storage Storage + storage.Type = StorageType(targetSec.Key("STORAGE_TYPE").String()) + if err := targetSec.MapTo(&storage.MinioConfig); err != nil { + return nil, fmt.Errorf("map minio config failed: %v", err) + } + + if storage.MinioConfig.BasePath == "" { + storage.MinioConfig.BasePath = name + "/" + } + + if overrideSec != nil { + storage.MinioConfig.ServeDirect = ConfigSectionKeyBool(overrideSec, "SERVE_DIRECT", storage.MinioConfig.ServeDirect) + storage.MinioConfig.BasePath = ConfigSectionKeyString(overrideSec, "MINIO_BASE_PATH", storage.MinioConfig.BasePath) + storage.MinioConfig.Bucket = ConfigSectionKeyString(overrideSec, "MINIO_BUCKET", storage.MinioConfig.Bucket) + } + return &storage, nil +} diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go index 9a83f31d91..20886d4c4e 100644 --- a/modules/setting/storage_test.go +++ b/modules/setting/storage_test.go @@ -27,12 +27,15 @@ MINIO_BUCKET = gitea-storage assert.NoError(t, loadAttachmentFrom(cfg)) assert.EqualValues(t, "gitea-attachment", Attachment.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "attachments/", Attachment.Storage.MinioConfig.BasePath) assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, "gitea-lfs", LFS.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath) assert.NoError(t, loadAvatarsFrom(cfg)) assert.EqualValues(t, "gitea-storage", Avatar.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "avatars/", Avatar.Storage.MinioConfig.BasePath) } func Test_getStorageUseOtherNameAsType(t *testing.T) { @@ -49,9 +52,11 @@ MINIO_BUCKET = gitea-storage assert.NoError(t, loadAttachmentFrom(cfg)) assert.EqualValues(t, "gitea-storage", Attachment.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "attachments/", Attachment.Storage.MinioConfig.BasePath) assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, "gitea-storage", LFS.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath) } func Test_getStorageInheritStorageType(t *testing.T) { @@ -117,6 +122,9 @@ func Test_getStorageInheritStorageTypeLocal(t *testing.T) { [storage] STORAGE_TYPE = local `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"}, + {loadLFSFrom, &LFS.Storage, "/appdata/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/repo-archive"}, {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, @@ -131,6 +139,9 @@ func Test_getStorageInheritStorageTypeLocalPath(t *testing.T) { STORAGE_TYPE = local PATH = /data/gitea `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/data/gitea/attachments"}, + {loadLFSFrom, &LFS.Storage, "/data/gitea/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/data/gitea/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/repo-archive"}, {loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"}, @@ -145,6 +156,9 @@ func Test_getStorageInheritStorageTypeLocalRelativePath(t *testing.T) { STORAGE_TYPE = local PATH = storages `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/appdata/storages/attachments"}, + {loadLFSFrom, &LFS.Storage, "/appdata/storages/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/appdata/storages/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/appdata/storages/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/storages/repo-archive"}, {loadActionsFrom, &Actions.LogStorage, "/appdata/storages/actions_log"}, @@ -162,6 +176,9 @@ PATH = /data/gitea [repo-archive] PATH = /data/gitea/the-archives-dir `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/data/gitea/attachments"}, + {loadLFSFrom, &LFS.Storage, "/data/gitea/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/data/gitea/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/the-archives-dir"}, {loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"}, @@ -178,6 +195,9 @@ PATH = /data/gitea [repo-archive] `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/data/gitea/attachments"}, + {loadLFSFrom, &LFS.Storage, "/data/gitea/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/data/gitea/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/repo-archive"}, {loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"}, @@ -195,6 +215,9 @@ PATH = /data/gitea [repo-archive] PATH = the-archives-dir `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/data/gitea/attachments"}, + {loadLFSFrom, &LFS.Storage, "/data/gitea/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/data/gitea/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/the-archives-dir"}, {loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"}, @@ -209,6 +232,9 @@ func Test_getStorageInheritStorageTypeLocalPathOverride3(t *testing.T) { STORAGE_TYPE = local PATH = /data/gitea/archives `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"}, + {loadLFSFrom, &LFS.Storage, "/appdata/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/archives"}, {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, @@ -217,6 +243,23 @@ PATH = /data/gitea/archives }) } +func Test_getStorageInheritStorageTypeLocalPathOverride3_5(t *testing.T) { + testLocalStoragePath(t, "/appdata", ` +[storage.repo-archive] +STORAGE_TYPE = local +PATH = a-relative-path +`, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"}, + {loadLFSFrom, &LFS.Storage, "/appdata/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"}, + {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, + {loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/a-relative-path"}, + {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, + {loadAvatarsFrom, &Avatar.Storage, "/appdata/avatars"}, + {loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/repo-avatars"}, + }) +} + func Test_getStorageInheritStorageTypeLocalPathOverride4(t *testing.T) { testLocalStoragePath(t, "/appdata", ` [storage.repo-archive] @@ -226,6 +269,9 @@ PATH = /data/gitea/archives [repo-archive] PATH = /tmp/gitea/archives `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"}, + {loadLFSFrom, &LFS.Storage, "/appdata/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/tmp/gitea/archives"}, {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, @@ -242,6 +288,9 @@ PATH = /data/gitea/archives [repo-archive] `, []testLocalStoragePathCase{ + {loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"}, + {loadLFSFrom, &LFS.Storage, "/appdata/lfs"}, + {loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"}, {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/archives"}, {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, @@ -249,3 +298,117 @@ PATH = /data/gitea/archives {loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/repo-avatars"}, }) } + +func Test_getStorageInheritStorageTypeLocalPathOverride72(t *testing.T) { + testLocalStoragePath(t, "/appdata", ` +[repo-archive] +STORAGE_TYPE = local +PATH = archives +`, []testLocalStoragePathCase{ + {loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/archives"}, + }) +} + +func Test_getStorageConfiguration20(t *testing.T) { + cfg, err := NewConfigProviderFromData(` +[repo-archive] +STORAGE_TYPE = my_storage +PATH = archives +`) + assert.NoError(t, err) + + assert.Error(t, loadRepoArchiveFrom(cfg)) +} + +func Test_getStorageConfiguration21(t *testing.T) { + testLocalStoragePath(t, "/appdata", ` +[storage.repo-archive] +`, []testLocalStoragePathCase{ + {loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/repo-archive"}, + }) +} + +func Test_getStorageConfiguration22(t *testing.T) { + testLocalStoragePath(t, "/appdata", ` +[storage.repo-archive] +PATH = archives +`, []testLocalStoragePathCase{ + {loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/archives"}, + }) +} + +func Test_getStorageConfiguration23(t *testing.T) { + cfg, err := NewConfigProviderFromData(` +[repo-archive] +STORAGE_TYPE = minio +MINIO_ACCESS_KEY_ID = my_access_key +MINIO_SECRET_ACCESS_KEY = my_secret_key +`) + assert.NoError(t, err) + + _, err = getStorage(cfg, "", "", nil) + assert.Error(t, err) + + assert.NoError(t, loadRepoArchiveFrom(cfg)) + cp := RepoArchive.Storage.ToShadowCopy() + assert.EqualValues(t, "******", cp.MinioConfig.AccessKeyID) + assert.EqualValues(t, "******", cp.MinioConfig.SecretAccessKey) +} + +func Test_getStorageConfiguration24(t *testing.T) { + cfg, err := NewConfigProviderFromData(` +[repo-archive] +STORAGE_TYPE = my_archive + +[storage.my_archive] +; unsupported, storage type should be defined explicitly +PATH = archives +`) + assert.NoError(t, err) + assert.Error(t, loadRepoArchiveFrom(cfg)) +} + +func Test_getStorageConfiguration25(t *testing.T) { + cfg, err := NewConfigProviderFromData(` +[repo-archive] +STORAGE_TYPE = my_archive + +[storage.my_archive] +; unsupported, storage type should be known type +STORAGE_TYPE = unknown // should be local or minio +PATH = archives +`) + assert.NoError(t, err) + assert.Error(t, loadRepoArchiveFrom(cfg)) +} + +func Test_getStorageConfiguration26(t *testing.T) { + cfg, err := NewConfigProviderFromData(` +[repo-archive] +STORAGE_TYPE = minio +MINIO_ACCESS_KEY_ID = my_access_key +MINIO_SECRET_ACCESS_KEY = my_secret_key +; wrong configuration +MINIO_USE_SSL = abc +`) + assert.NoError(t, err) + // assert.Error(t, loadRepoArchiveFrom(cfg)) + // FIXME: this should return error but now ini package's MapTo() doesn't check type + assert.NoError(t, loadRepoArchiveFrom(cfg)) +} + +func Test_getStorageConfiguration27(t *testing.T) { + cfg, err := NewConfigProviderFromData(` +[storage.repo-archive] +STORAGE_TYPE = minio +MINIO_ACCESS_KEY_ID = my_access_key +MINIO_SECRET_ACCESS_KEY = my_secret_key +MINIO_USE_SSL = true +`) + assert.NoError(t, err) + assert.NoError(t, loadRepoArchiveFrom(cfg)) + assert.EqualValues(t, "my_access_key", RepoArchive.Storage.MinioConfig.AccessKeyID) + assert.EqualValues(t, "my_secret_key", RepoArchive.Storage.MinioConfig.SecretAccessKey) + assert.EqualValues(t, true, RepoArchive.Storage.MinioConfig.UseSSL) + assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) +}