Fix all possible setting error related storages and added some tests (g…

…o-gitea#23911)

Follow up go-gitea#22405

Fix go-gitea#20703 

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

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

---------

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

cmd/dump.go

@@ -353,9 +353,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 {

cmd/migrate_storage.go

@@ -179,30 +179,32 @@ 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")
  return nil
  }
  dstStorage, err = storage.NewLocalStorage(
  stdCtx,
- storage.LocalStorageConfig{
+ &setting.Storage{
  Path: p,
  })
- case string(storage.MinioStorageType):
+ case string(setting.MinioStorageType):
  dstStorage, err = storage.NewMinioStorage(
  stdCtx,
- storage.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"))

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)

custom/conf/app.example.ini

@@ -2392,6 +2392,10 @@ LEVEL = Info
 ;; 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
 ;;
@@ -2452,6 +2456,19 @@ LEVEL = Info
 ;; 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
@@ -2471,6 +2488,9 @@ LEVEL = Info
 ;;
 ;; 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/
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -2520,6 +2540,7 @@ LEVEL = Info
 ; [actions]
 ;; Enable/Disable actions capabilities
 ;ENABLED = false
+;;
 ;; 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
 

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`)
 
@@ -1306,6 +1352,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`: **repo-archive/**: 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
@@ -1324,6 +1375,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 "<https://gitea.com/actions/checkout>" 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
 

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

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

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

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