Fix storage path logic especially for relative paths (#26441) (#26481)

Backport #26441 by @lunny

This PR rewrites the function `getStorage` and make it more clear.

Include tests from #26435, thanks @earl-warren

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Earl Warren <contact@earl-warren.org>

modules/git/blame.go

@@ -5,11 +5,14 @@ package git
 
 import (
 	"bufio"
+	"bytes"
 	"context"
 	"fmt"
 	"io"
 	"os"
 	"regexp"
+
+	"code.gitea.io/gitea/modules/log"
 )
 
 // BlamePart represents block of blame - continuous lines with one sha
@@ -115,15 +118,19 @@ func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*B
 	done := make(chan error, 1)
 
 	go func(cmd *Command, dir string, stdout io.WriteCloser, done chan error) {
-		if err := cmd.Run(&RunOpts{
+		stderr := bytes.Buffer{}
+		// TODO: it doesn't work for directories (the directories shouldn't be "blamed"), and the "err" should be returned by "Read" but not by "Close"
+		err := cmd.Run(&RunOpts{
 			UseContextTimeout: true,
 			Dir:               dir,
 			Stdout:            stdout,
-			Stderr:            os.Stderr,
-		}); err == nil {
-			stdout.Close()
-		}
+			Stderr:            &stderr,
+		})
 		done <- err
+		_ = stdout.Close()
+		if err != nil {
+			log.Error("Error running git blame (dir: %v): %v, stderr: %v", repoPath, err, stderr.String())
+		}
 	}(cmd, repoPath, stdout, done)
 
 	bufferedReader := bufio.NewReader(reader)

modules/setting/storage.go

@@ -91,134 +91,172 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S
 		return nil, errors.New("no name for storage")
 	}
 
