config: validate and print warnings for invalid options (#7705)

Co-authored-by: Joe Chen <jc@unknwon.io>

internal/conf/conf.go

@@ -346,6 +346,9 @@ func Init(customConf string) error {
 	LFS.ObjectsPath = ensureAbs(LFS.ObjectsPath)
 
 	handleDeprecated()
+	for _, warning := range checkInvalidOptions(File) {
+		log.Warn("%s", warning)
+	}
 
 	if err = File.Section("cache").MapTo(&Cache); err != nil {
 		return errors.Wrap(err, "mapping [cache] section")

internal/conf/static.go

@@ -1,11 +1,15 @@
 package conf
 
 import (
+	"fmt"
 	"net/url"
 	"os"
 	"time"
 
 	"github.com/gogs/go-libravatar"
+	"gopkg.in/ini.v1"
+
+	"gogs.io/gogs/conf"
 )
 
 // ℹ️ README: This file contains static values that should only be set at initialization time.
@@ -430,6 +434,83 @@ func handleDeprecated() {
 	// }
 }
 
+// checkInvalidOptions checks invalid (renamed/deleted) configuration sections
+// and options and returns a list of warnings.
+//
+// LEGACY [0.15]: Delete this function.
+func checkInvalidOptions(config *ini.File) (warnings []string) {
+	renamedSections := map[string]string{
+		"mailer":  "email",
+		"service": "auth",
+	}
+	for oldSection, newSection := range renamedSections {
+		if config.Section(oldSection).KeyStrings() != nil {
+			warnings = append(warnings, fmt.Sprintf("section [%s] is invalid, use [%s] instead", oldSection, newSection))
+		}
+	}
+
+	type optionPath struct {
+		section string
+		option  string
+	}
+	renamedOptionPaths := map[optionPath]optionPath{
+		{"security", "REVERSE_PROXY_AUTHENTICATION_USER"}: {"auth", "REVERSE_PROXY_AUTHENTICATION_HEADER"},
+		{"auth", "ACTIVE_CODE_LIVE_MINUTES"}:              {"auth", "ACTIVATE_CODE_LIVES"},
+		{"auth", "RESET_PASSWD_CODE_LIVE_MINUTES"}:        {"auth", "RESET_PASSWORD_CODE_LIVES"},
+		{"auth", "ENABLE_CAPTCHA"}:                        {"auth", "ENABLE_REGISTRATION_CAPTCHA"},
+		{"auth", "ENABLE_NOTIFY_MAIL"}:                    {"user", "ENABLE_EMAIL_NOTIFICATION"},
+		{"auth", "REGISTER_EMAIL_CONFIRM"}:                {"auth", "REQUIRE_EMAIL_CONFIRMATION"},
+		{"session", "GC_INTERVAL_TIME"}:                   {"session", "GC_INTERVAL"},
+		{"session", "SESSION_LIFE_TIME"}:                  {"session", "MAX_LIFE_TIME"},
+		{"server", "ROOT_URL"}:                            {"server", "EXTERNAL_URL"},
+		{"server", "LANDING_PAGE"}:                        {"server", "LANDING_URL"},
+		{"database", "DB_TYPE"}:                           {"database", "TYPE"},
+		{"database", "PASSWD"}:                            {"database", "PASSWORD"},
+	}
+	for oldPath, newPath := range renamedOptionPaths {
+		if config.Section(oldPath.section).HasKey(oldPath.option) {
+			warnings = append(
+				warnings,
+				fmt.Sprintf("option [%s] %s is invalid, use [%s] %s instead",
+					oldPath.section, oldPath.option,
+					newPath.section, newPath.option,
+				),
+			)
+		}
+	}
+
+	// Check options that don't exist anymore.
+	defaultConfigData, err := conf.Files.ReadFile("app.ini")
+	if err != nil {
+		// Warning is best-effort, OK to skip on error.
+		return warnings
+	}
+	defaultConfig, err := ini.LoadSources(
+		ini.LoadOptions{IgnoreInlineComment: true},
+		defaultConfigData,
+	)
+	if err != nil {
+		// Warning is best-effort, OK to skip on error.
+		return warnings
+	}
+	for _, section := range config.Sections() {
+		// Skip sections already warned about.
+		if _, ok := renamedSections[section.Name()]; ok {
+			continue
+		}
+		for _, option := range section.Keys() {
+			if _, ok := renamedOptionPaths[optionPath{section.Name(), option.Name()}]; ok {
+				continue
+			}
+			if !defaultConfig.Section(section.Name()).HasKey(option.Name()) {
+				warnings = append(warnings, fmt.Sprintf("option [%s] %s is invalid", section.Name(), option.Name()))
+			}
+		}
+	}
+
+	return warnings
+}
+
 // HookMode indicates whether program starts as Git server-side hook callback.
 // All operations should be done synchronously to prevent program exits before finishing.
 //

internal/conf/static_test.go

@@ -1,9 +1,11 @@
 package conf
 
 import (
+	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"gopkg.in/ini.v1"
 )
 
 func Test_i18n_DateLang(t *testing.T) {
@@ -29,3 +31,49 @@ func Test_i18n_DateLang(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckInvalidOptions(t *testing.T) {
+	cfg := ini.Empty()
+	_, _ = cfg.Section("mailer").NewKey("ENABLED", "true")
+	_, _ = cfg.Section("service").NewKey("START_TYPE", "true")
+	_, _ = cfg.Section("security").NewKey("REVERSE_PROXY_AUTHENTICATION_USER", "true")
+	_, _ = cfg.Section("auth").NewKey("ACTIVE_CODE_LIVE_MINUTES", "10")
+	_, _ = cfg.Section("auth").NewKey("RESET_PASSWD_CODE_LIVE_MINUTES", "10")
+	_, _ = cfg.Section("auth").NewKey("ENABLE_CAPTCHA", "true")
+	_, _ = cfg.Section("auth").NewKey("ENABLE_NOTIFY_MAIL", "true")
+	_, _ = cfg.Section("auth").NewKey("REGISTER_EMAIL_CONFIRM", "true")
+	_, _ = cfg.Section("session").NewKey("GC_INTERVAL_TIME", "10")
+	_, _ = cfg.Section("session").NewKey("SESSION_LIFE_TIME", "10")
+	_, _ = cfg.Section("server").NewKey("ROOT_URL", "true")
+	_, _ = cfg.Section("server").NewKey("LANDING_PAGE", "true")
+	_, _ = cfg.Section("database").NewKey("DB_TYPE", "true")
+	_, _ = cfg.Section("database").NewKey("PASSWD", "true")
+	_, _ = cfg.Section("other").NewKey("SHOW_FOOTER_BRANDING", "true")
+	_, _ = cfg.Section("other").NewKey("SHOW_FOOTER_TEMPLATE_LOAD_TIME", "true")
+	_, _ = cfg.Section("email").NewKey("ENABLED", "true")
+	_, _ = cfg.Section("server").NewKey("NONEXISTENT_OPTION", "true")
+
+	wantWarnings := []string{
+		"option [auth] ACTIVE_CODE_LIVE_MINUTES is invalid, use [auth] ACTIVATE_CODE_LIVES instead",
+		"option [auth] ENABLE_CAPTCHA is invalid, use [auth] ENABLE_REGISTRATION_CAPTCHA instead",
+		"option [auth] ENABLE_NOTIFY_MAIL is invalid, use [user] ENABLE_EMAIL_NOTIFICATION instead",
+		"option [auth] REGISTER_EMAIL_CONFIRM is invalid, use [auth] REQUIRE_EMAIL_CONFIRMATION instead",
+		"option [auth] RESET_PASSWD_CODE_LIVE_MINUTES is invalid, use [auth] RESET_PASSWORD_CODE_LIVES instead",
+		"option [database] DB_TYPE is invalid, use [database] TYPE instead",
+		"option [database] PASSWD is invalid, use [database] PASSWORD instead",
+		"option [security] REVERSE_PROXY_AUTHENTICATION_USER is invalid, use [auth] REVERSE_PROXY_AUTHENTICATION_HEADER instead",
+		"option [session] GC_INTERVAL_TIME is invalid, use [session] GC_INTERVAL instead",
+		"option [session] SESSION_LIFE_TIME is invalid, use [session] MAX_LIFE_TIME instead",
+		"section [mailer] is invalid, use [email] instead",
+		"section [service] is invalid, use [auth] instead",
+		"option [server] ROOT_URL is invalid, use [server] EXTERNAL_URL instead",
+		"option [server] LANDING_PAGE is invalid, use [server] LANDING_URL instead",
+
+		"option [server] NONEXISTENT_OPTION is invalid",
+	}
+
+	gotWarnings := checkInvalidOptions(cfg)
+	sort.Strings(wantWarnings)
+	sort.Strings(gotWarnings)
+	assert.Equal(t, wantWarnings, gotWarnings)
+}

internal/conf/testdata/custom.ini

@@ -28,9 +28,8 @@ PASSWORD = 87654321
 ACTIVATE_CODE_LIVES = 10
 RESET_PASSWORD_CODE_LIVES = 10
 REQUIRE_EMAIL_CONFIRMATION = true
-ENABLE_CAPTCHA = true
-ENABLE_NOTIFY_MAIL = true
-REVERSE_PROXY_AUTHENTICATION_HEADER=X-FORWARDED-FOR
+ENABLE_REGISTRATION_CAPTCHA = true
+REVERSE_PROXY_AUTHENTICATION_HEADER = X-FORWARDED-FOR
 
 [user]
 ENABLE_EMAIL_NOTIFICATION = true