diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 8f7c4658ce..ec65b3382e 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -592,9 +592,9 @@ LEVEL = Info ;BUFFER_LEN = 10000 ;; ;; Sub logger modes, a single comma means use default MODE above, empty means disable it -;logger.access.MODE= -;logger.router.MODE=, -;logger.xorm.MODE=, +;LOGGER_ACCESS_MODE= +;LOGGER_ROUTER_MODE=, +;LOGGER_XORM_MODE=, ;; ;; Collect SSH logs (Creates log from ssh git request) ;; diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 19f3b9008a..a34859de56 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -331,6 +331,14 @@ func deprecatedSetting(rootCfg ConfigProvider, oldSection, oldKey, newSection, n } } +func deprecatedSettingWarning(rootCfg ConfigProvider, oldSection, oldKey, newSection, newKey string) { //nolint:unparam + if rootCfg.Section(oldSection).HasKey(oldKey) { + msg := fmt.Sprintf("Deprecated config option `[%s]` `%s` present. Use `[%s]` `%s` instead.", oldSection, oldKey, newSection, newKey) + log.Error("%v", msg) + DeprecatedWarnings = append(DeprecatedWarnings, msg) + } +} + // 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/log.go b/modules/setting/log.go index 6d069d0e9c..a23662239e 100644 --- a/modules/setting/log.go +++ b/modules/setting/log.go @@ -56,41 +56,73 @@ func loadLogGlobalFrom(rootCfg ConfigProvider) { func prepareLoggerConfig(rootCfg ConfigProvider) { sec := rootCfg.Section("log") - if !sec.HasKey("logger.default.MODE") { - sec.Key("logger.default.MODE").MustString(",") + // Priority: `LOGGER_DEFAULT_MODE` -> `logger.default.MODE` + deprecatedSettingWarning(rootCfg, "log", "logger.default.MODE", "log", "LOGGER_DEFAULT_MODE") + hasNoValue := !sec.HasKey("LOGGER_DEFAULT_MODE") + if hasNoValue && sec.HasKey("logger.default.MODE") { + sec.Key("LOGGER_DEFAULT_MODE").SetValue(sec.Key("logger.default.MODE").String()) + hasNoValue = false + } + if hasNoValue { + sec.Key("LOGGER_DEFAULT_MODE").SetValue(",") // use default logger } - 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) + // Priority: `ENABLE_ACCESS_LOG` -> `LOGGER_ACCESS_MODE` -> `logger.access.MODE` -> `ACCESS` + deprecatedSettingWarning(rootCfg, "log", "ACCESS", "log", "LOGGER_ACCESS_MODE") + deprecatedSettingWarning(rootCfg, "log", "ENABLE_ACCESS_LOG", "log", "LOGGER_ACCESS_MODE") + deprecatedSettingWarning(rootCfg, "log", "logger.access.MODE", "log", "LOGGER_ACCESS_MODE") + hasNoValue = !sec.HasKey("LOGGER_ACCESS_MODE") + if hasNoValue && sec.HasKey("logger.access.MODE") { + sec.Key("LOGGER_ACCESS_MODE").SetValue(sec.Key("logger.access.MODE").String()) + hasNoValue = false + } + if val := sec.Key("ACCESS").String(); hasNoValue && val != "" { + sec.Key("LOGGER_ACCESS_MODE").SetValue(val) } if sec.HasKey("ENABLE_ACCESS_LOG") && !sec.Key("ENABLE_ACCESS_LOG").MustBool() { - sec.Key("logger.access.MODE").SetValue("") + 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) + // Priority: `DISABLE_ROUTER_LOG` -> `LOGGER_ROUTER_MODE` -> `logger.router.MODE` -> `ROUTER` + deprecatedSettingWarning(rootCfg, "log", "ROUTER", "log", "LOGGER_ROUTER_MODE") + deprecatedSettingWarning(rootCfg, "log", "DISABLE_ROUTER_LOG", "log", "LOGGER_ROUTER_MODE") + deprecatedSettingWarning(rootCfg, "log", "logger.router.MODE", "log", "LOGGER_ROUTER_MODE") + hasNoValue = !sec.HasKey("LOGGER_ROUTER_MODE") + if hasNoValue && sec.HasKey("logger.router.MODE") { + sec.Key("LOGGER_ROUTER_MODE").SetValue(sec.Key("logger.router.MODE").String()) + hasNoValue = false } - if !sec.HasKey("logger.router.MODE") { - sec.Key("logger.router.MODE").MustString(",") // use default logger + if val := sec.Key("ROUTER").String(); hasNoValue && val != "" { + sec.Key("LOGGER_ROUTER_MODE").SetValue(val) + hasNoValue = false } if sec.HasKey("DISABLE_ROUTER_LOG") && sec.Key("DISABLE_ROUTER_LOG").MustBool() { - sec.Key("logger.router.MODE").SetValue("") + sec.Key("LOGGER_ROUTER_MODE").SetValue("") + hasNoValue = false + } + if hasNoValue { + sec.Key("LOGGER_ROUTER_MODE").SetValue(",") // use default logger } - 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) + // Priority: `ENABLE_XORM_LOG` -> `LOGGER_XORM_MODE` -> `logger.xorm.MODE` -> `XORM` + deprecatedSettingWarning(rootCfg, "log", "XORM", "log", "LOGGER_XORM_MODE") + deprecatedSettingWarning(rootCfg, "log", "ENABLE_XORM_LOG", "log", "LOGGER_XORM_MODE") + deprecatedSettingWarning(rootCfg, "log", "logger.xorm.MODE", "log", "LOGGER_XORM_MODE") + hasNoValue = !sec.HasKey("LOGGER_XORM_MODE") + if hasNoValue && sec.HasKey("logger.xorm.MODE") { + sec.Key("LOGGER_XORM_MODE").SetValue(sec.Key("logger.xorm.MODE").String()) + hasNoValue = false } - if !sec.HasKey("logger.xorm.MODE") { - sec.Key("logger.xorm.MODE").MustString(",") // use default logger + if val := sec.Key("XORM").String(); hasNoValue && val != "" { + sec.Key("LOGGER_XORM_MODE").SetValue(val) + hasNoValue = false } if sec.HasKey("ENABLE_XORM_LOG") && !sec.Key("ENABLE_XORM_LOG").MustBool() { - sec.Key("logger.xorm.MODE").SetValue("") + sec.Key("LOGGER_XORM_MODE").SetValue("") + hasNoValue = false + } + if hasNoValue { + sec.Key("LOGGER_XORM_MODE").SetValue(",") // use default logger } } @@ -217,14 +249,14 @@ func initManagedLoggers(manager *log.LoggerManager, cfg ConfigProvider) { func initLoggerByName(manager *log.LoggerManager, rootCfg ConfigProvider, loggerName string) { sec := rootCfg.Section("log") - keyPrefix := "logger." + loggerName + key := "LOGGER_" + strings.ToUpper(loggerName) + "_MODE" - disabled := sec.HasKey(keyPrefix+".MODE") && sec.Key(keyPrefix+".MODE").String() == "" + disabled := sec.HasKey(key) && sec.Key(key).String() == "" if disabled { return } - modeVal := sec.Key(keyPrefix + ".MODE").String() + modeVal := sec.Key(key).String() if modeVal == "," { modeVal = Log.Mode } diff --git a/modules/setting/log_test.go b/modules/setting/log_test.go index 528921c669..a37c8f07a9 100644 --- a/modules/setting/log_test.go +++ b/modules/setting/log_test.go @@ -251,8 +251,8 @@ ENABLE_ACCESS_LOG = false func TestLogConfigNewConfig(t *testing.T) { manager, managerClose := initLoggersByConfig(t, ` [log] -logger.access.MODE = console -logger.xorm.MODE = console, console-1 +LOGGER_ACCESS_MODE = console +LOGGER_XORM_MODE = console, console-1 [log.console] LEVEL = warn @@ -421,7 +421,7 @@ func TestLegacyLoggerMigrations(t *testing.T) { } t.Run("default", func(t *testing.T) { - runCases(t, "logger.default.MODE", Cases{ + runCases(t, "LOGGER_DEFAULT_MODE", Cases{ { "uses default value for default logger", "", @@ -438,7 +438,7 @@ logger.default.MODE = file }) t.Run("access", func(t *testing.T) { - runCases(t, "logger.access.MODE", Cases{ + runCases(t, "LOGGER_ACCESS_MODE", Cases{ { "uses default value for access logger", "", @@ -475,6 +475,14 @@ logger.access.MODE = console `, "console", }, + { + "LOGGER_ACCESS_MODE has precedence over logger.access.MODE for access logger", + `[log] +LOGGER_ACCESS_MODE = file +logger.access.MODE = console +`, + "file", + }, { "ENABLE_ACCESS_LOG doesn't enable access logger", `[log] @@ -486,7 +494,7 @@ ENABLE_ACCESS_LOG = true }) t.Run("router", func(t *testing.T) { - runCases(t, "logger.router.MODE", Cases{ + runCases(t, "LOGGER_ROUTER_MODE", Cases{ { "uses default value for router logger", "", @@ -523,11 +531,19 @@ logger.router.MODE = console `, "console", }, + { + "LOGGER_ROUTER_MODE has precedence over logger.router.MODE for router logger", + `[log] +LOGGER_ROUTER_MODE = file +logger.router.MODE = console +`, + "file", + }, }) }) t.Run("xorm", func(t *testing.T) { - runCases(t, "logger.xorm.MODE", Cases{ + runCases(t, "LOGGER_XORM_MODE", Cases{ { "uses default value for xorm logger", "", @@ -564,6 +580,14 @@ logger.xorm.MODE = console `, "console", }, + { + "LOGGER_XORM_MODE has precedence over logger.xorm.MODE for xorm logger", + `[log] +LOGGER_XORM_MODE = file +logger.xorm.MODE = console +`, + "file", + }, }) }) } diff --git a/tests/mysql.ini.tmpl b/tests/mysql.ini.tmpl index 3315d85a3f..674884cea5 100644 --- a/tests/mysql.ini.tmpl +++ b/tests/mysql.ini.tmpl @@ -77,7 +77,7 @@ PROVIDER_CONFIG = tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-mysql/data/sessions MODE = {{TEST_LOGGER}} ROOT_PATH = {{REPO_TEST_DIR}}mysql-log ENABLE_SSH_LOG = true -logger.xorm.MODE = file +LOGGER_XORM_MODE = file [log.test] LEVEL = Info diff --git a/tests/pgsql.ini.tmpl b/tests/pgsql.ini.tmpl index 1e9b981800..386137d219 100644 --- a/tests/pgsql.ini.tmpl +++ b/tests/pgsql.ini.tmpl @@ -82,7 +82,7 @@ PROVIDER_CONFIG = tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-pgsql/data/sessions MODE = {{TEST_LOGGER}} ROOT_PATH = {{REPO_TEST_DIR}}pgsql-log ENABLE_SSH_LOG = true -logger.xorm.MODE = file +LOGGER_XORM_MODE = file [log.test] LEVEL = Info diff --git a/tests/sqlite.ini.tmpl b/tests/sqlite.ini.tmpl index df6cea44ca..b3baa314a2 100644 --- a/tests/sqlite.ini.tmpl +++ b/tests/sqlite.ini.tmpl @@ -79,7 +79,7 @@ PROVIDER_CONFIG = tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-sqlite/data/sessions MODE = {{TEST_LOGGER}} ROOT_PATH = {{REPO_TEST_DIR}}sqlite-log ENABLE_SSH_LOG = true -logger.xorm.MODE = file +LOGGER_XORM_MODE = file [log.test] LEVEL = Info