From 02747fe6d1ded8e2192fb60bdc7f5b2fa23f9a1a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 4 Apr 2023 15:17:06 +0800 Subject: [PATCH 01/28] Fix all possible setting error related storages and added some tests --- custom/conf/app.example.ini | 23 +++ modules/setting/actions.go | 13 +- modules/setting/actions_test.go | 64 +++++++ modules/setting/attachment.go | 4 +- modules/setting/attachment_test.go | 114 ++++++++++++ modules/setting/lfs.go | 38 ++-- modules/setting/lfs_test.go | 64 +++++++ modules/setting/packages.go | 14 +- modules/setting/packages_test.go | 57 ++++++ modules/setting/picture.go | 15 +- modules/setting/repository.go | 8 +- modules/setting/repository_archive.go | 20 +++ modules/setting/repository_archive_test.go | 68 +++++++ modules/setting/setting.go | 15 +- modules/setting/storage_test.go | 200 +++++---------------- 15 files changed, 521 insertions(+), 196 deletions(-) create mode 100644 modules/setting/actions_test.go create mode 100644 modules/setting/attachment_test.go create mode 100644 modules/setting/lfs_test.go create mode 100644 modules/setting/repository_archive.go create mode 100644 modules/setting/repository_archive_test.go diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 0543c85d50daa..13f6ec1663ea8 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2484,6 +2484,10 @@ ROUTER = console ;; Enable/Disable package registry capabilities ;ENABLED = true ;; +;STORAGE_TYPE = local +;; override the minio base path if storage type is minio +;MINIO_BASE_PATH = packages/ +;; ;; Path for chunked uploads. Defaults to APP_DATA_PATH + `tmp/package-upload` ;CHUNKED_UPLOAD_PATH = tmp/package-upload ;; @@ -2534,6 +2538,19 @@ ROUTER = console ;; storage type ;STORAGE_TYPE = local +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; repo-archive storage will override storage +;; +;[repo-archive] +;STORAGE_TYPE = local +;; +;; Where your lfs files reside, default is data/lfs. +;PATH = data/repo-archive +;; +;; override the minio base path if storage type is minio +;MINIO_BASE_PATH = repo-archive/ + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; settings for repository archives, will override storage setting @@ -2553,6 +2570,9 @@ ROUTER = console ;; ;; Where your lfs files reside, default is data/lfs. ;PATH = data/lfs +;; +;; override the minio base path if storage type is minio +;MINIO_BASE_PATH = lfs/ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -2602,6 +2622,9 @@ ROUTER = console ; [actions] ;; Enable/Disable actions capabilities ;ENABLED = false +;; +;STORAGE_TYPE = local +;; ;; Default address to get action plugins, e.g. the default value means downloading from "https://gitea.com/actions/checkout" for "uses: actions/checkout@v3" ;DEFAULT_ACTIONS_URL = https://gitea.com diff --git a/modules/setting/actions.go b/modules/setting/actions.go index b11500dab4a44..ade7ee9e49c65 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -3,9 +3,7 @@ package setting -import ( - "code.gitea.io/gitea/modules/log" -) +import "fmt" // Actions settings var ( @@ -19,11 +17,14 @@ var ( } ) -func loadActionsFrom(rootCfg ConfigProvider) { +func loadActionsFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("actions") if err := sec.MapTo(&Actions); err != nil { - log.Fatal("Failed to map Actions settings: %v", err) + return fmt.Errorf("failed to map Actions settings: %v", err) } + sec.Key("MINIO_BASE_PATH").MustString("actions_log/") + storageType := sec.Key("STORAGE_TYPE").MustString("") - Actions.Storage = getStorage(rootCfg, "actions_log", "", nil) + Actions.Storage = getStorage(rootCfg, "actions_log", storageType, sec) + return nil } diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go new file mode 100644 index 0000000000000..07316a01314af --- /dev/null +++ b/modules/setting/actions_test.go @@ -0,0 +1,64 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package setting + +import ( + "testing" + + "github.com/stretchr/testify/assert" + ini "gopkg.in/ini.v1" +) + +func Test_getStorageInheritNameSectionTypeForActions(t *testing.T) { + iniStr := ` + [storage] + STORAGE_TYPE = minio + ` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadActionsFrom(cfg)) + + assert.EqualValues(t, "minio", Actions.Storage.Type) + assert.EqualValues(t, "actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) + + iniStr = ` +[storage.actions_log] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadActionsFrom(cfg)) + + assert.EqualValues(t, "minio", Actions.Storage.Type) + assert.EqualValues(t, "actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) + + iniStr = ` +[actions] +STORAGE_TYPE = my_minio + +[storage.my_minio] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadActionsFrom(cfg)) + + assert.EqualValues(t, "minio", Actions.Storage.Type) + assert.EqualValues(t, "actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) + + iniStr = ` +[actions] +STORAGE_TYPE = my_minio +MINIO_BASE_PATH = my_actions_log/ + +[storage.my_minio] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadActionsFrom(cfg)) + + assert.EqualValues(t, "minio", Actions.Storage.Type) + assert.EqualValues(t, "my_actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) +} diff --git a/modules/setting/attachment.go b/modules/setting/attachment.go index 4d4b8e3b02430..4121fb87ae2be 100644 --- a/modules/setting/attachment.go +++ b/modules/setting/attachment.go @@ -20,7 +20,7 @@ var Attachment = struct { Enabled: true, } -func loadAttachmentFrom(rootCfg ConfigProvider) { +func loadAttachmentFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("attachment") storageType := sec.Key("STORAGE_TYPE").MustString("") @@ -30,4 +30,6 @@ func loadAttachmentFrom(rootCfg ConfigProvider) { Attachment.MaxSize = sec.Key("MAX_SIZE").MustInt64(4) Attachment.MaxFiles = sec.Key("MAX_FILES").MustInt(5) Attachment.Enabled = sec.Key("ENABLED").MustBool(true) + + return nil } diff --git a/modules/setting/attachment_test.go b/modules/setting/attachment_test.go new file mode 100644 index 0000000000000..c427b9b94b299 --- /dev/null +++ b/modules/setting/attachment_test.go @@ -0,0 +1,114 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package setting + +import ( + "testing" + + "github.com/stretchr/testify/assert" + ini "gopkg.in/ini.v1" +) + +func Test_getStorageCustomType(t *testing.T) { + iniStr := ` +[attachment] +STORAGE_TYPE = my_minio +MINIO_BUCKET = gitea-attachment + +[storage.my_minio] +STORAGE_TYPE = minio +MINIO_ENDPOINT = my_minio:9000 +` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + + assert.NoError(t, loadAttachmentFrom(cfg)) + + assert.EqualValues(t, "minio", Attachment.Storage.Type) + assert.EqualValues(t, "my_minio:9000", Attachment.Storage.Section.Key("MINIO_ENDPOINT").String()) + assert.EqualValues(t, "gitea-attachment", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) +} + +func Test_getStorageNameSectionOverridesTypeSection(t *testing.T) { + iniStr := ` +[attachment] +STORAGE_TYPE = minio + +[storage.attachments] +MINIO_BUCKET = gitea-attachment + +[storage.minio] +MINIO_BUCKET = gitea +` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + + assert.NoError(t, loadAttachmentFrom(cfg)) + + assert.EqualValues(t, "minio", Attachment.Storage.Type) + assert.EqualValues(t, "gitea-attachment", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) +} + +func Test_getStorageTypeSectionOverridesStorageSection(t *testing.T) { + iniStr := ` +[attachment] +STORAGE_TYPE = minio + +[storage.minio] +MINIO_BUCKET = gitea-minio + +[storage] +MINIO_BUCKET = gitea +` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + + assert.NoError(t, loadAttachmentFrom(cfg)) + + assert.EqualValues(t, "minio", Attachment.Storage.Type) + assert.EqualValues(t, "gitea-minio", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) +} + +func Test_getStorageSpecificOverridesStorage(t *testing.T) { + iniStr := ` +[attachment] +STORAGE_TYPE = minio +MINIO_BUCKET = gitea-attachment + +[storage.attachments] +MINIO_BUCKET = gitea + +[storage] +STORAGE_TYPE = local +` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + + assert.NoError(t, loadAttachmentFrom(cfg)) + + assert.EqualValues(t, "minio", Attachment.Storage.Type) + assert.EqualValues(t, "gitea-attachment", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) +} + +func Test_getStorageGetDefaults(t *testing.T) { + cfg, err := ini.Load([]byte("")) + assert.NoError(t, err) + + assert.NoError(t, loadAttachmentFrom(cfg)) + + assert.EqualValues(t, "gitea", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) +} + +func Test_getStorageInheritNameSectionType(t *testing.T) { + iniStr := ` +[storage.attachments] +STORAGE_TYPE = minio +` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + + assert.NoError(t, loadAttachmentFrom(cfg)) + + assert.EqualValues(t, "minio", Attachment.Storage.Type) +} diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index e04cde100b683..33d5749240401 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -5,10 +5,10 @@ package setting import ( "encoding/base64" + "fmt" "time" "code.gitea.io/gitea/modules/generate" - "code.gitea.io/gitea/modules/log" ini "gopkg.in/ini.v1" ) @@ -25,10 +25,10 @@ var LFS = struct { Storage }{} -func loadLFSFrom(rootCfg ConfigProvider) { +func loadLFSFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("server") if err := sec.MapTo(&LFS); err != nil { - log.Fatal("Failed to map LFS settings: %v", err) + return fmt.Errorf("failed to map LFS settings: %v", err) } lfsSec := rootCfg.Section("lfs") @@ -50,21 +50,23 @@ func loadLFSFrom(rootCfg ConfigProvider) { LFS.HTTPAuthExpiry = sec.Key("LFS_HTTP_AUTH_EXPIRY").MustDuration(20 * time.Minute) - if LFS.StartServer { - LFS.JWTSecretBytes = make([]byte, 32) - n, err := base64.RawURLEncoding.Decode(LFS.JWTSecretBytes, []byte(LFS.JWTSecretBase64)) - - if err != nil || n != 32 { - LFS.JWTSecretBase64, err = generate.NewJwtSecretBase64() - if err != nil { - log.Fatal("Error generating JWT Secret for custom config: %v", err) - return - } - - // Save secret - CreateOrAppendToCustomConf("server.LFS_JWT_SECRET", func(cfg *ini.File) { - cfg.Section("server").Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64) - }) + if !LFS.StartServer { + return nil + } + + LFS.JWTSecretBytes = make([]byte, 32) + n, err := base64.RawURLEncoding.Decode(LFS.JWTSecretBytes, []byte(LFS.JWTSecretBase64)) + + if err != nil || n != 32 { + LFS.JWTSecretBase64, err = generate.NewJwtSecretBase64() + if err != nil { + return fmt.Errorf("error generating JWT Secret for custom config: %v", err) } + + // Save secret + CreateOrAppendToCustomConf("server.LFS_JWT_SECRET", func(cfg *ini.File) { + cfg.Section("server").Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64) + }) } + return nil } diff --git a/modules/setting/lfs_test.go b/modules/setting/lfs_test.go new file mode 100644 index 0000000000000..4d902ccf71a5e --- /dev/null +++ b/modules/setting/lfs_test.go @@ -0,0 +1,64 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package setting + +import ( + "testing" + + "github.com/stretchr/testify/assert" + ini "gopkg.in/ini.v1" +) + +func Test_getStorageInheritNameSectionTypeForLFS(t *testing.T) { + iniStr := ` + [storage] + STORAGE_TYPE = minio + ` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadLFSFrom(cfg)) + + assert.EqualValues(t, "minio", LFS.Storage.Type) + assert.EqualValues(t, "lfs/", cfg.Section("lfs").Key("MINIO_BASE_PATH").MustString("")) + + iniStr = ` +[storage.lfs] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadLFSFrom(cfg)) + + assert.EqualValues(t, "minio", LFS.Storage.Type) + assert.EqualValues(t, "lfs/", cfg.Section("lfs").Key("MINIO_BASE_PATH").MustString("")) + + iniStr = ` +[lfs] +STORAGE_TYPE = my_minio + +[storage.my_minio] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadLFSFrom(cfg)) + + assert.EqualValues(t, "minio", LFS.Storage.Type) + assert.EqualValues(t, "lfs/", cfg.Section("lfs").Key("MINIO_BASE_PATH").MustString("")) + + iniStr = ` +[lfs] +STORAGE_TYPE = my_minio +MINIO_BASE_PATH = my_lfs/ + +[storage.my_minio] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadLFSFrom(cfg)) + + assert.EqualValues(t, "minio", LFS.Storage.Type) + assert.EqualValues(t, "my_lfs/", cfg.Section("lfs").Key("MINIO_BASE_PATH").MustString("")) +} diff --git a/modules/setting/packages.go b/modules/setting/packages.go index ac0ad62bca3d1..24fd356e78682 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -4,13 +4,12 @@ package setting import ( + "fmt" "math" "net/url" "os" "path/filepath" - "code.gitea.io/gitea/modules/log" - "github.com/dustin/go-humanize" ini "gopkg.in/ini.v1" ) @@ -47,13 +46,15 @@ var ( } ) -func loadPackagesFrom(rootCfg ConfigProvider) { +func loadPackagesFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("packages") if err := sec.MapTo(&Packages); err != nil { - log.Fatal("Failed to map Packages settings: %v", err) + return fmt.Errorf("failed to map Packages settings: %v", err) } - Packages.Storage = getStorage(rootCfg, "packages", "", nil) + storageType := sec.Key("STORAGE_TYPE").MustString("") + + Packages.Storage = getStorage(rootCfg, "packages", storageType, sec) appURL, _ := url.Parse(AppURL) Packages.RegistryHost = appURL.Host @@ -64,7 +65,7 @@ func loadPackagesFrom(rootCfg ConfigProvider) { } if err := os.MkdirAll(Packages.ChunkedUploadPath, os.ModePerm); err != nil { - log.Error("Unable to create chunked upload directory: %s (%v)", Packages.ChunkedUploadPath, err) + return fmt.Errorf("unable to create chunked upload directory: %s (%v)", Packages.ChunkedUploadPath, err) } Packages.LimitTotalOwnerSize = mustBytes(sec, "LIMIT_TOTAL_OWNER_SIZE") @@ -84,6 +85,7 @@ func loadPackagesFrom(rootCfg ConfigProvider) { Packages.LimitSizeRubyGems = mustBytes(sec, "LIMIT_SIZE_RUBYGEMS") Packages.LimitSizeSwift = mustBytes(sec, "LIMIT_SIZE_SWIFT") Packages.LimitSizeVagrant = mustBytes(sec, "LIMIT_SIZE_VAGRANT") + return nil } func mustBytes(section *ini.Section, key string) int64 { diff --git a/modules/setting/packages_test.go b/modules/setting/packages_test.go index 04d6e55834611..25e1d38e1a953 100644 --- a/modules/setting/packages_test.go +++ b/modules/setting/packages_test.go @@ -28,3 +28,60 @@ func TestMustBytes(t *testing.T) { assert.EqualValues(t, 1782579, test("1.7mib")) assert.EqualValues(t, -1, test("1 yib")) // too large } + +func Test_getStorageInheritNameSectionTypeForPackages(t *testing.T) { + // packages storage inherits from storage if nothing configured + iniStr := ` +[storage] +STORAGE_TYPE = minio +` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadPackagesFrom(cfg)) + + assert.EqualValues(t, "minio", Packages.Storage.Type) + assert.EqualValues(t, "packages/", cfg.Section("packages").Key("MINIO_BASE_PATH").MustString("")) + + // we can also configure packages storage directly + iniStr = ` +[storage.packages] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadPackagesFrom(cfg)) + + assert.EqualValues(t, "minio", Packages.Storage.Type) + assert.EqualValues(t, "packages/", cfg.Section("packages").Key("MINIO_BASE_PATH").MustString("")) + + // or we can indicate the storage type in the packages section + iniStr = ` +[packages] +STORAGE_TYPE = my_minio + +[storage.my_minio] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadPackagesFrom(cfg)) + + assert.EqualValues(t, "minio", Packages.Storage.Type) + assert.EqualValues(t, "packages/", cfg.Section("packages").Key("MINIO_BASE_PATH").MustString("")) + + // or we can indicate the storage type and minio base path in the packages section + iniStr = ` +[packages] +STORAGE_TYPE = my_minio +MINIO_BASE_PATH = my_packages/ + +[storage.my_minio] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadPackagesFrom(cfg)) + + assert.EqualValues(t, "minio", Packages.Storage.Type) + assert.EqualValues(t, "my_packages/", cfg.Section("packages").Key("MINIO_BASE_PATH").MustString("")) +} diff --git a/modules/setting/picture.go b/modules/setting/picture.go index 6d7c8b33ce46f..6b77d0eefd8af 100644 --- a/modules/setting/picture.go +++ b/modules/setting/picture.go @@ -3,6 +3,8 @@ package setting +import "code.gitea.io/gitea/modules/log" + // settings var ( // Picture settings @@ -32,7 +34,7 @@ var ( }{} ) -func loadPictureFrom(rootCfg ConfigProvider) { +func loadAvatarsFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("picture") avatarSec := rootCfg.Section("avatar") @@ -64,6 +66,13 @@ func loadPictureFrom(rootCfg ConfigProvider) { EnableFederatedAvatar = sec.Key("ENABLE_FEDERATED_AVATAR").MustBool(GetDefaultEnableFederatedAvatar(DisableGravatar)) deprecatedSettingDB(rootCfg, "", "ENABLE_FEDERATED_AVATAR") + return nil +} + +func loadPictureFrom(rootCfg ConfigProvider) { + if err := loadAvatarsFrom(rootCfg); err != nil { + log.Fatal("%v", err) + } loadRepoAvatarFrom(rootCfg) } @@ -82,7 +91,7 @@ func GetDefaultEnableFederatedAvatar(disableGravatar bool) bool { return v } -func loadRepoAvatarFrom(rootCfg ConfigProvider) { +func loadRepoAvatarFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("picture") repoAvatarSec := rootCfg.Section("repo-avatar") @@ -95,4 +104,6 @@ func loadRepoAvatarFrom(rootCfg ConfigProvider) { RepoAvatar.Fallback = sec.Key("REPOSITORY_AVATAR_FALLBACK").MustString("none") RepoAvatar.FallbackImage = sec.Key("REPOSITORY_AVATAR_FALLBACK_IMAGE").MustString("/assets/img/repo_default.png") + + return nil } diff --git a/modules/setting/repository.go b/modules/setting/repository.go index bae3c658a4855..771ffe9a9650e 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -261,10 +261,6 @@ var ( } RepoRootPath string ScriptType = "bash" - - RepoArchive = struct { - Storage - }{} ) func loadRepositoryFrom(rootCfg ConfigProvider) { @@ -351,5 +347,7 @@ func loadRepositoryFrom(rootCfg ConfigProvider) { Repository.Upload.TempPath = path.Join(AppWorkPath, Repository.Upload.TempPath) } - RepoArchive.Storage = getStorage(rootCfg, "repo-archive", "", nil) + if err := loadRepoArchiveFrom(rootCfg); err != nil { + log.Fatal("%v", err) + } } diff --git a/modules/setting/repository_archive.go b/modules/setting/repository_archive.go new file mode 100644 index 0000000000000..fc0d1010acaac --- /dev/null +++ b/modules/setting/repository_archive.go @@ -0,0 +1,20 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package setting + +import "fmt" + +var RepoArchive = struct { + Storage +}{} + +func loadRepoArchiveFrom(rootCfg ConfigProvider) error { + sec := rootCfg.Section("repo-archive") + if err := sec.MapTo(&RepoArchive); err != nil { + return fmt.Errorf("mapto repoarchive failed: %v", err) + } + storageType := sec.Key("STORAGE_TYPE").MustString("") + RepoArchive.Storage = getStorage(rootCfg, "repo-archive", storageType, sec) + return nil +} diff --git a/modules/setting/repository_archive_test.go b/modules/setting/repository_archive_test.go new file mode 100644 index 0000000000000..f9b63381bf16a --- /dev/null +++ b/modules/setting/repository_archive_test.go @@ -0,0 +1,68 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package setting + +import ( + "testing" + + "github.com/stretchr/testify/assert" + ini "gopkg.in/ini.v1" +) + +func Test_getStorageInheritNameSectionTypeForRepoArchive(t *testing.T) { + // packages storage inherits from storage if nothing configured + iniStr := ` +[storage] +STORAGE_TYPE = minio +` + cfg, err := ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadRepoArchiveFrom(cfg)) + + assert.EqualValues(t, "minio", RepoArchive.Storage.Type) + assert.EqualValues(t, "repo-archive/", cfg.Section("repo-archive").Key("MINIO_BASE_PATH").MustString("")) + + // we can also configure packages storage directly + iniStr = ` +[storage.repo-archive] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadRepoArchiveFrom(cfg)) + + assert.EqualValues(t, "minio", RepoArchive.Storage.Type) + assert.EqualValues(t, "repo-archive/", cfg.Section("repo-archive").Key("MINIO_BASE_PATH").MustString("")) + + // or we can indicate the storage type in the packages section + iniStr = ` +[repo-archive] +STORAGE_TYPE = my_minio + +[storage.my_minio] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadRepoArchiveFrom(cfg)) + + assert.EqualValues(t, "minio", RepoArchive.Storage.Type) + assert.EqualValues(t, "repo-archive/", cfg.Section("repo-archive").Key("MINIO_BASE_PATH").MustString("")) + + // or we can indicate the storage type and minio base path in the packages section + iniStr = ` +[repo-archive] +STORAGE_TYPE = my_minio +MINIO_BASE_PATH = my_archive/ + +[storage.my_minio] +STORAGE_TYPE = minio +` + cfg, err = ini.Load([]byte(iniStr)) + assert.NoError(t, err) + assert.NoError(t, loadRepoArchiveFrom(cfg)) + + assert.EqualValues(t, "minio", RepoArchive.Storage.Type) + assert.EqualValues(t, "my_archive/", cfg.Section("repo-archive").Key("MINIO_BASE_PATH").MustString("")) +} diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 4d7a7caab8de3..1075bc2a502a1 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -210,7 +210,9 @@ func InitProviderAllowEmpty() { // InitProviderAndLoadCommonSettingsForTest initializes config provider and load common setttings for tests func InitProviderAndLoadCommonSettingsForTest(extraConfigs ...string) { CfgProvider = newFileProviderFromConf(CustomConf, true, strings.Join(extraConfigs, "\n")) - loadCommonSettingsFrom(CfgProvider) + if err := loadCommonSettingsFrom(CfgProvider); err != nil { + log.Fatal("%v", err) + } if err := PrepareAppDataPath(); err != nil { log.Fatal("Can not prepare APP_DATA_PATH: %v", err) } @@ -249,11 +251,13 @@ func newFileProviderFromConf(customConf string, allowEmpty bool, extraConfig str // LoadCommonSettings loads common configurations from a configuration provider. func LoadCommonSettings() { - loadCommonSettingsFrom(CfgProvider) + if err := loadCommonSettingsFrom(CfgProvider); err != nil { + log.Fatal("%v", err) + } } // loadCommonSettingsFrom loads common configurations from a configuration provider. -func loadCommonSettingsFrom(cfg ConfigProvider) { +func loadCommonSettingsFrom(cfg ConfigProvider) error { // WARNNING: don't change the sequence except you know what you are doing. loadRunModeFrom(cfg) loadLogFrom(cfg) @@ -266,7 +270,9 @@ func loadCommonSettingsFrom(cfg ConfigProvider) { loadTimeFrom(cfg) loadRepositoryFrom(cfg) loadPictureFrom(cfg) - loadPackagesFrom(cfg) + if err := loadPackagesFrom(cfg); err != nil { + return err + } loadActionsFrom(cfg) loadUIFrom(cfg) loadAdminFrom(cfg) @@ -278,6 +284,7 @@ func loadCommonSettingsFrom(cfg ConfigProvider) { loadMirrorFrom(cfg) loadMarkupFrom(cfg) loadOtherFrom(cfg) + return nil } func loadRunModeFrom(rootCfg ConfigProvider) { diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go index 7737d233b9b06..797456b36adb8 100644 --- a/modules/setting/storage_test.go +++ b/modules/setting/storage_test.go @@ -10,106 +10,6 @@ import ( ini "gopkg.in/ini.v1" ) -func Test_getStorageCustomType(t *testing.T) { - iniStr := ` -[attachment] -STORAGE_TYPE = my_minio -MINIO_BUCKET = gitea-attachment - -[storage.my_minio] -STORAGE_TYPE = minio -MINIO_ENDPOINT = my_minio:9000 -` - cfg, err := ini.Load([]byte(iniStr)) - assert.NoError(t, err) - - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "minio", storage.Type) - assert.EqualValues(t, "my_minio:9000", storage.Section.Key("MINIO_ENDPOINT").String()) - assert.EqualValues(t, "gitea-attachment", storage.Section.Key("MINIO_BUCKET").String()) -} - -func Test_getStorageNameSectionOverridesTypeSection(t *testing.T) { - iniStr := ` -[attachment] -STORAGE_TYPE = minio - -[storage.attachments] -MINIO_BUCKET = gitea-attachment - -[storage.minio] -MINIO_BUCKET = gitea -` - cfg, err := ini.Load([]byte(iniStr)) - assert.NoError(t, err) - - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "minio", storage.Type) - assert.EqualValues(t, "gitea-attachment", storage.Section.Key("MINIO_BUCKET").String()) -} - -func Test_getStorageTypeSectionOverridesStorageSection(t *testing.T) { - iniStr := ` -[attachment] -STORAGE_TYPE = minio - -[storage.minio] -MINIO_BUCKET = gitea-minio - -[storage] -MINIO_BUCKET = gitea -` - cfg, err := ini.Load([]byte(iniStr)) - assert.NoError(t, err) - - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "minio", storage.Type) - assert.EqualValues(t, "gitea-minio", storage.Section.Key("MINIO_BUCKET").String()) -} - -func Test_getStorageSpecificOverridesStorage(t *testing.T) { - iniStr := ` -[attachment] -STORAGE_TYPE = minio -MINIO_BUCKET = gitea-attachment - -[storage.attachments] -MINIO_BUCKET = gitea - -[storage] -STORAGE_TYPE = local -` - cfg, err := ini.Load([]byte(iniStr)) - assert.NoError(t, err) - - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "minio", storage.Type) - assert.EqualValues(t, "gitea-attachment", storage.Section.Key("MINIO_BUCKET").String()) -} - -func Test_getStorageGetDefaults(t *testing.T) { - cfg, err := ini.Load([]byte("")) - assert.NoError(t, err) - - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "gitea", storage.Section.Key("MINIO_BUCKET").String()) -} - func Test_getStorageMultipleName(t *testing.T) { iniStr := ` [lfs] @@ -124,27 +24,14 @@ MINIO_BUCKET = gitea-storage cfg, err := ini.Load([]byte(iniStr)) assert.NoError(t, err) - { - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "gitea-attachment", storage.Section.Key("MINIO_BUCKET").String()) - } - { - sec := cfg.Section("lfs") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "lfs", storageType, sec) - - assert.EqualValues(t, "gitea-lfs", storage.Section.Key("MINIO_BUCKET").String()) - } - { - sec := cfg.Section("avatar") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "avatars", storageType, sec) - - assert.EqualValues(t, "gitea-storage", storage.Section.Key("MINIO_BUCKET").String()) - } + assert.NoError(t, loadAttachmentFrom(cfg)) + assert.EqualValues(t, "gitea-attachment", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) + + assert.NoError(t, loadLFSFrom(cfg)) + assert.EqualValues(t, "gitea-lfs", LFS.Storage.Section.Key("MINIO_BUCKET").String()) + + assert.NoError(t, loadAvatarsFrom(cfg)) + assert.EqualValues(t, "gitea-storage", Avatar.Storage.Section.Key("MINIO_BUCKET").String()) } func Test_getStorageUseOtherNameAsType(t *testing.T) { @@ -158,20 +45,11 @@ MINIO_BUCKET = gitea-storage cfg, err := ini.Load([]byte(iniStr)) assert.NoError(t, err) - { - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "gitea-storage", storage.Section.Key("MINIO_BUCKET").String()) - } - { - sec := cfg.Section("lfs") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "lfs", storageType, sec) + assert.NoError(t, loadAttachmentFrom(cfg)) + assert.EqualValues(t, "gitea-storage", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) - assert.EqualValues(t, "gitea-storage", storage.Section.Key("MINIO_BUCKET").String()) - } + assert.NoError(t, loadLFSFrom(cfg)) + assert.EqualValues(t, "gitea-storage", LFS.Storage.Section.Key("MINIO_BUCKET").String()) } func Test_getStorageInheritStorageType(t *testing.T) { @@ -182,24 +60,38 @@ STORAGE_TYPE = minio cfg, err := ini.Load([]byte(iniStr)) assert.NoError(t, err) - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "minio", storage.Type) -} - -func Test_getStorageInheritNameSectionType(t *testing.T) { - iniStr := ` -[storage.attachments] -STORAGE_TYPE = minio -` - cfg, err := ini.Load([]byte(iniStr)) - assert.NoError(t, err) - - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage := getStorage(cfg, "attachments", storageType, sec) - - assert.EqualValues(t, "minio", storage.Type) + assert.NoError(t, loadAttachmentFrom(cfg)) + assert.EqualValues(t, "minio", Attachment.Storage.Type) + assert.EqualValues(t, "gitea", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "attachments/", Attachment.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + + assert.NoError(t, loadLFSFrom(cfg)) + assert.EqualValues(t, "minio", LFS.Storage.Type) + assert.EqualValues(t, "gitea", LFS.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "lfs/", LFS.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + + assert.NoError(t, loadPackagesFrom(cfg)) + assert.EqualValues(t, "minio", Packages.Storage.Type) + assert.EqualValues(t, "gitea", Packages.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "packages/", Packages.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + + assert.NoError(t, loadRepoArchiveFrom(cfg)) + assert.EqualValues(t, "minio", RepoArchive.Storage.Type) + assert.EqualValues(t, "gitea", RepoArchive.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + + assert.NoError(t, loadActionsFrom(cfg)) + assert.EqualValues(t, "minio", Actions.Storage.Type) + assert.EqualValues(t, "gitea", Actions.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "actions_log/", Actions.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + + assert.NoError(t, loadAvatarsFrom(cfg)) + assert.EqualValues(t, "minio", Avatar.Storage.Type) + assert.EqualValues(t, "gitea", Avatar.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "avatars/", Avatar.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + + assert.NoError(t, loadRepoAvatarFrom(cfg)) + assert.EqualValues(t, "minio", RepoAvatar.Storage.Type) + assert.EqualValues(t, "gitea", RepoAvatar.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "repo-avatars/", RepoAvatar.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) } From ae66d8f6afa6a6dbee588097032c7f0eb5c8093b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 4 Apr 2023 15:29:58 +0800 Subject: [PATCH 02/28] Fix copyright year and some documentations --- .../doc/administration/config-cheat-sheet.en-us.md | 7 +++++++ modules/setting/actions_test.go | 2 +- modules/setting/attachment_test.go | 2 +- modules/setting/lfs_test.go | 2 +- modules/setting/picture.go | 9 --------- modules/setting/setting.go | 5 ++++- 6 files changed, 14 insertions(+), 13 deletions(-) diff --git a/docs/content/doc/administration/config-cheat-sheet.en-us.md b/docs/content/doc/administration/config-cheat-sheet.en-us.md index 67618d05b675e..2b3a4eac97c4e 100644 --- a/docs/content/doc/administration/config-cheat-sheet.en-us.md +++ b/docs/content/doc/administration/config-cheat-sheet.en-us.md @@ -1340,6 +1340,11 @@ is `data/repo-archive` and the default of `MINIO_BASE_PATH` is `repo-archive/`. - `MINIO_USE_SSL`: **false**: Minio enabled ssl only available when `STORAGE_TYPE` is `minio` - `MINIO_INSECURE_SKIP_VERIFY`: **false**: Minio skip SSL verification available when STORAGE_TYPE is `minio` +## Repository Archives (`repo-archive`) + +- `STORAGE_TYPE`: **local**: Storage type for actions logs, `local` for local disk or `minio` for s3 compatible object storage service, default is `local` or other name defined with `[storage.xxx]` +- `MINIO_BASE_PATH`: **actions_log/**: Minio base path on the bucket only available when STORAGE_TYPE is `minio` + ## Proxy (`proxy`) - `PROXY_ENABLED`: **false**: Enable the proxy if true, all requests to external via HTTP will be affected, if false, no proxy will be used even environment http_proxy/https_proxy @@ -1358,6 +1363,8 @@ PROXY_HOSTS = *.github.com - `ENABLED`: **false**: Enable/Disable actions capabilities - `DEFAULT_ACTIONS_URL`: **https://gitea.com**: Default address to get action plugins, e.g. the default value means downloading from "" for "uses: actions/checkout@v3" +- `STORAGE_TYPE`: **local**: Storage type for actions logs, `local` for local disk or `minio` for s3 compatible object storage service, default is `local` or other name defined with `[storage.xxx]` +- `MINIO_BASE_PATH`: **actions_log/**: Minio base path on the bucket only available when STORAGE_TYPE is `minio` `DEFAULT_ACTIONS_URL` indicates where should we find the relative path action plugin. i.e. when use an action in a workflow file like diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index 07316a01314af..6d7fc66e48d6e 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package setting diff --git a/modules/setting/attachment_test.go b/modules/setting/attachment_test.go index c427b9b94b299..c7ca66bb1582a 100644 --- a/modules/setting/attachment_test.go +++ b/modules/setting/attachment_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package setting diff --git a/modules/setting/lfs_test.go b/modules/setting/lfs_test.go index 4d902ccf71a5e..f96874baeccc0 100644 --- a/modules/setting/lfs_test.go +++ b/modules/setting/lfs_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package setting diff --git a/modules/setting/picture.go b/modules/setting/picture.go index 6b77d0eefd8af..2232aff508f8d 100644 --- a/modules/setting/picture.go +++ b/modules/setting/picture.go @@ -3,8 +3,6 @@ package setting -import "code.gitea.io/gitea/modules/log" - // settings var ( // Picture settings @@ -69,13 +67,6 @@ func loadAvatarsFrom(rootCfg ConfigProvider) error { return nil } -func loadPictureFrom(rootCfg ConfigProvider) { - if err := loadAvatarsFrom(rootCfg); err != nil { - log.Fatal("%v", err) - } - loadRepoAvatarFrom(rootCfg) -} - func GetDefaultDisableGravatar() bool { return OfflineMode } diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 1075bc2a502a1..3ac51441e0d7e 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -269,7 +269,10 @@ func loadCommonSettingsFrom(cfg ConfigProvider) error { loadLFSFrom(cfg) loadTimeFrom(cfg) loadRepositoryFrom(cfg) - loadPictureFrom(cfg) + if err := loadAvatarsFrom(cfg); err != nil { + return err + } + loadRepoAvatarFrom(cfg) if err := loadPackagesFrom(cfg); err != nil { return err } From f70d4c632649706f77f38a55d18092a58e4767d5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 4 Apr 2023 21:27:55 +0800 Subject: [PATCH 03/28] Fix lint --- modules/setting/setting.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 3ac51441e0d7e..3f2b70bf4e84d 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -265,18 +265,26 @@ func loadCommonSettingsFrom(cfg ConfigProvider) error { loadSSHFrom(cfg) loadOAuth2From(cfg) loadSecurityFrom(cfg) - loadAttachmentFrom(cfg) - loadLFSFrom(cfg) + if err := loadAttachmentFrom(cfg); err != nil { + return err + } + if err := loadLFSFrom(cfg); err != nil { + return err + } loadTimeFrom(cfg) loadRepositoryFrom(cfg) if err := loadAvatarsFrom(cfg); err != nil { return err } - loadRepoAvatarFrom(cfg) + if err := loadRepoAvatarFrom(cfg); err != nil { + return err + } if err := loadPackagesFrom(cfg); err != nil { return err } - loadActionsFrom(cfg) + if err := loadActionsFrom(cfg); err != nil { + return err + } loadUIFrom(cfg) loadAdminFrom(cfg) loadAPIFrom(cfg) From 40407b56d26c28e7d46fae55472c10c9e928c8c5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 10 Apr 2023 17:38:14 +0800 Subject: [PATCH 04/28] add some comments for getStorage --- modules/setting/storage.go | 42 ++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 50d4c8439e2b1..556a1d5a133d0 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -6,6 +6,7 @@ package setting import ( "path/filepath" "reflect" + "sync" ini "gopkg.in/ini.v1" ) @@ -30,20 +31,35 @@ func (s *Storage) MapTo(v interface{}) error { return nil } -func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section) Storage { - const sectionName = "storage" - sec := rootCfg.Section(sectionName) +const sectionName = "storage" - // Global Defaults - sec.Key("MINIO_ENDPOINT").MustString("localhost:9000") - sec.Key("MINIO_ACCESS_KEY_ID").MustString("") - sec.Key("MINIO_SECRET_ACCESS_KEY").MustString("") - sec.Key("MINIO_BUCKET").MustString("gitea") - sec.Key("MINIO_LOCATION").MustString("us-east-1") - sec.Key("MINIO_USE_SSL").MustBool(false) - sec.Key("MINIO_INSECURE_SKIP_VERIFY").MustBool(false) - sec.Key("MINIO_CHECKSUM_ALGORITHM").MustString("default") +var ( + storageSec *ini.Section + storageSecOnce sync.Once +) +func getStorageSection(rootCfg ConfigProvider) *ini.Section { + storageSecOnce.Do(func() { + storageSec = rootCfg.Section(sectionName) + // Global Defaults + storageSec.Key("MINIO_ENDPOINT").MustString("localhost:9000") + storageSec.Key("MINIO_ACCESS_KEY_ID").MustString("") + storageSec.Key("MINIO_SECRET_ACCESS_KEY").MustString("") + storageSec.Key("MINIO_BUCKET").MustString("gitea") + storageSec.Key("MINIO_LOCATION").MustString("us-east-1") + storageSec.Key("MINIO_USE_SSL").MustBool(false) + storageSec.Key("MINIO_INSECURE_SKIP_VERIFY").MustBool(false) + storageSec.Key("MINIO_CHECKSUM_ALGORITHM").MustString("default") + }) + return storageSec +} + +// getStorage will read storage configurations from 4 possible ways +// 1 read configurations from [$name] if the item exist +// 2 read configurations from [storage.$name] if the item exist +// 3 read configurations from [storage.$typ] if the item exist +// 4 read configurations from [storage] if the item exist +func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section) Storage { if targetSec == nil { targetSec, _ = rootCfg.NewSection(name) } @@ -66,7 +82,7 @@ func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section storage.Type = nextType // Support custom STORAGE_TYPE } } - overrides = append(overrides, sec) + overrides = append(overrides, getStorageSection(rootCfg)) for _, override := range overrides { for _, key := range override.Keys() { From 7d16902222980f713ebc16604ed4fcccd5b54967 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 10 Apr 2023 20:40:34 +0800 Subject: [PATCH 05/28] Fix bug --- modules/setting/storage.go | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 556a1d5a133d0..fbe1181bb5c7d 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -6,7 +6,6 @@ package setting import ( "path/filepath" "reflect" - "sync" ini "gopkg.in/ini.v1" ) @@ -31,26 +30,19 @@ func (s *Storage) MapTo(v interface{}) error { return nil } -const sectionName = "storage" - -var ( - storageSec *ini.Section - storageSecOnce sync.Once -) +const storageSectionName = "storage" func getStorageSection(rootCfg ConfigProvider) *ini.Section { - storageSecOnce.Do(func() { - storageSec = rootCfg.Section(sectionName) - // Global Defaults - storageSec.Key("MINIO_ENDPOINT").MustString("localhost:9000") - storageSec.Key("MINIO_ACCESS_KEY_ID").MustString("") - storageSec.Key("MINIO_SECRET_ACCESS_KEY").MustString("") - storageSec.Key("MINIO_BUCKET").MustString("gitea") - storageSec.Key("MINIO_LOCATION").MustString("us-east-1") - storageSec.Key("MINIO_USE_SSL").MustBool(false) - storageSec.Key("MINIO_INSECURE_SKIP_VERIFY").MustBool(false) - storageSec.Key("MINIO_CHECKSUM_ALGORITHM").MustString("default") - }) + storageSec := rootCfg.Section(storageSectionName) + // Global Defaults + storageSec.Key("MINIO_ENDPOINT").MustString("localhost:9000") + storageSec.Key("MINIO_ACCESS_KEY_ID").MustString("") + storageSec.Key("MINIO_SECRET_ACCESS_KEY").MustString("") + storageSec.Key("MINIO_BUCKET").MustString("gitea") + storageSec.Key("MINIO_LOCATION").MustString("us-east-1") + storageSec.Key("MINIO_USE_SSL").MustBool(false) + storageSec.Key("MINIO_INSECURE_SKIP_VERIFY").MustBool(false) + storageSec.Key("MINIO_CHECKSUM_ALGORITHM").MustString("default") return storageSec } @@ -69,12 +61,12 @@ func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section storage.Type = typ overrides := make([]*ini.Section, 0, 3) - nameSec, err := rootCfg.GetSection(sectionName + "." + name) + nameSec, err := rootCfg.GetSection(storageSectionName + "." + name) if err == nil { overrides = append(overrides, nameSec) } - typeSec, err := rootCfg.GetSection(sectionName + "." + typ) + typeSec, err := rootCfg.GetSection(storageSectionName + "." + typ) if err == nil { overrides = append(overrides, typeSec) nextType := typeSec.Key("STORAGE_TYPE").String() From e85e9fb306019f4b5dfd9d635b2278391aeb1a88 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 12 Apr 2023 16:18:47 +0800 Subject: [PATCH 06/28] Update modules/setting/storage.go Co-authored-by: wxiaoguang --- modules/setting/storage.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index fbe1181bb5c7d..6d33a5bf4e7ed 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -47,10 +47,11 @@ func getStorageSection(rootCfg ConfigProvider) *ini.Section { } // getStorage will read storage configurations from 4 possible ways -// 1 read configurations from [$name] if the item exist -// 2 read configurations from [storage.$name] if the item exist -// 3 read configurations from [storage.$typ] if the item exist -// 4 read configurations from [storage] if the item exist +// 1 read configurations from [$name] if the setting keys exist (eg: name="attachments") +// 2 read configurations from [storage.$name] if the keys exist +// 3 read configurations from [storage.$type] if the keys exist (eg: type="local" or "minio") +// 4 read configurations from [storage] if the keys exist +// The keys in earlier section have higher priority. func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section) Storage { if targetSec == nil { targetSec, _ = rootCfg.NewSection(name) From 48113e101e9b8276b8851aa67e9e69a64ee0da72 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 4 May 2023 21:18:18 +0800 Subject: [PATCH 07/28] Remove support for [actions] storage type because actions may have many storages --- custom/conf/app.example.ini | 2 -- modules/setting/actions.go | 3 ++- modules/setting/actions_test.go | 29 ----------------------------- modules/setting/repository.go | 2 +- 4 files changed, 3 insertions(+), 33 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 0f665b66724e1..a590312f78726 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2618,8 +2618,6 @@ ROUTER = console ;; Enable/Disable actions capabilities ;ENABLED = false ;; -;STORAGE_TYPE = local -;; ;; Default address to get action plugins, e.g. the default value means downloading from "https://gitea.com/actions/checkout" for "uses: actions/checkout@v3" ;DEFAULT_ACTIONS_URL = https://gitea.com diff --git a/modules/setting/actions.go b/modules/setting/actions.go index ade7ee9e49c65..d9cb9926d464c 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -25,6 +25,7 @@ func loadActionsFrom(rootCfg ConfigProvider) error { sec.Key("MINIO_BASE_PATH").MustString("actions_log/") storageType := sec.Key("STORAGE_TYPE").MustString("") - Actions.Storage = getStorage(rootCfg, "actions_log", storageType, sec) + // don't support to read configuration from [actions] + Actions.Storage = getStorage(rootCfg, "actions_log", storageType, nil) return nil } diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index 6d7fc66e48d6e..baaff7ebbabc2 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -32,33 +32,4 @@ STORAGE_TYPE = minio assert.EqualValues(t, "minio", Actions.Storage.Type) assert.EqualValues(t, "actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) - - iniStr = ` -[actions] -STORAGE_TYPE = my_minio - -[storage.my_minio] -STORAGE_TYPE = minio -` - cfg, err = ini.Load([]byte(iniStr)) - assert.NoError(t, err) - assert.NoError(t, loadActionsFrom(cfg)) - - assert.EqualValues(t, "minio", Actions.Storage.Type) - assert.EqualValues(t, "actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) - - iniStr = ` -[actions] -STORAGE_TYPE = my_minio -MINIO_BASE_PATH = my_actions_log/ - -[storage.my_minio] -STORAGE_TYPE = minio -` - cfg, err = ini.Load([]byte(iniStr)) - assert.NoError(t, err) - assert.NoError(t, loadActionsFrom(cfg)) - - assert.EqualValues(t, "minio", Actions.Storage.Type) - assert.EqualValues(t, "my_actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) } diff --git a/modules/setting/repository.go b/modules/setting/repository.go index d2437aa1a22f1..2b8bb3fceae88 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -348,6 +348,6 @@ func loadRepositoryFrom(rootCfg ConfigProvider) { } if err := loadRepoArchiveFrom(rootCfg); err != nil { - log.Fatal("%v", err) + log.Fatal("loadRepoArchiveFrom: %v", err) } } From 49ed13813fc03a3d44ee257ec26d6285c35ad538 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 5 May 2023 12:09:22 +0800 Subject: [PATCH 08/28] Fix lint --- modules/setting/actions_test.go | 5 ++--- modules/setting/attachment_test.go | 13 ++++++------- modules/setting/lfs_test.go | 9 ++++----- modules/setting/packages_test.go | 8 ++++---- modules/setting/repository_archive_test.go | 9 ++++----- modules/setting/setting.go | 6 ++++-- 6 files changed, 24 insertions(+), 26 deletions(-) diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index baaff7ebbabc2..14a5fad5b0853 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - ini "gopkg.in/ini.v1" ) func Test_getStorageInheritNameSectionTypeForActions(t *testing.T) { @@ -15,7 +14,7 @@ func Test_getStorageInheritNameSectionTypeForActions(t *testing.T) { [storage] STORAGE_TYPE = minio ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadActionsFrom(cfg)) @@ -26,7 +25,7 @@ func Test_getStorageInheritNameSectionTypeForActions(t *testing.T) { [storage.actions_log] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadActionsFrom(cfg)) diff --git a/modules/setting/attachment_test.go b/modules/setting/attachment_test.go index c7ca66bb1582a..b4983810a899d 100644 --- a/modules/setting/attachment_test.go +++ b/modules/setting/attachment_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - ini "gopkg.in/ini.v1" ) func Test_getStorageCustomType(t *testing.T) { @@ -20,7 +19,7 @@ MINIO_BUCKET = gitea-attachment STORAGE_TYPE = minio MINIO_ENDPOINT = my_minio:9000 ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadAttachmentFrom(cfg)) @@ -41,7 +40,7 @@ MINIO_BUCKET = gitea-attachment [storage.minio] MINIO_BUCKET = gitea ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadAttachmentFrom(cfg)) @@ -61,7 +60,7 @@ MINIO_BUCKET = gitea-minio [storage] MINIO_BUCKET = gitea ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadAttachmentFrom(cfg)) @@ -82,7 +81,7 @@ MINIO_BUCKET = gitea [storage] STORAGE_TYPE = local ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadAttachmentFrom(cfg)) @@ -92,7 +91,7 @@ STORAGE_TYPE = local } func Test_getStorageGetDefaults(t *testing.T) { - cfg, err := ini.Load([]byte("")) + cfg, err := newConfigProviderFromData("") assert.NoError(t, err) assert.NoError(t, loadAttachmentFrom(cfg)) @@ -105,7 +104,7 @@ func Test_getStorageInheritNameSectionType(t *testing.T) { [storage.attachments] STORAGE_TYPE = minio ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadAttachmentFrom(cfg)) diff --git a/modules/setting/lfs_test.go b/modules/setting/lfs_test.go index f96874baeccc0..266748eae6ca8 100644 --- a/modules/setting/lfs_test.go +++ b/modules/setting/lfs_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - ini "gopkg.in/ini.v1" ) func Test_getStorageInheritNameSectionTypeForLFS(t *testing.T) { @@ -15,7 +14,7 @@ func Test_getStorageInheritNameSectionTypeForLFS(t *testing.T) { [storage] STORAGE_TYPE = minio ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadLFSFrom(cfg)) @@ -26,7 +25,7 @@ func Test_getStorageInheritNameSectionTypeForLFS(t *testing.T) { [storage.lfs] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadLFSFrom(cfg)) @@ -40,7 +39,7 @@ STORAGE_TYPE = my_minio [storage.my_minio] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadLFSFrom(cfg)) @@ -55,7 +54,7 @@ MINIO_BASE_PATH = my_lfs/ [storage.my_minio] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadLFSFrom(cfg)) diff --git a/modules/setting/packages_test.go b/modules/setting/packages_test.go index 25e1d38e1a953..39494c5d8a7b0 100644 --- a/modules/setting/packages_test.go +++ b/modules/setting/packages_test.go @@ -35,7 +35,7 @@ func Test_getStorageInheritNameSectionTypeForPackages(t *testing.T) { [storage] STORAGE_TYPE = minio ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadPackagesFrom(cfg)) @@ -47,7 +47,7 @@ STORAGE_TYPE = minio [storage.packages] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadPackagesFrom(cfg)) @@ -62,7 +62,7 @@ STORAGE_TYPE = my_minio [storage.my_minio] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadPackagesFrom(cfg)) @@ -78,7 +78,7 @@ MINIO_BASE_PATH = my_packages/ [storage.my_minio] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadPackagesFrom(cfg)) diff --git a/modules/setting/repository_archive_test.go b/modules/setting/repository_archive_test.go index f9b63381bf16a..eb0d85f5e9afd 100644 --- a/modules/setting/repository_archive_test.go +++ b/modules/setting/repository_archive_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - ini "gopkg.in/ini.v1" ) func Test_getStorageInheritNameSectionTypeForRepoArchive(t *testing.T) { @@ -16,7 +15,7 @@ func Test_getStorageInheritNameSectionTypeForRepoArchive(t *testing.T) { [storage] STORAGE_TYPE = minio ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadRepoArchiveFrom(cfg)) @@ -28,7 +27,7 @@ STORAGE_TYPE = minio [storage.repo-archive] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadRepoArchiveFrom(cfg)) @@ -43,7 +42,7 @@ STORAGE_TYPE = my_minio [storage.my_minio] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadRepoArchiveFrom(cfg)) @@ -59,7 +58,7 @@ MINIO_BASE_PATH = my_archive/ [storage.my_minio] STORAGE_TYPE = minio ` - cfg, err = ini.Load([]byte(iniStr)) + cfg, err = newConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadRepoArchiveFrom(cfg)) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 31af2dbd568da..1681298074d9e 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -209,10 +209,12 @@ func Init(opts *Options) { var err error CfgProvider, err = newConfigProviderFromFile(opts) if err != nil { - log.Fatal("Init[%v]: %v", opts, err) + log.Fatal("newConfigProviderFromFile[%v]: %v", opts, err) } if !opts.DisableLoadCommonSettings { - loadCommonSettingsFrom(CfgProvider) + if err := loadCommonSettingsFrom(CfgProvider); err != nil { + log.Fatal("loadCommonSettingsFrom[%v]: %v", opts, err) + } } } From a781649fd82c83e2ccbfbb0434133ecf232abd27 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 5 May 2023 12:54:27 +0800 Subject: [PATCH 09/28] Fix read storage configuration from actions --- modules/setting/actions.go | 4 +--- modules/setting/actions_test.go | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/modules/setting/actions.go b/modules/setting/actions.go index d9cb9926d464c..0392d610f8f24 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -22,10 +22,8 @@ func loadActionsFrom(rootCfg ConfigProvider) error { if err := sec.MapTo(&Actions); err != nil { return fmt.Errorf("failed to map Actions settings: %v", err) } - sec.Key("MINIO_BASE_PATH").MustString("actions_log/") - storageType := sec.Key("STORAGE_TYPE").MustString("") // don't support to read configuration from [actions] - Actions.Storage = getStorage(rootCfg, "actions_log", storageType, nil) + Actions.Storage = getStorage(rootCfg, "actions_log", "", nil) return nil } diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index 14a5fad5b0853..54688d99c357f 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -19,7 +19,7 @@ func Test_getStorageInheritNameSectionTypeForActions(t *testing.T) { assert.NoError(t, loadActionsFrom(cfg)) assert.EqualValues(t, "minio", Actions.Storage.Type) - assert.EqualValues(t, "actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "actions_log/", Actions.Storage.Section.Key("MINIO_BASE_PATH").String()) iniStr = ` [storage.actions_log] @@ -30,5 +30,5 @@ STORAGE_TYPE = minio assert.NoError(t, loadActionsFrom(cfg)) assert.EqualValues(t, "minio", Actions.Storage.Type) - assert.EqualValues(t, "actions_log/", cfg.Section("actions").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "actions_log/", Actions.Storage.Section.Key("MINIO_BASE_PATH").String()) } From 93714fdd9a7417e186235099892bc04194022a87 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 5 May 2023 18:54:26 +0800 Subject: [PATCH 10/28] refactor getStorage function --- modules/setting/actions.go | 11 +++-- modules/setting/actions_test.go | 14 ++++++ modules/setting/attachment.go | 10 ++-- modules/setting/config_provider.go | 12 ++++- modules/setting/lfs.go | 8 ++- modules/setting/packages.go | 7 +-- modules/setting/picture.go | 16 ++++-- modules/setting/repository_archive.go | 7 +-- modules/setting/storage.go | 70 ++++++++++++++++----------- routers/web/web.go | 4 +- 10 files changed, 107 insertions(+), 52 deletions(-) diff --git a/modules/setting/actions.go b/modules/setting/actions.go index 0392d610f8f24..ad9087cd544af 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -3,12 +3,14 @@ package setting -import "fmt" +import ( + "fmt" +) // Actions settings var ( Actions = struct { - Storage // how the created logs should be stored + *Storage // how the created logs should be stored Enabled bool DefaultActionsURL string `ini:"DEFAULT_ACTIONS_URL"` }{ @@ -24,6 +26,7 @@ func loadActionsFrom(rootCfg ConfigProvider) error { } // don't support to read configuration from [actions] - Actions.Storage = getStorage(rootCfg, "actions_log", "", nil) - return nil + var err error + Actions.Storage, err = getStorage(rootCfg, nil, "actions_log", "") + return err } diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index 54688d99c357f..13b2c8e855530 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -31,4 +31,18 @@ STORAGE_TYPE = minio assert.EqualValues(t, "minio", Actions.Storage.Type) assert.EqualValues(t, "actions_log/", Actions.Storage.Section.Key("MINIO_BASE_PATH").String()) + + iniStr = ` +[storage.actions_log] +STORAGE_TYPE = my_storage + +[storage.my_storage] +STORAGE_TYPE = minio +` + cfg, err = newConfigProviderFromData(iniStr) + assert.NoError(t, err) + assert.NoError(t, loadActionsFrom(cfg)) + + assert.EqualValues(t, "minio", Actions.Storage.Type) + assert.EqualValues(t, "actions_log/", Actions.Storage.Section.Key("MINIO_BASE_PATH").String()) } diff --git a/modules/setting/attachment.go b/modules/setting/attachment.go index 4121fb87ae2be..6fed5a43dab33 100644 --- a/modules/setting/attachment.go +++ b/modules/setting/attachment.go @@ -5,13 +5,13 @@ package setting // Attachment settings var Attachment = struct { - Storage + *Storage AllowedTypes string MaxSize int64 MaxFiles int Enabled bool }{ - Storage: Storage{ + Storage: &Storage{ ServeDirect: false, }, AllowedTypes: "image/jpeg,image/png,application/zip,application/gzip", @@ -24,7 +24,11 @@ func loadAttachmentFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("attachment") storageType := sec.Key("STORAGE_TYPE").MustString("") - Attachment.Storage = getStorage(rootCfg, "attachments", storageType, sec) + var err error + Attachment.Storage, err = getStorage(rootCfg, sec, "attachments", storageType) + if err != nil { + return err + } Attachment.AllowedTypes = sec.Key("ALLOWED_TYPES").MustString(".csv,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.xls,.xlsx,.zip") Attachment.MaxSize = sec.Key("MAX_SIZE").MustInt64(4) diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 1685958298635..4fc3a353f0698 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -114,11 +114,19 @@ func (p *iniFileConfigProvider) Section(section string) ConfigSection { } func (p *iniFileConfigProvider) NewSection(name string) (ConfigSection, error) { - return p.File.NewSection(name) + sec, err := p.File.NewSection(name) + if err != nil { + return nil, err + } + return sec, nil } func (p *iniFileConfigProvider) GetSection(name string) (ConfigSection, error) { - return p.File.GetSection(name) + sec, err := p.File.GetSection(name) + if err != nil { + return nil, err + } + return sec, nil } func (p *iniFileConfigProvider) DeleteSection(name string) error { diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 0cfc924ae559b..bd9fb5b6a8c5a 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -20,7 +20,7 @@ var LFS = struct { MaxFileSize int64 `ini:"LFS_MAX_FILE_SIZE"` LocksPagingNum int `ini:"LFS_LOCKS_PAGING_NUM"` - Storage + *Storage }{} func loadLFSFrom(rootCfg ConfigProvider) error { @@ -38,7 +38,11 @@ func loadLFSFrom(rootCfg ConfigProvider) error { deprecatedSetting(rootCfg, "server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0") lfsSec.Key("PATH").MustString(sec.Key("LFS_CONTENT_PATH").String()) - LFS.Storage = getStorage(rootCfg, "lfs", storageType, lfsSec) + var err error + LFS.Storage, err = getStorage(rootCfg, lfsSec, "lfs", storageType) + if err != nil { + return err + } // Rest of LFS service settings if LFS.LocksPagingNum == 0 { diff --git a/modules/setting/packages.go b/modules/setting/packages.go index 0b955b4508b7e..5dfbb556eb604 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -16,7 +16,7 @@ import ( // Package registry settings var ( Packages = struct { - Storage + *Storage Enabled bool ChunkedUploadPath string RegistryHost string @@ -48,13 +48,14 @@ var ( func loadPackagesFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("packages") - if err := sec.MapTo(&Packages); err != nil { + err := sec.MapTo(&Packages) + if err != nil { return fmt.Errorf("failed to map Packages settings: %v", err) } storageType := sec.Key("STORAGE_TYPE").MustString("") - Packages.Storage = getStorage(rootCfg, "packages", storageType, sec) + Packages.Storage, err = getStorage(rootCfg, sec, "packages", storageType) appURL, _ := url.Parse(AppURL) Packages.RegistryHost = appURL.Host diff --git a/modules/setting/picture.go b/modules/setting/picture.go index 2232aff508f8d..c14de55dbef72 100644 --- a/modules/setting/picture.go +++ b/modules/setting/picture.go @@ -7,7 +7,7 @@ package setting var ( // Picture settings Avatar = struct { - Storage + *Storage MaxWidth int MaxHeight int @@ -25,7 +25,7 @@ var ( EnableFederatedAvatar bool // Depreciated: migrated to database RepoAvatar = struct { - Storage + *Storage Fallback string FallbackImage string @@ -41,7 +41,11 @@ func loadAvatarsFrom(rootCfg ConfigProvider) error { avatarSec.Key("PATH").MustString( sec.Key("AVATAR_UPLOAD_PATH").String()) - Avatar.Storage = getStorage(rootCfg, "avatars", storageType, avatarSec) + var err error + Avatar.Storage, err = getStorage(rootCfg, avatarSec, "avatars", storageType) + if err != nil { + return err + } Avatar.MaxWidth = sec.Key("AVATAR_MAX_WIDTH").MustInt(4096) Avatar.MaxHeight = sec.Key("AVATAR_MAX_HEIGHT").MustInt(3072) @@ -91,7 +95,11 @@ func loadRepoAvatarFrom(rootCfg ConfigProvider) error { repoAvatarSec.Key("PATH").MustString( sec.Key("REPOSITORY_AVATAR_UPLOAD_PATH").String()) - RepoAvatar.Storage = getStorage(rootCfg, "repo-avatars", storageType, repoAvatarSec) + var err error + RepoAvatar.Storage, err = getStorage(rootCfg, repoAvatarSec, "repo-avatars", storageType) + if err != nil { + return err + } RepoAvatar.Fallback = sec.Key("REPOSITORY_AVATAR_FALLBACK").MustString("none") RepoAvatar.FallbackImage = sec.Key("REPOSITORY_AVATAR_FALLBACK_IMAGE").MustString("/assets/img/repo_default.png") diff --git a/modules/setting/repository_archive.go b/modules/setting/repository_archive.go index fc0d1010acaac..c734cd976f4e5 100644 --- a/modules/setting/repository_archive.go +++ b/modules/setting/repository_archive.go @@ -6,7 +6,7 @@ package setting import "fmt" var RepoArchive = struct { - Storage + *Storage }{} func loadRepoArchiveFrom(rootCfg ConfigProvider) error { @@ -15,6 +15,7 @@ func loadRepoArchiveFrom(rootCfg ConfigProvider) error { return fmt.Errorf("mapto repoarchive failed: %v", err) } storageType := sec.Key("STORAGE_TYPE").MustString("") - RepoArchive.Storage = getStorage(rootCfg, "repo-archive", storageType, sec) - return nil + var err error + RepoArchive.Storage, err = getStorage(rootCfg, sec, "repo-archive", storageType) + return err } diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 4fd5a8081ceb5..14b2fbdbafef2 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -4,6 +4,8 @@ package setting import ( + "errors" + "fmt" "path/filepath" "reflect" ) @@ -50,50 +52,60 @@ func getStorageSection(rootCfg ConfigProvider) ConfigSection { // 3 read configurations from [storage.$type] if the keys exist (eg: type="local" or "minio") // 4 read configurations from [storage] if the keys exist // The keys in earlier section have higher priority. -func getStorage(rootCfg ConfigProvider, name, typ string, targetSec ConfigSection) Storage { - if targetSec == nil { - targetSec, _ = rootCfg.NewSection(name) - } - - var storage Storage - storage.Section = targetSec - storage.Type = typ - - overrides := make([]ConfigSection, 0, 3) - nameSec, err := rootCfg.GetSection(storageSectionName + "." + name) - if err == nil { - overrides = append(overrides, nameSec) +func getStorage(rootCfg ConfigProvider, startSec ConfigSection, name, typ string) (*Storage, error) { + if name == "" { + return nil, errors.New("getStorage: name cannot be empty") } - typeSec, err := rootCfg.GetSection(storageSectionName + "." + typ) - if err == nil { - overrides = append(overrides, typeSec) - nextType := typeSec.Key("STORAGE_TYPE").String() - if len(nextType) > 0 { - storage.Type = nextType // Support custom STORAGE_TYPE + targetSec := startSec + if targetSec == nil { + targetSec, _ = rootCfg.GetSection(storageSectionName + "." + name) + } else { + if targetSec.Key("STORAGE_TYPE").String() == "" { + targetSec = nil } } - overrides = append(overrides, getStorageSection(rootCfg)) - for _, override := range overrides { - for _, key := range override.Keys() { - if !targetSec.HasKey(key.Name()) { - _, _ = targetSec.NewKey(key.Name(), key.Value()) + if targetSec == nil && typ != "" { + targetSec, _ = rootCfg.GetSection(storageSectionName + "." + typ) + } + if targetSec == nil { + targetSec, _ = rootCfg.GetSection(storageSectionName) + } + if targetSec == nil { // finally fallback + targetSec = startSec + if targetSec == nil { + var err error + targetSec, err = rootCfg.NewSection(storageSectionName + "." + name) + if err != nil { + return nil, err } } - if len(storage.Type) == 0 { - storage.Type = override.Key("STORAGE_TYPE").String() + } + + storageType := targetSec.Key("STORAGE_TYPE").MustString("local") + if storageType != "local" && storageType != "minio" { + var err error + targetSec, err = rootCfg.GetSection(storageSectionName + "." + storageType) + if err != nil { + return nil, fmt.Errorf("unknown storage type: %s", storageType) } + storageType = targetSec.Key("STORAGE_TYPE").String() + } + + storage := Storage{ + Section: targetSec, + Type: storageType, + ServeDirect: targetSec.Key("SERVE_DIRECT").MustBool(false), + Path: targetSec.Key("PATH").MustString(filepath.Join(AppDataPath, name)), } - storage.ServeDirect = storage.Section.Key("SERVE_DIRECT").MustBool(false) // 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 + return &storage, nil } diff --git a/routers/web/web.go b/routers/web/web.go index e904db321d816..5cf451cff0dfa 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -109,8 +109,8 @@ func Routes(ctx gocontext.Context) *web.Route { routes.Head("/", misc.DummyOK) // for health check - doesn't need to be passed through gzip handler routes.RouteMethods("/assets/*", "GET, HEAD", CorsHandler(), public.AssetsHandlerFunc("/assets/")) - routes.RouteMethods("/avatars/*", "GET, HEAD", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars)) - routes.RouteMethods("/repo-avatars/*", "GET, HEAD", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) + routes.RouteMethods("/avatars/*", "GET, HEAD", storageHandler(*setting.Avatar.Storage, "avatars", storage.Avatars)) + routes.RouteMethods("/repo-avatars/*", "GET, HEAD", storageHandler(*setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) routes.RouteMethods("/apple-touch-icon.png", "GET, HEAD", misc.StaticRedirect("/assets/img/apple-touch-icon.png")) routes.RouteMethods("/favicon.ico", "GET, HEAD", misc.StaticRedirect("/assets/img/favicon.png")) From 163d086817fe2392839f67223fab32016564f6b1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 5 May 2023 19:01:07 +0800 Subject: [PATCH 11/28] Fix comment --- modules/setting/storage.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 14b2fbdbafef2..064f297b62ee5 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -47,7 +47,7 @@ func getStorageSection(rootCfg ConfigProvider) ConfigSection { } // getStorage will read storage configurations from 4 possible ways -// 1 read configurations from [$name] if the setting keys exist (eg: name="attachments") +// 1 read configurations from given section if the setting keys exist (eg: name="attachments") // 2 read configurations from [storage.$name] if the keys exist // 3 read configurations from [storage.$type] if the keys exist (eg: type="local" or "minio") // 4 read configurations from [storage] if the keys exist @@ -75,11 +75,7 @@ func getStorage(rootCfg ConfigProvider, startSec ConfigSection, name, typ string if targetSec == nil { // finally fallback targetSec = startSec if targetSec == nil { - var err error - targetSec, err = rootCfg.NewSection(storageSectionName + "." + name) - if err != nil { - return nil, err - } + targetSec = getStorageSection(rootCfg) } } From fa85e0629734e4b5d9d5987a458661f5b0738c41 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 6 May 2023 10:41:30 +0800 Subject: [PATCH 12/28] Fix storage override sequence --- modules/setting/storage.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 064f297b62ee5..b05c1baeeb599 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -47,22 +47,23 @@ func getStorageSection(rootCfg ConfigProvider) ConfigSection { } // getStorage will read storage configurations from 4 possible ways -// 1 read configurations from given section if the setting keys exist (eg: name="attachments") -// 2 read configurations from [storage.$name] if the keys exist +// 1 read configurations from [storage.$name] if the keys exist +// 2 read configurations from given section if the setting keys exist (eg: name="attachments") // 3 read configurations from [storage.$type] if the keys exist (eg: type="local" or "minio") // 4 read configurations from [storage] if the keys exist // The keys in earlier section have higher priority. -func getStorage(rootCfg ConfigProvider, startSec ConfigSection, name, typ string) (*Storage, error) { +func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ string) (*Storage, error) { if name == "" { return nil, errors.New("getStorage: name cannot be empty") } - targetSec := startSec + nameSec, _ := rootCfg.GetSection(storageSectionName + "." + name) + + targetSec := nameSec if targetSec == nil { - targetSec, _ = rootCfg.GetSection(storageSectionName + "." + name) - } else { - if targetSec.Key("STORAGE_TYPE").String() == "" { - targetSec = nil + targetSec = startSec + if targetSec != nil && targetSec.Key("STORAGE_TYPE").String() == "" { + targetSec = nil // startSec's STORAGE_TYPE could be ignored } } From e3aa03efce7bfbb35ee66d4d0333048ec0acb2e8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 7 May 2023 14:17:03 +0800 Subject: [PATCH 13/28] Improve getStorage --- modules/setting/actions.go | 2 +- modules/setting/attachment.go | 3 +-- modules/setting/lfs.go | 3 +-- modules/setting/packages.go | 7 ++++--- modules/setting/picture.go | 7 +++---- modules/setting/repository_archive.go | 4 ++-- modules/setting/storage.go | 22 +++++++++++++++++++--- 7 files changed, 31 insertions(+), 17 deletions(-) diff --git a/modules/setting/actions.go b/modules/setting/actions.go index ad9087cd544af..7c03a7787a431 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -27,6 +27,6 @@ func loadActionsFrom(rootCfg ConfigProvider) error { // don't support to read configuration from [actions] var err error - Actions.Storage, err = getStorage(rootCfg, nil, "actions_log", "") + Actions.Storage, err = getStorage(rootCfg, "actions_log", nil, "") return err } diff --git a/modules/setting/attachment.go b/modules/setting/attachment.go index 6fed5a43dab33..2af392bb94f1b 100644 --- a/modules/setting/attachment.go +++ b/modules/setting/attachment.go @@ -22,10 +22,9 @@ var Attachment = struct { func loadAttachmentFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") var err error - Attachment.Storage, err = getStorage(rootCfg, sec, "attachments", storageType) + Attachment.Storage, err = getStorage(rootCfg, "attachments", sec, "") if err != nil { return err } diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index bd9fb5b6a8c5a..9a42b1e38c3eb 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -30,7 +30,6 @@ func loadLFSFrom(rootCfg ConfigProvider) error { } lfsSec := rootCfg.Section("lfs") - storageType := lfsSec.Key("STORAGE_TYPE").MustString("") // Specifically default PATH to LFS_CONTENT_PATH // DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version @@ -39,7 +38,7 @@ func loadLFSFrom(rootCfg ConfigProvider) error { lfsSec.Key("PATH").MustString(sec.Key("LFS_CONTENT_PATH").String()) var err error - LFS.Storage, err = getStorage(rootCfg, lfsSec, "lfs", storageType) + LFS.Storage, err = getStorage(rootCfg, "lfs", lfsSec, "") if err != nil { return err } diff --git a/modules/setting/packages.go b/modules/setting/packages.go index 523a754927688..9eebc5b865b42 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -54,9 +54,10 @@ func loadPackagesFrom(rootCfg ConfigProvider) error { return fmt.Errorf("failed to map Packages settings: %v", err) } - storageType := sec.Key("STORAGE_TYPE").MustString("") - - Packages.Storage, err = getStorage(rootCfg, sec, "packages", storageType) + Packages.Storage, err = getStorage(rootCfg, "packages", sec, "") + if err != nil { + return fmt.Errorf("failed to get storage: %v", err) + } appURL, _ := url.Parse(AppURL) Packages.RegistryHost = appURL.Host diff --git a/modules/setting/picture.go b/modules/setting/picture.go index c14de55dbef72..45ff0db1efc21 100644 --- a/modules/setting/picture.go +++ b/modules/setting/picture.go @@ -38,11 +38,10 @@ func loadAvatarsFrom(rootCfg ConfigProvider) error { avatarSec := rootCfg.Section("avatar") storageType := sec.Key("AVATAR_STORAGE_TYPE").MustString("") // Specifically default PATH to AVATAR_UPLOAD_PATH - avatarSec.Key("PATH").MustString( - sec.Key("AVATAR_UPLOAD_PATH").String()) + avatarSec.Key("PATH").MustString(sec.Key("AVATAR_UPLOAD_PATH").String()) var err error - Avatar.Storage, err = getStorage(rootCfg, avatarSec, "avatars", storageType) + Avatar.Storage, err = getStorage(rootCfg, "avatars", avatarSec, storageType) if err != nil { return err } @@ -96,7 +95,7 @@ func loadRepoAvatarFrom(rootCfg ConfigProvider) error { sec.Key("REPOSITORY_AVATAR_UPLOAD_PATH").String()) var err error - RepoAvatar.Storage, err = getStorage(rootCfg, repoAvatarSec, "repo-avatars", storageType) + RepoAvatar.Storage, err = getStorage(rootCfg, "repo-avatars", repoAvatarSec, storageType) if err != nil { return err } diff --git a/modules/setting/repository_archive.go b/modules/setting/repository_archive.go index c734cd976f4e5..f289f7d8e4e4d 100644 --- a/modules/setting/repository_archive.go +++ b/modules/setting/repository_archive.go @@ -14,8 +14,8 @@ func loadRepoArchiveFrom(rootCfg ConfigProvider) error { if err := sec.MapTo(&RepoArchive); err != nil { return fmt.Errorf("mapto repoarchive failed: %v", err) } - storageType := sec.Key("STORAGE_TYPE").MustString("") + var err error - RepoArchive.Storage, err = getStorage(rootCfg, sec, "repo-archive", storageType) + RepoArchive.Storage, err = getStorage(rootCfg, "repo-archive", sec, "") return err } diff --git a/modules/setting/storage.go b/modules/setting/storage.go index b05c1baeeb599..f6b3f9b5c0f5f 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -48,7 +48,7 @@ func getStorageSection(rootCfg ConfigProvider) ConfigSection { // getStorage will read storage configurations from 4 possible ways // 1 read configurations from [storage.$name] if the keys exist -// 2 read configurations from given section if the setting keys exist (eg: name="attachments") +// 2 read configurations from given section if it's not nil // 3 read configurations from [storage.$type] if the keys exist (eg: type="local" or "minio") // 4 read configurations from [storage] if the keys exist // The keys in earlier section have higher priority. @@ -88,13 +88,29 @@ func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ return nil, fmt.Errorf("unknown storage type: %s", storageType) } storageType = targetSec.Key("STORAGE_TYPE").String() + if storageType != "local" && storageType != "minio" { + return nil, fmt.Errorf("%s should have STORAGE_TYPE as local or minio", storageSectionName+"."+storageType) + } + } + + // just nameSec and startSec could contains override configurations + overrideSec := nameSec + if overrideSec == nil { + overrideSec = startSec + } + + serveDirect := false + path := "" + if overrideSec != nil { + serveDirect = overrideSec.Key("SERVE_DIRECT").MustBool(false) + path = overrideSec.Key("PATH").MustString(filepath.Join(AppDataPath, name)) } storage := Storage{ Section: targetSec, Type: storageType, - ServeDirect: targetSec.Key("SERVE_DIRECT").MustBool(false), - Path: targetSec.Key("PATH").MustString(filepath.Join(AppDataPath, name)), + ServeDirect: serveDirect, + Path: path, } // Specific defaults From 7f6369aa4de26017154d20714f8817a389a211fd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 7 May 2023 22:13:21 +0800 Subject: [PATCH 14/28] Fix override --- modules/setting/storage.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index f6b3f9b5c0f5f..62962e84e23af 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -93,7 +93,7 @@ func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ } } - // just nameSec and startSec could contains override configurations + // just nameSec and startSec could contain override configurations overrideSec := nameSec if overrideSec == nil { overrideSec = startSec @@ -101,9 +101,13 @@ func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ serveDirect := false path := "" + minioBucket := "" + minioBasePath := "" if overrideSec != nil { serveDirect = overrideSec.Key("SERVE_DIRECT").MustBool(false) path = overrideSec.Key("PATH").MustString(filepath.Join(AppDataPath, name)) + minioBucket = overrideSec.Key("MINIO_BUCKET").MustString(targetSec.Key("MINIO_BUCKET").String()) + minioBasePath = overrideSec.Key("MINIO_BASE_PATH").MustString(name + "/") } storage := Storage{ @@ -118,7 +122,8 @@ func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ storage.Path = filepath.Join(AppWorkPath, storage.Path) storage.Section.Key("PATH").SetValue(storage.Path) } - storage.Section.Key("MINIO_BASE_PATH").MustString(name + "/") + storage.Section.Key("MINIO_BUCKET").SetValue(minioBucket) + storage.Section.Key("MINIO_BASE_PATH").SetValue(minioBasePath) return &storage, nil } From f139b8c66363deb410f97ec99141cbb6ab7f7c07 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 8 May 2023 18:48:17 +0800 Subject: [PATCH 15/28] Fix storage --- modules/setting/storage.go | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 62962e84e23af..0e0e64126480e 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -60,10 +60,26 @@ func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ nameSec, _ := rootCfg.GetSection(storageSectionName + "." + name) targetSec := nameSec + storageType := "" if targetSec == nil { targetSec = startSec - if targetSec != nil && targetSec.Key("STORAGE_TYPE").String() == "" { - targetSec = nil // startSec's STORAGE_TYPE could be ignored + if targetSec != nil { + storageType = targetSec.Key("STORAGE_TYPE").String() + if storageType == "" { + targetSec = nil // startSec's STORAGE_TYPE could be ignored + } else { + storageSec, _ := rootCfg.GetSection(storageSectionName + "." + storageType) + if storageSec != nil { + targetSec = storageSec + if storageType != "local" && storageType != "minio" { + storageType = targetSec.Key("STORAGE_TYPE").MustString("local") + } + } else { + if storageType != "local" && storageType != "minio" { + return nil, fmt.Errorf("unknown storage type: %s", storageType) + } + } + } } } @@ -80,7 +96,7 @@ func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ } } - storageType := targetSec.Key("STORAGE_TYPE").MustString("local") + storageType = targetSec.Key("STORAGE_TYPE").MustString("local") if storageType != "local" && storageType != "minio" { var err error targetSec, err = rootCfg.GetSection(storageSectionName + "." + storageType) @@ -100,14 +116,14 @@ func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ } serveDirect := false - path := "" - minioBucket := "" - minioBasePath := "" + path := filepath.Join(AppDataPath, name) + minioBucket := targetSec.Key("MINIO_BUCKET").String() + minioBasePath := name + "/" if overrideSec != nil { serveDirect = overrideSec.Key("SERVE_DIRECT").MustBool(false) - path = overrideSec.Key("PATH").MustString(filepath.Join(AppDataPath, name)) - minioBucket = overrideSec.Key("MINIO_BUCKET").MustString(targetSec.Key("MINIO_BUCKET").String()) - minioBasePath = overrideSec.Key("MINIO_BASE_PATH").MustString(name + "/") + path = overrideSec.Key("PATH").MustString(path) + minioBucket = overrideSec.Key("MINIO_BUCKET").MustString(minioBucket) + minioBasePath = overrideSec.Key("MINIO_BASE_PATH").MustString(minioBasePath) } storage := Storage{ From e9853b1c3b58449f5cdfee6ec2153643c214b4ed Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 14 May 2023 22:19:17 +0800 Subject: [PATCH 16/28] Use explicit storage --- cmd/dump.go | 6 +++--- models/migrations/v1_10/v96.go | 2 +- models/migrations/v1_11/v112.go | 2 +- models/migrations/v1_11/v115.go | 8 ++++---- models/unittest/testdb.go | 2 +- modules/setting/actions.go | 4 ++-- modules/setting/attachment.go | 2 +- modules/setting/lfs.go | 2 +- modules/setting/packages.go | 2 +- modules/setting/picture.go | 4 ++-- modules/setting/repository_archive.go | 2 +- modules/storage/storage.go | 4 ++-- routers/api/v1/repo/file.go | 4 ++-- routers/install/install.go | 2 +- routers/web/repo/attachment.go | 2 +- routers/web/repo/download.go | 2 +- routers/web/repo/repo.go | 2 +- services/lfs/server.go | 2 +- 18 files changed, 27 insertions(+), 27 deletions(-) diff --git a/cmd/dump.go b/cmd/dump.go index 32ccc5566c8a9..f11e90459bf7b 100644 --- a/cmd/dump.go +++ b/cmd/dump.go @@ -356,9 +356,9 @@ func runDump(ctx *cli.Context) error { } excludes = append(excludes, setting.RepoRootPath) - excludes = append(excludes, setting.LFS.Path) - excludes = append(excludes, setting.Attachment.Path) - excludes = append(excludes, setting.Packages.Path) + excludes = append(excludes, setting.LFS.Storage.Path) + excludes = append(excludes, setting.Attachment.Storage.Path) + excludes = append(excludes, setting.Packages.Storage.Path) excludes = append(excludes, setting.Log.RootPath) excludes = append(excludes, absFileName) if err := addRecursiveExclude(w, "data", setting.AppDataPath, excludes, verbose); err != nil { diff --git a/models/migrations/v1_10/v96.go b/models/migrations/v1_10/v96.go index 422defe8387a4..34c8240031c20 100644 --- a/models/migrations/v1_10/v96.go +++ b/models/migrations/v1_10/v96.go @@ -53,7 +53,7 @@ func DeleteOrphanedAttachments(x *xorm.Engine) error { for _, attachment := range attachments { uuid := attachment.UUID - if err := util.RemoveAll(filepath.Join(setting.Attachment.Path, uuid[0:1], uuid[1:2], uuid)); err != nil { + if err := util.RemoveAll(filepath.Join(setting.Attachment.Storage.Path, uuid[0:1], uuid[1:2], uuid)); err != nil { return err } } diff --git a/models/migrations/v1_11/v112.go b/models/migrations/v1_11/v112.go index ff1f972204e7c..0857663119535 100644 --- a/models/migrations/v1_11/v112.go +++ b/models/migrations/v1_11/v112.go @@ -30,7 +30,7 @@ func RemoveAttachmentMissedRepo(x *xorm.Engine) error { for i := 0; i < len(attachments); i++ { uuid := attachments[i].UUID - if err = util.RemoveAll(filepath.Join(setting.Attachment.Path, uuid[0:1], uuid[1:2], uuid)); err != nil { + if err = util.RemoveAll(filepath.Join(setting.Attachment.Storage.Path, uuid[0:1], uuid[1:2], uuid)); err != nil { fmt.Printf("Error: %v", err) //nolint:forbidigo } } diff --git a/models/migrations/v1_11/v115.go b/models/migrations/v1_11/v115.go index da935f6514e47..8c631cfd0bbf3 100644 --- a/models/migrations/v1_11/v115.go +++ b/models/migrations/v1_11/v115.go @@ -61,7 +61,7 @@ func RenameExistingUserAvatarName(x *xorm.Engine) error { for _, user := range users { oldAvatar := user.Avatar - if stat, err := os.Stat(filepath.Join(setting.Avatar.Path, oldAvatar)); err != nil || !stat.Mode().IsRegular() { + if stat, err := os.Stat(filepath.Join(setting.Avatar.Storage.Path, oldAvatar)); err != nil || !stat.Mode().IsRegular() { if err == nil { err = fmt.Errorf("Error: \"%s\" is not a regular file", oldAvatar) } @@ -86,7 +86,7 @@ func RenameExistingUserAvatarName(x *xorm.Engine) error { return fmt.Errorf("[user: %s] user table update: %w", user.LowerName, err) } - deleteList.Add(filepath.Join(setting.Avatar.Path, oldAvatar)) + deleteList.Add(filepath.Join(setting.Avatar.Storage.Path, oldAvatar)) migrated++ select { case <-ticker.C: @@ -135,7 +135,7 @@ func RenameExistingUserAvatarName(x *xorm.Engine) error { // copyOldAvatarToNewLocation copies oldAvatar to newAvatarLocation // and returns newAvatar location func copyOldAvatarToNewLocation(userID int64, oldAvatar string) (string, error) { - fr, err := os.Open(filepath.Join(setting.Avatar.Path, oldAvatar)) + fr, err := os.Open(filepath.Join(setting.Avatar.Storage.Path, oldAvatar)) if err != nil { return "", fmt.Errorf("os.Open: %w", err) } @@ -151,7 +151,7 @@ func copyOldAvatarToNewLocation(userID int64, oldAvatar string) (string, error) return newAvatar, nil } - if err := os.WriteFile(filepath.Join(setting.Avatar.Path, newAvatar), data, 0o666); err != nil { + if err := os.WriteFile(filepath.Join(setting.Avatar.Storage.Path, newAvatar), data, 0o666); err != nil { return "", fmt.Errorf("os.WriteFile: %w", err) } diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 10a70ad9f8e8f..5351ff1139989 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -126,7 +126,7 @@ func MainTest(m *testing.M, testOpts *TestOptions) { setting.Packages.Storage.Path = filepath.Join(setting.AppDataPath, "packages") - setting.Actions.Storage.Path = filepath.Join(setting.AppDataPath, "actions_log") + setting.Actions.LogStorage.Path = filepath.Join(setting.AppDataPath, "actions_log") setting.Git.HomePath = filepath.Join(setting.AppDataPath, "home") diff --git a/modules/setting/actions.go b/modules/setting/actions.go index 7c03a7787a431..c60c30d1f93cc 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -10,7 +10,7 @@ import ( // Actions settings var ( Actions = struct { - *Storage // how the created logs should be stored + LogStorage *Storage // how the created logs should be stored Enabled bool DefaultActionsURL string `ini:"DEFAULT_ACTIONS_URL"` }{ @@ -27,6 +27,6 @@ func loadActionsFrom(rootCfg ConfigProvider) error { // don't support to read configuration from [actions] var err error - Actions.Storage, err = getStorage(rootCfg, "actions_log", nil, "") + Actions.LogStorage, err = getStorage(rootCfg, "actions_log", nil, "") return err } diff --git a/modules/setting/attachment.go b/modules/setting/attachment.go index 2af392bb94f1b..251a3a5119bf3 100644 --- a/modules/setting/attachment.go +++ b/modules/setting/attachment.go @@ -5,7 +5,7 @@ package setting // Attachment settings var Attachment = struct { - *Storage + Storage *Storage AllowedTypes string MaxSize int64 MaxFiles int diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index b0a243c733d2b..8493df8fe222a 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -20,7 +20,7 @@ var LFS = struct { MaxFileSize int64 `ini:"LFS_MAX_FILE_SIZE"` LocksPagingNum int `ini:"LFS_LOCKS_PAGING_NUM"` - *Storage + Storage *Storage }{} func loadLFSFrom(rootCfg ConfigProvider) error { diff --git a/modules/setting/packages.go b/modules/setting/packages.go index 860feb2dd2acb..519fd4fd12c83 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -16,7 +16,7 @@ import ( // Package registry settings var ( Packages = struct { - *Storage + Storage *Storage Enabled bool ChunkedUploadPath string RegistryHost string diff --git a/modules/setting/picture.go b/modules/setting/picture.go index 2cf262ef91e76..212ea2cf9e080 100644 --- a/modules/setting/picture.go +++ b/modules/setting/picture.go @@ -7,7 +7,7 @@ package setting var ( Avatar = struct { - *Storage + Storage *Storage MaxWidth int MaxHeight int @@ -27,7 +27,7 @@ var ( EnableFederatedAvatar bool // Depreciated: migrated to database RepoAvatar = struct { - *Storage + Storage *Storage Fallback string FallbackImage string diff --git a/modules/setting/repository_archive.go b/modules/setting/repository_archive.go index f289f7d8e4e4d..6aada840fa549 100644 --- a/modules/setting/repository_archive.go +++ b/modules/setting/repository_archive.go @@ -6,7 +6,7 @@ package setting import "fmt" var RepoArchive = struct { - *Storage + Storage *Storage }{} func loadRepoArchiveFrom(rootCfg ConfigProvider) error { diff --git a/modules/storage/storage.go b/modules/storage/storage.go index caecab306e630..7ab271ae93f41 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -214,7 +214,7 @@ func initActions() (err error) { Actions = discardStorage("Actions isn't enabled") return nil } - log.Info("Initialising Actions storage with type: %s", setting.Actions.Storage.Type) - Actions, err = NewStorage(setting.Actions.Storage.Type, &setting.Actions.Storage) + log.Info("Initialising Actions storage with type: %s", setting.Actions.LogStorage.Type) + Actions, err = NewStorage(setting.Actions.LogStorage.Type, &setting.Actions.LogStorage) return err } diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index eb63dda590247..f5d86aad4f192 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -199,7 +199,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { return } - if setting.LFS.ServeDirect { + if setting.LFS.Storage.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) if u != nil && err == nil { @@ -319,7 +319,7 @@ func download(ctx *context.APIContext, archiveName string, archiver *repo_model. downloadName := ctx.Repo.Repository.Name + "-" + archiveName rPath := archiver.RelativePath() - if setting.RepoArchive.ServeDirect { + if setting.RepoArchive.Storage.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.RepoArchives.URL(rPath, downloadName) if u != nil && err == nil { diff --git a/routers/install/install.go b/routers/install/install.go index 714ddd5548454..45ca1ae4c7604 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -118,7 +118,7 @@ func Install(ctx *context.Context) { // Application general settings form.AppName = setting.AppName form.RepoRootPath = setting.RepoRootPath - form.LFSRootPath = setting.LFS.Path + form.LFSRootPath = setting.LFS.Storage.Path // Note(unknown): it's hard for Windows users change a running user, // so just use current one if config says default. diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index c46ec29841a73..f332dc54dd459 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -131,7 +131,7 @@ func ServeAttachment(ctx *context.Context, uuid string) { return } - if setting.Attachment.ServeDirect { + if setting.Attachment.Storage.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name) diff --git a/routers/web/repo/download.go b/routers/web/repo/download.go index 1c87f9bed73cd..745c1f89fbbbd 100644 --- a/routers/web/repo/download.go +++ b/routers/web/repo/download.go @@ -53,7 +53,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified time.Time return nil } - if setting.LFS.ServeDirect { + if setting.LFS.Storage.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) if u != nil && err == nil { diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index f697d9433e15e..f95b1b38464b1 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -428,7 +428,7 @@ func download(ctx *context.Context, archiveName string, archiver *repo_model.Rep downloadName := ctx.Repo.Repository.Name + "-" + archiveName rPath := archiver.RelativePath() - if setting.RepoArchive.ServeDirect { + if setting.RepoArchive.Storage.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.RepoArchives.URL(rPath, downloadName) if u != nil && err == nil { diff --git a/services/lfs/server.go b/services/lfs/server.go index 64e1203394547..c4033305f8500 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -446,7 +446,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa if download { var link *lfs_module.Link - if setting.LFS.ServeDirect { + if setting.LFS.Storage.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid) if u != nil && err == nil { From a73e6199fb2986d87cf86fe0ecf1ca89b31634b3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 2 Jun 2023 23:52:01 +0800 Subject: [PATCH 17/28] revert getstorage change --- modules/setting/actions.go | 10 +-- modules/setting/attachment.go | 6 +- modules/setting/lfs.go | 6 +- modules/setting/packages.go | 5 +- modules/setting/picture.go | 15 +--- modules/setting/repository_archive.go | 5 +- modules/setting/storage.go | 108 +++++++------------------- 7 files changed, 41 insertions(+), 114 deletions(-) diff --git a/modules/setting/actions.go b/modules/setting/actions.go index bf297961ce920..55c635188d0aa 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -27,16 +27,12 @@ func loadActionsFrom(rootCfg ConfigProvider) error { } // don't support to read configuration from [actions] - var err error - Actions.LogStorage, err = getStorage(rootCfg, "actions_log", nil, "") - if err != nil { - return err - } + Actions.LogStorage = getStorage(rootCfg, "actions_log", "", nil) actionsSec := rootCfg.Section("actions.artifacts") storageType := actionsSec.Key("STORAGE_TYPE").MustString("") - Actions.ArtifactStorage, err = getStorage(rootCfg, "actions_artifacts", actionsSec, storageType) + Actions.ArtifactStorage = getStorage(rootCfg, "actions_artifacts", storageType, actionsSec) - return err + return nil } diff --git a/modules/setting/attachment.go b/modules/setting/attachment.go index 251a3a5119bf3..26ca4e8662134 100644 --- a/modules/setting/attachment.go +++ b/modules/setting/attachment.go @@ -23,11 +23,7 @@ var Attachment = struct { func loadAttachmentFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("attachment") - var err error - Attachment.Storage, err = getStorage(rootCfg, "attachments", sec, "") - if err != nil { - return err - } + Attachment.Storage = getStorage(rootCfg, "attachments", "", sec) Attachment.AllowedTypes = sec.Key("ALLOWED_TYPES").MustString(".csv,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.xls,.xlsx,.zip") Attachment.MaxSize = sec.Key("MAX_SIZE").MustInt64(4) diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 8493df8fe222a..65c7b37092370 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -37,11 +37,7 @@ func loadLFSFrom(rootCfg ConfigProvider) error { deprecatedSetting(rootCfg, "server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0") lfsSec.Key("PATH").MustString(sec.Key("LFS_CONTENT_PATH").String()) - var err error - LFS.Storage, err = getStorage(rootCfg, "lfs", lfsSec, "") - if err != nil { - return err - } + LFS.Storage = getStorage(rootCfg, "lfs", "", lfsSec) // Rest of LFS service settings if LFS.LocksPagingNum == 0 { diff --git a/modules/setting/packages.go b/modules/setting/packages.go index ad348aa977501..e3c2c059ba04d 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -57,10 +57,7 @@ func loadPackagesFrom(rootCfg ConfigProvider) error { return fmt.Errorf("failed to map Packages settings: %v", err) } - Packages.Storage, err = getStorage(rootCfg, "packages", sec, "") - if err != nil { - return fmt.Errorf("failed to get storage: %v", err) - } + Packages.Storage = getStorage(rootCfg, "packages", "", sec) appURL, _ := url.Parse(AppURL) Packages.RegistryHost = appURL.Host diff --git a/modules/setting/picture.go b/modules/setting/picture.go index 212ea2cf9e080..b2e5794cf3a9a 100644 --- a/modules/setting/picture.go +++ b/modules/setting/picture.go @@ -42,11 +42,7 @@ func loadAvatarsFrom(rootCfg ConfigProvider) error { // Specifically default PATH to AVATAR_UPLOAD_PATH avatarSec.Key("PATH").MustString(sec.Key("AVATAR_UPLOAD_PATH").String()) - var err error - Avatar.Storage, err = getStorage(rootCfg, "avatars", avatarSec, storageType) - if err != nil { - return err - } + Avatar.Storage = getStorage(rootCfg, "avatars", storageType, avatarSec) Avatar.MaxWidth = sec.Key("AVATAR_MAX_WIDTH").MustInt(4096) Avatar.MaxHeight = sec.Key("AVATAR_MAX_HEIGHT").MustInt(4096) @@ -94,14 +90,9 @@ func loadRepoAvatarFrom(rootCfg ConfigProvider) error { repoAvatarSec := rootCfg.Section("repo-avatar") storageType := sec.Key("REPOSITORY_AVATAR_STORAGE_TYPE").MustString("") // Specifically default PATH to AVATAR_UPLOAD_PATH - repoAvatarSec.Key("PATH").MustString( - sec.Key("REPOSITORY_AVATAR_UPLOAD_PATH").String()) + repoAvatarSec.Key("PATH").MustString(sec.Key("REPOSITORY_AVATAR_UPLOAD_PATH").String()) - var err error - RepoAvatar.Storage, err = getStorage(rootCfg, "repo-avatars", repoAvatarSec, storageType) - if err != nil { - return err - } + RepoAvatar.Storage = getStorage(rootCfg, "repo-avatars", storageType, repoAvatarSec) RepoAvatar.Fallback = sec.Key("REPOSITORY_AVATAR_FALLBACK").MustString("none") RepoAvatar.FallbackImage = sec.Key("REPOSITORY_AVATAR_FALLBACK_IMAGE").MustString(AppSubURL + "/assets/img/repo_default.png") diff --git a/modules/setting/repository_archive.go b/modules/setting/repository_archive.go index 6aada840fa549..3f42e41f8497f 100644 --- a/modules/setting/repository_archive.go +++ b/modules/setting/repository_archive.go @@ -15,7 +15,6 @@ func loadRepoArchiveFrom(rootCfg ConfigProvider) error { return fmt.Errorf("mapto repoarchive failed: %v", err) } - var err error - RepoArchive.Storage, err = getStorage(rootCfg, "repo-archive", sec, "") - return err + RepoArchive.Storage = getStorage(rootCfg, "repo-archive", "", sec) + return nil } diff --git a/modules/setting/storage.go b/modules/setting/storage.go index c4c37f76ab5bd..2e4c22a52b59b 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -4,8 +4,6 @@ package setting import ( - "errors" - "fmt" "path/filepath" "reflect" ) @@ -46,98 +44,52 @@ func getStorageSection(rootCfg ConfigProvider) ConfigSection { return storageSec } -// getStorage will read storage configurations from 4 possible ways -// 1 read configurations from [storage.$name] if the keys exist -// 2 read configurations from given section if it's not nil -// 3 read configurations from [storage.$type] if the keys exist (eg: type="local" or "minio") -// 4 read configurations from [storage] if the keys exist -// The keys in earlier section have higher priority. -func getStorage(rootCfg ConfigProvider, name string, startSec ConfigSection, typ string) (*Storage, error) { - if name == "" { - return nil, errors.New("getStorage: name cannot be empty") - } - - nameSec, _ := rootCfg.GetSection(storageSectionName + "." + name) +func getStorage(rootCfg ConfigProvider, name, typ string, targetSec ConfigSection) *Storage { + sec := getStorageSection(rootCfg) - targetSec := nameSec - storageType := "" if targetSec == nil { - targetSec = startSec - if targetSec != nil { - storageType = targetSec.Key("STORAGE_TYPE").String() - if storageType == "" { - targetSec = nil // startSec's STORAGE_TYPE could be ignored - } else { - storageSec, _ := rootCfg.GetSection(storageSectionName + "." + storageType) - if storageSec != nil { - targetSec = storageSec - if storageType != "local" && storageType != "minio" { - storageType = targetSec.Key("STORAGE_TYPE").MustString("local") - } - } else if storageType != "local" && storageType != "minio" { - return nil, fmt.Errorf("unknown storage type: %s", storageType) - } - } - } + targetSec, _ = rootCfg.NewSection(name) } - if targetSec == nil && typ != "" { - targetSec, _ = rootCfg.GetSection(storageSectionName + "." + typ) - } - if targetSec == nil { - targetSec, _ = rootCfg.GetSection(storageSectionName) + var storage Storage + storage.Section = targetSec + storage.Type = typ + + overrides := make([]ConfigSection, 0, 3) + nameSec, err := rootCfg.GetSection(storageSectionName + "." + name) + if err == nil { + overrides = append(overrides, nameSec) } - if targetSec == nil { // finally fallback - targetSec = startSec - if targetSec == nil { - targetSec = getStorageSection(rootCfg) + + typeSec, err := rootCfg.GetSection(storageSectionName + "." + typ) + if err == nil { + overrides = append(overrides, typeSec) + nextType := typeSec.Key("STORAGE_TYPE").String() + if len(nextType) > 0 { + storage.Type = nextType // Support custom STORAGE_TYPE } } + overrides = append(overrides, sec) - storageType = targetSec.Key("STORAGE_TYPE").MustString("local") - if storageType != "local" && storageType != "minio" { - var err error - targetSec, err = rootCfg.GetSection(storageSectionName + "." + storageType) - if err != nil { - return nil, fmt.Errorf("unknown storage type: %s", storageType) + for _, override := range overrides { + for _, key := range override.Keys() { + if !targetSec.HasKey(key.Name()) { + _, _ = targetSec.NewKey(key.Name(), key.Value()) + } } - storageType = targetSec.Key("STORAGE_TYPE").String() - if storageType != "local" && storageType != "minio" { - return nil, fmt.Errorf("%s should have STORAGE_TYPE as local or minio", storageSectionName+"."+storageType) + if len(storage.Type) == 0 { + storage.Type = override.Key("STORAGE_TYPE").String() } } - - // just nameSec and startSec could contain override configurations - overrideSec := nameSec - if overrideSec == nil { - overrideSec = startSec - } - - serveDirect := false - path := filepath.Join(AppDataPath, name) - minioBucket := targetSec.Key("MINIO_BUCKET").String() - minioBasePath := name + "/" - if overrideSec != nil { - serveDirect = overrideSec.Key("SERVE_DIRECT").MustBool(false) - path = overrideSec.Key("PATH").MustString(path) - minioBucket = overrideSec.Key("MINIO_BUCKET").MustString(minioBucket) - minioBasePath = overrideSec.Key("MINIO_BASE_PATH").MustString(minioBasePath) - } - - storage := Storage{ - Section: targetSec, - Type: storageType, - ServeDirect: serveDirect, - Path: path, - } + storage.ServeDirect = storage.Section.Key("SERVE_DIRECT").MustBool(false) // 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_BUCKET").SetValue(minioBucket) - storage.Section.Key("MINIO_BASE_PATH").SetValue(minioBasePath) + storage.Section.Key("MINIO_BASE_PATH").MustString(name + "/") - return &storage, nil + return &storage } From 26e3aefc88e33e0bce84d3b0e16a273668e55c2a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 7 Jun 2023 12:07:54 +0800 Subject: [PATCH 18/28] Fix all tests --- cmd/migrate_storage.go | 6 +- modules/setting/actions.go | 12 +- modules/setting/actions_test.go | 6 +- modules/setting/attachment.go | 19 +-- modules/setting/attachment_test.go | 67 +++++---- modules/setting/config_provider.go | 20 +++ modules/setting/lfs.go | 20 ++- modules/setting/lfs_test.go | 22 ++- modules/setting/packages.go | 17 ++- modules/setting/packages_test.go | 118 ++++++++++++++- modules/setting/picture.go | 12 +- modules/setting/repository_archive.go | 13 +- modules/setting/repository_archive_test.go | 52 ++++++- modules/setting/storage.go | 165 ++++++++++++++------- modules/setting/storage_test.go | 60 +++----- modules/storage/local.go | 6 +- modules/storage/local_test.go | 3 +- modules/storage/minio.go | 31 +--- modules/storage/minio_test.go | 4 +- modules/storage/storage.go | 9 +- modules/storage/storage_test.go | 2 +- routers/api/v1/repo/file.go | 4 +- routers/web/base.go | 2 +- routers/web/repo/attachment.go | 2 +- routers/web/repo/download.go | 2 +- routers/web/repo/repo.go | 2 +- services/lfs/server.go | 2 +- 27 files changed, 471 insertions(+), 207 deletions(-) diff --git a/cmd/migrate_storage.go b/cmd/migrate_storage.go index 291e7695b5e40..83b15c877e8b1 100644 --- a/cmd/migrate_storage.go +++ b/cmd/migrate_storage.go @@ -179,7 +179,7 @@ func runMigrateStorage(ctx *cli.Context) error { switch strings.ToLower(ctx.String("storage")) { case "": fallthrough - case string(storage.LocalStorageType): + case string(setting.LocalStorageType): p := ctx.String("path") if p == "" { log.Fatal("Path must be given when storage is loal") @@ -190,10 +190,10 @@ func runMigrateStorage(ctx *cli.Context) error { storage.LocalStorageConfig{ Path: p, }) - case string(storage.MinioStorageType): + case string(setting.MinioStorageType): dstStorage, err = storage.NewMinioStorage( stdCtx, - storage.MinioStorageConfig{ + setting.MinioStorageConfig{ Endpoint: ctx.String("minio-endpoint"), AccessKeyID: ctx.String("minio-access-key-id"), SecretAccessKey: ctx.String("minio-secret-access-key"), diff --git a/modules/setting/actions.go b/modules/setting/actions.go index 55c635188d0aa..5ce6b0b41dfef 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -22,17 +22,21 @@ var ( func loadActionsFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("actions") - if err := sec.MapTo(&Actions); err != nil { + err := sec.MapTo(&Actions) + if err != nil { return fmt.Errorf("failed to map Actions settings: %v", err) } // don't support to read configuration from [actions] - Actions.LogStorage = getStorage(rootCfg, "actions_log", "", nil) + Actions.LogStorage, err = getStorage(rootCfg, "actions_log", "", nil) + if err != nil { + return err + } actionsSec := rootCfg.Section("actions.artifacts") storageType := actionsSec.Key("STORAGE_TYPE").MustString("") - Actions.ArtifactStorage = getStorage(rootCfg, "actions_artifacts", storageType, actionsSec) + Actions.ArtifactStorage, err = getStorage(rootCfg, "actions_artifacts", storageType, actionsSec) - return nil + return err } diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index cfa1829fdcae5..77defc1b673e6 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -19,7 +19,7 @@ func Test_getStorageInheritNameSectionTypeForActions(t *testing.T) { assert.NoError(t, loadActionsFrom(cfg)) assert.EqualValues(t, "minio", Actions.LogStorage.Type) - assert.EqualValues(t, "actions_log/", Actions.LogStorage.Section.Key("MINIO_BASE_PATH").String()) + assert.EqualValues(t, "actions_log/", Actions.LogStorage.MinioConfig.BasePath) iniStr = ` [storage.actions_log] @@ -30,7 +30,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadActionsFrom(cfg)) assert.EqualValues(t, "minio", Actions.LogStorage.Type) - assert.EqualValues(t, "actions_log/", Actions.LogStorage.Section.Key("MINIO_BASE_PATH").String()) + assert.EqualValues(t, "actions_log/", Actions.LogStorage.MinioConfig.BasePath) iniStr = ` [storage.actions_log] @@ -44,5 +44,5 @@ STORAGE_TYPE = minio assert.NoError(t, loadActionsFrom(cfg)) assert.EqualValues(t, "minio", Actions.LogStorage.Type) - assert.EqualValues(t, "actions_log/", Actions.LogStorage.Section.Key("MINIO_BASE_PATH").String()) + assert.EqualValues(t, "actions_log/", Actions.LogStorage.MinioConfig.BasePath) } diff --git a/modules/setting/attachment.go b/modules/setting/attachment.go index 26ca4e8662134..491564c9dcc08 100644 --- a/modules/setting/attachment.go +++ b/modules/setting/attachment.go @@ -11,24 +11,25 @@ var Attachment = struct { MaxFiles int Enabled bool }{ - Storage: &Storage{ - ServeDirect: false, - }, - AllowedTypes: "image/jpeg,image/png,application/zip,application/gzip", + Storage: &Storage{}, + AllowedTypes: ".csv,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.xls,.xlsx,.zip", MaxSize: 4, MaxFiles: 5, Enabled: true, } -func loadAttachmentFrom(rootCfg ConfigProvider) error { - sec := rootCfg.Section("attachment") - - Attachment.Storage = getStorage(rootCfg, "attachments", "", sec) +func loadAttachmentFrom(rootCfg ConfigProvider) (err error) { + sec, _ := rootCfg.GetSection("attachment") + if sec == nil { + Attachment.Storage, err = getStorage(rootCfg, "attachments", "", nil) + return err + } Attachment.AllowedTypes = sec.Key("ALLOWED_TYPES").MustString(".csv,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.xls,.xlsx,.zip") Attachment.MaxSize = sec.Key("MAX_SIZE").MustInt64(4) Attachment.MaxFiles = sec.Key("MAX_FILES").MustInt(5) Attachment.Enabled = sec.Key("ENABLED").MustBool(true) - return nil + Attachment.Storage, err = getStorage(rootCfg, "attachments", "", sec) + return err } diff --git a/modules/setting/attachment_test.go b/modules/setting/attachment_test.go index a56a63b30eb19..6c6dfafb36712 100644 --- a/modules/setting/attachment_test.go +++ b/modules/setting/attachment_test.go @@ -25,28 +25,8 @@ MINIO_ENDPOINT = my_minio:9000 assert.NoError(t, loadAttachmentFrom(cfg)) assert.EqualValues(t, "minio", Attachment.Storage.Type) - assert.EqualValues(t, "my_minio:9000", Attachment.Storage.Section.Key("MINIO_ENDPOINT").String()) - assert.EqualValues(t, "gitea-attachment", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) -} - -func Test_getStorageNameSectionOverridesTypeSection(t *testing.T) { - iniStr := ` -[attachment] -STORAGE_TYPE = minio - -[storage.attachments] -MINIO_BUCKET = gitea-attachment - -[storage.minio] -MINIO_BUCKET = gitea -` - cfg, err := NewConfigProviderFromData(iniStr) - assert.NoError(t, err) - - assert.NoError(t, loadAttachmentFrom(cfg)) - - assert.EqualValues(t, "minio", Attachment.Storage.Type) - assert.EqualValues(t, "gitea-attachment", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "my_minio:9000", Attachment.Storage.MinioConfig.Endpoint) + assert.EqualValues(t, "gitea-attachment", Attachment.Storage.MinioConfig.Bucket) } func Test_getStorageTypeSectionOverridesStorageSection(t *testing.T) { @@ -66,7 +46,7 @@ MINIO_BUCKET = gitea assert.NoError(t, loadAttachmentFrom(cfg)) assert.EqualValues(t, "minio", Attachment.Storage.Type) - assert.EqualValues(t, "gitea-minio", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "gitea-minio", Attachment.Storage.MinioConfig.Bucket) } func Test_getStorageSpecificOverridesStorage(t *testing.T) { @@ -87,7 +67,7 @@ STORAGE_TYPE = local assert.NoError(t, loadAttachmentFrom(cfg)) assert.EqualValues(t, "minio", Attachment.Storage.Type) - assert.EqualValues(t, "gitea-attachment", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "gitea-attachment", Attachment.Storage.MinioConfig.Bucket) } func Test_getStorageGetDefaults(t *testing.T) { @@ -96,7 +76,8 @@ func Test_getStorageGetDefaults(t *testing.T) { assert.NoError(t, loadAttachmentFrom(cfg)) - assert.EqualValues(t, "gitea", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) + // default storage is local, so bucket is empty + assert.EqualValues(t, "", Attachment.Storage.MinioConfig.Bucket) } func Test_getStorageInheritNameSectionType(t *testing.T) { @@ -111,3 +92,39 @@ STORAGE_TYPE = minio assert.EqualValues(t, "minio", Attachment.Storage.Type) } + +func Test_AttachmentStorage(t *testing.T) { + iniStr := ` +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +[storage] +STORAGE_TYPE = minio +MINIO_ENDPOINT = s3.my-domain.net +MINIO_BUCKET = gitea +MINIO_LOCATION = homenet +MINIO_USE_SSL = true +MINIO_ACCESS_KEY_ID = correct_key +MINIO_SECRET_ACCESS_KEY = correct_key +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadAttachmentFrom(cfg)) + storage := Attachment.Storage + + assert.EqualValues(t, "minio", storage.Type) + assert.EqualValues(t, "gitea", storage.MinioConfig.Bucket) +} + +func Test_AttachmentStorage1(t *testing.T) { + iniStr := ` +[storage] +STORAGE_TYPE = minio +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadAttachmentFrom(cfg)) + assert.EqualValues(t, "minio", Attachment.Storage.Type) + assert.EqualValues(t, "gitea", Attachment.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "attachments/", Attachment.Storage.MinioConfig.BasePath) +} diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 8b317d94e32c7..c8a4538404e9f 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "time" @@ -313,3 +314,22 @@ func NewConfigProviderForLocale(source any, others ...any) (ConfigProvider, erro func init() { ini.PrettyFormat = false } + +// MustSectionKeyString to avoid use ini's MustString which will write data back to the ini's memory +// this function is safe to be instead of ini's MustString +func MustSectionKeyString(section ConfigSection, key string, defaults ...string) string { + v := section.Key(key).String() + if v == "" && len(defaults) > 0 { + return defaults[0] + } + return v +} + +func MustSectionKeyBool(section ConfigSection, key string, defaults ...bool) bool { + v := section.Key(key).String() + if v == "" && len(defaults) > 0 { + return defaults[0] + } + b, _ := strconv.ParseBool(v) + return b +} diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 65c7b37092370..fffd5436939cb 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -25,19 +25,29 @@ var LFS = struct { func loadLFSFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("server") - if err := sec.MapTo(&LFS); err != nil { + err := sec.MapTo(&LFS) + if err != nil { return fmt.Errorf("failed to map LFS settings: %v", err) } - lfsSec := rootCfg.Section("lfs") + lfsSec, _ := rootCfg.GetSection("lfs") // Specifically default PATH to LFS_CONTENT_PATH // DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version // if these are removed, the warning will not be shown deprecatedSetting(rootCfg, "server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0") - lfsSec.Key("PATH").MustString(sec.Key("LFS_CONTENT_PATH").String()) - - LFS.Storage = getStorage(rootCfg, "lfs", "", lfsSec) + lfsContentPath := sec.Key("LFS_CONTENT_PATH").String() + if lfsContentPath != "" { + LFS.Storage = &Storage{ + Type: LocalStorageType, + Path: lfsContentPath, + } + } else { + LFS.Storage, err = getStorage(rootCfg, "lfs", "", lfsSec) + if err != nil { + return err + } + } // Rest of LFS service settings if LFS.LocksPagingNum == 0 { diff --git a/modules/setting/lfs_test.go b/modules/setting/lfs_test.go index aefba79f34193..3313cae0eb38a 100644 --- a/modules/setting/lfs_test.go +++ b/modules/setting/lfs_test.go @@ -19,7 +19,7 @@ func Test_getStorageInheritNameSectionTypeForLFS(t *testing.T) { assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, "minio", LFS.Storage.Type) - assert.EqualValues(t, "lfs/", cfg.Section("lfs").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath) iniStr = ` [storage.lfs] @@ -30,7 +30,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, "minio", LFS.Storage.Type) - assert.EqualValues(t, "lfs/", cfg.Section("lfs").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath) iniStr = ` [lfs] @@ -44,7 +44,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, "minio", LFS.Storage.Type) - assert.EqualValues(t, "lfs/", cfg.Section("lfs").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath) iniStr = ` [lfs] @@ -59,5 +59,19 @@ STORAGE_TYPE = minio assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, "minio", LFS.Storage.Type) - assert.EqualValues(t, "my_lfs/", cfg.Section("lfs").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "my_lfs/", LFS.Storage.MinioConfig.BasePath) +} + +func Test_LFSStorage1(t *testing.T) { + iniStr := ` +[storage] +STORAGE_TYPE = minio +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadLFSFrom(cfg)) + assert.EqualValues(t, "minio", LFS.Storage.Type) + assert.EqualValues(t, "gitea", LFS.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath) } diff --git a/modules/setting/packages.go b/modules/setting/packages.go index e3c2c059ba04d..dc8d98d29fbc5 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -50,14 +50,21 @@ var ( } ) -func loadPackagesFrom(rootCfg ConfigProvider) error { - sec := rootCfg.Section("packages") - err := sec.MapTo(&Packages) - if err != nil { +func loadPackagesFrom(rootCfg ConfigProvider) (err error) { + sec, _ := rootCfg.GetSection("packages") + if sec == nil { + Packages.Storage, err = getStorage(rootCfg, "packages", "", nil) + return err + } + + if err = sec.MapTo(&Packages); err != nil { return fmt.Errorf("failed to map Packages settings: %v", err) } - Packages.Storage = getStorage(rootCfg, "packages", "", sec) + Packages.Storage, err = getStorage(rootCfg, "packages", "", sec) + if err != nil { + return err + } appURL, _ := url.Parse(AppURL) Packages.RegistryHost = appURL.Host diff --git a/modules/setting/packages_test.go b/modules/setting/packages_test.go index 2499b9ced4261..87de276041764 100644 --- a/modules/setting/packages_test.go +++ b/modules/setting/packages_test.go @@ -41,7 +41,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadPackagesFrom(cfg)) assert.EqualValues(t, "minio", Packages.Storage.Type) - assert.EqualValues(t, "packages/", cfg.Section("packages").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "packages/", Packages.Storage.MinioConfig.BasePath) // we can also configure packages storage directly iniStr = ` @@ -53,7 +53,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadPackagesFrom(cfg)) assert.EqualValues(t, "minio", Packages.Storage.Type) - assert.EqualValues(t, "packages/", cfg.Section("packages").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "packages/", Packages.Storage.MinioConfig.BasePath) // or we can indicate the storage type in the packages section iniStr = ` @@ -68,7 +68,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadPackagesFrom(cfg)) assert.EqualValues(t, "minio", Packages.Storage.Type) - assert.EqualValues(t, "packages/", cfg.Section("packages").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "packages/", Packages.Storage.MinioConfig.BasePath) // or we can indicate the storage type and minio base path in the packages section iniStr = ` @@ -84,5 +84,115 @@ STORAGE_TYPE = minio assert.NoError(t, loadPackagesFrom(cfg)) assert.EqualValues(t, "minio", Packages.Storage.Type) - assert.EqualValues(t, "my_packages/", cfg.Section("packages").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "my_packages/", Packages.Storage.MinioConfig.BasePath) +} + +func Test_PackageStorage1(t *testing.T) { + iniStr := ` +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +[packages] +MINIO_BASE_PATH = packages/ +SERVE_DIRECT = true +[storage] +STORAGE_TYPE = minio +MINIO_ENDPOINT = s3.my-domain.net +MINIO_BUCKET = gitea +MINIO_LOCATION = homenet +MINIO_USE_SSL = true +MINIO_ACCESS_KEY_ID = correct_key +MINIO_SECRET_ACCESS_KEY = correct_key +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadPackagesFrom(cfg)) + storage := Packages.Storage + + assert.EqualValues(t, "minio", storage.Type) + assert.EqualValues(t, "gitea", storage.MinioConfig.Bucket) + assert.EqualValues(t, "packages/", storage.MinioConfig.BasePath) + assert.True(t, storage.MinioConfig.ServeDirect) +} + +func Test_PackageStorage2(t *testing.T) { + iniStr := ` +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +[storage.packages] +MINIO_BASE_PATH = packages/ +SERVE_DIRECT = true +[storage] +STORAGE_TYPE = minio +MINIO_ENDPOINT = s3.my-domain.net +MINIO_BUCKET = gitea +MINIO_LOCATION = homenet +MINIO_USE_SSL = true +MINIO_ACCESS_KEY_ID = correct_key +MINIO_SECRET_ACCESS_KEY = correct_key +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadPackagesFrom(cfg)) + storage := Packages.Storage + + assert.EqualValues(t, "minio", storage.Type) + assert.EqualValues(t, "gitea", storage.MinioConfig.Bucket) + assert.EqualValues(t, "packages/", storage.MinioConfig.BasePath) + assert.True(t, storage.MinioConfig.ServeDirect) +} + +func Test_PackageStorage3(t *testing.T) { + iniStr := ` +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +[packages] +STORAGE_TYPE = my_cfg +MINIO_BASE_PATH = my_packages/ +SERVE_DIRECT = true +[storage.my_cfg] +STORAGE_TYPE = minio +MINIO_ENDPOINT = s3.my-domain.net +MINIO_BUCKET = gitea +MINIO_LOCATION = homenet +MINIO_USE_SSL = true +MINIO_ACCESS_KEY_ID = correct_key +MINIO_SECRET_ACCESS_KEY = correct_key +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadPackagesFrom(cfg)) + storage := Packages.Storage + + assert.EqualValues(t, "minio", storage.Type) + assert.EqualValues(t, "gitea", storage.MinioConfig.Bucket) + assert.EqualValues(t, "my_packages/", storage.MinioConfig.BasePath) + assert.True(t, storage.MinioConfig.ServeDirect) +} + +func Test_PackageStorage4(t *testing.T) { + iniStr := ` +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +[storage.packages] +STORAGE_TYPE = my_cfg +MINIO_BASE_PATH = my_packages/ +SERVE_DIRECT = true +[storage.my_cfg] +STORAGE_TYPE = minio +MINIO_ENDPOINT = s3.my-domain.net +MINIO_BUCKET = gitea +MINIO_LOCATION = homenet +MINIO_USE_SSL = true +MINIO_ACCESS_KEY_ID = correct_key +MINIO_SECRET_ACCESS_KEY = correct_key +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadPackagesFrom(cfg)) + storage := Packages.Storage + + assert.EqualValues(t, "minio", storage.Type) + assert.EqualValues(t, "gitea", storage.MinioConfig.Bucket) + assert.EqualValues(t, "my_packages/", storage.MinioConfig.BasePath) + assert.True(t, storage.MinioConfig.ServeDirect) } diff --git a/modules/setting/picture.go b/modules/setting/picture.go index b2e5794cf3a9a..fafae45babafe 100644 --- a/modules/setting/picture.go +++ b/modules/setting/picture.go @@ -42,7 +42,11 @@ func loadAvatarsFrom(rootCfg ConfigProvider) error { // Specifically default PATH to AVATAR_UPLOAD_PATH avatarSec.Key("PATH").MustString(sec.Key("AVATAR_UPLOAD_PATH").String()) - Avatar.Storage = getStorage(rootCfg, "avatars", storageType, avatarSec) + var err error + Avatar.Storage, err = getStorage(rootCfg, "avatars", storageType, avatarSec) + if err != nil { + return err + } Avatar.MaxWidth = sec.Key("AVATAR_MAX_WIDTH").MustInt(4096) Avatar.MaxHeight = sec.Key("AVATAR_MAX_HEIGHT").MustInt(4096) @@ -92,7 +96,11 @@ func loadRepoAvatarFrom(rootCfg ConfigProvider) error { // Specifically default PATH to AVATAR_UPLOAD_PATH repoAvatarSec.Key("PATH").MustString(sec.Key("REPOSITORY_AVATAR_UPLOAD_PATH").String()) - RepoAvatar.Storage = getStorage(rootCfg, "repo-avatars", storageType, repoAvatarSec) + var err error + RepoAvatar.Storage, err = getStorage(rootCfg, "repo-avatars", storageType, repoAvatarSec) + if err != nil { + return err + } RepoAvatar.Fallback = sec.Key("REPOSITORY_AVATAR_FALLBACK").MustString("none") RepoAvatar.FallbackImage = sec.Key("REPOSITORY_AVATAR_FALLBACK_IMAGE").MustString(AppSubURL + "/assets/img/repo_default.png") diff --git a/modules/setting/repository_archive.go b/modules/setting/repository_archive.go index 3f42e41f8497f..9d24afa9e7f10 100644 --- a/modules/setting/repository_archive.go +++ b/modules/setting/repository_archive.go @@ -9,12 +9,17 @@ var RepoArchive = struct { Storage *Storage }{} -func loadRepoArchiveFrom(rootCfg ConfigProvider) error { - sec := rootCfg.Section("repo-archive") +func loadRepoArchiveFrom(rootCfg ConfigProvider) (err error) { + sec, _ := rootCfg.GetSection("repo-archive") + if sec == nil { + RepoArchive.Storage, err = getStorage(rootCfg, "repo-archive", "", nil) + return err + } + if err := sec.MapTo(&RepoArchive); err != nil { return fmt.Errorf("mapto repoarchive failed: %v", err) } - RepoArchive.Storage = getStorage(rootCfg, "repo-archive", "", sec) - return nil + RepoArchive.Storage, err = getStorage(rootCfg, "repo-archive", "", sec) + return err } diff --git a/modules/setting/repository_archive_test.go b/modules/setting/repository_archive_test.go index db3ffb6e9648b..a0f91f0da12f3 100644 --- a/modules/setting/repository_archive_test.go +++ b/modules/setting/repository_archive_test.go @@ -20,7 +20,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadRepoArchiveFrom(cfg)) assert.EqualValues(t, "minio", RepoArchive.Storage.Type) - assert.EqualValues(t, "repo-archive/", cfg.Section("repo-archive").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) // we can also configure packages storage directly iniStr = ` @@ -32,7 +32,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadRepoArchiveFrom(cfg)) assert.EqualValues(t, "minio", RepoArchive.Storage.Type) - assert.EqualValues(t, "repo-archive/", cfg.Section("repo-archive").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) // or we can indicate the storage type in the packages section iniStr = ` @@ -47,7 +47,7 @@ STORAGE_TYPE = minio assert.NoError(t, loadRepoArchiveFrom(cfg)) assert.EqualValues(t, "minio", RepoArchive.Storage.Type) - assert.EqualValues(t, "repo-archive/", cfg.Section("repo-archive").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) // or we can indicate the storage type and minio base path in the packages section iniStr = ` @@ -63,5 +63,49 @@ STORAGE_TYPE = minio assert.NoError(t, loadRepoArchiveFrom(cfg)) assert.EqualValues(t, "minio", RepoArchive.Storage.Type) - assert.EqualValues(t, "my_archive/", cfg.Section("repo-archive").Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "my_archive/", RepoArchive.Storage.MinioConfig.BasePath) +} + +func Test_RepoArchiveStorage(t *testing.T) { + iniStr := ` +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +[storage] +STORAGE_TYPE = minio +MINIO_ENDPOINT = s3.my-domain.net +MINIO_BUCKET = gitea +MINIO_LOCATION = homenet +MINIO_USE_SSL = true +MINIO_ACCESS_KEY_ID = correct_key +MINIO_SECRET_ACCESS_KEY = correct_key +` + cfg, err := NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadRepoArchiveFrom(cfg)) + storage := RepoArchive.Storage + + assert.EqualValues(t, "minio", storage.Type) + assert.EqualValues(t, "gitea", storage.MinioConfig.Bucket) + + iniStr = ` +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +[storage.repo-archive] +STORAGE_TYPE = s3 +[storage.s3] +STORAGE_TYPE = minio +MINIO_ENDPOINT = s3.my-domain.net +MINIO_BUCKET = gitea +MINIO_LOCATION = homenet +MINIO_USE_SSL = true +MINIO_ACCESS_KEY_ID = correct_key +MINIO_SECRET_ACCESS_KEY = correct_key +` + cfg, err = NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadRepoArchiveFrom(cfg)) + storage = RepoArchive.Storage + + assert.EqualValues(t, "minio", storage.Type) + assert.EqualValues(t, "gitea", storage.MinioConfig.Bucket) } diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 2e4c22a52b59b..1fa0cc30404e0 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -4,35 +4,47 @@ package setting import ( + "errors" + "fmt" "path/filepath" - "reflect" ) -// Storage represents configuration of storages -type Storage struct { - Type string - Path string - Section ConfigSection - ServeDirect bool +// StorageType is a type of Storage +type StorageType string + +// LocalStorageType is the type descriptor for local storage +const LocalStorageType StorageType = "local" + +// MinioStorageType is the type descriptor for minio storage +const MinioStorageType StorageType = "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"` + ServeDirect bool `ini:"SERVE_DIRECT"` } -// MapTo implements the Mappable interface -func (s *Storage) MapTo(v interface{}) error { - pathValue := reflect.ValueOf(v).Elem().FieldByName("Path") - if pathValue.IsValid() && pathValue.Kind() == reflect.String { - pathValue.SetString(s.Path) - } - if s.Section != nil { - return s.Section.MapTo(v) - } - return nil +// Storage represents configuration of storages +type Storage struct { + Type StorageType // local or minio + Path string // for local type + MinioConfig MinioStorageConfig // for minio type } const storageSectionName = "storage" -func getStorageSection(rootCfg ConfigProvider) ConfigSection { +func getDefaultStorageSection(rootCfg ConfigProvider) ConfigSection { storageSec := rootCfg.Section(storageSectionName) // Global Defaults + storageSec.Key("STORAGE_TYPE").MustString("local") storageSec.Key("MINIO_ENDPOINT").MustString("localhost:9000") storageSec.Key("MINIO_ACCESS_KEY_ID").MustString("") storageSec.Key("MINIO_SECRET_ACCESS_KEY").MustString("") @@ -44,52 +56,99 @@ func getStorageSection(rootCfg ConfigProvider) ConfigSection { return storageSec } -func getStorage(rootCfg ConfigProvider, name, typ string, targetSec ConfigSection) *Storage { - sec := getStorageSection(rootCfg) +func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*Storage, error) { + if name == "" { + return nil, errors.New("no name for storage") + } - if targetSec == nil { - targetSec, _ = rootCfg.NewSection(name) + var targetSec ConfigSection = nil + if typ != "" { + var err error + targetSec, err = rootCfg.GetSection(storageSectionName + "." + typ) + if err != nil { + if typ != "local" && typ != "minio" { + 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 typ != "local" && typ != "minio" { + return nil, fmt.Errorf("unknow storage type %q", typ) + } + targetSec.Key("STORAGE_TYPE").SetValue(typ) + } else if targetType != "local" && targetType != "minio" { + return nil, fmt.Errorf("unknow storage type %q for section storage.%v", targetType, typ) + } + } } - var storage Storage - storage.Section = targetSec - storage.Type = typ + packageNameSec, _ := rootCfg.GetSection(storageSectionName + "." + name) - overrides := make([]ConfigSection, 0, 3) - nameSec, err := rootCfg.GetSection(storageSectionName + "." + name) - if err == nil { - overrides = append(overrides, nameSec) + if targetSec == nil { + targetSec = sec } - - typeSec, err := rootCfg.GetSection(storageSectionName + "." + typ) - if err == nil { - overrides = append(overrides, typeSec) - nextType := typeSec.Key("STORAGE_TYPE").String() - if len(nextType) > 0 { - storage.Type = nextType // Support custom STORAGE_TYPE + if targetSec == nil { + targetSec = packageNameSec + } + if targetSec == nil { + targetSec = getDefaultStorageSection(rootCfg) + } else { + targetType := targetSec.Key("STORAGE_TYPE").String() + switch { + case targetType == "": + targetSec = getDefaultStorageSection(rootCfg) + default: + newTargetSec, _ := rootCfg.GetSection(storageSectionName + "." + targetType) + if newTargetSec == nil { + if targetType != "local" && targetType != "minio" { + return nil, fmt.Errorf("invalid storage section %s.%q", storageSectionName, targetType) + } + } else { + targetSec = newTargetSec + if targetType == "local" || targetType == "minio" { + tp := targetSec.Key("STORAGE_TYPE").String() + if tp == "" { + targetSec.Key("STORAGE_TYPE").SetValue(targetType) + } + } + } } } - overrides = append(overrides, sec) - for _, override := range overrides { - for _, key := range override.Keys() { - if !targetSec.HasKey(key.Name()) { - _, _ = targetSec.NewKey(key.Name(), key.Value()) - } + targetType := targetSec.Key("STORAGE_TYPE").String() + if targetType != "local" && targetType != "minio" { + return nil, fmt.Errorf("invalid storage type %q", targetType) + } + + var storage Storage + storage.Type = StorageType(targetType) + + switch targetType { + case string(LocalStorageType): + storage.Path = targetSec.Key("PATH").MustString(filepath.Join(AppDataPath, name)) + if !filepath.IsAbs(storage.Path) { + storage.Path = filepath.Join(AppWorkPath, storage.Path) + targetSec.Key("PATH").SetValue(storage.Path) } - if len(storage.Type) == 0 { - storage.Type = override.Key("STORAGE_TYPE").String() + case string(MinioStorageType): + storage.MinioConfig.BasePath = name + "/" + + if err := targetSec.MapTo(&storage.MinioConfig); err != nil { + return nil, fmt.Errorf("map minio config failed: %v", err) + } + // extra config section will be read SERVE_DIRECT, PATH, MINIO_BASE_PATH to override the targetsec + extraConfigSec := sec + if extraConfigSec == nil { + extraConfigSec = packageNameSec } - } - storage.ServeDirect = storage.Section.Key("SERVE_DIRECT").MustBool(false) - // 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) + if extraConfigSec != nil { + storage.MinioConfig.ServeDirect = MustSectionKeyBool(extraConfigSec, "SERVE_DIRECT", storage.MinioConfig.ServeDirect) + storage.MinioConfig.BasePath = MustSectionKeyString(extraConfigSec, "MINIO_BASE_PATH", storage.MinioConfig.BasePath) + storage.MinioConfig.Bucket = MustSectionKeyString(extraConfigSec, "MINIO_BUCKET", storage.MinioConfig.Bucket) + } } - storage.Section.Key("MINIO_BASE_PATH").MustString(name + "/") - return &storage + return &storage, nil } diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go index 6dae2f5a47e25..4eda7646e81d0 100644 --- a/modules/setting/storage_test.go +++ b/modules/setting/storage_test.go @@ -18,19 +18,20 @@ MINIO_BUCKET = gitea-lfs MINIO_BUCKET = gitea-attachment [storage] +STORAGE_TYPE = minio MINIO_BUCKET = gitea-storage ` cfg, err := NewConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadAttachmentFrom(cfg)) - assert.EqualValues(t, "gitea-attachment", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "gitea-attachment", Attachment.Storage.MinioConfig.Bucket) assert.NoError(t, loadLFSFrom(cfg)) - assert.EqualValues(t, "gitea-lfs", LFS.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "gitea-lfs", LFS.Storage.MinioConfig.Bucket) assert.NoError(t, loadAvatarsFrom(cfg)) - assert.EqualValues(t, "gitea-storage", Avatar.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "gitea-storage", Avatar.Storage.MinioConfig.Bucket) } func Test_getStorageUseOtherNameAsType(t *testing.T) { @@ -39,16 +40,17 @@ func Test_getStorageUseOtherNameAsType(t *testing.T) { STORAGE_TYPE = lfs [storage.lfs] +STORAGE_TYPE = minio MINIO_BUCKET = gitea-storage ` cfg, err := NewConfigProviderFromData(iniStr) assert.NoError(t, err) assert.NoError(t, loadAttachmentFrom(cfg)) - assert.EqualValues(t, "gitea-storage", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "gitea-storage", Attachment.Storage.MinioConfig.Bucket) assert.NoError(t, loadLFSFrom(cfg)) - assert.EqualValues(t, "gitea-storage", LFS.Storage.Section.Key("MINIO_BUCKET").String()) + assert.EqualValues(t, "gitea-storage", LFS.Storage.MinioConfig.Bucket) } func Test_getStorageInheritStorageType(t *testing.T) { @@ -59,52 +61,32 @@ STORAGE_TYPE = minio cfg, err := NewConfigProviderFromData(iniStr) assert.NoError(t, err) - assert.NoError(t, loadAttachmentFrom(cfg)) - assert.EqualValues(t, "minio", Attachment.Storage.Type) - assert.EqualValues(t, "gitea", Attachment.Storage.Section.Key("MINIO_BUCKET").String()) - assert.EqualValues(t, "attachments/", Attachment.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) - - assert.NoError(t, loadLFSFrom(cfg)) - assert.EqualValues(t, "minio", LFS.Storage.Type) - assert.EqualValues(t, "gitea", LFS.Storage.Section.Key("MINIO_BUCKET").String()) - assert.EqualValues(t, "lfs/", LFS.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) - assert.NoError(t, loadPackagesFrom(cfg)) assert.EqualValues(t, "minio", Packages.Storage.Type) - assert.EqualValues(t, "gitea", Packages.Storage.Section.Key("MINIO_BUCKET").String()) - assert.EqualValues(t, "packages/", Packages.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "gitea", Packages.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "packages/", Packages.Storage.MinioConfig.BasePath) assert.NoError(t, loadRepoArchiveFrom(cfg)) assert.EqualValues(t, "minio", RepoArchive.Storage.Type) - assert.EqualValues(t, "gitea", RepoArchive.Storage.Section.Key("MINIO_BUCKET").String()) - assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "gitea", RepoArchive.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) assert.NoError(t, loadActionsFrom(cfg)) assert.EqualValues(t, "minio", Actions.LogStorage.Type) - assert.EqualValues(t, "gitea", Actions.LogStorage.Section.Key("MINIO_BUCKET").String()) - assert.EqualValues(t, "actions_log/", Actions.LogStorage.Section.Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "gitea", Actions.LogStorage.MinioConfig.Bucket) + assert.EqualValues(t, "actions_log/", Actions.LogStorage.MinioConfig.BasePath) + + assert.EqualValues(t, "minio", Actions.ArtifactStorage.Type) + assert.EqualValues(t, "gitea", Actions.ArtifactStorage.MinioConfig.Bucket) + assert.EqualValues(t, "actions_artifacts/", Actions.ArtifactStorage.MinioConfig.BasePath) assert.NoError(t, loadAvatarsFrom(cfg)) assert.EqualValues(t, "minio", Avatar.Storage.Type) - assert.EqualValues(t, "gitea", Avatar.Storage.Section.Key("MINIO_BUCKET").String()) - assert.EqualValues(t, "avatars/", Avatar.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) + assert.EqualValues(t, "gitea", Avatar.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "avatars/", Avatar.Storage.MinioConfig.BasePath) assert.NoError(t, loadRepoAvatarFrom(cfg)) assert.EqualValues(t, "minio", RepoAvatar.Storage.Type) - assert.EqualValues(t, "gitea", RepoAvatar.Storage.Section.Key("MINIO_BUCKET").String()) - assert.EqualValues(t, "repo-avatars/", RepoAvatar.Storage.Section.Key("MINIO_BASE_PATH").MustString("")) - - iniStr = ` -[storage.attachments] -STORAGE_TYPE = minio -` - cfg, err = NewConfigProviderFromData(iniStr) - assert.NoError(t, err) - - sec := cfg.Section("attachment") - storageType := sec.Key("STORAGE_TYPE").MustString("") - storage, err := getStorage(cfg, "attachments", sec, storageType) - assert.NoError(t, err) - - assert.EqualValues(t, "minio", storage.Type) + assert.EqualValues(t, "gitea", RepoAvatar.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "repo-avatars/", RepoAvatar.Storage.MinioConfig.BasePath) } diff --git a/modules/storage/local.go b/modules/storage/local.go index 73ef306979abe..f5fc3d88c5675 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -12,14 +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"` @@ -164,5 +162,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 1c4b856ab6afd..1cade6c5b4208 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "testing" + "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" ) @@ -55,5 +56,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, LocalStorageConfig{Path: dir}) } diff --git a/modules/storage/minio.go b/modules/storage/minio.go index c78f351e9ce03..9b15ead1282e7 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,11 @@ 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) +func NewMinioStorage(ctx context.Context, cfg any) (ObjectStorage, error) { + config, ok := cfg.(setting.MinioStorageConfig) + if !ok { + return nil, fmt.Errorf("config struct is not MinioStorageConfig but %#v", cfg) } - config := configInterface.(MinioStorageConfig) - if config.ChecksumAlgorithm != "" && config.ChecksumAlgorithm != "default" && config.ChecksumAlgorithm != "md5" { return nil, fmt.Errorf("invalid minio checksum algorithm: %s", config.ChecksumAlgorithm) } @@ -258,5 +241,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 bfddf33032260..11b490617305a 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,7 +15,7 @@ func TestMinioStorageIterator(t *testing.T) { t.Skip("minioStorage not present outside of CI") return } - testStorageIterator(t, string(MinioStorageType), MinioStorageConfig{ + testStorageIterator(t, setting.MinioStorageType, setting.MinioStorageConfig{ Endpoint: "127.0.0.1:9000", AccessKeyID: "123456", SecretAccessKey: "12345678", diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 5b6efccb6a489..dc3aec2bb9bb9 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -37,8 +37,7 @@ 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) @@ -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 interface{}) (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) } diff --git a/modules/storage/storage_test.go b/modules/storage/storage_test.go index b517a9e71a16e..bd0f366178923 100644 --- a/modules/storage/storage_test.go +++ b/modules/storage/storage_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" ) -func testStorageIterator(t *testing.T, typStr string, cfg interface{}) { +func testStorageIterator(t *testing.T, typStr Type, cfg interface{}) { l, err := NewStorage(typStr, cfg) assert.NoError(t, err) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 12f7d4fd7c8e6..25e39f9d55a62 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -200,7 +200,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { return } - if setting.LFS.Storage.ServeDirect { + if setting.LFS.Storage.MinioConfig.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) if u != nil && err == nil { @@ -320,7 +320,7 @@ func download(ctx *context.APIContext, archiveName string, archiver *repo_model. downloadName := ctx.Repo.Repository.Name + "-" + archiveName rPath := archiver.RelativePath() - if setting.RepoArchive.Storage.ServeDirect { + if setting.RepoArchive.Storage.MinioConfig.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.RepoArchives.URL(rPath, downloadName) if u != nil && err == nil { diff --git a/routers/web/base.go b/routers/web/base.go index 1f6c4fbfc5956..5ebdadf32846b 100644 --- a/routers/web/base.go +++ b/routers/web/base.go @@ -23,7 +23,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor prefix = strings.Trim(prefix, "/") funcInfo := routing.GetFuncInfo(storageHandler, prefix) return func(next http.Handler) http.Handler { - if storageSetting.ServeDirect { + if storageSetting.MinioConfig.ServeDirect { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { if req.Method != "GET" && req.Method != "HEAD" { next.ServeHTTP(w, req) diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index 52c28374a65ac..af1842ad109a5 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -126,7 +126,7 @@ func ServeAttachment(ctx *context.Context, uuid string) { return } - if setting.Attachment.Storage.ServeDirect { + if setting.Attachment.Storage.MinioConfig.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name) diff --git a/routers/web/repo/download.go b/routers/web/repo/download.go index 9fd15a6441dd4..fd67b82ef277a 100644 --- a/routers/web/repo/download.go +++ b/routers/web/repo/download.go @@ -53,7 +53,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified time.Time return nil } - if setting.LFS.Storage.ServeDirect { + if setting.LFS.Storage.MinioConfig.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) if u != nil && err == nil { diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 47c89923046e4..a1e1346b38c4b 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -427,7 +427,7 @@ func download(ctx *context.Context, archiveName string, archiver *repo_model.Rep downloadName := ctx.Repo.Repository.Name + "-" + archiveName rPath := archiver.RelativePath() - if setting.RepoArchive.Storage.ServeDirect { + if setting.RepoArchive.Storage.MinioConfig.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.RepoArchives.URL(rPath, downloadName) if u != nil && err == nil { diff --git a/services/lfs/server.go b/services/lfs/server.go index 38ebe560e9652..a18e752d4791b 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -452,7 +452,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa if download { var link *lfs_module.Link - if setting.LFS.Storage.ServeDirect { + if setting.LFS.Storage.MinioConfig.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid) if u != nil && err == nil { From 793936d9d71f0abcdc5b73c60e1e363fd4db09bb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 7 Jun 2023 12:22:21 +0800 Subject: [PATCH 19/28] Fix lint --- modules/setting/storage.go | 8 ++++---- modules/storage/local_test.go | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 1fa0cc30404e0..01fc40656fb3d 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -61,7 +61,7 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S return nil, errors.New("no name for storage") } - var targetSec ConfigSection = nil + var targetSec ConfigSection if typ != "" { var err error targetSec, err = rootCfg.GetSection(storageSectionName + "." + typ) @@ -83,13 +83,13 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S } } - packageNameSec, _ := rootCfg.GetSection(storageSectionName + "." + name) + storageNameSec, _ := rootCfg.GetSection(storageSectionName + "." + name) if targetSec == nil { targetSec = sec } if targetSec == nil { - targetSec = packageNameSec + targetSec = storageNameSec } if targetSec == nil { targetSec = getDefaultStorageSection(rootCfg) @@ -140,7 +140,7 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S // extra config section will be read SERVE_DIRECT, PATH, MINIO_BASE_PATH to override the targetsec extraConfigSec := sec if extraConfigSec == nil { - extraConfigSec = packageNameSec + extraConfigSec = storageNameSec } if extraConfigSec != nil { diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go index 1cade6c5b4208..3e06f05c70905 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -9,6 +9,7 @@ import ( "testing" "code.gitea.io/gitea/modules/setting" + "github.com/stretchr/testify/assert" ) From 82b4ec29043963f9b054b62d844129b56047f880 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 7 Jun 2023 16:40:41 +0800 Subject: [PATCH 20/28] Remove unused interface --- cmd/migrate_storage.go | 24 +++++++------- cmd/migrate_storage_test.go | 3 +- modules/setting/storage.go | 7 +++-- modules/storage/helper.go | 56 --------------------------------- modules/storage/local.go | 14 +-------- modules/storage/local_test.go | 2 +- modules/storage/minio.go | 7 ++--- modules/storage/minio_test.go | 14 +++++---- modules/storage/storage.go | 22 ++++++------- modules/storage/storage_test.go | 4 ++- tests/test_utils.go | 2 +- 11 files changed, 46 insertions(+), 109 deletions(-) diff --git a/cmd/migrate_storage.go b/cmd/migrate_storage.go index 83b15c877e8b1..511db6cbf7519 100644 --- a/cmd/migrate_storage.go +++ b/cmd/migrate_storage.go @@ -187,22 +187,24 @@ func runMigrateStorage(ctx *cli.Context) error { } dstStorage, err = storage.NewLocalStorage( stdCtx, - storage.LocalStorageConfig{ + &setting.Storage{ Path: p, }) case string(setting.MinioStorageType): dstStorage, err = storage.NewMinioStorage( stdCtx, - setting.MinioStorageConfig{ - Endpoint: ctx.String("minio-endpoint"), - AccessKeyID: ctx.String("minio-access-key-id"), - SecretAccessKey: ctx.String("minio-secret-access-key"), - Bucket: ctx.String("minio-bucket"), - Location: ctx.String("minio-location"), - BasePath: ctx.String("minio-base-path"), - UseSSL: ctx.Bool("minio-use-ssl"), - InsecureSkipVerify: ctx.Bool("minio-insecure-skip-verify"), - ChecksumAlgorithm: ctx.String("minio-checksum-algorithm"), + &setting.Storage{ + MinioConfig: setting.MinioStorageConfig{ + Endpoint: ctx.String("minio-endpoint"), + AccessKeyID: ctx.String("minio-access-key-id"), + SecretAccessKey: ctx.String("minio-secret-access-key"), + Bucket: ctx.String("minio-bucket"), + Location: ctx.String("minio-location"), + BasePath: ctx.String("minio-base-path"), + UseSSL: ctx.Bool("minio-use-ssl"), + InsecureSkipVerify: ctx.Bool("minio-insecure-skip-verify"), + ChecksumAlgorithm: ctx.String("minio-checksum-algorithm"), + }, }) default: return fmt.Errorf("unsupported storage type: %s", ctx.String("storage")) diff --git a/cmd/migrate_storage_test.go b/cmd/migrate_storage_test.go index 7f3de894ba3e2..644e0dc18bc76 100644 --- a/cmd/migrate_storage_test.go +++ b/cmd/migrate_storage_test.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" packages_module "code.gitea.io/gitea/modules/packages" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" packages_service "code.gitea.io/gitea/services/packages" @@ -57,7 +58,7 @@ func TestMigratePackages(t *testing.T) { dstStorage, err := storage.NewLocalStorage( ctx, - storage.LocalStorageConfig{ + &setting.Storage{ Path: p, }) assert.NoError(t, err) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 01fc40656fb3d..c9d8ac680389e 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -34,9 +34,10 @@ type MinioStorageConfig struct { // Storage represents configuration of storages type Storage struct { - Type StorageType // local or minio - Path string // for local type - MinioConfig MinioStorageConfig // for minio type + Type StorageType // local or minio + Path string // for local type + TemporaryPath string + MinioConfig MinioStorageConfig // for minio type } const storageSectionName = "storage" diff --git a/modules/storage/helper.go b/modules/storage/helper.go index d1959830b977d..f8dff9e64d912 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 f5fc3d88c5675..9bb532f1df812 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -18,12 +18,6 @@ import ( var _ ObjectStorage = &LocalStorage{} -// 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 @@ -32,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) } diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go index 3e06f05c70905..e230323f67953 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -57,5 +57,5 @@ func TestBuildLocalPath(t *testing.T) { func TestLocalStorageIterator(t *testing.T) { dir := filepath.Join(os.TempDir(), "TestLocalStorageIteratorTestDir") - testStorageIterator(t, setting.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 9b15ead1282e7..81774fb9cfada 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -72,11 +72,8 @@ func convertMinioErr(err error) error { } // NewMinioStorage returns a minio storage -func NewMinioStorage(ctx context.Context, cfg any) (ObjectStorage, error) { - config, ok := cfg.(setting.MinioStorageConfig) - if !ok { - return nil, fmt.Errorf("config struct is not MinioStorageConfig but %#v", cfg) - } +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) } diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index 11b490617305a..af392b7e2215e 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -15,11 +15,13 @@ func TestMinioStorageIterator(t *testing.T) { t.Skip("minioStorage not present outside of CI") return } - testStorageIterator(t, setting.MinioStorageType, setting.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 dc3aec2bb9bb9..c3396f0c7fbd3 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -40,12 +40,12 @@ func IsErrInvalidConfiguration(err error) bool { 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 } @@ -150,7 +150,7 @@ func Init() error { } // NewStorage takes a storage type and some config and returns an ObjectStorage or an error -func NewStorage(typStr Type, cfg interface{}) (ObjectStorage, error) { +func NewStorage(typStr Type, cfg *setting.Storage) (ObjectStorage, error) { if len(typStr) == 0 { typStr = setting.LocalStorageType } @@ -164,7 +164,7 @@ func NewStorage(typStr Type, 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 } @@ -174,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 } @@ -184,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 } @@ -206,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 } @@ -217,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 bd0f366178923..5e3e9c7dba937 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 Type, cfg interface{}) { +func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) { l, err := NewStorage(typStr, cfg) assert.NoError(t, err) diff --git a/tests/test_utils.go b/tests/test_utils.go index 6f0fb25560d9b..628e98d98c34d 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -214,7 +214,7 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() { // load LFS object fixtures // (LFS storage can be on any of several backends, including remote servers, so we init it with the storage API) - lfsFixtures, err := storage.NewStorage("", storage.LocalStorageConfig{Path: path.Join(filepath.Dir(setting.AppPath), "tests/gitea-lfs-meta")}) + lfsFixtures, err := storage.NewStorage("", &setting.Storage{Path: path.Join(filepath.Dir(setting.AppPath), "tests/gitea-lfs-meta")}) assert.NoError(t, err) assert.NoError(t, storage.Clean(storage.LFS)) assert.NoError(t, lfsFixtures.IterateObjects("", func(path string, _ storage.Object) error { From a055e53c126c6c75d9b3bb9eaf489659deefce64 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 7 Jun 2023 18:13:33 +0800 Subject: [PATCH 21/28] improve code --- modules/setting/actions_test.go | 7 +++++ modules/setting/attachment_test.go | 3 +++ modules/setting/lfs.go | 4 +-- modules/setting/storage.go | 42 ++++++++++++++++++++++-------- routers/web/base.go | 2 +- routers/web/web.go | 4 +-- templates/admin/config.tmpl | 2 +- tests/test_utils.go | 4 ++- 8 files changed, 50 insertions(+), 18 deletions(-) diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index 77defc1b673e6..04918cb10e096 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -4,6 +4,7 @@ package setting import ( + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -20,6 +21,8 @@ func Test_getStorageInheritNameSectionTypeForActions(t *testing.T) { assert.EqualValues(t, "minio", Actions.LogStorage.Type) assert.EqualValues(t, "actions_log/", Actions.LogStorage.MinioConfig.BasePath) + assert.EqualValues(t, "minio", Actions.ArtifactStorage.Type) + assert.EqualValues(t, "actions_artifacts/", Actions.ArtifactStorage.MinioConfig.BasePath) iniStr = ` [storage.actions_log] @@ -31,6 +34,8 @@ STORAGE_TYPE = minio assert.EqualValues(t, "minio", Actions.LogStorage.Type) assert.EqualValues(t, "actions_log/", Actions.LogStorage.MinioConfig.BasePath) + assert.EqualValues(t, "local", Actions.ArtifactStorage.Type) + assert.EqualValues(t, "actions_artifacts", filepath.Base(Actions.ArtifactStorage.Path)) iniStr = ` [storage.actions_log] @@ -45,4 +50,6 @@ STORAGE_TYPE = minio assert.EqualValues(t, "minio", Actions.LogStorage.Type) assert.EqualValues(t, "actions_log/", Actions.LogStorage.MinioConfig.BasePath) + assert.EqualValues(t, "local", Actions.ArtifactStorage.Type) + assert.EqualValues(t, "actions_artifacts", filepath.Base(Actions.ArtifactStorage.Path)) } diff --git a/modules/setting/attachment_test.go b/modules/setting/attachment_test.go index 6c6dfafb36712..3e8d2da4d9094 100644 --- a/modules/setting/attachment_test.go +++ b/modules/setting/attachment_test.go @@ -27,6 +27,7 @@ MINIO_ENDPOINT = my_minio:9000 assert.EqualValues(t, "minio", Attachment.Storage.Type) assert.EqualValues(t, "my_minio:9000", Attachment.Storage.MinioConfig.Endpoint) assert.EqualValues(t, "gitea-attachment", Attachment.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "attachments/", Attachment.Storage.MinioConfig.BasePath) } func Test_getStorageTypeSectionOverridesStorageSection(t *testing.T) { @@ -47,6 +48,7 @@ MINIO_BUCKET = gitea assert.EqualValues(t, "minio", Attachment.Storage.Type) assert.EqualValues(t, "gitea-minio", Attachment.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "attachments/", Attachment.Storage.MinioConfig.BasePath) } func Test_getStorageSpecificOverridesStorage(t *testing.T) { @@ -68,6 +70,7 @@ STORAGE_TYPE = local assert.EqualValues(t, "minio", Attachment.Storage.Type) assert.EqualValues(t, "gitea-attachment", Attachment.Storage.MinioConfig.Bucket) + assert.EqualValues(t, "attachments/", Attachment.Storage.MinioConfig.BasePath) } func Test_getStorageGetDefaults(t *testing.T) { diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index fffd5436939cb..40d3e4ad899df 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -25,8 +25,7 @@ var LFS = struct { func loadLFSFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("server") - err := sec.MapTo(&LFS) - if err != nil { + if err := sec.MapTo(&LFS); err != nil { return fmt.Errorf("failed to map LFS settings: %v", err) } @@ -43,6 +42,7 @@ func loadLFSFrom(rootCfg ConfigProvider) error { Path: lfsContentPath, } } else { + var err error LFS.Storage, err = getStorage(rootCfg, "lfs", "", lfsSec) if err != nil { return err diff --git a/modules/setting/storage.go b/modules/setting/storage.go index c9d8ac680389e..72ebce65976ee 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -12,11 +12,27 @@ import ( // StorageType is a type of Storage type StorageType string -// LocalStorageType is the type descriptor for local storage -const LocalStorageType StorageType = "local" +const ( + // LocalStorageType is the type descriptor for local storage + LocalStorageType StorageType = "local" + // MinioStorageType is the type descriptor for minio storage + MinioStorageType StorageType = "minio" +) + +var storageTypes = []StorageType{ + LocalStorageType, + MinioStorageType, +} -// MinioStorageType is the type descriptor for minio storage -const MinioStorageType StorageType = "minio" +// IsValidStorageType returns true if the given storage type is valid +func IsValidStorageType(storageType StorageType) bool { + for _, t := range storageTypes { + if t == storageType { + return true + } + } + return false +} // MinioStorageConfig represents the configuration for a minio storage type MinioStorageConfig struct { @@ -67,18 +83,18 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S var err error targetSec, err = rootCfg.GetSection(storageSectionName + "." + typ) if err != nil { - if typ != "local" && typ != "minio" { + 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 typ != "local" && typ != "minio" { + if !IsValidStorageType(StorageType(typ)) { return nil, fmt.Errorf("unknow storage type %q", typ) } targetSec.Key("STORAGE_TYPE").SetValue(typ) - } else if targetType != "local" && targetType != "minio" { + } else if !IsValidStorageType(StorageType(targetType)) { return nil, fmt.Errorf("unknow storage type %q for section storage.%v", targetType, typ) } } @@ -98,16 +114,20 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S targetType := targetSec.Key("STORAGE_TYPE").String() switch { case targetType == "": - targetSec = getDefaultStorageSection(rootCfg) + 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 targetType != "local" && targetType != "minio" { + if !IsValidStorageType(StorageType(targetType)) { return nil, fmt.Errorf("invalid storage section %s.%q", storageSectionName, targetType) } } else { targetSec = newTargetSec - if targetType == "local" || targetType == "minio" { + if IsValidStorageType(StorageType(targetType)) { tp := targetSec.Key("STORAGE_TYPE").String() if tp == "" { targetSec.Key("STORAGE_TYPE").SetValue(targetType) @@ -118,7 +138,7 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S } targetType := targetSec.Key("STORAGE_TYPE").String() - if targetType != "local" && targetType != "minio" { + if !IsValidStorageType(StorageType(targetType)) { return nil, fmt.Errorf("invalid storage type %q", targetType) } diff --git a/routers/web/base.go b/routers/web/base.go index 5ebdadf32846b..e4b7d8ce8db2e 100644 --- a/routers/web/base.go +++ b/routers/web/base.go @@ -19,7 +19,7 @@ import ( "code.gitea.io/gitea/modules/web/routing" ) -func storageHandler(storageSetting setting.Storage, prefix string, objStore storage.ObjectStorage) func(next http.Handler) http.Handler { +func storageHandler(storageSetting *setting.Storage, prefix string, objStore storage.ObjectStorage) func(next http.Handler) http.Handler { prefix = strings.Trim(prefix, "/") funcInfo := routing.GetFuncInfo(storageHandler, prefix) return func(next http.Handler) http.Handler { diff --git a/routers/web/web.go b/routers/web/web.go index 3e408205f7c71..f5037a848ea59 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -109,8 +109,8 @@ func Routes(ctx gocontext.Context) *web.Route { routes.Head("/", misc.DummyOK) // for health check - doesn't need to be passed through gzip handler routes.RouteMethods("/assets/*", "GET, HEAD", CorsHandler(), public.AssetsHandlerFunc("/assets/")) - routes.RouteMethods("/avatars/*", "GET, HEAD", storageHandler(*setting.Avatar.Storage, "avatars", storage.Avatars)) - routes.RouteMethods("/repo-avatars/*", "GET, HEAD", storageHandler(*setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) + routes.RouteMethods("/avatars/*", "GET, HEAD", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars)) + routes.RouteMethods("/repo-avatars/*", "GET, HEAD", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) routes.RouteMethods("/apple-touch-icon.png", "GET, HEAD", misc.StaticRedirect("/assets/img/apple-touch-icon.png")) routes.RouteMethods("/apple-touch-icon-precomposed.png", "GET, HEAD", misc.StaticRedirect("/assets/img/apple-touch-icon.png")) routes.RouteMethods("/favicon.ico", "GET, HEAD", misc.StaticRedirect("/assets/img/favicon.png")) diff --git a/templates/admin/config.tmpl b/templates/admin/config.tmpl index 0c52830ab080d..9b434797df2a8 100644 --- a/templates/admin/config.tmpl +++ b/templates/admin/config.tmpl @@ -102,7 +102,7 @@
{{if .LFS.StartServer}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}
{{if .LFS.StartServer}}
{{.locale.Tr "admin.config.lfs_content_path"}}
-
{{.LFS.Path}}
+
{{JsonUtils.EncodeToString .LFS.Storage}}
{{.locale.Tr "admin.config.lfs_http_auth_expiry"}}
{{.LFS.HTTPAuthExpiry}}
{{end}} diff --git a/tests/test_utils.go b/tests/test_utils.go index 628e98d98c34d..fd46d163a987d 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -214,7 +214,9 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() { // load LFS object fixtures // (LFS storage can be on any of several backends, including remote servers, so we init it with the storage API) - lfsFixtures, err := storage.NewStorage("", &setting.Storage{Path: path.Join(filepath.Dir(setting.AppPath), "tests/gitea-lfs-meta")}) + lfsFixtures, err := storage.NewStorage(setting.LocalStorageType, &setting.Storage{ + Path: path.Join(filepath.Dir(setting.AppPath), "tests/gitea-lfs-meta"), + }) assert.NoError(t, err) assert.NoError(t, storage.Clean(storage.LFS)) assert.NoError(t, lfsFixtures.IterateObjects("", func(path string, _ storage.Object) error { From a4fe93f1069421a5946879de07939085664df5d0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 8 Jun 2023 12:04:42 +0800 Subject: [PATCH 22/28] Fix test --- modules/setting/actions.go | 5 ++-- modules/setting/actions_test.go | 42 +++++++++++++++++++++++++++++++++ modules/setting/storage.go | 3 +-- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/modules/setting/actions.go b/modules/setting/actions.go index 5ce6b0b41dfef..1c8075cd6cc55 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -33,10 +33,9 @@ func loadActionsFrom(rootCfg ConfigProvider) error { return err } - actionsSec := rootCfg.Section("actions.artifacts") - storageType := actionsSec.Key("STORAGE_TYPE").MustString("") + actionsSec, _ := rootCfg.GetSection("actions.artifacts") - Actions.ArtifactStorage, err = getStorage(rootCfg, "actions_artifacts", storageType, actionsSec) + Actions.ArtifactStorage, err = getStorage(rootCfg, "actions_artifacts", "", actionsSec) return err } diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index 04918cb10e096..a1cc8fe333b0f 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -52,4 +52,46 @@ STORAGE_TYPE = minio assert.EqualValues(t, "actions_log/", Actions.LogStorage.MinioConfig.BasePath) assert.EqualValues(t, "local", Actions.ArtifactStorage.Type) assert.EqualValues(t, "actions_artifacts", filepath.Base(Actions.ArtifactStorage.Path)) + + iniStr = ` +[storage.actions_artifacts] +STORAGE_TYPE = my_storage + +[storage.my_storage] +STORAGE_TYPE = minio +` + cfg, err = NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + assert.NoError(t, loadActionsFrom(cfg)) + + assert.EqualValues(t, "local", Actions.LogStorage.Type) + assert.EqualValues(t, "actions_log", filepath.Base(Actions.LogStorage.Path)) + assert.EqualValues(t, "minio", Actions.ArtifactStorage.Type) + assert.EqualValues(t, "actions_artifacts/", Actions.ArtifactStorage.MinioConfig.BasePath) + + iniStr = ` +[storage.actions_artifacts] +STORAGE_TYPE = my_storage + +[storage.my_storage] +STORAGE_TYPE = minio +` + cfg, err = NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + assert.NoError(t, loadActionsFrom(cfg)) + + assert.EqualValues(t, "local", Actions.LogStorage.Type) + assert.EqualValues(t, "actions_log", filepath.Base(Actions.LogStorage.Path)) + assert.EqualValues(t, "minio", Actions.ArtifactStorage.Type) + assert.EqualValues(t, "actions_artifacts/", Actions.ArtifactStorage.MinioConfig.BasePath) + + iniStr = `` + cfg, err = NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + assert.NoError(t, loadActionsFrom(cfg)) + + assert.EqualValues(t, "local", Actions.LogStorage.Type) + assert.EqualValues(t, "actions_log", filepath.Base(Actions.LogStorage.Path)) + assert.EqualValues(t, "local", Actions.ArtifactStorage.Type) + assert.EqualValues(t, "actions_artifacts", filepath.Base(Actions.ArtifactStorage.Path)) } diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 72ebce65976ee..97737c7bb5b0f 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -147,10 +147,9 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S switch targetType { case string(LocalStorageType): - storage.Path = targetSec.Key("PATH").MustString(filepath.Join(AppDataPath, name)) + storage.Path = MustSectionKeyString(targetSec, "PATH", filepath.Join(AppDataPath, name)) if !filepath.IsAbs(storage.Path) { storage.Path = filepath.Join(AppWorkPath, storage.Path) - targetSec.Key("PATH").SetValue(storage.Path) } case string(MinioStorageType): storage.MinioConfig.BasePath = name + "/" From c113c60e22298a9c762a6ae08d180508c4f09cf5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 8 Jun 2023 14:05:20 +0800 Subject: [PATCH 23/28] shadow possible token leak --- modules/setting/storage.go | 16 ++++++++-------- modules/templates/helper.go | 12 ++++++++++++ templates/admin/config.tmpl | 2 +- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 97737c7bb5b0f..4f958bd6f675e 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -36,22 +36,22 @@ func IsValidStorageType(storageType StorageType) bool { // 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"` + Endpoint string `ini:"MINIO_ENDPOINT" json:",omitempty"` + AccessKeyID string `ini:"MINIO_ACCESS_KEY_ID" json:",omitempty"` + SecretAccessKey string `ini:"MINIO_SECRET_ACCESS_KEY" json:",omitempty"` + Bucket string `ini:"MINIO_BUCKET" json:",omitempty"` + Location string `ini:"MINIO_LOCATION" json:",omitempty"` + BasePath string `ini:"MINIO_BASE_PATH" json:",omitempty"` UseSSL bool `ini:"MINIO_USE_SSL"` InsecureSkipVerify bool `ini:"MINIO_INSECURE_SKIP_VERIFY"` - ChecksumAlgorithm string `ini:"MINIO_CHECKSUM_ALGORITHM"` + ChecksumAlgorithm string `ini:"MINIO_CHECKSUM_ALGORITHM" json:",omitempty"` ServeDirect bool `ini:"SERVE_DIRECT"` } // Storage represents configuration of storages type Storage struct { Type StorageType // local or minio - Path string // for local type + Path string `json:",omitempty"` // for local type TemporaryPath string MinioConfig MinioStorageConfig // for minio type } diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 2f2ef44049794..2e7fc8efe7a59 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -181,9 +181,21 @@ func NewFuncMap() template.FuncMap { "FilenameIsImage": FilenameIsImage, "TabSizeClass": TabSizeClass, + "ShadowStorage": shadowStorage, } } +func shadowStorage(storage *setting.Storage) setting.Storage { + shadowStorage := *storage + if shadowStorage.MinioConfig.AccessKeyID != "" { + shadowStorage.MinioConfig.AccessKeyID = "******" + } + if shadowStorage.MinioConfig.SecretAccessKey != "" { + shadowStorage.MinioConfig.SecretAccessKey = "******" + } + return shadowStorage +} + // Safe render raw as HTML func Safe(raw string) template.HTML { return template.HTML(raw) diff --git a/templates/admin/config.tmpl b/templates/admin/config.tmpl index 9b434797df2a8..f3013e203cb6c 100644 --- a/templates/admin/config.tmpl +++ b/templates/admin/config.tmpl @@ -102,7 +102,7 @@
{{if .LFS.StartServer}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}
{{if .LFS.StartServer}}
{{.locale.Tr "admin.config.lfs_content_path"}}
-
{{JsonUtils.EncodeToString .LFS.Storage}}
+
{{JsonUtils.EncodeToString (ShadowStorage .LFS.Storage)}}
{{.locale.Tr "admin.config.lfs_http_auth_expiry"}}
{{.LFS.HTTPAuthExpiry}}
{{end}} From 1a7a50b2e6acccd797c6f691f6b3f766d5d5853f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 8 Jun 2023 14:24:52 +0800 Subject: [PATCH 24/28] Follow wxiaoguang's suggestion --- modules/setting/storage.go | 11 +++++++++++ modules/templates/helper.go | 12 ------------ templates/admin/config.tmpl | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 4f958bd6f675e..d4473d8c17194 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -56,6 +56,17 @@ type Storage struct { MinioConfig MinioStorageConfig // for minio type } +func (storage *Storage) ToShadowCopy() Storage { + shadowStorage := *storage + if shadowStorage.MinioConfig.AccessKeyID != "" { + shadowStorage.MinioConfig.AccessKeyID = "******" + } + if shadowStorage.MinioConfig.SecretAccessKey != "" { + shadowStorage.MinioConfig.SecretAccessKey = "******" + } + return shadowStorage +} + const storageSectionName = "storage" func getDefaultStorageSection(rootCfg ConfigProvider) ConfigSection { diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 2e7fc8efe7a59..2f2ef44049794 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -181,21 +181,9 @@ func NewFuncMap() template.FuncMap { "FilenameIsImage": FilenameIsImage, "TabSizeClass": TabSizeClass, - "ShadowStorage": shadowStorage, } } -func shadowStorage(storage *setting.Storage) setting.Storage { - shadowStorage := *storage - if shadowStorage.MinioConfig.AccessKeyID != "" { - shadowStorage.MinioConfig.AccessKeyID = "******" - } - if shadowStorage.MinioConfig.SecretAccessKey != "" { - shadowStorage.MinioConfig.SecretAccessKey = "******" - } - return shadowStorage -} - // Safe render raw as HTML func Safe(raw string) template.HTML { return template.HTML(raw) diff --git a/templates/admin/config.tmpl b/templates/admin/config.tmpl index f3013e203cb6c..2850cc8d370dc 100644 --- a/templates/admin/config.tmpl +++ b/templates/admin/config.tmpl @@ -102,7 +102,7 @@
{{if .LFS.StartServer}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}
{{if .LFS.StartServer}}
{{.locale.Tr "admin.config.lfs_content_path"}}
-
{{JsonUtils.EncodeToString (ShadowStorage .LFS.Storage)}}
+
{{JsonUtils.EncodeToString .LFS.Storage.ToShadowCopy}}
{{.locale.Tr "admin.config.lfs_http_auth_expiry"}}
{{.LFS.HTTPAuthExpiry}}
{{end}} From 9461c2109df8199781dcd34b784230880aa231f3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 8 Jun 2023 14:26:26 +0800 Subject: [PATCH 25/28] fix --- modules/setting/storage.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index d4473d8c17194..a8895d26a6f48 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -50,9 +50,9 @@ type MinioStorageConfig struct { // Storage represents configuration of storages type Storage struct { - Type StorageType // local or minio - Path string `json:",omitempty"` // for local type - TemporaryPath string + Type StorageType // local or minio + Path string `json:",omitempty"` // for local type + TemporaryPath string `json:",omitempty"` MinioConfig MinioStorageConfig // for minio type } From a772b6cfca0fe92cf2906dbe5d42ac7f3d75fe33 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 8 Jun 2023 15:03:42 +0800 Subject: [PATCH 26/28] update storage configuration documents --- .../config-cheat-sheet.en-us.md | 56 ++++++++++++++++-- .../config-cheat-sheet.zh-cn.md | 57 +++++++++++++++++-- 2 files changed, 104 insertions(+), 9 deletions(-) diff --git a/docs/content/doc/administration/config-cheat-sheet.en-us.md b/docs/content/doc/administration/config-cheat-sheet.en-us.md index a915fcbdb1d51..7b94c7a4882e1 100644 --- a/docs/content/doc/administration/config-cheat-sheet.en-us.md +++ b/docs/content/doc/administration/config-cheat-sheet.en-us.md @@ -1254,8 +1254,9 @@ is `data/lfs` and the default of `MINIO_BASE_PATH` is `lfs/`. ## Storage (`storage`) -Default storage configuration for attachments, lfs, avatars and etc. +Default storage configuration for attachments, lfs, avatars, repo-avatars, repo-archive, packages, actions_log, actions_artifact. +- `STORAGE_TYPE`: **local**: Storage type, `local` for local disk or `minio` for s3 compatible object storage service. - `SERVE_DIRECT`: **false**: Allows the storage driver to redirect to authenticated URLs to serve files directly. Currently, only Minio/S3 is supported via signed URLs, local does nothing. - `MINIO_ENDPOINT`: **localhost:9000**: Minio endpoint to connect only available when `STORAGE_TYPE` is `minio` - `MINIO_ACCESS_KEY_ID`: Minio accessKeyID to connect only available when `STORAGE_TYPE` is `minio` @@ -1265,10 +1266,10 @@ Default storage configuration for attachments, lfs, avatars and etc. - `MINIO_USE_SSL`: **false**: Minio enabled ssl only available when `STORAGE_TYPE` is `minio` - `MINIO_INSECURE_SKIP_VERIFY`: **false**: Minio skip SSL verification available when STORAGE_TYPE is `minio` -And you can also define a customize storage like below: +The recommanded storage configuration for minio like below: ```ini -[storage.my_minio] +[storage] STORAGE_TYPE = minio ; Minio endpoint to connect only available when STORAGE_TYPE is `minio` MINIO_ENDPOINT = localhost:9000 @@ -1284,9 +1285,54 @@ MINIO_LOCATION = us-east-1 MINIO_USE_SSL = false ; Minio skip SSL verification available when STORAGE_TYPE is `minio` MINIO_INSECURE_SKIP_VERIFY = false +SERVE_DIRECT = true +``` + +Defaultly every storage has their default base path like below + +| storage | default base path | +| ----------------- | ------------------ | +| attachments | attachments/ | +| lfs | lfs/ | +| avatars | avatars/ | +| repo-avatars | repo-avatars/ | +| repo-archive | repo-archive/ | +| packages | packages/ | +| actions_log | actions_log/ | +| actions_artifacts | actions_artifacts/ | + +And bucket, basepath or `SERVE_DIRECT` could be special or overrided, if you want to use a different you can: + +```ini +[storage.actions_log] +MINIO_BUCKET = gitea_actions_log +SERVE_DIRECT = true +MINIO_BASE_PATH = my_actions_log/ ; default is actions_log/ if blank ``` -And used by `[attachment]`, `[lfs]` and etc. as `STORAGE_TYPE`. +If you want to customerize a different storage for `lfs` if above default storage defined + +```ini +[lfs] +STORAGE_TYPE = my_minio + +[storage.my_minio] +STORAGE_TYPE = minio +; Minio endpoint to connect only available when STORAGE_TYPE is `minio` +MINIO_ENDPOINT = localhost:9000 +; Minio accessKeyID to connect only available when STORAGE_TYPE is `minio` +MINIO_ACCESS_KEY_ID = +; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio` +MINIO_SECRET_ACCESS_KEY = +; Minio bucket to store the attachments only available when STORAGE_TYPE is `minio` +MINIO_BUCKET = gitea +; Minio location to create bucket only available when STORAGE_TYPE is `minio` +MINIO_LOCATION = us-east-1 +; Minio enabled ssl only available when STORAGE_TYPE is `minio` +MINIO_USE_SSL = false +; Minio skip SSL verification available when STORAGE_TYPE is `minio` +MINIO_INSECURE_SKIP_VERIFY = false +``` ## Repository Archive Storage (`storage.repo-archive`) @@ -1309,7 +1355,7 @@ is `data/repo-archive` and the default of `MINIO_BASE_PATH` is `repo-archive/`. ## Repository Archives (`repo-archive`) - `STORAGE_TYPE`: **local**: Storage type for actions logs, `local` for local disk or `minio` for s3 compatible object storage service, default is `local` or other name defined with `[storage.xxx]` -- `MINIO_BASE_PATH`: **actions_log/**: Minio base path on the bucket only available when STORAGE_TYPE is `minio` +- `MINIO_BASE_PATH`: **repo-archive/**: Minio base path on the bucket only available when STORAGE_TYPE is `minio` ## Proxy (`proxy`) diff --git a/docs/content/doc/administration/config-cheat-sheet.zh-cn.md b/docs/content/doc/administration/config-cheat-sheet.zh-cn.md index ba28b98123fa9..ebf939b025e66 100644 --- a/docs/content/doc/administration/config-cheat-sheet.zh-cn.md +++ b/docs/content/doc/administration/config-cheat-sheet.zh-cn.md @@ -414,7 +414,7 @@ LFS 的存储配置。 如果 `STORAGE_TYPE` 为空，则此配置将从 `[stora ## Storage (`storage`) -Attachments, lfs, avatars and etc 的默认存储配置。 +Attachments, lfs, avatars, repo-avatars, repo-archive, packages, actions_log, actions_artifact 的默认存储配置。 - `STORAGE_TYPE`: **local**: 附件存储类型，`local` 将存储到本地文件夹， `minio` 将存储到 s3 兼容的对象存储服务中。 - `SERVE_DIRECT`: **false**: 允许直接重定向到存储系统。当前，仅 Minio/S3 是支持的。 @@ -425,11 +425,13 @@ Attachments, lfs, avatars and etc 的默认存储配置。 - `MINIO_LOCATION`: **us-east-1**: Minio location to create bucket，仅当 `STORAGE_TYPE` 是 `minio` 时有效。 - `MINIO_USE_SSL`: **false**: Minio enabled ssl，仅当 `STORAGE_TYPE` 是 `minio` 时有效。 -你也可以自定义一个存储的名字如下： +以下为推荐的 recommanded storage configuration for minio like below: ```ini -[storage.my_minio] +[storage] STORAGE_TYPE = minio +; uncomment when STORAGE_TYPE = local +; PATH = storage root path ; Minio endpoint to connect only available when STORAGE_TYPE is `minio` MINIO_ENDPOINT = localhost:9000 ; Minio accessKeyID to connect only available when STORAGE_TYPE is `minio` @@ -444,9 +446,56 @@ MINIO_LOCATION = us-east-1 MINIO_USE_SSL = false ; Minio skip SSL verification available when STORAGE_TYPE is `minio` MINIO_INSECURE_SKIP_VERIFY = false +SERVE_DIRECT = true +``` + +默认的，每一个存储都会有各自默认的 BasePath 在同一个minio中，默认值如下： + +| storage | default base path | +| ----------------- | ------------------ | +| attachments | attachments/ | +| lfs | lfs/ | +| avatars | avatars/ | +| repo-avatars | repo-avatars/ | +| repo-archive | repo-archive/ | +| packages | packages/ | +| actions_log | actions_log/ | +| actions_artifacts | actions_artifacts/ | + +同时 bucket, basepath or `SERVE_DIRECT` 是可以被覆写的，像如下所示： + +```ini +[storage.actions_log] +MINIO_BUCKET = gitea_actions_log +SERVE_DIRECT = true +MINIO_BASE_PATH = my_actions_log/ ; default is actions_log/ if blank ``` -然后你在 `[attachment]`, `[lfs]` 等中可以把这个名字用作 `STORAGE_TYPE` 的值。 +当然你也可以完全自定义，像如下 + +```ini +[lfs] +STORAGE_TYPE = my_minio +MINIO_BASE_PATH = my_lfs_basepath + +[storage.my_minio] +STORAGE_TYPE = minio +; Minio endpoint to connect only available when STORAGE_TYPE is `minio` +MINIO_ENDPOINT = localhost:9000 +; Minio accessKeyID to connect only available when STORAGE_TYPE is `minio` +MINIO_ACCESS_KEY_ID = +; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio` +MINIO_SECRET_ACCESS_KEY = +; Minio bucket to store the attachments only available when STORAGE_TYPE is `minio` +MINIO_BUCKET = gitea +; Minio location to create bucket only available when STORAGE_TYPE is `minio` +MINIO_LOCATION = us-east-1 +; Minio enabled ssl only available when STORAGE_TYPE is `minio` +MINIO_USE_SSL = false +; Minio skip SSL verification available when STORAGE_TYPE is `minio` +MINIO_INSECURE_SKIP_VERIFY = false +SERVE_DIRECT = true +``` ## Repository Archive Storage (`storage.repo-archive`) From e47fb07e9d147333e85bab80892a77bb3363e58b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 Jun 2023 13:51:39 +0800 Subject: [PATCH 27/28] Follow wxiaoguang's suggestion --- modules/setting/config_provider.go | 31 ++++++++++++------------------ modules/setting/lfs.go | 17 +++++----------- modules/setting/storage.go | 8 ++++---- tests/test_utils.go | 2 +- 4 files changed, 22 insertions(+), 36 deletions(-) diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index c8a4538404e9f..328c299752245 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -96,6 +96,18 @@ func ConfigSectionKeyString(sec ConfigSection, key string, def ...string) string return "" } +func ConfigSectionKeyBool(sec ConfigSection, key string, def ...bool) bool { + k := ConfigSectionKey(sec, key) + if k != nil && k.String() != "" { + b, _ := strconv.ParseBool(k.String()) + return b + } + if len(def) > 0 { + return def[0] + } + return false +} + // ConfigInheritedKey works like ini.Section.Key(), but it always returns a new key instance, it is O(n) because NewKey is O(n) // and the returned key is safe to be used with "MustXxx", it doesn't change the parent's values. // Otherwise, ini.Section.Key().MustXxx would pollute the parent section's keys. @@ -314,22 +326,3 @@ func NewConfigProviderForLocale(source any, others ...any) (ConfigProvider, erro func init() { ini.PrettyFormat = false } - -// MustSectionKeyString to avoid use ini's MustString which will write data back to the ini's memory -// this function is safe to be instead of ini's MustString -func MustSectionKeyString(section ConfigSection, key string, defaults ...string) string { - v := section.Key(key).String() - if v == "" && len(defaults) > 0 { - return defaults[0] - } - return v -} - -func MustSectionKeyBool(section ConfigSection, key string, defaults ...bool) bool { - v := section.Key(key).String() - if v == "" && len(defaults) > 0 { - return defaults[0] - } - b, _ := strconv.ParseBool(v) - return b -} diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 40d3e4ad899df..90813fc15ffb4 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -35,18 +35,11 @@ func loadLFSFrom(rootCfg ConfigProvider) error { // DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version // if these are removed, the warning will not be shown deprecatedSetting(rootCfg, "server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0") - lfsContentPath := sec.Key("LFS_CONTENT_PATH").String() - if lfsContentPath != "" { - LFS.Storage = &Storage{ - Type: LocalStorageType, - Path: lfsContentPath, - } - } else { - var err error - LFS.Storage, err = getStorage(rootCfg, "lfs", "", lfsSec) - if err != nil { - return err - } + + var err error + LFS.Storage, err = getStorage(rootCfg, "lfs", "", lfsSec) + if err != nil { + return err } // Rest of LFS service settings diff --git a/modules/setting/storage.go b/modules/setting/storage.go index a8895d26a6f48..ed804a910a461 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -158,7 +158,7 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S switch targetType { case string(LocalStorageType): - storage.Path = MustSectionKeyString(targetSec, "PATH", filepath.Join(AppDataPath, name)) + storage.Path = ConfigSectionKeyString(targetSec, "PATH", filepath.Join(AppDataPath, name)) if !filepath.IsAbs(storage.Path) { storage.Path = filepath.Join(AppWorkPath, storage.Path) } @@ -175,9 +175,9 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S } if extraConfigSec != nil { - storage.MinioConfig.ServeDirect = MustSectionKeyBool(extraConfigSec, "SERVE_DIRECT", storage.MinioConfig.ServeDirect) - storage.MinioConfig.BasePath = MustSectionKeyString(extraConfigSec, "MINIO_BASE_PATH", storage.MinioConfig.BasePath) - storage.MinioConfig.Bucket = MustSectionKeyString(extraConfigSec, "MINIO_BUCKET", storage.MinioConfig.Bucket) + 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) } } diff --git a/tests/test_utils.go b/tests/test_utils.go index fd46d163a987d..5540d92e9230a 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -215,7 +215,7 @@ func PrepareTestEnv(t testing.TB, skip ...int) func() { // load LFS object fixtures // (LFS storage can be on any of several backends, including remote servers, so we init it with the storage API) lfsFixtures, err := storage.NewStorage(setting.LocalStorageType, &setting.Storage{ - Path: path.Join(filepath.Dir(setting.AppPath), "tests/gitea-lfs-meta"), + Path: filepath.Join(filepath.Dir(setting.AppPath), "tests/gitea-lfs-meta"), }) assert.NoError(t, err) assert.NoError(t, storage.Clean(storage.LFS)) From 2d5726bc1b591bc6db39b8021c4efcb840718677 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 Jun 2023 14:20:48 +0800 Subject: [PATCH 28/28] Follow wxiaoguang's suggestion --- modules/setting/config_provider.go | 6 ++++++ modules/setting/lfs.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 328c299752245..526d69bbdc7dd 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -300,6 +300,12 @@ func deprecatedSetting(rootCfg ConfigProvider, oldSection, oldKey, newSection, n } } +func deprecatedSettingFatal(rootCfg ConfigProvider, oldSection, oldKey, newSection, newKey, version string) { + if rootCfg.Section(oldSection).HasKey(oldKey) { + log.Fatal("Deprecated fallback `[%s]` `%s` present. Use `[%s]` `%s` instead. This fallback will be/has been removed in %s", oldSection, oldKey, newSection, newKey, version) + } +} + // deprecatedSettingDB add a hint that the configuration has been moved to database but still kept in app.ini func deprecatedSettingDB(rootCfg ConfigProvider, oldSection, oldKey string) { if rootCfg.Section(oldSection).HasKey(oldKey) { diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 90813fc15ffb4..d68349be86114 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -34,7 +34,7 @@ func loadLFSFrom(rootCfg ConfigProvider) error { // Specifically default PATH to LFS_CONTENT_PATH // DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version // if these are removed, the warning will not be shown - deprecatedSetting(rootCfg, "server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0") + deprecatedSettingFatal(rootCfg, "server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0") var err error LFS.Storage, err = getStorage(rootCfg, "lfs", "", lfsSec)