diff --git a/conf/configuration.go b/conf/configuration.go index a9264d3b7..3d04e725c 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -258,6 +258,13 @@ type searchOptions struct { FullString bool } +// fatalFunc is called for fatal config errors. Defaults to printing + os.Exit(1). +// Overridden in tests to allow testing fatal paths. +var fatalFunc = func(msg string) { + _, _ = fmt.Fprintln(os.Stderr, "FATAL:", msg) + os.Exit(1) +} + var ( Server = &configOptions{} hooks []func() @@ -275,6 +282,7 @@ func LoadFromFile(confFile string) { func Load(noConfigDump bool) { parseIniFileConfiguration() + remapEnvVarKeysFromConfig() // Map deprecated options to their new names for backwards compatibility mapDeprecatedOption("ReverseProxyWhitelist", "ExtAuth.TrustedSources") @@ -466,6 +474,35 @@ func logRemovedOptions(options ...string) { } } +// remapEnvVarKeysFromConfig detects ND_-prefixed keys in the config file (users mistakenly +// using environment variable names) and remaps them to canonical Viper keys with a warning. +func remapEnvVarKeysFromConfig() { + for _, key := range viper.AllKeys() { + if !strings.HasPrefix(key, "nd_") || !viper.InConfig(key) { + continue + } + stripped := strings.TrimPrefix(key, "nd_") + canonicalKey := strings.ReplaceAll(stripped, "_", ".") + displayNDKey := "ND_" + strings.ToUpper(stripped) + displayCanonical := toPascalCase(canonicalKey) + + if viper.InConfig(canonicalKey) { + fatalFunc(fmt.Sprintf( + "Config file contains both '%s' and '%s'. Remove the ND_-prefixed version. "+ + "The 'ND_' prefix is only needed for environment variables, not config file keys.", + displayNDKey, displayCanonical, + )) + return + } + + viper.Set(canonicalKey, viper.Get(key)) + _, _ = fmt.Fprintf(os.Stderr, "WARNING: Config key '%s' uses environment variable naming. Use '%s' instead. "+ + "The 'ND_' prefix is only needed for environment variables.\n", + displayNDKey, displayCanonical, + ) + } +} + // mapDeprecatedOption is used to provide backwards compatibility for deprecated options. It should be called after // the config has been read by viper, but before unmarshalling it into the Config struct. func mapDeprecatedOption(legacyName, newName string) { @@ -617,6 +654,21 @@ func normalizeSearchBackend(value string) string { } } +// toPascalCase converts a dotted lowercase config key to PascalCase for display. +// Example: "scanner.schedule" → "Scanner.Schedule" +func toPascalCase(key string) string { + if key == "" { + return "" + } + parts := strings.Split(key, ".") + for i, part := range parts { + if len(part) > 0 { + parts[i] = strings.ToUpper(part[:1]) + part[1:] + } + } + return strings.Join(parts, ".") +} + // AddHook is used to register initialization code that should run as soon as the config is loaded func AddHook(hook func()) { hooks = append(hooks, hook) diff --git a/conf/configuration_test.go b/conf/configuration_test.go index 73fec4196..9aba04197 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -108,6 +108,62 @@ var _ = Describe("Configuration", func() { Entry("falls back to 'fts' for empty string", "", "fts"), ) + DescribeTable("ToPascalCase", + func(input, expected string) { + Expect(conf.ToPascalCase(input)).To(Equal(expected)) + }, + Entry("simple key", "address", "Address"), + Entry("dotted key", "scanner.schedule", "Scanner.Schedule"), + Entry("already capitalized", "Address", "Address"), + Entry("multi-segment", "lastfm.enabled", "Lastfm.Enabled"), + Entry("empty string", "", ""), + ) + + Describe("remapEnvVarKeysFromConfig", func() { + BeforeEach(func() { + viper.Reset() + conf.SetViperDefaults() + viper.SetDefault("datafolder", GinkgoT().TempDir()) + viper.SetDefault("loglevel", "error") + conf.ResetConf() + }) + + It("remaps ND_-prefixed keys to canonical keys", func() { + filename := filepath.Join("testdata", "cfg_nd_keys.toml") + conf.InitConfig(filename, false) + conf.Load(true) + + Expect(conf.Server.Address).To(Equal("127.0.0.1")) + Expect(conf.Server.Port).To(Equal(4531)) + Expect(conf.Server.Scanner.Schedule).To(Equal("@every 1h")) + }) + + It("exits with fatal error when both ND_ and canonical key exist", func() { + cleanup := conf.SetFatalFunc(func(msg string) { + panic(msg) + }) + defer cleanup() + + filename := filepath.Join("testdata", "cfg_nd_conflict.toml") + conf.InitConfig(filename, false) + + Expect(func() { conf.Load(true) }).To(PanicWith(And( + ContainSubstring("ND_ADDRESS"), + ContainSubstring("Address"), + ContainSubstring("only needed for environment variables"), + ))) + }) + + It("does nothing when no ND_ keys are present", func() { + filename := filepath.Join("testdata", "cfg.toml") + conf.InitConfig(filename, false) + conf.Load(true) + + // Verify normal config loading still works + Expect(conf.Server.MusicFolder).To(Equal("/toml/music")) + }) + }) + DescribeTable("should load configuration from", func(format string) { filename := filepath.Join("testdata", "cfg."+format) diff --git a/conf/export_test.go b/conf/export_test.go index d1d1bb3a9..6498215dc 100644 --- a/conf/export_test.go +++ b/conf/export_test.go @@ -11,3 +11,11 @@ var ParseLanguages = parseLanguages var ValidateURL = validateURL var NormalizeSearchBackend = normalizeSearchBackend + +var ToPascalCase = toPascalCase + +func SetFatalFunc(f func(string)) func() { + old := fatalFunc + fatalFunc = f + return func() { fatalFunc = old } +} diff --git a/conf/testdata/cfg_nd_conflict.toml b/conf/testdata/cfg_nd_conflict.toml new file mode 100644 index 000000000..2e8b94bc3 --- /dev/null +++ b/conf/testdata/cfg_nd_conflict.toml @@ -0,0 +1,2 @@ +ND_ADDRESS = "127.0.0.1" +Address = "0.0.0.0" diff --git a/conf/testdata/cfg_nd_keys.toml b/conf/testdata/cfg_nd_keys.toml new file mode 100644 index 000000000..de441ce66 --- /dev/null +++ b/conf/testdata/cfg_nd_keys.toml @@ -0,0 +1,3 @@ +ND_ADDRESS = "127.0.0.1" +ND_PORT = 4531 +ND_SCANNER_SCHEDULE = "@every 1h"