From d0402a3df90fe636ba5e37ea83dc7c2eeea51309 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 11 Jun 2023 17:35:17 +0800 Subject: [PATCH 1/5] Remove all depreciated setting fallback for v1.19 but keep warnning --- modules/setting/lfs.go | 1 - modules/setting/mailer.go | 52 --------------------------------------- modules/setting/mirror.go | 3 --- modules/setting/server.go | 19 ++++++-------- 4 files changed, 7 insertions(+), 68 deletions(-) diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index a5c3631681be..c061f3698534 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -36,7 +36,6 @@ func loadLFSFrom(rootCfg ConfigProvider) { // 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", storageType, lfsSec) diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index a2bc2df44499..c8c39d393f72 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -68,66 +68,14 @@ func loadMailerFrom(rootCfg ConfigProvider) { // 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") diff --git a/modules/setting/mirror.go b/modules/setting/mirror.go index cd6b8d456248..c2ec90c0b773 100644 --- a/modules/setting/mirror.go +++ b/modules/setting/mirror.go @@ -30,9 +30,6 @@ func loadMirrorFrom(rootCfg ConfigProvider) { // 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 rootCfg.Section("repository").Key("DISABLE_MIRRORS").MustBool(false) { - 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/server.go b/modules/setting/server.go index d937faca1012..d97b4e8c4b62 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -183,37 +183,32 @@ func loadServerFrom(rootCfg ConfigProvider) { // 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) } + deprecatedSetting(rootCfg, "server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19.0") + if EnableAcme { 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) } + deprecatedSetting(rootCfg, "server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19.0") + 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") } + deprecatedSetting(rootCfg, "server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19.0") 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("") } + deprecatedSetting(rootCfg, "server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19.0") + } else { CertFile = sec.Key("CERT_FILE").String() KeyFile = sec.Key("KEY_FILE").String() From 3c12dac2bc720437288d435c5f71de8a3b1d4d6c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 Jun 2023 16:02:09 +0800 Subject: [PATCH 2/5] Fatal if the depricated setting has still been using --- modules/setting/config_provider.go | 6 ++++++ modules/setting/lfs.go | 2 +- modules/setting/mailer.go | 18 +++++++++--------- modules/setting/mirror.go | 2 +- modules/setting/server.go | 8 ++++---- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 8b317d94e32c..f9482dc80d17 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -287,6 +287,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 c061f3698534..530f88bdcd17 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -35,7 +35,7 @@ func loadLFSFrom(rootCfg ConfigProvider) { // 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") LFS.Storage = getStorage(rootCfg, "lfs", storageType, lfsSec) diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index c8c39d393f72..20ec13085578 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -67,15 +67,15 @@ func loadMailerFrom(rootCfg ConfigProvider) { // 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") - deprecatedSetting(rootCfg, "mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19.0") - deprecatedSetting(rootCfg, "mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19.0") - deprecatedSetting(rootCfg, "mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19.0") - deprecatedSetting(rootCfg, "mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19.0") - deprecatedSetting(rootCfg, "mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19.0") - deprecatedSetting(rootCfg, "mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19.0") - deprecatedSetting(rootCfg, "mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19.0") - deprecatedSetting(rootCfg, "mailer", "ENABLE_HTML_ALTERNATIVE", "mailer", "SEND_AS_PLAIN_TEXT", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "MAILER_TYPE", "mailer", "PROTOCOL", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19.0") + deprecatedSettingFatal(rootCfg, "mailer", "ENABLE_HTML_ALTERNATIVE", "mailer", "SEND_AS_PLAIN_TEXT", "v1.19.0") 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") diff --git a/modules/setting/mirror.go b/modules/setting/mirror.go index c2ec90c0b773..2a14a41b21e7 100644 --- a/modules/setting/mirror.go +++ b/modules/setting/mirror.go @@ -29,7 +29,7 @@ func loadMirrorFrom(rootCfg ConfigProvider) { // - 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") + deprecatedSettingFatal(rootCfg, "repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19.0") 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 d97b4e8c4b62..ce508fe4509f 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -184,7 +184,7 @@ func loadServerFrom(rootCfg ConfigProvider) { if sec.HasKey("ENABLE_ACME") { EnableAcme = sec.Key("ENABLE_ACME").MustBool(false) } - deprecatedSetting(rootCfg, "server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19.0") + deprecatedSettingFatal(rootCfg, "server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19.0") if EnableAcme { AcmeURL = sec.Key("ACME_URL").MustString("") @@ -193,7 +193,7 @@ func loadServerFrom(rootCfg ConfigProvider) { if sec.HasKey("ACME_ACCEPTTOS") { AcmeTOS = sec.Key("ACME_ACCEPTTOS").MustBool(false) } - deprecatedSetting(rootCfg, "server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19.0") + deprecatedSettingFatal(rootCfg, "server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19.0") if !AcmeTOS { log.Fatal("ACME TOS is not accepted (ACME_ACCEPTTOS).") @@ -202,12 +202,12 @@ func loadServerFrom(rootCfg ConfigProvider) { if sec.HasKey("ACME_DIRECTORY") { AcmeLiveDirectory = sec.Key("ACME_DIRECTORY").MustString("https") } - deprecatedSetting(rootCfg, "server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19.0") + deprecatedSettingFatal(rootCfg, "server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19.0") if sec.HasKey("ACME_EMAIL") { AcmeEmail = sec.Key("ACME_EMAIL").MustString("") } - deprecatedSetting(rootCfg, "server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19.0") + deprecatedSettingFatal(rootCfg, "server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19.0") } else { CertFile = sec.Key("CERT_FILE").String() From ff5612a89dc9d3fec8d334577d10cc046739c740 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 Jun 2023 17:45:41 +0800 Subject: [PATCH 3/5] Remove deprecated settings in integration tests --- tests/mssql.ini.tmpl | 2 +- tests/mysql.ini.tmpl | 2 +- tests/pgsql.ini.tmpl | 2 +- tests/sqlite.ini.tmpl | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/mssql.ini.tmpl b/tests/mssql.ini.tmpl index 10e70d35fc81..93ade74805b1 100644 --- a/tests/mssql.ini.tmpl +++ b/tests/mssql.ini.tmpl @@ -58,7 +58,7 @@ PATH = tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-mssql/data/attachments [mailer] ENABLED = true -MAILER_TYPE = dummy +PROTOCOL = dummy FROM = mssql-{{TEST_TYPE}}-test@gitea.io [service] diff --git a/tests/mysql.ini.tmpl b/tests/mysql.ini.tmpl index 9d6bbd65e6e8..10aa3cabbee8 100644 --- a/tests/mysql.ini.tmpl +++ b/tests/mysql.ini.tmpl @@ -58,7 +58,7 @@ SSH_TRUSTED_USER_CA_KEYS = ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCb4DC1dMFnJ6pXW [mailer] ENABLED = true -MAILER_TYPE = dummy +PROTOCOL = dummy FROM = mysql-{{TEST_TYPE}}-test@gitea.io [service] diff --git a/tests/pgsql.ini.tmpl b/tests/pgsql.ini.tmpl index dd45ab717aae..2b54843c30a6 100644 --- a/tests/pgsql.ini.tmpl +++ b/tests/pgsql.ini.tmpl @@ -59,7 +59,7 @@ PATH = tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-pgsql/data/attachments [mailer] ENABLED = true -MAILER_TYPE = dummy +PROTOCOL = dummy FROM = pgsql-{{TEST_TYPE}}-test@gitea.io [service] diff --git a/tests/sqlite.ini.tmpl b/tests/sqlite.ini.tmpl index 969c15698daa..fbfaf0a42a8e 100644 --- a/tests/sqlite.ini.tmpl +++ b/tests/sqlite.ini.tmpl @@ -55,7 +55,7 @@ PATH = tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-sqlite/data/attachments [mailer] ENABLED = true -MAILER_TYPE = dummy +PROTOCOL = dummy FROM = sqlite-{{TEST_TYPE}}-test@gitea.io [service] From 1ba4dd1d154b62e22dcbc3b7d2c73e3118dbf60d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 Jun 2023 18:32:55 +0800 Subject: [PATCH 4/5] Fix test --- modules/setting/mailer_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/modules/setting/mailer_test.go b/modules/setting/mailer_test.go index fbabf113786f..728e51dfaf5d 100644 --- a/modules/setting/mailer_test.go +++ b/modules/setting/mailer_test.go @@ -4,6 +4,7 @@ package setting import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -26,11 +27,12 @@ func Test_loadMailerFrom(t *testing.T) { } for host, kase := range kases { t.Run(host, func(t *testing.T) { - cfg, _ := NewConfigProviderFromData("") - sec := cfg.Section("mailer") - sec.NewKey("ENABLED", "true") - sec.NewKey("HOST", host) - + cfg, _ := NewConfigProviderFromData(fmt.Sprintf(` +[mailer] +ENABLED = true +SMTP_ADDR = %s +SMTP_PORT = %s +`, kase.SMTPAddr, kase.SMTPPort)) // Check mailer setting loadMailerFrom(cfg) From 5b5ac4f7e7c55b971509cebc9b19e9108e9bff69 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 5 Jul 2023 17:19:10 +0800 Subject: [PATCH 5/5] Only fatal once even there are many fatal ini keys --- modules/setting/config_provider.go | 10 +++++++++- modules/setting/setting.go | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 94dd989850ae..ae1a78624ba2 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -322,9 +322,17 @@ func deprecatedSetting(rootCfg ConfigProvider, oldSection, oldKey, newSection, n } } +var fatalDeprecatedSetting []string + 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) + fatalDeprecatedSetting = append(fatalDeprecatedSetting, fmt.Sprintf("Deprecated fallback `[%s]` `%s` present. Use `[%s]` `%s` instead. This fallback will be/has been removed in %s", oldSection, oldKey, newSection, newKey, version)) + } +} + +func doDeprecatedSettingFatal() { + if len(fatalDeprecatedSetting) > 0 { + log.Fatal(strings.Join(fatalDeprecatedSetting, "\n")) } } diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 0d69847dbeab..43705b7a97b7 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -214,6 +214,8 @@ func LoadSettings() { loadProjectFrom(CfgProvider) loadMimeTypeMapFrom(CfgProvider) loadFederationFrom(CfgProvider) + + doDeprecatedSettingFatal() } // LoadSettingsForInstall initializes the settings for install