From b74f666705822998c76fe5d214badf5cbe7cd0ce Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Wed, 5 Mar 2025 17:06:58 +0100 Subject: [PATCH 1/5] move deprecated settings into a single function --- modules/setting/git.go | 10 ------ modules/setting/lfs.go | 12 ------- modules/setting/log.go | 25 -------------- modules/setting/mailer.go | 70 -------------------------------------- modules/setting/mirror.go | 9 ----- modules/setting/removed.go | 65 +++++++++++++++++++++++++++++++++++ modules/setting/server.go | 31 +++-------------- modules/setting/service.go | 3 -- modules/setting/setting.go | 2 +- modules/setting/task.go | 26 -------------- 10 files changed, 71 insertions(+), 182 deletions(-) create mode 100644 modules/setting/removed.go delete mode 100644 modules/setting/task.go diff --git a/modules/setting/git.go b/modules/setting/git.go index 48a4e7f30deb5..149f38bcc4d62 100644 --- a/modules/setting/git.go +++ b/modules/setting/git.go @@ -97,16 +97,6 @@ func loadGitFrom(rootCfg ConfigProvider) { GitConfig.SetOption("core.logAllRefUpdates", "true") GitConfig.SetOption("gc.reflogExpire", "90") - secGitReflog := rootCfg.Section("git.reflog") - if secGitReflog.HasKey("ENABLED") { - deprecatedSetting(rootCfg, "git.reflog", "ENABLED", "git.config", "core.logAllRefUpdates", "1.21") - GitConfig.SetOption("core.logAllRefUpdates", secGitReflog.Key("ENABLED").In("true", []string{"true", "false"})) - } - if secGitReflog.HasKey("EXPIRATION") { - deprecatedSetting(rootCfg, "git.reflog", "EXPIRATION", "git.config", "core.reflogExpire", "1.21") - GitConfig.SetOption("gc.reflogExpire", secGitReflog.Key("EXPIRATION").String()) - } - for _, key := range secGitConfig.Keys() { GitConfig.SetOption(key.Name(), key.String()) } diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 7f2d0ae159061..cbf511d288dce 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -40,18 +40,6 @@ func loadLFSFrom(rootCfg ConfigProvider) error { 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") - - if val := sec.Key("LFS_CONTENT_PATH").String(); val != "" { - if lfsSec == nil { - lfsSec = rootCfg.Section("lfs") - } - lfsSec.Key("PATH").MustString(val) - } - var err error LFS.Storage, err = getStorage(rootCfg, "lfs", "", lfsSec) if err != nil { diff --git a/modules/setting/log.go b/modules/setting/log.go index 50c57799945e5..ca9249dce0c10 100644 --- a/modules/setting/log.go +++ b/modules/setting/log.go @@ -60,38 +60,13 @@ func prepareLoggerConfig(rootCfg ConfigProvider) { sec.Key("logger.default.MODE").MustString(",") } - deprecatedSetting(rootCfg, "log", "ACCESS", "log", "logger.access.MODE", "1.21") - deprecatedSetting(rootCfg, "log", "ENABLE_ACCESS_LOG", "log", "logger.access.MODE", "1.21") - if val := sec.Key("ACCESS").String(); val != "" { - sec.Key("logger.access.MODE").MustString(val) - } - if sec.HasKey("ENABLE_ACCESS_LOG") && !sec.Key("ENABLE_ACCESS_LOG").MustBool() { - sec.Key("logger.access.MODE").SetValue("") - } - - deprecatedSetting(rootCfg, "log", "ROUTER", "log", "logger.router.MODE", "1.21") - deprecatedSetting(rootCfg, "log", "DISABLE_ROUTER_LOG", "log", "logger.router.MODE", "1.21") - if val := sec.Key("ROUTER").String(); val != "" { - sec.Key("logger.router.MODE").MustString(val) - } if !sec.HasKey("logger.router.MODE") { sec.Key("logger.router.MODE").MustString(",") // use default logger } - if sec.HasKey("DISABLE_ROUTER_LOG") && sec.Key("DISABLE_ROUTER_LOG").MustBool() { - sec.Key("logger.router.MODE").SetValue("") - } - deprecatedSetting(rootCfg, "log", "XORM", "log", "logger.xorm.MODE", "1.21") - deprecatedSetting(rootCfg, "log", "ENABLE_XORM_LOG", "log", "logger.xorm.MODE", "1.21") - if val := sec.Key("XORM").String(); val != "" { - sec.Key("logger.xorm.MODE").MustString(val) - } if !sec.HasKey("logger.xorm.MODE") { sec.Key("logger.xorm.MODE").MustString(",") // use default logger } - if sec.HasKey("ENABLE_XORM_LOG") && !sec.Key("ENABLE_XORM_LOG").MustBool() { - sec.Key("logger.xorm.MODE").SetValue("") - } } func LogPrepareFilenameForWriter(fileName, defaultFileName string) string { diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index e79ff304474b8..3a9fad7a7de8d 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -73,76 +73,6 @@ func loadMailerFrom(rootCfg ConfigProvider) { return } - // Handle Deprecations and map on to new configuration - // 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, "mailer", "MAILER_TYPE", "mailer", "PROTOCOL", "v1.19.0") - if sec.HasKey("MAILER_TYPE") && !sec.HasKey("PROTOCOL") { - if sec.Key("MAILER_TYPE").String() == "sendmail" { - sec.Key("PROTOCOL").MustString("sendmail") - } - } - - deprecatedSetting(rootCfg, "mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19.0") - if sec.HasKey("HOST") && !sec.HasKey("SMTP_ADDR") { - givenHost := sec.Key("HOST").String() - addr, port, err := net.SplitHostPort(givenHost) - if err != nil && strings.Contains(err.Error(), "missing port in address") { - addr = givenHost - } else if err != nil { - log.Fatal("Invalid mailer.HOST (%s): %v", givenHost, err) - } - if addr == "" { - addr = "127.0.0.1" - } - sec.Key("SMTP_ADDR").MustString(addr) - sec.Key("SMTP_PORT").MustString(port) - } - - deprecatedSetting(rootCfg, "mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19.0") - if sec.HasKey("IS_TLS_ENABLED") && !sec.HasKey("PROTOCOL") { - if sec.Key("IS_TLS_ENABLED").MustBool() { - sec.Key("PROTOCOL").MustString("smtps") - } else { - sec.Key("PROTOCOL").MustString("smtp+starttls") - } - } - - deprecatedSetting(rootCfg, "mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19.0") - if sec.HasKey("DISABLE_HELO") && !sec.HasKey("ENABLE_HELO") { - sec.Key("ENABLE_HELO").MustBool(!sec.Key("DISABLE_HELO").MustBool()) - } - - deprecatedSetting(rootCfg, "mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19.0") - if sec.HasKey("SKIP_VERIFY") && !sec.HasKey("FORCE_TRUST_SERVER_CERT") { - sec.Key("FORCE_TRUST_SERVER_CERT").MustBool(sec.Key("SKIP_VERIFY").MustBool()) - } - - deprecatedSetting(rootCfg, "mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19.0") - if sec.HasKey("USE_CERTIFICATE") && !sec.HasKey("USE_CLIENT_CERT") { - sec.Key("USE_CLIENT_CERT").MustBool(sec.Key("USE_CERTIFICATE").MustBool()) - } - - deprecatedSetting(rootCfg, "mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19.0") - if sec.HasKey("CERT_FILE") && !sec.HasKey("CLIENT_CERT_FILE") { - sec.Key("CERT_FILE").MustString(sec.Key("CERT_FILE").String()) - } - - deprecatedSetting(rootCfg, "mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19.0") - if sec.HasKey("KEY_FILE") && !sec.HasKey("CLIENT_KEY_FILE") { - sec.Key("KEY_FILE").MustString(sec.Key("KEY_FILE").String()) - } - - deprecatedSetting(rootCfg, "mailer", "ENABLE_HTML_ALTERNATIVE", "mailer", "SEND_AS_PLAIN_TEXT", "v1.19.0") - if sec.HasKey("ENABLE_HTML_ALTERNATIVE") && !sec.HasKey("SEND_AS_PLAIN_TEXT") { - sec.Key("SEND_AS_PLAIN_TEXT").MustBool(!sec.Key("ENABLE_HTML_ALTERNATIVE").MustBool(false)) - } - - if sec.HasKey("PROTOCOL") && sec.Key("PROTOCOL").String() == "smtp+startls" { - log.Error("Deprecated fallback `[mailer]` `PROTOCOL = smtp+startls` present. Use `[mailer]` `PROTOCOL = smtp+starttls`` instead. This fallback will be removed in v1.19.0") - sec.Key("PROTOCOL").SetValue("smtp+starttls") - } - // Set default values & validate sec.Key("NAME").MustString(AppName) sec.Key("PROTOCOL").In("", []string{"smtp", "smtps", "smtp+starttls", "smtp+unix", "sendmail", "dummy"}) diff --git a/modules/setting/mirror.go b/modules/setting/mirror.go index 3aa530a1f4847..b111ba443f4a4 100644 --- a/modules/setting/mirror.go +++ b/modules/setting/mirror.go @@ -25,15 +25,6 @@ var Mirror = struct { } func loadMirrorFrom(rootCfg ConfigProvider) { - // Handle old configuration through `[repository]` `DISABLE_MIRRORS` - // - please note this was badly named and only disabled the creation of new pull mirrors - // 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, "repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19.0") - if ConfigSectionKeyBool(rootCfg.Section("repository"), "DISABLE_MIRRORS") { - Mirror.DisableNewPull = true - } - if err := rootCfg.Section("mirror").MapTo(&Mirror); err != nil { log.Fatal("Failed to map Mirror settings: %v", err) } diff --git a/modules/setting/removed.go b/modules/setting/removed.go new file mode 100644 index 0000000000000..7fde4e66a45b1 --- /dev/null +++ b/modules/setting/removed.go @@ -0,0 +1,65 @@ +package setting + +import ( + "errors" + "fmt" + + "code.gitea.io/gitea/modules/log" +) + +// checkForRemovedSettings checks the entire configuration for removed keys and gathers them fataling if any is present +// Arbitrary permament removal is 3 releases +func checkForRemovedSettings(rootCfg ConfigProvider) { + var errs []error + + removedSettings := []struct { + oldSection, oldKey, newSection, newKey, version string + }{ + {"service", "EMAIL_DOMAIN_WHITELIST", "service", "EMAIL_DOMAIN_ALLOWLIST", "1.21"}, + {"mailer", "MAILER_TYPE", "mailer", "PROTOCOL", "v1.19.0"}, + {"mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19.0"}, + {"mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19.0"}, + {"mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19.0"}, + {"mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19.0"}, + {"mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19.0"}, + {"mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19.0"}, + {"mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19.0"}, + {"task", "QUEUE_TYPE", "queue.task", "TYPE", "v1.19.0"}, + {"task", "QUEUE_CONN_STR", "queue.task", "CONN_STR", "v1.19.0"}, + {"task", "QUEUE_LENGTH", "queue.task", "LENGTH", "v1.19.0"}, + {"server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19.0"}, + {"server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19.0"}, + {"server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19.0"}, + {"server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19.0"}, + {"git.reflog", "ENABLED", "git.config", "core.logAllRefUpdates", "1.21"}, + {"git.reflog", "EXPIRATION", "git.config", "core.reflogExpire", "1.21"}, + {"repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19.0"}, + {"server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0"}, + {"log", "XORM", "log", "logger.xorm.MODE", "1.21"}, + {"log", "ENABLE_XORM_LOG", "log", "logger.xorm.MODE", "1.21"}, + {"log", "ROUTER", "log", "logger.router.MODE", "1.21"}, + {"log", "DISABLE_ROUTER_LOG", "log", "logger.router.MODE", "1.21"}, + {"log", "ACCESS", "log", "logger.access.MODE", "1.21"}, + {"log", "ENABLE_ACCESS_LOG", "log", "logger.access.MODE", "1.21"}, + } + + for _, rs := range removedSettings { + errs = append(errs, removedSetting(rootCfg, rs.oldSection, rs.oldKey, rs.newSection, rs.newKey, rs.version)) + } + + if rootCfg.Section("mailer").Key("PROTOCOL").String() == "smtp+startls" { + errs = append(errs, fmt.Errorf("removed fallback `[mailer]` `PROTOCOL = smtp+startls` present. Use `[mailer]` `PROTOCOL = smtp+starttls`` instead")) + } + + err := errors.Join(errs...) + if err != nil { + log.Fatal("Encountered errors while loading configuration: %v", err) + } +} + +func removedSetting(rootCfg ConfigProvider, oldSection, oldKey, newSection, newKey, version string) error { + if rootCfg.Section(oldSection).HasKey(oldKey) { + return fmt.Errorf("Config option `[%s].%s` was removed in %s. Please use `[%s].%s`", oldSection, oldKey, version, newSection, newKey) + } + return nil +} diff --git a/modules/setting/server.go b/modules/setting/server.go index e15b790906738..48f6608c806e0 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -169,14 +169,7 @@ func loadServerFrom(rootCfg ConfigProvider) { HTTPAddr = sec.Key("HTTP_ADDR").MustString("0.0.0.0") HTTPPort = sec.Key("HTTP_PORT").MustString("3000") - // 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 - if sec.HasKey("ENABLE_ACME") { - EnableAcme = sec.Key("ENABLE_ACME").MustBool(false) - } else { - deprecatedSetting(rootCfg, "server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19.0") - EnableAcme = sec.Key("ENABLE_LETSENCRYPT").MustBool(false) - } + EnableAcme = sec.Key("ENABLE_ACME").MustBool(false) Protocol = HTTP protocolCfg := sec.Key("PROTOCOL").String() @@ -191,29 +184,15 @@ func loadServerFrom(rootCfg ConfigProvider) { AcmeURL = sec.Key("ACME_URL").MustString("") AcmeCARoot = sec.Key("ACME_CA_ROOT").MustString("") - if sec.HasKey("ACME_ACCEPTTOS") { - AcmeTOS = sec.Key("ACME_ACCEPTTOS").MustBool(false) - } else { - deprecatedSetting(rootCfg, "server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19.0") - AcmeTOS = sec.Key("LETSENCRYPT_ACCEPTTOS").MustBool(false) - } + AcmeTOS = sec.Key("ACME_ACCEPTTOS").MustBool(false) + if !AcmeTOS { log.Fatal("ACME TOS is not accepted (ACME_ACCEPTTOS).") } - if sec.HasKey("ACME_DIRECTORY") { - AcmeLiveDirectory = sec.Key("ACME_DIRECTORY").MustString("https") - } else { - deprecatedSetting(rootCfg, "server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19.0") - AcmeLiveDirectory = sec.Key("LETSENCRYPT_DIRECTORY").MustString("https") - } + AcmeLiveDirectory = sec.Key("ACME_DIRECTORY").MustString("https") - if sec.HasKey("ACME_EMAIL") { - AcmeEmail = sec.Key("ACME_EMAIL").MustString("") - } else { - deprecatedSetting(rootCfg, "server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19.0") - AcmeEmail = sec.Key("LETSENCRYPT_EMAIL").MustString("") - } + AcmeEmail = sec.Key("ACME_EMAIL").MustString("") if AcmeEmail == "" { log.Fatal("ACME Email is not set (ACME_EMAIL).") } diff --git a/modules/setting/service.go b/modules/setting/service.go index 8c1843eeb7548..7aa50c0aaad81 100644 --- a/modules/setting/service.go +++ b/modules/setting/service.go @@ -152,9 +152,6 @@ func loadServiceFrom(rootCfg ConfigProvider) { } else { Service.RegisterManualConfirm = false } - if sec.HasKey("EMAIL_DOMAIN_WHITELIST") { - deprecatedSetting(rootCfg, "service", "EMAIL_DOMAIN_WHITELIST", "service", "EMAIL_DOMAIN_ALLOWLIST", "1.21") - } Service.EmailDomainAllowList = CompileEmailGlobList(sec, "EMAIL_DOMAIN_WHITELIST", "EMAIL_DOMAIN_ALLOWLIST") Service.EmailDomainBlockList = CompileEmailGlobList(sec, "EMAIL_DOMAIN_BLOCKLIST") Service.ShowRegistrationButton = sec.Key("SHOW_REGISTRATION_BUTTON").MustBool(!(Service.DisableRegistration || Service.AllowOnlyExternalRegistration)) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 20da796b58d3d..1e1144e9e3a7c 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -200,6 +200,7 @@ func mustCurrentRunUserMatch(rootCfg ConfigProvider) { // LoadSettings initializes the settings for normal start up func LoadSettings() { initAllLoggers() + checkForRemovedSettings(CfgProvider) loadDBSetting(CfgProvider) loadServiceFrom(CfgProvider) @@ -212,7 +213,6 @@ func LoadSettings() { loadWebhookFrom(CfgProvider) loadMigrationsFrom(CfgProvider) loadIndexerFrom(CfgProvider) - loadTaskFrom(CfgProvider) LoadQueueSettings() loadProjectFrom(CfgProvider) loadMimeTypeMapFrom(CfgProvider) diff --git a/modules/setting/task.go b/modules/setting/task.go deleted file mode 100644 index f75b4f14813f8..0000000000000 --- a/modules/setting/task.go +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package setting - -// 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 -// - will need to set default for [queue.task] LENGTH to 1000 though -func loadTaskFrom(rootCfg ConfigProvider) { - taskSec := rootCfg.Section("task") - queueTaskSec := rootCfg.Section("queue.task") - - deprecatedSetting(rootCfg, "task", "QUEUE_TYPE", "queue.task", "TYPE", "v1.19.0") - deprecatedSetting(rootCfg, "task", "QUEUE_CONN_STR", "queue.task", "CONN_STR", "v1.19.0") - deprecatedSetting(rootCfg, "task", "QUEUE_LENGTH", "queue.task", "LENGTH", "v1.19.0") - - switch taskSec.Key("QUEUE_TYPE").MustString("channel") { - case "channel": - queueTaskSec.Key("TYPE").MustString("persistable-channel") - queueTaskSec.Key("CONN_STR").MustString(taskSec.Key("QUEUE_CONN_STR").MustString("")) - case "redis": - queueTaskSec.Key("TYPE").MustString("redis") - queueTaskSec.Key("CONN_STR").MustString(taskSec.Key("QUEUE_CONN_STR").MustString("addrs=127.0.0.1:6379 db=0")) - } - queueTaskSec.Key("LENGTH").MustInt(taskSec.Key("QUEUE_LENGTH").MustInt(1000)) -} From 540b1922147d0dfcb75b1f2c4d0b9c8882df3501 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Fri, 14 Mar 2025 23:21:40 +0100 Subject: [PATCH 2/5] align tests --- modules/setting/git_test.go | 12 +++--- modules/setting/lfs_test.go | 5 +-- modules/setting/log_test.go | 75 ++++------------------------------ modules/setting/mailer_test.go | 11 ++--- modules/setting/removed.go | 9 +--- modules/setting/setting.go | 4 +- 6 files changed, 23 insertions(+), 93 deletions(-) diff --git a/modules/setting/git_test.go b/modules/setting/git_test.go index 441c514d8c160..ceb3e0fba3e0e 100644 --- a/modules/setting/git_test.go +++ b/modules/setting/git_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGitConfig(t *testing.T) { @@ -50,16 +51,15 @@ func TestGitReflog(t *testing.T) { assert.EqualValues(t, "true", GitConfig.GetOption("core.logAllRefUpdates")) assert.EqualValues(t, "90", GitConfig.GetOption("gc.reflogExpire")) +} - // custom reflog config by legacy options - cfg, err = NewConfigProviderFromData(` +func TestLegacyRefLog(t *testing.T) { + cfg, err := NewConfigProviderFromData(` [git.reflog] ENABLED = false EXPIRATION = 123 `) - assert.NoError(t, err) - loadGitFrom(cfg) + require.NoError(t, err) - assert.EqualValues(t, "false", GitConfig.GetOption("core.logAllRefUpdates")) - assert.EqualValues(t, "123", GitConfig.GetOption("gc.reflogExpire")) + require.Error(t, checkForRemovedSettings(cfg)) } diff --git a/modules/setting/lfs_test.go b/modules/setting/lfs_test.go index d27dd7c5bf87f..5646f2bc3186c 100644 --- a/modules/setting/lfs_test.go +++ b/modules/setting/lfs_test.go @@ -40,10 +40,7 @@ LFS_CONTENT_PATH = deprecatedpath ` cfg, err = NewConfigProviderFromData(iniStr) assert.NoError(t, err) - assert.NoError(t, loadLFSFrom(cfg)) - - assert.EqualValues(t, "local", LFS.Storage.Type) - assert.Contains(t, LFS.Storage.Path, "deprecatedpath") + assert.Error(t, checkForRemovedSettings(cfg)) iniStr = ` [storage.lfs] diff --git a/modules/setting/log_test.go b/modules/setting/log_test.go index 87b14f0b1d8d7..6a1879b600b47 100644 --- a/modules/setting/log_test.go +++ b/modules/setting/log_test.go @@ -150,90 +150,29 @@ MODE = console func TestLogConfigLegacyMode(t *testing.T) { tempDir := t.TempDir() - tempPath := func(file string) string { - return filepath.Join(tempDir, file) - } - - manager, managerClose := initLoggersByConfig(t, ` + cfg, err := NewConfigProviderFromData(` [log] -ROOT_PATH = `+tempDir+` +ROOT_PATH = ` + tempDir + ` MODE = file ROUTER = file ACCESS = file `) - defer managerClose() + require.NoError(t, err) - writerDump := ` -{ - "file": { - "BufferLen": 10000, - "Colorize": false, - "Expression": "", - "Flags": "stdflags", - "Level": "info", - "Prefix": "", - "StacktraceLevel": "none", - "WriterOption": { - "Compress": true, - "CompressionLevel": -1, - "DailyRotate": true, - "FileName": "$FILENAME", - "LogRotate": true, - "MaxDays": 7, - "MaxSize": 268435456 - }, - "WriterType": "file" - } -} -` - writerDumpAccess := ` -{ - "file.access": { - "BufferLen": 10000, - "Colorize": false, - "Expression": "", - "Flags": "none", - "Level": "info", - "Prefix": "", - "StacktraceLevel": "none", - "WriterOption": { - "Compress": true, - "CompressionLevel": -1, - "DailyRotate": true, - "FileName": "$FILENAME", - "LogRotate": true, - "MaxDays": 7, - "MaxSize": 268435456 - }, - "WriterType": "file" - } -} -` - dump := manager.GetLogger(log.DEFAULT).DumpWriters() - require.JSONEq(t, strings.ReplaceAll(writerDump, "$FILENAME", tempPath("gitea.log")), toJSON(dump)) - - dump = manager.GetLogger("access").DumpWriters() - require.JSONEq(t, strings.ReplaceAll(writerDumpAccess, "$FILENAME", tempPath("access.log")), toJSON(dump)) - - dump = manager.GetLogger("router").DumpWriters() - require.JSONEq(t, strings.ReplaceAll(writerDump, "$FILENAME", tempPath("gitea.log")), toJSON(dump)) + require.Error(t, checkForRemovedSettings(cfg)) } func TestLogConfigLegacyModeDisable(t *testing.T) { - manager, managerClose := initLoggersByConfig(t, ` + cfg, err := NewConfigProviderFromData(` [log] ROUTER = file ACCESS = file DISABLE_ROUTER_LOG = true ENABLE_ACCESS_LOG = false `) - defer managerClose() + require.NoError(t, err) - dump := manager.GetLogger("access").DumpWriters() - require.JSONEq(t, "{}", toJSON(dump)) - - dump = manager.GetLogger("router").DumpWriters() - require.JSONEq(t, "{}", toJSON(dump)) + require.Error(t, checkForRemovedSettings(cfg)) } func TestLogConfigNewConfig(t *testing.T) { diff --git a/modules/setting/mailer_test.go b/modules/setting/mailer_test.go index fbabf113786f7..d128d10f7563e 100644 --- a/modules/setting/mailer_test.go +++ b/modules/setting/mailer_test.go @@ -10,7 +10,7 @@ import ( ) func Test_loadMailerFrom(t *testing.T) { - kases := map[string]*Mailer{ + cases := map[string]*Mailer{ "smtp.mydomain.com": { SMTPAddr: "smtp.mydomain.com", SMTPPort: "465", @@ -24,18 +24,15 @@ func Test_loadMailerFrom(t *testing.T) { SMTPPort: "123", }, } - for host, kase := range kases { + for host := range cases { t.Run(host, func(t *testing.T) { cfg, _ := NewConfigProviderFromData("") sec := cfg.Section("mailer") sec.NewKey("ENABLED", "true") sec.NewKey("HOST", host) - // Check mailer setting - loadMailerFrom(cfg) - - assert.EqualValues(t, kase.SMTPAddr, MailService.SMTPAddr) - assert.EqualValues(t, kase.SMTPPort, MailService.SMTPPort) + // Ensure removed settings catches the removed config setting + assert.Error(t, checkForRemovedSettings(cfg)) }) } } diff --git a/modules/setting/removed.go b/modules/setting/removed.go index 7fde4e66a45b1..60ab250019a27 100644 --- a/modules/setting/removed.go +++ b/modules/setting/removed.go @@ -3,13 +3,11 @@ package setting import ( "errors" "fmt" - - "code.gitea.io/gitea/modules/log" ) // checkForRemovedSettings checks the entire configuration for removed keys and gathers them fataling if any is present // Arbitrary permament removal is 3 releases -func checkForRemovedSettings(rootCfg ConfigProvider) { +func checkForRemovedSettings(rootCfg ConfigProvider) error { var errs []error removedSettings := []struct { @@ -51,10 +49,7 @@ func checkForRemovedSettings(rootCfg ConfigProvider) { errs = append(errs, fmt.Errorf("removed fallback `[mailer]` `PROTOCOL = smtp+startls` present. Use `[mailer]` `PROTOCOL = smtp+starttls`` instead")) } - err := errors.Join(errs...) - if err != nil { - log.Fatal("Encountered errors while loading configuration: %v", err) - } + return errors.Join(errs...) } func removedSetting(rootCfg ConfigProvider, oldSection, oldKey, newSection, newKey, version string) error { diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 1e1144e9e3a7c..75d865e95c5e9 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -200,7 +200,9 @@ func mustCurrentRunUserMatch(rootCfg ConfigProvider) { // LoadSettings initializes the settings for normal start up func LoadSettings() { initAllLoggers() - checkForRemovedSettings(CfgProvider) + if err := checkForRemovedSettings(CfgProvider); err != nil { + log.Fatal("Encountered removed settings while loading configuration: %v", err) + } loadDBSetting(CfgProvider) loadServiceFrom(CfgProvider) From 3cdf7525ebfc456ba2d5c0a8a722f3ba907b9189 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Fri, 14 Mar 2025 23:33:49 +0100 Subject: [PATCH 3/5] add copyright header --- modules/setting/removed.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/setting/removed.go b/modules/setting/removed.go index 60ab250019a27..f4fac471300d5 100644 --- a/modules/setting/removed.go +++ b/modules/setting/removed.go @@ -1,3 +1,6 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + package setting import ( From 4c18f5c346e531d49504937263901b764dd8e096 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Sat, 15 Mar 2025 10:30:38 +0100 Subject: [PATCH 4/5] unify error syntax change tests to depend on length instead of error existance --- modules/setting/git_test.go | 2 +- modules/setting/lfs_test.go | 2 +- modules/setting/log_test.go | 4 +-- modules/setting/mailer_test.go | 2 +- modules/setting/removed.go | 58 ++++++++++++++++++++-------------- modules/setting/setting.go | 3 +- 6 files changed, 41 insertions(+), 30 deletions(-) diff --git a/modules/setting/git_test.go b/modules/setting/git_test.go index ceb3e0fba3e0e..31737c9842a16 100644 --- a/modules/setting/git_test.go +++ b/modules/setting/git_test.go @@ -61,5 +61,5 @@ EXPIRATION = 123 `) require.NoError(t, err) - require.Error(t, checkForRemovedSettings(cfg)) + require.Len(t, checkForRemovedSettings(cfg), 2) } diff --git a/modules/setting/lfs_test.go b/modules/setting/lfs_test.go index 5646f2bc3186c..adb68e1567a4d 100644 --- a/modules/setting/lfs_test.go +++ b/modules/setting/lfs_test.go @@ -40,7 +40,7 @@ LFS_CONTENT_PATH = deprecatedpath ` cfg, err = NewConfigProviderFromData(iniStr) assert.NoError(t, err) - assert.Error(t, checkForRemovedSettings(cfg)) + assert.Len(t, checkForRemovedSettings(cfg), 1) iniStr = ` [storage.lfs] diff --git a/modules/setting/log_test.go b/modules/setting/log_test.go index 6a1879b600b47..3d1a5ecdcb2fa 100644 --- a/modules/setting/log_test.go +++ b/modules/setting/log_test.go @@ -159,7 +159,7 @@ ACCESS = file `) require.NoError(t, err) - require.Error(t, checkForRemovedSettings(cfg)) + require.Len(t, checkForRemovedSettings(cfg), 2) } func TestLogConfigLegacyModeDisable(t *testing.T) { @@ -172,7 +172,7 @@ ENABLE_ACCESS_LOG = false `) require.NoError(t, err) - require.Error(t, checkForRemovedSettings(cfg)) + require.Len(t, checkForRemovedSettings(cfg), 4) } func TestLogConfigNewConfig(t *testing.T) { diff --git a/modules/setting/mailer_test.go b/modules/setting/mailer_test.go index d128d10f7563e..eba9e768346c6 100644 --- a/modules/setting/mailer_test.go +++ b/modules/setting/mailer_test.go @@ -32,7 +32,7 @@ func Test_loadMailerFrom(t *testing.T) { sec.NewKey("HOST", host) // Ensure removed settings catches the removed config setting - assert.Error(t, checkForRemovedSettings(cfg)) + assert.Len(t, checkForRemovedSettings(cfg), 1) }) } } diff --git a/modules/setting/removed.go b/modules/setting/removed.go index f4fac471300d5..1906113eaa005 100644 --- a/modules/setting/removed.go +++ b/modules/setting/removed.go @@ -4,38 +4,37 @@ package setting import ( - "errors" "fmt" ) // checkForRemovedSettings checks the entire configuration for removed keys and gathers them fataling if any is present // Arbitrary permament removal is 3 releases -func checkForRemovedSettings(rootCfg ConfigProvider) error { +func checkForRemovedSettings(rootCfg ConfigProvider) []error { var errs []error removedSettings := []struct { oldSection, oldKey, newSection, newKey, version string }{ {"service", "EMAIL_DOMAIN_WHITELIST", "service", "EMAIL_DOMAIN_ALLOWLIST", "1.21"}, - {"mailer", "MAILER_TYPE", "mailer", "PROTOCOL", "v1.19.0"}, - {"mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19.0"}, - {"mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19.0"}, - {"mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19.0"}, - {"mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19.0"}, - {"mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19.0"}, - {"mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19.0"}, - {"mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19.0"}, - {"task", "QUEUE_TYPE", "queue.task", "TYPE", "v1.19.0"}, - {"task", "QUEUE_CONN_STR", "queue.task", "CONN_STR", "v1.19.0"}, - {"task", "QUEUE_LENGTH", "queue.task", "LENGTH", "v1.19.0"}, - {"server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19.0"}, - {"server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19.0"}, - {"server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19.0"}, - {"server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19.0"}, + {"mailer", "MAILER_TYPE", "mailer", "PROTOCOL", "v1.19"}, + {"mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19"}, + {"mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19"}, + {"mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19"}, + {"mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19"}, + {"mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19"}, + {"mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19"}, + {"mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19"}, + {"task", "QUEUE_TYPE", "queue.task", "TYPE", "v1.19"}, + {"task", "QUEUE_CONN_STR", "queue.task", "CONN_STR", "v1.19"}, + {"task", "QUEUE_LENGTH", "queue.task", "LENGTH", "v1.19"}, + {"server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19"}, + {"server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19"}, + {"server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19"}, + {"server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19"}, {"git.reflog", "ENABLED", "git.config", "core.logAllRefUpdates", "1.21"}, {"git.reflog", "EXPIRATION", "git.config", "core.reflogExpire", "1.21"}, - {"repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19.0"}, - {"server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0"}, + {"repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19"}, + {"server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19"}, {"log", "XORM", "log", "logger.xorm.MODE", "1.21"}, {"log", "ENABLE_XORM_LOG", "log", "logger.xorm.MODE", "1.21"}, {"log", "ROUTER", "log", "logger.router.MODE", "1.21"}, @@ -45,19 +44,30 @@ func checkForRemovedSettings(rootCfg ConfigProvider) error { } for _, rs := range removedSettings { - errs = append(errs, removedSetting(rootCfg, rs.oldSection, rs.oldKey, rs.newSection, rs.newKey, rs.version)) + if err := removedSetting(rootCfg, rs.oldSection, rs.oldKey, rs.newSection, rs.newKey, rs.version); err != nil { + errs = append(errs, err) + } } - if rootCfg.Section("mailer").Key("PROTOCOL").String() == "smtp+startls" { - errs = append(errs, fmt.Errorf("removed fallback `[mailer]` `PROTOCOL = smtp+startls` present. Use `[mailer]` `PROTOCOL = smtp+starttls`` instead")) + if err := removedOption(rootCfg, "mailer", "PROTOCOL", "smtp+startls", "smtp+starttls", "v1.19"); err != nil { + errs = append(errs, err) } - return errors.Join(errs...) + return errs } +// removedOption checks if configuration has an oldValue under key in section and returns error for user if it does +func removedOption(rootCfg ConfigProvider, section, key, oldValue, newValue, version string) error { + if rootCfg.Section(section).Key(key).String() == oldValue { + return fmt.Errorf("Config option `[%s].%s=%s` was removed in %s. Please use `[%s].%s=%s` instead", section, key, oldValue, version, section, key, newValue) + } + return nil +} + +// removedSetting checks if oldKey exists in oldSection and returns an error for user if it does func removedSetting(rootCfg ConfigProvider, oldSection, oldKey, newSection, newKey, version string) error { if rootCfg.Section(oldSection).HasKey(oldKey) { - return fmt.Errorf("Config option `[%s].%s` was removed in %s. Please use `[%s].%s`", oldSection, oldKey, version, newSection, newKey) + return fmt.Errorf("Config option `[%s].%s` was removed in %s. Please use `[%s].%s` instead", oldSection, oldKey, version, newSection, newKey) } return nil } diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 75d865e95c5e9..3c05b02dfe916 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -5,6 +5,7 @@ package setting import ( + "errors" "fmt" "os" "runtime" @@ -200,7 +201,7 @@ func mustCurrentRunUserMatch(rootCfg ConfigProvider) { // LoadSettings initializes the settings for normal start up func LoadSettings() { initAllLoggers() - if err := checkForRemovedSettings(CfgProvider); err != nil { + if err := errors.Join(checkForRemovedSettings(CfgProvider)...); err != nil { log.Fatal("Encountered removed settings while loading configuration: %v", err) } From 8428403e775ed8ff2f87313bbe6ddcb9875e44e6 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Sun, 16 Mar 2025 23:09:06 +0100 Subject: [PATCH 5/5] add basic comments about the removal --- modules/setting/git.go | 1 + modules/setting/lfs.go | 1 + modules/setting/log.go | 1 + modules/setting/mailer.go | 1 + modules/setting/mirror.go | 1 + modules/setting/server.go | 1 + modules/setting/service.go | 1 + modules/setting/setting.go | 1 + 8 files changed, 8 insertions(+) diff --git a/modules/setting/git.go b/modules/setting/git.go index 149f38bcc4d62..93ef85b2c1a28 100644 --- a/modules/setting/git.go +++ b/modules/setting/git.go @@ -91,6 +91,7 @@ func loadGitFrom(rootCfg ConfigProvider) { log.Fatal("Failed to map Git settings: %v", err) } + // legacy reflog config options have been moved to removed.go secGitConfig := rootCfg.Section("git.config") GitConfig.Options = make(map[string]string) GitConfig.SetOption("diff.algorithm", "histogram") diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index cbf511d288dce..1c1fba693304b 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -38,6 +38,7 @@ func loadLFSFrom(rootCfg ConfigProvider) error { mustMapSetting(rootCfg, "server", &LFS) sec := rootCfg.Section("server") + // legacy LFS_CONTENT_PATH option has been moved to removed.go lfsSec, _ := rootCfg.GetSection("lfs") var err error diff --git a/modules/setting/log.go b/modules/setting/log.go index ca9249dce0c10..d5f460e6862ac 100644 --- a/modules/setting/log.go +++ b/modules/setting/log.go @@ -54,6 +54,7 @@ func loadLogGlobalFrom(rootCfg ConfigProvider) { } func prepareLoggerConfig(rootCfg ConfigProvider) { + // legacy log config options have been moved to removed.go sec := rootCfg.Section("log") if !sec.HasKey("logger.default.MODE") { diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index 3a9fad7a7de8d..aa7888196646f 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -72,6 +72,7 @@ func loadMailerFrom(rootCfg ConfigProvider) { if !sec.Key("ENABLED").MustBool() { return } + // legacy mailer config options have been moved to removed.go // Set default values & validate sec.Key("NAME").MustString(AppName) diff --git a/modules/setting/mirror.go b/modules/setting/mirror.go index b111ba443f4a4..8c4934f97e816 100644 --- a/modules/setting/mirror.go +++ b/modules/setting/mirror.go @@ -25,6 +25,7 @@ var Mirror = struct { } func loadMirrorFrom(rootCfg ConfigProvider) { + // legacy mirror config options have been moved to removed.go if err := rootCfg.Section("mirror").MapTo(&Mirror); err != nil { log.Fatal("Failed to map Mirror settings: %v", err) } diff --git a/modules/setting/server.go b/modules/setting/server.go index 48f6608c806e0..da3c5f9a3731e 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -162,6 +162,7 @@ func MakeAbsoluteAssetURL(appURL, staticURLPrefix string) string { } func loadServerFrom(rootCfg ConfigProvider) { + // legacy server config options have been moved to removed.go sec := rootCfg.Section("server") AppName = rootCfg.Section("").Key("APP_NAME").MustString("Gitea: Git with a cup of tea") diff --git a/modules/setting/service.go b/modules/setting/service.go index 7aa50c0aaad81..4029d540f31c2 100644 --- a/modules/setting/service.go +++ b/modules/setting/service.go @@ -152,6 +152,7 @@ func loadServiceFrom(rootCfg ConfigProvider) { } else { Service.RegisterManualConfirm = false } + // legacy email config options have been moved to removed.go Service.EmailDomainAllowList = CompileEmailGlobList(sec, "EMAIL_DOMAIN_WHITELIST", "EMAIL_DOMAIN_ALLOWLIST") Service.EmailDomainBlockList = CompileEmailGlobList(sec, "EMAIL_DOMAIN_BLOCKLIST") Service.ShowRegistrationButton = sec.Key("SHOW_REGISTRATION_BUTTON").MustBool(!(Service.DisableRegistration || Service.AllowOnlyExternalRegistration)) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 3c05b02dfe916..372a575acd2a5 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -201,6 +201,7 @@ func mustCurrentRunUserMatch(rootCfg ConfigProvider) { // LoadSettings initializes the settings for normal start up func LoadSettings() { initAllLoggers() + // legacy task config options have been moved to removed.go if err := errors.Join(checkForRemovedSettings(CfgProvider)...); err != nil { log.Fatal("Encountered removed settings while loading configuration: %v", err) }