-	var targetSec ConfigSection
-	// check typ first
-	if typ != "" {
-		var err error
-		targetSec, err = rootCfg.GetSection(storageSectionName + "." + typ)
-		if err != nil {
-			if !IsValidStorageType(StorageType(typ)) {
-				return nil, fmt.Errorf("get section via storage type %q failed: %v", typ, err)
-			}
-		}
-		if targetSec != nil {
-			targetType := targetSec.Key("STORAGE_TYPE").String()
-			if targetType == "" {
-				if !IsValidStorageType(StorageType(typ)) {
-					return nil, fmt.Errorf("unknow storage type %q", typ)
-				}
-				targetSec.Key("STORAGE_TYPE").SetValue(typ)
-			} else if !IsValidStorageType(StorageType(targetType)) {
-				return nil, fmt.Errorf("unknow storage type %q for section storage.%v", targetType, typ)
-			}
-		}
+	targetSec, tp, err := getStorageTargetSection(rootCfg, name, typ, sec)
+	if err != nil {
+		return nil, err
 	}
 
-	if targetSec == nil && sec != nil {
-		secTyp := sec.Key("STORAGE_TYPE").String()
-		if IsValidStorageType(StorageType(secTyp)) {
-			targetSec = sec
-		} else if secTyp != "" {
-			targetSec, _ = rootCfg.GetSection(storageSectionName + "." + secTyp)
-		}
-	}
+	overrideSec := getStorageOverrideSection(rootCfg, targetSec, sec, tp, name)
 
-	targetSecIsStoragename := false
-	storageNameSec, _ := rootCfg.GetSection(storageSectionName + "." + name)
-	if targetSec == nil {
-		targetSec = storageNameSec
-		targetSecIsStoragename = storageNameSec != nil
+	targetType := targetSec.Key("STORAGE_TYPE").String()
+	switch targetType {
+	case string(LocalStorageType):
+		return getStorageForLocal(targetSec, overrideSec, tp, name)
+	case string(MinioStorageType):
+		return getStorageForMinio(targetSec, overrideSec, tp, name)
+	default:
+		return nil, fmt.Errorf("unsupported storage type %q", targetType)
 	}
+}
 
-	if targetSec == nil {
-		targetSec = getDefaultStorageSection(rootCfg)
-	} else {
-		targetType := targetSec.Key("STORAGE_TYPE").String()
-		switch {
-		case targetType == "":
-			if targetSec != storageNameSec && storageNameSec != nil {
-				targetSec = storageNameSec
-				targetSecIsStoragename = true
-				if targetSec.Key("STORAGE_TYPE").String() == "" {
-					return nil, fmt.Errorf("storage section %s.%s has no STORAGE_TYPE", storageSectionName, name)
-				}
-			} else {
-				if targetSec.Key("PATH").String() == "" {
-					targetSec = getDefaultStorageSection(rootCfg)
-				} else {
-					targetSec.Key("STORAGE_TYPE").SetValue("local")
-				}
-			}
-		default:
-			newTargetSec, _ := rootCfg.GetSection(storageSectionName + "." + targetType)
-			if newTargetSec == nil {
-				if !IsValidStorageType(StorageType(targetType)) {
-					return nil, fmt.Errorf("invalid storage section %s.%q", storageSectionName, targetType)
-				}
-			} else {
-				targetSec = newTargetSec
-				if IsValidStorageType(StorageType(targetType)) {
-					tp := targetSec.Key("STORAGE_TYPE").String()
-					if tp == "" {
-						targetSec.Key("STORAGE_TYPE").SetValue(targetType)
-					}
-				}
-			}
+type targetSecType int
+
+const (
+	targetSecIsTyp             targetSecType = iota // target section is [storage.type] which the type from parameter
+	targetSecIsStorage                              // target section is [storage]
+	targetSecIsDefault                              // target section is the default value
+	targetSecIsStorageWithName                      // target section is [storage.name]
+	targetSecIsSec                                  // target section is from the name seciont [name]
+)
+
+func getStorageSectionByType(rootCfg ConfigProvider, typ string) (ConfigSection, targetSecType, error) {
+	targetSec, err := rootCfg.GetSection(storageSectionName + "." + typ)
+	if err != nil {
+		if !IsValidStorageType(StorageType(typ)) {
+			return nil, 0, fmt.Errorf("get section via storage type %q failed: %v", typ, err)
 		}
+		// if typ is a valid storage type, but there is no [storage.local] or [storage.minio] section
+		// it's not an error
+		return nil, 0, nil
 	}
 
 	targetType := targetSec.Key("STORAGE_TYPE").String()
-	if !IsValidStorageType(StorageType(targetType)) {
-		return nil, fmt.Errorf("invalid storage type %q", targetType)
+	if targetType == "" {
+		if !IsValidStorageType(StorageType(typ)) {
+			return nil, 0, fmt.Errorf("unknow storage type %q", typ)
+		}
+		targetSec.Key("STORAGE_TYPE").SetValue(typ)
+	} else if !IsValidStorageType(StorageType(targetType)) {
+		return nil, 0, fmt.Errorf("unknow storage type %q for section storage.%v", targetType, typ)
 	}
 
-	// extra config section will be read SERVE_DIRECT, PATH, MINIO_BASE_PATH, MINIO_BUCKET to override the targetsec when possible
-	extraConfigSec := sec
-	if extraConfigSec == nil {
-		extraConfigSec = storageNameSec
+	return targetSec, targetSecIsTyp, nil
+}
+
+func getStorageTargetSection(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (ConfigSection, targetSecType, error) {
+	// check typ first
+	if typ == "" {
+		if sec != nil { // check sec's type secondly
+			typ = sec.Key("STORAGE_TYPE").String()
+			if IsValidStorageType(StorageType(typ)) {
+				if targetSec, _ := rootCfg.GetSection(storageSectionName + "." + typ); targetSec == nil {
+					return sec, targetSecIsSec, nil
+				}
+			}
+		}
 	}
 
-	var storage Storage
-	storage.Type = StorageType(targetType)
-
-	switch targetType {
-	case string(LocalStorageType):
-		targetPath := ConfigSectionKeyString(targetSec, "PATH", "")
-		if targetPath == "" {
-			targetPath = AppDataPath
-		} else if !filepath.IsAbs(targetPath) {
-			targetPath = filepath.Join(AppDataPath, targetPath)
+	if typ != "" {
+		targetSec, tp, err := getStorageSectionByType(rootCfg, typ)
+		if targetSec != nil || err != nil {
+			return targetSec, tp, err
 		}
+	}
 
-		var fallbackPath string
-		if targetSecIsStoragename {
-			fallbackPath = targetPath
-		} else {
-			fallbackPath = filepath.Join(targetPath, name)
-		}
+	// check stoarge name thirdly
+	targetSec, _ := rootCfg.GetSection(storageSectionName + "." + name)
+	if targetSec != nil {
+		targetType := targetSec.Key("STORAGE_TYPE").String()
+		switch {
+		case targetType == "":
+			if targetSec.Key("PATH").String() == "" { // both storage type and path are empty, use default
+				return getDefaultStorageSection(rootCfg), targetSecIsDefault, nil
+			}
 
-		if extraConfigSec == nil {
-			storage.Path = fallbackPath
-		} else {
-			storage.Path = ConfigSectionKeyString(extraConfigSec, "PATH", fallbackPath)
-			if !filepath.IsAbs(storage.Path) {
-				storage.Path = filepath.Join(targetPath, storage.Path)
+			targetSec.Key("STORAGE_TYPE").SetValue("local")
+		default:
+			targetSec, tp, err := getStorageSectionByType(rootCfg, targetType)
+			if targetSec != nil || err != nil {
+				return targetSec, tp, err
 			}
 		}
 
-	case string(MinioStorageType):
-		if err := targetSec.MapTo(&storage.MinioConfig); err != nil {
-			return nil, fmt.Errorf("map minio config failed: %v", err)
+		return targetSec, targetSecIsStorageWithName, nil
+	}
+
+	return getDefaultStorageSection(rootCfg), targetSecIsDefault, nil
+}
+
+// getStorageOverrideSection override section will be read SERVE_DIRECT, PATH, MINIO_BASE_PATH, MINIO_BUCKET to override the targetsec when possible
+func getStorageOverrideSection(rootConfig ConfigProvider, targetSec, sec ConfigSection, targetSecType targetSecType, name string) ConfigSection {
+	if targetSecType == targetSecIsSec {
+		return nil
+	}
+
+	if sec != nil {
+		return sec
+	}
+
+	if targetSecType != targetSecIsStorageWithName {
+		nameSec, _ := rootConfig.GetSection(storageSectionName + "." + name)
+		return nameSec
+	}
+	return nil
+}
+
+func getStorageForLocal(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) {
+	storage := Storage{
+		Type: StorageType(targetSec.Key("STORAGE_TYPE").String()),
+	}
+
+	targetPath := ConfigSectionKeyString(targetSec, "PATH", "")
+	var fallbackPath string
+	if targetPath == "" { // no path
+		fallbackPath = filepath.Join(AppDataPath, name)
+	} else {
+		if tp == targetSecIsStorage || tp == targetSecIsDefault {
+			fallbackPath = filepath.Join(targetPath, name)
+		} else {
+			fallbackPath = targetPath
 		}
+		if !filepath.IsAbs(fallbackPath) {
+			fallbackPath = filepath.Join(AppDataPath, fallbackPath)
+		}
+	}
 
-		storage.MinioConfig.BasePath = name + "/"
-
-		if extraConfigSec != nil {
-			storage.MinioConfig.ServeDirect = ConfigSectionKeyBool(extraConfigSec, "SERVE_DIRECT", storage.MinioConfig.ServeDirect)
-			storage.MinioConfig.BasePath = ConfigSectionKeyString(extraConfigSec, "MINIO_BASE_PATH", storage.MinioConfig.BasePath)
-			storage.MinioConfig.Bucket = ConfigSectionKeyString(extraConfigSec, "MINIO_BUCKET", storage.MinioConfig.Bucket)
+	if overrideSec == nil { // no override section
+		storage.Path = fallbackPath
+	} else {
+		storage.Path = ConfigSectionKeyString(overrideSec, "PATH", "")
+		if storage.Path == "" { // overrideSec has no path
+			storage.Path = fallbackPath
+		} else if !filepath.IsAbs(storage.Path) {
+			if targetPath == "" {
+				storage.Path = filepath.Join(AppDataPath, storage.Path)
+			} else {
+				storage.Path = filepath.Join(targetPath, storage.Path)
+			}
 		}
 	}
 
 	return &storage, nil
 }
+
+func getStorageForMinio(targetSec, overrideSec ConfigSection, tp targetSecType, name string) (*Storage, error) {
+	var storage Storage
+	storage.Type = StorageType(targetSec.Key("STORAGE_TYPE").String())
+	if err := targetSec.MapTo(&storage.MinioConfig); err != nil {
+		return nil, fmt.Errorf("map minio config failed: %v", err)
+	}
+
+	if storage.MinioConfig.BasePath == "" {
+		storage.MinioConfig.BasePath = name + "/"
+	}
+
+	if overrideSec != nil {
+		storage.MinioConfig.ServeDirect = ConfigSectionKeyBool(overrideSec, "SERVE_DIRECT", storage.MinioConfig.ServeDirect)
+		storage.MinioConfig.BasePath = ConfigSectionKeyString(overrideSec, "MINIO_BASE_PATH", storage.MinioConfig.BasePath)
+		storage.MinioConfig.Bucket = ConfigSectionKeyString(overrideSec, "MINIO_BUCKET", storage.MinioConfig.Bucket)
+	}
+	return &storage, nil
+}

modules/setting/storage_test.go

@@ -27,12 +27,15 @@ MINIO_BUCKET = gitea-storage
 
 	assert.NoError(t, loadAttachmentFrom(cfg))
 	assert.EqualValues(t, "gitea-attachment", Attachment.Storage.MinioConfig.Bucket)
+	assert.EqualValues(t, "attachments/", Attachment.Storage.MinioConfig.BasePath)
 
 	assert.NoError(t, loadLFSFrom(cfg))
 	assert.EqualValues(t, "gitea-lfs", LFS.Storage.MinioConfig.Bucket)
+	assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath)
 
 	assert.NoError(t, loadAvatarsFrom(cfg))
 	assert.EqualValues(t, "gitea-storage", Avatar.Storage.MinioConfig.Bucket)
+	assert.EqualValues(t, "avatars/", Avatar.Storage.MinioConfig.BasePath)
 }
 
 func Test_getStorageUseOtherNameAsType(t *testing.T) {
@@ -49,9 +52,11 @@ MINIO_BUCKET = gitea-storage
 
 	assert.NoError(t, loadAttachmentFrom(cfg))
 	assert.EqualValues(t, "gitea-storage", Attachment.Storage.MinioConfig.Bucket)
+	assert.EqualValues(t, "attachments/", Attachment.Storage.MinioConfig.BasePath)
 
 	assert.NoError(t, loadLFSFrom(cfg))
 	assert.EqualValues(t, "gitea-storage", LFS.Storage.MinioConfig.Bucket)
+	assert.EqualValues(t, "lfs/", LFS.Storage.MinioConfig.BasePath)
 }
 
 func Test_getStorageInheritStorageType(t *testing.T) {
@@ -117,6 +122,9 @@ func Test_getStorageInheritStorageTypeLocal(t *testing.T) {
 [storage]
 STORAGE_TYPE = local
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/appdata/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/appdata/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/repo-archive"},
 		{loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"},
@@ -131,6 +139,9 @@ func Test_getStorageInheritStorageTypeLocalPath(t *testing.T) {
 STORAGE_TYPE = local
 PATH = /data/gitea
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/data/gitea/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/data/gitea/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/data/gitea/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/repo-archive"},
 		{loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"},
@@ -145,6 +156,9 @@ func Test_getStorageInheritStorageTypeLocalRelativePath(t *testing.T) {
 STORAGE_TYPE = local
 PATH = storages
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/appdata/storages/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/appdata/storages/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/appdata/storages/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/appdata/storages/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/storages/repo-archive"},
 		{loadActionsFrom, &Actions.LogStorage, "/appdata/storages/actions_log"},
@@ -162,6 +176,9 @@ PATH = /data/gitea
 [repo-archive]
 PATH = /data/gitea/the-archives-dir
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/data/gitea/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/data/gitea/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/data/gitea/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/the-archives-dir"},
 		{loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"},
@@ -178,6 +195,9 @@ PATH = /data/gitea
 
 [repo-archive]
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/data/gitea/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/data/gitea/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/data/gitea/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/repo-archive"},
 		{loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"},
@@ -195,6 +215,9 @@ PATH = /data/gitea
 [repo-archive]
 PATH = the-archives-dir
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/data/gitea/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/data/gitea/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/data/gitea/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/the-archives-dir"},
 		{loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"},
@@ -209,6 +232,9 @@ func Test_getStorageInheritStorageTypeLocalPathOverride3(t *testing.T) {
 STORAGE_TYPE = local
 PATH = /data/gitea/archives
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/appdata/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/appdata/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/archives"},
 		{loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"},
@@ -217,6 +243,23 @@ PATH = /data/gitea/archives
 	})
 }
 
+func Test_getStorageInheritStorageTypeLocalPathOverride3_5(t *testing.T) {
+	testLocalStoragePath(t, "/appdata", `
+[storage.repo-archive]
+STORAGE_TYPE = local
+PATH = a-relative-path
+`, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/appdata/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"},
+		{loadPackagesFrom, &Packages.Storage, "/appdata/packages"},
+		{loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/a-relative-path"},
+		{loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"},
+		{loadAvatarsFrom, &Avatar.Storage, "/appdata/avatars"},
+		{loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/repo-avatars"},
+	})
+}
+
 func Test_getStorageInheritStorageTypeLocalPathOverride4(t *testing.T) {
 	testLocalStoragePath(t, "/appdata", `
 [storage.repo-archive]
@@ -226,6 +269,9 @@ PATH = /data/gitea/archives
 [repo-archive]
 PATH = /tmp/gitea/archives
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/appdata/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/appdata/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/tmp/gitea/archives"},
 		{loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"},
@@ -242,6 +288,9 @@ PATH = /data/gitea/archives
 
 [repo-archive]
 `, []testLocalStoragePathCase{
+		{loadAttachmentFrom, &Attachment.Storage, "/appdata/attachments"},
+		{loadLFSFrom, &LFS.Storage, "/appdata/lfs"},
+		{loadActionsFrom, &Actions.ArtifactStorage, "/appdata/actions_artifacts"},
 		{loadPackagesFrom, &Packages.Storage, "/appdata/packages"},
 		{loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/archives"},
 		{loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"},
@@ -249,3 +298,117 @@ PATH = /data/gitea/archives
 		{loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/repo-avatars"},
 	})
 }
+
+func Test_getStorageInheritStorageTypeLocalPathOverride72(t *testing.T) {
+	testLocalStoragePath(t, "/appdata", `
+[repo-archive]
+STORAGE_TYPE = local
+PATH = archives
+`, []testLocalStoragePathCase{
+		{loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/archives"},
+	})
+}
+
+func Test_getStorageConfiguration20(t *testing.T) {
+	cfg, err := NewConfigProviderFromData(`
+[repo-archive]
+STORAGE_TYPE = my_storage
+PATH = archives
+`)
+	assert.NoError(t, err)
+
+	assert.Error(t, loadRepoArchiveFrom(cfg))
+}
+
+func Test_getStorageConfiguration21(t *testing.T) {
+	testLocalStoragePath(t, "/appdata", `
+[storage.repo-archive]
+`, []testLocalStoragePathCase{
+		{loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/repo-archive"},
+	})
+}
+
+func Test_getStorageConfiguration22(t *testing.T) {
+	testLocalStoragePath(t, "/appdata", `
+[storage.repo-archive]
+PATH = archives
+`, []testLocalStoragePathCase{
+		{loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/archives"},
+	})
+}
+
+func Test_getStorageConfiguration23(t *testing.T) {
+	cfg, err := NewConfigProviderFromData(`
+[repo-archive]
+STORAGE_TYPE = minio
+MINIO_ACCESS_KEY_ID = my_access_key
+MINIO_SECRET_ACCESS_KEY = my_secret_key
+`)
+	assert.NoError(t, err)
+
+	_, err = getStorage(cfg, "", "", nil)
+	assert.Error(t, err)
+
+	assert.NoError(t, loadRepoArchiveFrom(cfg))
+	cp := RepoArchive.Storage.ToShadowCopy()
+	assert.EqualValues(t, "******", cp.MinioConfig.AccessKeyID)
+	assert.EqualValues(t, "******", cp.MinioConfig.SecretAccessKey)
+}
+
+func Test_getStorageConfiguration24(t *testing.T) {
+	cfg, err := NewConfigProviderFromData(`
+[repo-archive]
+STORAGE_TYPE = my_archive
+
+[storage.my_archive]
+; unsupported, storage type should be defined explicitly
+PATH = archives
+`)
+	assert.NoError(t, err)
+	assert.Error(t, loadRepoArchiveFrom(cfg))
+}
+
+func Test_getStorageConfiguration25(t *testing.T) {
+	cfg, err := NewConfigProviderFromData(`
+[repo-archive]
+STORAGE_TYPE = my_archive
+
+[storage.my_archive]
+; unsupported, storage type should be known type
+STORAGE_TYPE = unknown // should be local or minio
+PATH = archives
+`)
+	assert.NoError(t, err)
+	assert.Error(t, loadRepoArchiveFrom(cfg))
+}
+
+func Test_getStorageConfiguration26(t *testing.T) {
+	cfg, err := NewConfigProviderFromData(`
+[repo-archive]
+STORAGE_TYPE = minio
+MINIO_ACCESS_KEY_ID = my_access_key
+MINIO_SECRET_ACCESS_KEY = my_secret_key
+; wrong configuration
+MINIO_USE_SSL = abc
+`)
+	assert.NoError(t, err)
+	// assert.Error(t, loadRepoArchiveFrom(cfg))
+	// FIXME: this should return error but now ini package's MapTo() doesn't check type
+	assert.NoError(t, loadRepoArchiveFrom(cfg))
+}
+
+func Test_getStorageConfiguration27(t *testing.T) {
+	cfg, err := NewConfigProviderFromData(`
+[storage.repo-archive]
+STORAGE_TYPE = minio
+MINIO_ACCESS_KEY_ID = my_access_key
+MINIO_SECRET_ACCESS_KEY = my_secret_key
+MINIO_USE_SSL = true
+`)
+	assert.NoError(t, err)
+	assert.NoError(t, loadRepoArchiveFrom(cfg))
+	assert.EqualValues(t, "my_access_key", RepoArchive.Storage.MinioConfig.AccessKeyID)
+	assert.EqualValues(t, "my_secret_key", RepoArchive.Storage.MinioConfig.SecretAccessKey)
+	assert.EqualValues(t, true, RepoArchive.Storage.MinioConfig.UseSSL)
+	assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.MinioConfig.BasePath)
+}

options/locale/locale_en-US.ini

@@ -2284,6 +2284,7 @@ settings.tags.protection.none = There are no protected tags.
 settings.tags.protection.pattern.description = You can use a single name or a glob pattern or regular expression to match multiple tags. Read more in the <a target="_blank" rel="noopener" href="https://docs.gitea.io/en-us/protected-tags/">protected tags guide</a>.
 settings.bot_token = Bot Token
 settings.chat_id = Chat ID
+settings.thread_id = Thread ID
 settings.matrix.homeserver_url = Homeserver URL
 settings.matrix.room_id = Room ID
 settings.matrix.message_type = Message Type

routers/web/repo/webhook.go

@@ -425,12 +425,13 @@ func telegramHookParams(ctx *context.Context) webhookParams {
 
 	return webhookParams{
 		Type:        webhook_module.TELEGRAM,
-		URL:         fmt.Sprintf("https://api.telegram.org/bot%s/sendMessage?chat_id=%s", url.PathEscape(form.BotToken), url.QueryEscape(form.ChatID)),
+		URL:         fmt.Sprintf("https://api.telegram.org/bot%s/sendMessage?chat_id=%s&message_thread_id=%s", url.PathEscape(form.BotToken), url.QueryEscape(form.ChatID), url.QueryEscape(form.ThreadID)),
 		ContentType: webhook.ContentTypeJSON,
 		WebhookForm: form.WebhookForm,
 		Meta: &webhook_service.TelegramMeta{
 			BotToken: form.BotToken,
 			ChatID:   form.ChatID,
+			ThreadID: form.ThreadID,
 		},
 	}
 }

services/forms/repo_form.go

@@ -352,6 +352,7 @@ func (f *NewDingtalkHookForm) Validate(req *http.Request, errs binding.Errors) b
 type NewTelegramHookForm struct {
 	BotToken string `binding:"Required"`
 	ChatID   string `binding:"Required"`
+	ThreadID string
 	WebhookForm
 }
 

services/webhook/telegram.go

@@ -28,6 +28,7 @@ type (
 	TelegramMeta struct {
 		BotToken string `json:"bot_token"`
 		ChatID   string `json:"chat_id"`
+		ThreadID string `json:"thread_id"`
 	}
 )
 

templates/repo/settings/webhook/telegram.tmpl

@@ -10,6 +10,10 @@
 			<label for="chat_id">{{.locale.Tr "repo.settings.chat_id"}}</label>
 			<input id="chat_id" name="chat_id" type="text" value="{{.TelegramHook.ChatID}}" required>
 		</div>
+		<div class="field {{if .Err_ThreadID}}error{{end}}">
+			<label for="thread_id">{{.locale.Tr "repo.settings.thread_id"}}</label>
+			<input id="thread_id" name="thread_id" type="text" value="{{.TelegramHook.ThreadID}}">
+		</div>
 		{{template "repo/settings/webhook/settings" .}}
 	</form>
 {{end}}