diff --git a/combined/cmd/config_test.go b/combined/cmd/config_test.go index 070416cfc..a94eb8de8 100644 --- a/combined/cmd/config_test.go +++ b/combined/cmd/config_test.go @@ -1,15 +1,21 @@ package cmd import ( + "context" "os" "path/filepath" + "reflect" + "strings" "testing" + log "github.com/sirupsen/logrus" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestLoadConfigEnvironmentWithoutFile(t *testing.T) { + clearCombinedConfigEnvironment(t) t.Setenv("NB_SERVER_LOGLEVEL", "debug") cfg, err := LoadConfig("") @@ -17,7 +23,164 @@ func TestLoadConfigEnvironmentWithoutFile(t *testing.T) { assert.Equal(t, "debug", cfg.Server.LogLevel, "Environment should override defaults without a file") } +func TestLoadConfigIgnoresEmptyNumericEnvironment(t *testing.T) { + clearCombinedConfigEnvironment(t) + t.Setenv("NB_SERVER_METRICSPORT", "") + + cfg, err := LoadConfig("") + require.NoError(t, err) + assert.Equal(t, DefaultConfig().Server.MetricsPort, cfg.Server.MetricsPort, + "An empty numeric environment variable should retain the previous default") +} + +func TestLoadConfigPreservesLegacyYAMLSemantics(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + contents string + wantErr bool + validate func(*testing.T, *CombinedConfig) + }{ + { + name: "wrong case key is ignored", + contents: `server: + metricsport: 9191 +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "YAML field names should remain case-sensitive") + }, + }, + { + name: "unknown key is ignored", + contents: `server: + unknownSetting: true +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, DefaultConfig().Server, cfg.Server, "Unknown YAML fields should remain ignored") + }, + }, + { + name: "null scalar retains initialized default", + contents: `server: + metricsPort: null +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "Null scalar values should retain initialized defaults") + }, + }, + { + name: "empty nested value retains initialized defaults", + contents: `server: + auth: + storage: {} +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "sqlite3", cfg.Server.Auth.Storage.Type, + "Empty nested values should retain initialized defaults") + }, + }, + { + name: "empty pointer object remains present", + contents: `server: + auth: + owner: {} +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.NotNil(t, cfg.Server.Auth.Owner, "An explicitly configured empty object should remain present") + }, + }, + { + name: "empty sequence remains non nil", + contents: `server: + stunPorts: [] +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.NotNil(t, cfg.Server.StunPorts, "An explicitly configured empty sequence should remain non-nil") + assert.Empty(t, cfg.Server.StunPorts, "An explicitly configured empty sequence should remain empty") + }, + }, + { + name: "null sequence becomes nil", + contents: `server: + stunPorts: null +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.StunPorts, "A null sequence should retain legacy nil semantics") + }, + }, + { + name: "empty map remains non nil", + contents: `server: + perAccountSupportedSyncMessageVersions: {} +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.NotNil(t, cfg.Server.PerAccountSupportedSyncMessageVersions, + "An explicitly configured empty map should remain non-nil") + }, + }, + { + name: "map key case is retained", + contents: `server: + perAccountSupportedSyncMessageVersions: + AccountA: 1 +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, map[string]int{"AccountA": 1}, cfg.Server.PerAccountSupportedSyncMessageVersions, + "YAML map keys should retain their case") + }, + }, + { + name: "legacy boolean spelling remains accepted", + contents: `server: + disableAnonymousMetrics: yes +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.True(t, cfg.Server.DisableAnonymousMetrics, "Legacy YAML booleans should remain accepted") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + cfg, err := LoadConfig(configPath) + if test.wantErr { + assert.Error(t, err, "Legacy-invalid YAML should remain rejected") + return + } + if !assert.NoError(t, err) { + return + } + test.validate(t, cfg) + }) + } +} + +func TestLoadConfigTreatsEveryLegacyFileExtensionAsYAML(t *testing.T) { + clearCombinedConfigEnvironment(t) + for _, name := range []string{"config.json", "config.toml"} { + t.Run(name, func(t *testing.T) { + configPath := writeCombinedConfig(t, name, `server: + metricsPort: 9191 +`) + + cfg, err := LoadConfig(configPath) + if !assert.NoError(t, err, "Combined configuration files were historically decoded as YAML regardless of extension") { + return + } + assert.Equal(t, 9191, cfg.Server.MetricsPort, "YAML content should load regardless of its file extension") + }) + } +} + +func TestLegacyCombinedConfigFlagRemainsRegistered(t *testing.T) { + flag := rootCmd.PersistentFlags().Lookup("config") + require.NotNil(t, flag, "Legacy config flag should remain registered") + assert.Equal(t, "c", flag.Shorthand, "Legacy config shorthand should remain unchanged") +} + func TestLoadConfigEnvironmentOverridesFileAndDefaults(t *testing.T) { + clearCombinedConfigEnvironment(t) configPath := writeCombinedConfig(t, "config.yaml", ` server: exposedAddress: "https://netbird.example.com" @@ -44,6 +207,7 @@ management: } func TestLoadConfigEnvironmentCreatesOptionalConfig(t *testing.T) { + clearCombinedConfigEnvironment(t) configPath := writeCombinedConfig(t, "config.yaml", ` server: exposedAddress: "https://netbird.example.com" @@ -60,6 +224,7 @@ server: } func TestLoadConfigSupportsYAMLWithoutKnownExtension(t *testing.T) { + clearCombinedConfigEnvironment(t) for _, name := range []string{"config", "config.conf"} { t.Run(name, func(t *testing.T) { configPath := writeCombinedConfig(t, name, ` @@ -76,6 +241,7 @@ server: } func TestLoadConfigSupportsLegacyYAMLBooleans(t *testing.T) { + clearCombinedConfigEnvironment(t) configPath := writeCombinedConfig(t, "config.yaml", ` server: exposedAddress: "https://netbird.example.com" @@ -89,6 +255,7 @@ server: } func TestLoadConfigSupportsTOML(t *testing.T) { + clearCombinedConfigEnvironment(t) configPath := writeCombinedConfig(t, "config.toml", ` [server] exposedAddress = "https://netbird.example.com" @@ -104,6 +271,45 @@ logLevel = "warn" assert.Equal(t, ":443", cfg.Server.ListenAddress, "Defaults should survive decoding") } +func clearCombinedConfigEnvironment(t *testing.T) { + t.Helper() + clearCombinedEnvironmentType(t, reflect.TypeOf(CombinedConfig{}), "", make(map[reflect.Type]bool)) +} + +func clearCombinedEnvironmentType(t *testing.T, configType reflect.Type, prefix string, visiting map[reflect.Type]bool) { + t.Helper() + + for configType.Kind() == reflect.Pointer { + configType = configType.Elem() + } + if configType.Kind() != reflect.Struct || visiting[configType] { + return + } + visiting[configType] = true + defer delete(visiting, configType) + + for i := range configType.NumField() { + field := configType.Field(i) + if !field.IsExported() { + continue + } + key := strings.Split(field.Tag.Get("yaml"), ",")[0] + if key == "-" { + continue + } + if key == "" { + key = field.Name + } + if prefix != "" { + key = prefix + "." + key + } + environmentName := "NB_" + strings.ToUpper(strings.NewReplacer(".", "_", "-", "_").Replace(key)) + t.Setenv(environmentName, "") + require.NoError(t, os.Unsetenv(environmentName)) + clearCombinedEnvironmentType(t, field.Type, key, visiting) + } +} + func writeCombinedConfig(t *testing.T, name, contents string) string { t.Helper() @@ -111,3 +317,1014 @@ func writeCombinedConfig(t *testing.T, name, contents string) string { require.NoError(t, os.WriteFile(configPath, []byte(contents), 0o600)) return configPath } + +// legacyCombinedServerFile is a minimal file that satisfied Validate on main. +const legacyCombinedServerFile = `server: + exposedAddress: "https://netbird.example.com" + authSecret: "file-secret" +` + +func TestLoadConfigPreservesLegacyEnvironmentIsNotAConfigSource(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + contents string + env map[string]string + validate func(*testing.T, *CombinedConfig) + }{ + { + name: "NB_SERVER_LOGLEVEL does not override file", + contents: legacyCombinedServerFile + " logLevel: info\n", + env: map[string]string{"NB_SERVER_LOGLEVEL": "debug"}, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "info", cfg.Server.LogLevel, "Legacy loader consulted no environment variables; the file value must win") + assert.Equal(t, "info", cfg.Relay.LogLevel, "Legacy relay log level was inherited from the file value") + assert.Equal(t, "info", cfg.Signal.LogLevel, "Legacy signal log level was inherited from the file value") + assert.Equal(t, "info", cfg.Management.LogLevel, "Legacy management log level was inherited from the file value") + }, + }, + { + name: "NB_SERVER_AUTHSECRET does not override file", + contents: legacyCombinedServerFile, + env: map[string]string{"NB_SERVER_AUTHSECRET": "env-secret"}, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "file-secret", cfg.Server.AuthSecret, "Legacy loader consulted no environment variables; the file secret must win") + assert.Equal(t, "file-secret", cfg.Relay.AuthSecret, "Legacy relay secret was copied from the file value") + }, + }, + { + name: "NB_SERVER_METRICSPORT does not override default", + contents: legacyCombinedServerFile, + env: map[string]string{"NB_SERVER_METRICSPORT": "9191"}, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "Legacy loader consulted no environment variables; the default must be kept") + }, + }, + { + name: "NB_SERVER_AUTH_ISSUER does not set nested field", + contents: legacyCombinedServerFile, + env: map[string]string{"NB_SERVER_AUTH_ISSUER": "https://idp.example.com"}, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Empty(t, cfg.Server.Auth.Issuer, "Legacy loader consulted no environment variables for nested fields") + assert.Empty(t, cfg.Management.Auth.Issuer, "Legacy management auth was not populated from the environment") + }, + }, + { + name: "NB_SERVER_TLS_LETSENCRYPT_DOMAINS does not set list field", + contents: legacyCombinedServerFile, + env: map[string]string{"NB_SERVER_TLS_LETSENCRYPT_DOMAINS": "a.com"}, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.TLS.LetsEncrypt.Domains, "Legacy loader consulted no environment variables for list fields") + }, + }, + { + name: "NB_SERVER_LOG_LEVEL with word separator is ignored", + contents: legacyCombinedServerFile, + env: map[string]string{"NB_SERVER_LOG_LEVEL": "debug"}, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "info", cfg.Server.LogLevel, "Legacy loader consulted no environment variables") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for name, value := range test.env { + t.Setenv(name, value) + } + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +func TestLoadConfigPreservesLegacyEmptyPathReturnsPlainDefaults(t *testing.T) { + clearCombinedConfigEnvironment(t) + t.Setenv("NB_SERVER_EXPOSEDADDRESS", "https://env.example.com") + t.Setenv("NB_SERVER_AUTHSECRET", "env-secret") + + cfg, err := LoadConfig("") + require.NoError(t, err) + assert.Equal(t, DefaultConfig(), cfg, + "Legacy LoadConfig(\"\") returned DefaultConfig() before consulting anything else, including ApplySimplifiedDefaults") + assert.False(t, cfg.Relay.Enabled, "Legacy empty path never enabled the embedded relay") + assert.False(t, cfg.Signal.Enabled, "Legacy empty path never enabled the embedded signal") + assert.False(t, cfg.Management.Enabled, "Legacy empty path never enabled management") + assert.EqualError(t, cfg.Validate(), "server.exposedAddress is required", + "Legacy empty path produced a config that failed validation because the environment was not consulted") +} + +func TestLoadConfigPreservesLegacyEmptyStringEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + envName string + contents string + validate func(*testing.T, *CombinedConfig) + }{ + { + name: "empty NB_SERVER_LOGLEVEL keeps log level", + envName: "NB_SERVER_LOGLEVEL", + contents: legacyCombinedServerFile, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "info", cfg.Server.LogLevel, "Legacy loader ignored the environment; an empty variable must not blank the log level") + assert.Equal(t, "info", cfg.Relay.LogLevel, "Legacy relay log level inherited info") + assert.Equal(t, "info", cfg.Management.LogLevel, "Legacy management log level inherited info") + }, + }, + { + name: "empty NB_SERVER_EXPOSEDADDRESS keeps file value", + envName: "NB_SERVER_EXPOSEDADDRESS", + contents: legacyCombinedServerFile, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "https://netbird.example.com", cfg.Server.ExposedAddress, "Legacy loader ignored the environment; the file exposed address must be kept") + assert.True(t, cfg.Management.Enabled, "Legacy ApplySimplifiedDefaults ran with the file exposed address") + assert.NoError(t, cfg.Validate(), "Legacy config remained valid") + }, + }, + { + name: "empty NB_SERVER_DATADIR keeps default", + envName: "NB_SERVER_DATADIR", + contents: legacyCombinedServerFile, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "/var/lib/netbird/", cfg.Server.DataDir, "Legacy loader ignored the environment; the default data dir must be kept") + assert.Equal(t, "/var/lib/netbird/", cfg.Management.DataDir, "Legacy management data dir came from the default") + assert.NoError(t, cfg.Validate(), "Legacy config remained valid") + }, + }, + { + name: "empty NB_SERVER_AUTHSECRET keeps file value", + envName: "NB_SERVER_AUTHSECRET", + contents: legacyCombinedServerFile, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "file-secret", cfg.Server.AuthSecret, "Legacy loader ignored the environment; the file secret must be kept") + assert.NoError(t, cfg.Validate(), "Legacy config remained valid") + }, + }, + { + name: "empty NB_SERVER_LISTENADDRESS keeps default", + envName: "NB_SERVER_LISTENADDRESS", + contents: legacyCombinedServerFile, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, ":443", cfg.Server.ListenAddress, "Legacy loader ignored the environment; the default listen address must be kept") + }, + }, + { + name: "empty NB_SERVER_HEALTHCHECKADDRESS keeps default", + envName: "NB_SERVER_HEALTHCHECKADDRESS", + contents: legacyCombinedServerFile, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, ":9000", cfg.Server.HealthcheckAddress, "Legacy loader ignored the environment; the default healthcheck address must be kept") + }, + }, + { + name: "empty NB_SERVER_STORE_ENGINE keeps default", + envName: "NB_SERVER_STORE_ENGINE", + contents: legacyCombinedServerFile, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "sqlite", cfg.Server.Store.Engine, "Legacy loader ignored the environment; the default store engine must be kept") + assert.Equal(t, "sqlite", cfg.Management.Store.Engine, "Legacy management store engine came from the default") + }, + }, + { + name: "empty NB_SERVER_LOGFILE keeps default", + envName: "NB_SERVER_LOGFILE", + contents: legacyCombinedServerFile, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "console", cfg.Server.LogFile, "Legacy loader ignored the environment; the default console log target must be kept") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.envName, "") + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +func TestLoadConfigPreservesLegacyUnparsableEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + envName string + envValue string + validate func(*testing.T, *CombinedConfig) + }{ + {name: "metrics port word", envName: "NB_SERVER_METRICSPORT", envValue: "abc", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "Legacy loader ignored the environment; default metrics port must be kept") + }}, + {name: "metrics port padded", envName: "NB_SERVER_METRICSPORT", envValue: " 9191 ", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "Legacy loader ignored the environment; default metrics port must be kept") + }}, + {name: "metrics port float", envName: "NB_SERVER_METRICSPORT", envValue: "9191.0", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "Legacy loader ignored the environment; default metrics port must be kept") + }}, + {name: "metrics port leading zero", envName: "NB_SERVER_METRICSPORT", envValue: "09090", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "Legacy loader ignored the environment; default metrics port must be kept") + }}, + {name: "metrics port hex", envName: "NB_SERVER_METRICSPORT", envValue: "0x10", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "Legacy loader ignored the environment; default metrics port must be kept") + }}, + {name: "negative uint", envName: "NB_SERVER_REVERSEPROXY_TRUSTEDHTTPPROXIESCOUNT", envValue: "-1", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, uint(0), cfg.Server.ReverseProxy.TrustedHTTPProxiesCount, "Legacy loader ignored the environment; default proxy count must be kept") + }}, + {name: "sync version word", envName: "NB_SERVER_SUPPORTEDSYNCMESSAGEVERSIONS", envValue: "x", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.SupportedSyncMessageVersions, "Legacy loader ignored the environment; sync version pointer must remain nil") + }}, + {name: "boolean word", envName: "NB_SERVER_DISABLEANONYMOUSMETRICS", envValue: "maybe", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.False(t, cfg.Server.DisableAnonymousMetrics, "Legacy loader ignored the environment; default boolean must be kept") + }}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.envName, test.envValue) + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + if !assert.NoError(t, err, "Legacy loader never failed because of an environment variable") { + return + } + test.validate(t, cfg) + assert.NoError(t, cfg.Validate(), "Legacy config remained valid") + }) + } +} + +func TestLoadConfigPreservesLegacyEmptyBooleanEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + envName string + contents string + value func(*CombinedConfig) bool + }{ + { + name: "disableAnonymousMetrics", + envName: "NB_SERVER_DISABLEANONYMOUSMETRICS", + contents: legacyCombinedServerFile + " disableAnonymousMetrics: true\n", + value: func(cfg *CombinedConfig) bool { + return cfg.Server.DisableAnonymousMetrics && cfg.Management.DisableAnonymousMetrics + }, + }, + { + name: "disableGeoliteUpdate", + envName: "NB_SERVER_DISABLEGEOLITEUPDATE", + contents: legacyCombinedServerFile + " disableGeoliteUpdate: true\n", + value: func(cfg *CombinedConfig) bool { + return cfg.Server.DisableGeoliteUpdate && cfg.Management.DisableGeoliteUpdate + }, + }, + { + name: "tls.letsencrypt.enabled", + envName: "NB_SERVER_TLS_LETSENCRYPT_ENABLED", + contents: legacyCombinedServerFile + " tls:\n letsencrypt:\n enabled: true\n", + value: func(cfg *CombinedConfig) bool { return cfg.Server.TLS.LetsEncrypt.Enabled }, + }, + { + name: "auth.localAuthDisabled", + envName: "NB_SERVER_AUTH_LOCALAUTHDISABLED", + contents: legacyCombinedServerFile + " auth:\n localAuthDisabled: true\n", + value: func(cfg *CombinedConfig) bool { return cfg.Server.Auth.LocalAuthDisabled }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.envName, "") + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.True(t, test.value(cfg), "Legacy loader ignored the environment; a boolean set to true in the file must stay true") + }) + } +} + +func TestLoadConfigPreservesLegacyBooleanEnvironmentSpellingsIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + for _, value := range []string{"1", "0", "t", "f", "T", "F", "TRUE", "true", "True", "FALSE", "yes", "no", "y", "n", "on", "off", "ON", "Y"} { + t.Run("default_"+value, func(t *testing.T) { + t.Setenv("NB_SERVER_DISABLEANONYMOUSMETRICS", value) + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.False(t, cfg.Server.DisableAnonymousMetrics, + "Legacy booleans came only from YAML or the false default; environment spellings were not consulted") + }) + } + for _, value := range []string{"0", "f", "off", "no", "FALSE"} { + t.Run("file_true_"+value, func(t *testing.T) { + t.Setenv("NB_SERVER_DISABLEANONYMOUSMETRICS", value) + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile+" disableAnonymousMetrics: true\n") + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.True(t, cfg.Server.DisableAnonymousMetrics, + "Legacy booleans came only from YAML; an environment false spelling must not override the file") + }) + } +} + +func TestLoadConfigPreservesLegacyListEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + envName string + envValue string + validate func(*testing.T, *CombinedConfig) + }{ + {name: "stun ports csv", envName: "NB_SERVER_STUNPORTS", envValue: "3478,3479", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, []int{3478}, cfg.Server.StunPorts, "Legacy lists were only settable via YAML sequences") + }}, + {name: "stun ports csv with space", envName: "NB_SERVER_STUNPORTS", envValue: "3478, 3479", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, []int{3478}, cfg.Server.StunPorts, "Legacy lists were only settable via YAML sequences") + }}, + {name: "stun ports space separated", envName: "NB_SERVER_STUNPORTS", envValue: "3478 3479", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, []int{3478}, cfg.Server.StunPorts, "Legacy lists were only settable via YAML sequences") + }}, + {name: "stun ports trailing comma", envName: "NB_SERVER_STUNPORTS", envValue: "3478,", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, []int{3478}, cfg.Server.StunPorts, "Legacy lists were only settable via YAML sequences") + }}, + {name: "domains csv with space", envName: "NB_SERVER_TLS_LETSENCRYPT_DOMAINS", envValue: "a.com, b.com", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.TLS.LetsEncrypt.Domains, "Legacy lists were only settable via YAML sequences") + }}, + {name: "domains trailing comma", envName: "NB_SERVER_TLS_LETSENCRYPT_DOMAINS", envValue: "a.com,", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.TLS.LetsEncrypt.Domains, "Legacy lists were only settable via YAML sequences") + }}, + {name: "domains quoted", envName: "NB_SERVER_TLS_LETSENCRYPT_DOMAINS", envValue: `"a.com","b.com"`, validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.TLS.LetsEncrypt.Domains, "Legacy lists were only settable via YAML sequences") + }}, + {name: "trusted proxies", envName: "NB_SERVER_REVERSEPROXY_TRUSTEDHTTPPROXIES", envValue: "10.0.0.0/8,192.168.0.0/16", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.ReverseProxy.TrustedHTTPProxies, "Legacy lists were only settable via YAML sequences") + }}, + {name: "grant types", envName: "NB_SERVER_AUTH_GRANTTYPES", envValue: "authorization_code,refresh_token", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.Auth.GrantTypes, "Legacy lists were only settable via YAML sequences") + }}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.envName, test.envValue) + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + if !assert.NoError(t, err, "Legacy loader never failed because of an environment variable") { + return + } + test.validate(t, cfg) + assert.NoError(t, cfg.Validate(), "Legacy config remained valid") + }) + } +} + +func TestLoadConfigPreservesLegacyEmptyListEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + envName string + contents string + validate func(*testing.T, *CombinedConfig) + }{ + {name: "stun ports with exposed address", envName: "NB_SERVER_STUNPORTS", contents: legacyCombinedServerFile, validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, []int{3478}, cfg.Server.StunPorts, "Legacy default STUN ports were kept") + }}, + {name: "stun ports without exposed address", envName: "NB_SERVER_STUNPORTS", contents: "server: {}\n", validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, []int{3478}, cfg.Server.StunPorts, "Legacy default STUN ports were kept even without exposedAddress") + }}, + {name: "domains", envName: "NB_SERVER_TLS_LETSENCRYPT_DOMAINS", contents: legacyCombinedServerFile, validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.TLS.LetsEncrypt.Domains, "Legacy domains stayed nil when absent from the file") + }}, + {name: "relay addresses", envName: "NB_SERVER_RELAYS_ADDRESSES", contents: legacyCombinedServerFile, validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.Relays.Addresses, "Legacy relay addresses stayed nil when absent from the file") + }}, + {name: "stuns", envName: "NB_SERVER_STUNS", contents: legacyCombinedServerFile, validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Nil(t, cfg.Server.Stuns, "Legacy external STUN list stayed nil when absent from the file") + }}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.envName, "") + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +func TestLoadConfigPreservesLegacyMapEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + for _, value := range []string{"", "acc=1", `{"acc":1}`} { + t.Run("value_"+value, func(t *testing.T) { + t.Setenv("NB_SERVER_PERACCOUNTSUPPORTEDSYNCMESSAGEVERSIONS", value) + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + if !assert.NoError(t, err, "Legacy loader never failed because of an environment variable") { + return + } + assert.Nil(t, cfg.Server.PerAccountSupportedSyncMessageVersions, "Legacy map came only from YAML") + }) + } +} + +func TestLoadConfigPreservesLegacyStunsEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + envName string + }{ + {name: "stuns list", envName: "NB_SERVER_STUNS"}, + {name: "stuns uri", envName: "NB_SERVER_STUNS_URI"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.envName, "stun:stun.example.com:3478") + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + if !assert.NoError(t, err, "Legacy loader never failed because of an environment variable") { + return + } + assert.Nil(t, cfg.Server.Stuns, "Legacy external STUN servers were only configurable via YAML") + assert.True(t, cfg.Relay.Stun.Enabled, "Legacy local STUN stayed enabled") + assert.Equal(t, []HostConfig{{URI: "stun:netbird.example.com:3478"}}, cfg.Management.Stuns, + "Legacy clients were pointed at the local STUN server") + }) + } +} + +func TestLoadConfigPreservesLegacyOwnerEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + env map[string]string + }{ + {name: "email and password", env: map[string]string{"NB_SERVER_AUTH_OWNER_EMAIL": "a@b", "NB_SERVER_AUTH_OWNER_PASSWORD": "hash"}}, + {name: "empty email", env: map[string]string{"NB_SERVER_AUTH_OWNER_EMAIL": ""}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for name, value := range test.env { + t.Setenv(name, value) + } + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.Nil(t, cfg.Server.Auth.Owner, "Legacy owner was nil unless server.auth.owner was present in YAML") + }) + } +} + +func TestLoadConfigPreservesLegacyEmbeddedTopologyIgnoresEnvironment(t *testing.T) { + clearCombinedConfigEnvironment(t) + + t.Run("relay addresses", func(t *testing.T) { + t.Setenv("NB_SERVER_RELAYS_ADDRESSES", "rels://r1:443,rels://r2:443") + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.True(t, cfg.Relay.Enabled, "Legacy embedded relay stayed enabled because the environment was not consulted") + assert.Equal(t, []string{"rels://netbird.example.com"}, cfg.Management.Relays.Addresses, + "Legacy clients were pointed at the embedded relay") + }) + + t.Run("signal uri", func(t *testing.T) { + t.Setenv("NB_SERVER_SIGNALURI", "https://sig.example.com:443") + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.True(t, cfg.Signal.Enabled, "Legacy embedded signal stayed enabled because the environment was not consulted") + assert.Equal(t, "https://netbird.example.com", cfg.Management.SignalURI, + "Legacy clients were pointed at the embedded signal") + }) +} + +func TestLoadConfigPreservesLegacyStoreEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + for _, name := range []string{"NB_STORE_ENGINE_POSTGRES_DSN", "NB_STORE_ENGINE_SQLITE_FILE", "NB_ACTIVITY_EVENT_STORE_ENGINE", "NB_ACTIVITY_EVENT_POSTGRES_DSN", "NB_ACTIVITY_EVENT_SQLITE_FILE"} { + t.Setenv(name, "") + require.NoError(t, os.Unsetenv(name)) + } + + t.Run("store engine and dsn", func(t *testing.T) { + t.Setenv("NB_SERVER_STORE_ENGINE", "postgres") + t.Setenv("NB_SERVER_STORE_DSN", "host=x") + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.Equal(t, StoreConfig{Engine: "sqlite"}, cfg.Server.Store, "Legacy store engine came only from YAML") + assert.Equal(t, StoreConfig{Engine: "sqlite"}, cfg.Management.Store, "Legacy management store came only from YAML") + applyServerStoreEnv(cfg.Server.Store) + assert.Empty(t, os.Getenv("NB_STORE_ENGINE_POSTGRES_DSN"), "Legacy chained NB_STORE_ENGINE_POSTGRES_DSN export was derived from the file only") + }) + + t.Run("store file", func(t *testing.T) { + t.Setenv("NB_SERVER_STORE_FILE", "/db/x.db") + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.Empty(t, cfg.Management.Store.File, "Legacy store file came only from YAML") + applyServerStoreEnv(cfg.Server.Store) + assert.Empty(t, os.Getenv("NB_STORE_ENGINE_SQLITE_FILE"), "Legacy chained NB_STORE_ENGINE_SQLITE_FILE export was derived from the file only") + }) + + t.Run("encryption key", func(t *testing.T) { + t.Setenv("NB_SERVER_STORE_ENCRYPTIONKEY", "k") + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + mgmtConfig, err := cfg.ToManagementConfig() + require.NoError(t, err) + assert.Empty(t, mgmtConfig.DataStoreEncryptionKey, "Legacy encryption key came only from YAML, so one was auto-generated") + }) + + t.Run("activity store engine without dsn", func(t *testing.T) { + t.Setenv("NB_SERVER_ACTIVITYSTORE_ENGINE", "postgres") + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.Empty(t, cfg.Server.ActivityStore.Engine, "Legacy activity store engine came only from YAML") + assert.NoError(t, applyActivityStoreEnv(cfg.Server.ActivityStore), "Legacy startup did not fail on an environment-only activity store engine") + }) + + t.Run("auth store engine without dsn", func(t *testing.T) { + t.Setenv("NB_SERVER_AUTHSTORE_ENGINE", "postgres") + configPath := writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.Empty(t, cfg.Server.AuthStore.Engine, "Legacy auth store engine came only from YAML") + _, err = cfg.ToManagementConfig() + assert.NoError(t, err, "Legacy management config did not fail on an environment-only auth store engine") + }) +} + +func TestLoadConfigPreservesLegacyAdminCommandsIgnoreEnvironment(t *testing.T) { + clearCombinedConfigEnvironment(t) + for _, name := range []string{"NB_STORE_ENGINE_POSTGRES_DSN", "NB_STORE_ENGINE_SQLITE_FILE"} { + t.Setenv(name, "") + require.NoError(t, os.Unsetenv(name)) + } + + previousConfigPath := configPath + previousLevel := log.GetLevel() + t.Cleanup(func() { + configPath = previousConfigPath + log.SetLevel(previousLevel) + }) + + runAdmin := func(t *testing.T) (*CombinedConfig, error) { + t.Helper() + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + var loaded *CombinedConfig + err := withAdminConfig(cmd, func(_ context.Context, cfg *CombinedConfig) error { + loaded = cfg + return nil + }) + return loaded, err + } + + t.Run("store and data dir", func(t *testing.T) { + t.Setenv("NB_SERVER_STORE_ENGINE", "postgres") + t.Setenv("NB_SERVER_STORE_DSN", "host=x") + t.Setenv("NB_SERVER_DATADIR", "/other") + configPath = writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile+" dataDir: /srv/netbird\n") + + cfg, err := runAdmin(t) + require.NoError(t, err) + assert.Equal(t, "/srv/netbird", cfg.Management.DataDir, "Legacy admin commands operated on the data dir named in the file") + assert.Equal(t, StoreConfig{Engine: "sqlite"}, cfg.Management.Store, "Legacy admin commands operated on the store named in the file") + assert.Empty(t, os.Getenv("NB_STORE_ENGINE_POSTGRES_DSN"), "Legacy admin commands derived NB_STORE_ENGINE_POSTGRES_DSN from the file only") + }) + + t.Run("malformed numeric environment", func(t *testing.T) { + t.Setenv("NB_SERVER_METRICSPORT", "abc") + configPath = writeCombinedConfig(t, "config.yaml", legacyCombinedServerFile) + + _, err := runAdmin(t) + assert.NoError(t, err, "Legacy admin commands never failed because of an environment variable") + }) +} + +func TestLoadConfigPreservesLegacySectionNamedEnvironmentIgnored(t *testing.T) { + clearCombinedConfigEnvironment(t) + const contents = `server: + exposedAddress: "https://netbird.example.com" + authSecret: "file-secret" + tls: + certFile: /certs/cert.pem + keyFile: /certs/key.pem + store: + engine: postgres + dsn: host=x + auth: + issuer: https://netbird.example.com/oauth2 + owner: + email: owner@example.com + relays: + addresses: + - rels://r1:443 +` + tests := []struct { + name string + envName string + envValue string + }{ + {name: "NB_SERVER empty", envName: "NB_SERVER", envValue: ""}, + {name: "NB_SERVER_STORE", envName: "NB_SERVER_STORE", envValue: "x"}, + {name: "NB_SERVER_TLS empty", envName: "NB_SERVER_TLS", envValue: ""}, + {name: "NB_SERVER_AUTH", envName: "NB_SERVER_AUTH", envValue: "1"}, + {name: "NB_SERVER_AUTH_OWNER", envName: "NB_SERVER_AUTH_OWNER", envValue: "x"}, + {name: "NB_SERVER_RELAYS", envName: "NB_SERVER_RELAYS", envValue: "x"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.envName, test.envValue) + configPath := writeCombinedConfig(t, "config.yaml", contents) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + assert.Equal(t, "https://netbird.example.com", cfg.Server.ExposedAddress, "Legacy file section was decoded regardless of a section-named environment variable") + assert.Equal(t, "/certs/cert.pem", cfg.Server.TLS.CertFile, "Legacy TLS section was decoded regardless of a section-named environment variable") + assert.Equal(t, StoreConfig{Engine: "postgres", DSN: "host=x"}, cfg.Server.Store, "Legacy store section was decoded regardless of a section-named environment variable") + assert.Equal(t, "https://netbird.example.com/oauth2", cfg.Server.Auth.Issuer, "Legacy auth section was decoded regardless of a section-named environment variable") + if assert.NotNil(t, cfg.Server.Auth.Owner, "Legacy owner section was decoded regardless of a section-named environment variable") { + assert.Equal(t, "owner@example.com", cfg.Server.Auth.Owner.Email) + } + assert.Equal(t, []string{"rels://r1:443"}, cfg.Server.Relays.Addresses, "Legacy relays section was decoded regardless of a section-named environment variable") + assert.NoError(t, cfg.Validate(), "Legacy config remained valid") + }) + } +} + +func TestLegacyCombinedConfigHelpTextRemainsUnchanged(t *testing.T) { + flag := rootCmd.PersistentFlags().Lookup("config") + require.NotNil(t, flag, "Legacy config flag should remain registered") + assert.Equal(t, "path to YAML configuration file (required)", flag.Usage, "Legacy --config help text should remain unchanged") + assert.Contains(t, rootCmd.Long, "Configuration is loaded from a YAML file specified with --config.", + "Legacy command description should remain unchanged") + assert.NotContains(t, rootCmd.Long, "NB_SERVER_LOGLEVEL", + "Legacy command description did not advertise environment overrides") +} + +func TestLoadConfigPreservesLegacyRootAndListItemKeyCaseSensitivity(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + contents string + validate func(*testing.T, *CombinedConfig) + }{ + { + name: "capitalized root key is ignored", + contents: `Server: + metricsPort: 9191 +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, "Legacy YAML root keys were case-sensitive") + }, + }, + { + name: "uppercase root key is ignored", + contents: `SERVER: + exposedAddress: https://netbird.example.com +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Empty(t, cfg.Server.ExposedAddress, "Legacy YAML root keys were case-sensitive") + assert.EqualError(t, cfg.Validate(), "server.exposedAddress is required", "Legacy startup failed validation") + }, + }, + { + name: "wrong case list item key is ignored", + contents: `server: + stuns: + - URI: stun:x:3478 +`, + validate: func(t *testing.T, cfg *CombinedConfig) { + require.Len(t, cfg.Server.Stuns, 1, "Legacy list item was still created") + assert.Empty(t, cfg.Server.Stuns[0].URI, "Legacy YAML keys inside list items were case-sensitive") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +func TestLoadConfigPreservesLegacyMixedCaseDuplicateKeyResolution(t *testing.T) { + clearCombinedConfigEnvironment(t) + configPath := writeCombinedConfig(t, "config.yaml", `server: + metricsPort: 1 + MetricsPort: 2 +`) + + for i := 0; i < 25; i++ { + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + if !assert.Equal(t, 1, cfg.Server.MetricsPort, + "Legacy YAML applied only the exactly matching key deterministically on every load") { + return + } + } +} + +func TestLoadConfigTreatsAdditionalLegacyFileExtensionsAsYAML(t *testing.T) { + clearCombinedConfigEnvironment(t) + for _, name := range []string{"config.env", "config.dotenv", "config.ini", "config.properties", "config.props", "config.prop", "config.hcl", "config.tfvars", "config.JSON"} { + t.Run(name, func(t *testing.T) { + configPath := writeCombinedConfig(t, name, `server: + metricsPort: 9191 +`) + + cfg, err := LoadConfig(configPath) + if !assert.NoError(t, err, "Combined configuration files were historically decoded as YAML regardless of extension") { + return + } + assert.Equal(t, 9191, cfg.Server.MetricsPort, "YAML content should load regardless of its file extension") + }) + } +} + +func TestLoadConfigPreservesLegacyStrictYAMLTyping(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + contents string + }{ + {name: "quoted int", contents: "server:\n metricsPort: \"9191\"\n"}, + {name: "str tagged int", contents: "server:\n metricsPort: !!str 9191\n"}, + {name: "quoted uint", contents: "server:\n reverseProxy:\n trustedHTTPProxiesCount: \"3\"\n"}, + {name: "quoted int pointer", contents: "server:\n supportedSyncMessageVersions: \"2\"\n"}, + {name: "quoted map value", contents: "server:\n perAccountSupportedSyncMessageVersions: {abc: \"2\"}\n"}, + {name: "int as bool", contents: "server:\n disableAnonymousMetrics: 1\n"}, + {name: "float as bool", contents: "server:\n disableAnonymousMetrics: 1.0\n"}, + {name: "quoted true as bool", contents: "server:\n disableAnonymousMetrics: \"true\"\n"}, + {name: "quoted zero as bool", contents: "server:\n disableAnonymousMetrics: \"0\"\n"}, + {name: "t as bool", contents: "server:\n disableAnonymousMetrics: t\n"}, + {name: "yEs as bool", contents: "server:\n disableAnonymousMetrics: yEs\n"}, + {name: "scalar into int slice", contents: "server:\n stunPorts: 3479\n"}, + {name: "csv into int slice", contents: "server:\n stunPorts: \"3479,3480\"\n"}, + {name: "quoted element in int slice", contents: "server:\n stunPorts: [3479, \"3480\"]\n"}, + {name: "csv into string slice", contents: "server:\n tls:\n letsencrypt:\n domains: a.com,b.com\n"}, + {name: "empty string into int", contents: "server:\n metricsPort: \"\"\n"}, + {name: "float overflow into int", contents: "server:\n metricsPort: 99999999999999999999\n"}, + {name: "int overflow into int", contents: "server:\n metricsPort: 9223372036854775808\n"}, + {name: "negative into uint", contents: "server:\n reverseProxy:\n trustedHTTPProxiesCount: -1\n"}, + {name: "map into struct slice", contents: "server:\n stuns:\n uri: stun:x:3478\n"}, + {name: "map into int slice", contents: "server:\n stunPorts: {}\n"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + _, err := LoadConfig(configPath) + if !assert.Error(t, err, "Legacy yaml.v3 decoding rejected values whose YAML type did not match the Go field") { + return + } + assert.ErrorContains(t, err, "cannot unmarshal", "Legacy error came from yaml.v3 strict typing") + }) + } +} + +func TestLoadConfigPreservesLegacyUnquotedStringScalarText(t *testing.T) { + clearCombinedConfigEnvironment(t) + for _, value := range []string{"0x10", "0755", "0123456789", "1234567890123456789012345", "1.50", "1e3", ".inf", "true", "2001-12-14"} { + t.Run(value, func(t *testing.T) { + configPath := writeCombinedConfig(t, "config.yaml", "server:\n authSecret: "+value+"\n") + cfg, err := LoadConfig(configPath) + if !assert.NoError(t, err, "Legacy yaml.v3 decoded any unquoted scalar into a string field") { + return + } + assert.Equal(t, value, cfg.Server.AuthSecret, "Legacy yaml.v3 kept the original scalar text for string fields") + }) + } +} + +func TestLoadConfigPreservesLegacyLoadErrorWording(t *testing.T) { + clearCombinedConfigEnvironment(t) + + t.Run("missing file", func(t *testing.T) { + _, err := LoadConfig(filepath.Join(t.TempDir(), "missing.yaml")) + require.Error(t, err) + assert.ErrorContains(t, err, "failed to read config file:", "Legacy read errors were prefixed with 'failed to read config file:'") + assert.ErrorContains(t, err, "no such file or directory") + }) + + t.Run("directory", func(t *testing.T) { + _, err := LoadConfig(t.TempDir()) + require.Error(t, err) + assert.ErrorContains(t, err, "failed to read config file:", "Legacy read errors were prefixed with 'failed to read config file:'") + }) + + t.Run("syntax error", func(t *testing.T) { + configPath := writeCombinedConfig(t, "config.yaml", "server:\n\tmetricsPort: 1\n") + _, err := LoadConfig(configPath) + require.Error(t, err) + assert.ErrorContains(t, err, "failed to parse config file:", "Legacy parse errors were prefixed with 'failed to parse config file:'") + assert.ErrorContains(t, err, "found character that cannot start any token") + }) + + t.Run("type error", func(t *testing.T) { + configPath := writeCombinedConfig(t, "config.yaml", "server: foo\n") + _, err := LoadConfig(configPath) + require.Error(t, err) + assert.ErrorContains(t, err, "failed to parse config file:", "Legacy parse errors were prefixed with 'failed to parse config file:'") + assert.ErrorContains(t, err, "cannot unmarshal !!str `foo` into cmd.ServerConfig") + }) +} + +func TestLoadConfigPreservesLegacyEmptyIntegerEnvironmentKeepsFileValue(t *testing.T) { + clearCombinedConfigEnvironment(t) + const contents = legacyCombinedServerFile + ` reverseProxy: + trustedHTTPProxiesCount: 3 + accessLogRetentionDays: 30 + accessLogCleanupIntervalHours: 12 +` + tests := []struct { + name string + envName string + validate func(*testing.T, *CombinedConfig) + }{ + { + name: "empty NB_SERVER_REVERSEPROXY_TRUSTEDHTTPPROXIESCOUNT keeps uint from file", + envName: "NB_SERVER_REVERSEPROXY_TRUSTEDHTTPPROXIESCOUNT", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, uint(3), cfg.Server.ReverseProxy.TrustedHTTPProxiesCount, + "Legacy loader ignored the environment; an empty variable must not blank the uint proxy count set in the file") + assert.Equal(t, uint(3), cfg.Management.ReverseProxy.TrustedHTTPProxiesCount, + "Legacy management reverse proxy was copied from the file because the proxy count was non-zero") + }, + }, + { + name: "empty NB_SERVER_REVERSEPROXY_ACCESSLOGRETENTIONDAYS keeps int from file", + envName: "NB_SERVER_REVERSEPROXY_ACCESSLOGRETENTIONDAYS", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 30, cfg.Server.ReverseProxy.AccessLogRetentionDays, + "Legacy loader ignored the environment; an empty variable must not blank the retention days set in the file") + assert.Equal(t, 30, cfg.Management.ReverseProxy.AccessLogRetentionDays, + "Legacy management reverse proxy carried the 30 day retention from the file") + }, + }, + { + name: "empty NB_SERVER_REVERSEPROXY_ACCESSLOGCLEANUPINTERVALHOURS keeps int from file", + envName: "NB_SERVER_REVERSEPROXY_ACCESSLOGCLEANUPINTERVALHOURS", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 12, cfg.Server.ReverseProxy.AccessLogCleanupIntervalHours, + "Legacy loader ignored the environment; an empty variable must not blank the cleanup interval set in the file") + assert.Equal(t, 12, cfg.Management.ReverseProxy.AccessLogCleanupIntervalHours, + "Legacy management reverse proxy carried the 12 hour cleanup interval from the file") + }, + }, + { + name: "empty NB_SERVER_METRICSPORT keeps int from file", + envName: "NB_SERVER_METRICSPORT", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9191, cfg.Server.MetricsPort, + "Legacy loader ignored the environment; an empty variable must not blank the metrics port set in the file") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.envName, "") + configPath := writeCombinedConfig(t, "config.yaml", contents+" metricsPort: 9191\n") + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + assert.NoError(t, cfg.Validate(), "Legacy config remained valid") + }) + } +} + +func TestLoadConfigPreservesLegacyNullSequenceItemsAreDropped(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + contents string + validate func(*testing.T, *CombinedConfig) + }{ + { + name: "null stuns item", + contents: legacyCombinedServerFile + " stuns:\n - null\n", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Empty(t, cfg.Server.Stuns, "Legacy yaml.v3 dropped null sequence items, leaving no external STUN servers") + assert.True(t, cfg.Relay.Stun.Enabled, "Legacy local STUN stayed enabled because no external STUN server survived decoding") + assert.Equal(t, []HostConfig{{URI: "stun:netbird.example.com:3478"}}, cfg.Management.Stuns, + "Legacy clients were pointed at the local STUN server") + }, + }, + { + name: "null relay address item", + contents: legacyCombinedServerFile + " relays:\n addresses:\n - null\n", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Empty(t, cfg.Server.Relays.Addresses, "Legacy yaml.v3 dropped null sequence items, leaving no external relay addresses") + assert.True(t, cfg.Relay.Enabled, "Legacy embedded relay stayed enabled because no external relay address survived decoding") + assert.Equal(t, []string{"rels://netbird.example.com"}, cfg.Management.Relays.Addresses, + "Legacy clients were pointed at the embedded relay") + }, + }, + { + name: "null letsencrypt domain item", + contents: legacyCombinedServerFile + " tls:\n letsencrypt:\n enabled: true\n dataDir: /le\n domains:\n - null\n", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Empty(t, cfg.Server.TLS.LetsEncrypt.Domains, "Legacy yaml.v3 dropped null sequence items, leaving no Let's Encrypt domains") + assert.False(t, cfg.HasLetsEncrypt(), "Legacy Let's Encrypt was not considered configured without a surviving domain") + }, + }, + { + name: "null stun port item after a valid port", + contents: legacyCombinedServerFile + " stunPorts:\n - 3478\n - null\n", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, []int{3478}, cfg.Server.StunPorts, "Legacy yaml.v3 dropped the null port item and kept only 3478") + assert.NoError(t, cfg.Validate(), "Legacy config validated because no zero port was retained") + assert.Equal(t, []int{3478}, cfg.Relay.Stun.Ports, "Legacy local STUN listened only on 3478") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +func TestLoadConfigPreservesLegacyDottedKeysAreLiteralUnknownKeys(t *testing.T) { + clearCombinedConfigEnvironment(t) + tests := []struct { + name string + contents string + validate func(*testing.T, *CombinedConfig) + }{ + { + name: "dotted root key is ignored", + contents: legacyCombinedServerFile + "server.metricsPort: 1\n", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, 9090, cfg.Server.MetricsPort, + "Legacy yaml.v3 matched keys literally; 'server.metricsPort' was an unknown root key and left the default in place") + }, + }, + { + name: "dotted nested key is ignored", + contents: legacyCombinedServerFile + " tls.certFile: /a\n tls.keyFile: /b\n", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Empty(t, cfg.Server.TLS.CertFile, + "Legacy yaml.v3 matched keys literally; 'tls.certFile' was an unknown key under server and did not enable file-based TLS") + assert.Empty(t, cfg.Server.TLS.KeyFile, + "Legacy yaml.v3 matched keys literally; 'tls.keyFile' was an unknown key under server") + assert.False(t, cfg.HasTLSCert(), "Legacy file-based TLS stayed disabled") + }, + }, + { + name: "dotted store key is ignored", + contents: legacyCombinedServerFile + " store.engine: postgres\n", + validate: func(t *testing.T, cfg *CombinedConfig) { + assert.Equal(t, "sqlite", cfg.Server.Store.Engine, + "Legacy yaml.v3 matched keys literally; 'store.engine' was an unknown key under server and kept the sqlite default") + assert.Equal(t, "sqlite", cfg.Management.Store.Engine, "Legacy management store engine came from the default") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := writeCombinedConfig(t, "config.yaml", test.contents) + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + assert.NoError(t, cfg.Validate(), "Legacy config remained valid") + }) + } +} + +func TestLoadConfigPreservesLegacyRejectionOfRealTOMLContent(t *testing.T) { + clearCombinedConfigEnvironment(t) + configPath := writeCombinedConfig(t, "config.toml", `[server] +exposedAddress = "https://netbird.example.com" +authSecret = "s" +metricsPort = 9191 +`) + + _, err := LoadConfig(configPath) + if !assert.Error(t, err, "Legacy loader always decoded the file as YAML regardless of the .toml extension; a TOML table header parsed as a YAML sequence and was rejected, so the server did not start") { + return + } + assert.ErrorContains(t, err, "failed to parse config file:", "Legacy parse errors were prefixed with 'failed to parse config file:'") + assert.ErrorContains(t, err, "cannot unmarshal !!seq into cmd.CombinedConfig", + "Legacy yaml.v3 rejected the TOML table header '[server]' as a sequence at the document root") +} diff --git a/management/cmd/config_test.go b/management/cmd/config_test.go index 009ba702f..45f7ce842 100644 --- a/management/cmd/config_test.go +++ b/management/cmd/config_test.go @@ -1,16 +1,25 @@ package cmd import ( + "bytes" + "context" + "encoding/json" "os" "path/filepath" + "reflect" + "strings" "testing" "time" + "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + nbconfig "github.com/netbirdio/netbird/management/internals/server/config" ) func TestLoadManagementConfigUsesDefaultDataDirForEmptyValue(t *testing.T) { + clearManagementConfigEnvironment(t) configPath := filepath.Join(t.TempDir(), "management.json") require.NoError(t, os.WriteFile(configPath, []byte(`{"Datadir":""}`), 0o600)) @@ -19,7 +28,352 @@ func TestLoadManagementConfigUsesDefaultDataDirForEmptyValue(t *testing.T) { assert.Equal(t, defaultMgmtDataDir, cfg.Datadir, "Empty legacy values should use the default data directory") } +func TestLoadManagementConfigPreservesExtraConfigKeyCase(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "IdpManagerConfig": { + "ExtraConfig": { + "ServiceAccountKey": "service-account-key", + "CustomerId": "customer-id" + } + } +}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + require.NotNil(t, cfg.IdpManagerConfig, "IDP configuration should be decoded") + assert.Equal(t, map[string]string{ + "ServiceAccountKey": "service-account-key", + "CustomerId": "customer-id", + }, map[string]string(cfg.IdpManagerConfig.ExtraConfig), "IDP-specific keys should retain their case") +} + +func TestLoadManagementConfigPreservesEmptyHTTPConfig(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{"HttpConfig":{}}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + assert.NotNil(t, cfg.HttpConfig, "An explicitly configured empty HTTP section should remain present") +} + +func TestLoadManagementConfigPreservesJSONShapes(t *testing.T) { + clearManagementConfigEnvironment(t) + tests := []struct { + name string + contents string + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "mixed case struct field", + contents: `{"dAtAdIr":"/mixed-case"}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, "/mixed-case", cfg.Datadir, "JSON struct fields should remain case-insensitive") + }, + }, + { + name: "empty nested pointer", + contents: `{"IdpManagerConfig":{"ClientConfig":{}}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.IdpManagerConfig, "IDP configuration should remain present") + assert.NotNil(t, cfg.IdpManagerConfig.ClientConfig, + "An explicitly configured empty nested pointer should remain present") + }, + }, + { + name: "empty map", + contents: `{"IdpManagerConfig":{"ExtraConfig":{}}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.IdpManagerConfig, "IDP configuration should remain present") + assert.NotNil(t, cfg.IdpManagerConfig.ExtraConfig, + "An explicitly configured empty map should remain non-nil") + }, + }, + { + name: "empty slice", + contents: `{"Stuns":[]}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.NotNil(t, cfg.Stuns, "An explicitly configured empty slice should remain non-nil") + assert.Empty(t, cfg.Stuns, "An explicitly configured empty slice should remain empty") + }, + }, + { + name: "empty account map", + contents: `{"PerAccountHighestSupportedSyncMessageVersion":{}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.NotNil(t, cfg.PerAccountHighestSupportedSyncMessageVersion, + "An explicitly configured empty account map should remain non-nil") + }, + }, + { + name: "null pointer", + contents: `{"HttpConfig":null}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Nil(t, cfg.HttpConfig, "A null pointer should remain nil") + }, + }, + { + name: "unknown field", + contents: `{"Datadir":"/known-data","UnknownSetting":true}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, "/known-data", cfg.Datadir, "Unknown JSON fields should remain ignored") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +func TestLoadAdminMgmtConfigPreservesLegacyEmptyDataDir(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{}`), 0o600)) + + oldConfigPath := nbconfig.MgmtConfigPath + oldAdminDatadir := adminDatadir + nbconfig.MgmtConfigPath = configPath + adminDatadir = "" + t.Cleanup(func() { + nbconfig.MgmtConfigPath = oldConfigPath + adminDatadir = oldAdminDatadir + }) + + cfg, datadir, err := loadAdminMgmtConfig(context.Background(), false) + require.NoError(t, err) + assert.Empty(t, cfg.Datadir, "Admin commands should retain the legacy empty data directory") + assert.Empty(t, datadir, "Admin commands should retain the legacy empty effective data directory") +} + +func TestEnsureEncryptionKeyDoesNotPersistUnreferencedEnvironmentSecrets(t *testing.T) { + const environmentSecret = "environment-client-secret" + + clearManagementConfigEnvironment(t) + t.Setenv("NB_IDPMANAGERCONFIG_CLIENTCONFIG_CLIENTSECRET", environmentSecret) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "IdpManagerConfig": { + "ClientConfig": {} + } +}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + require.NoError(t, EnsureEncryptionKey(context.Background(), configPath, cfg)) + + persistedConfig, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.False(t, bytes.Contains(persistedConfig, []byte(environmentSecret)), + "An environment value absent from the file should not be persisted") +} + +func TestEnsureEncryptionKeyPreservesTemplateExpandedValues(t *testing.T) { + const templateSecret = "template-client-secret" + + clearManagementConfigEnvironment(t) + t.Setenv("MANAGEMENT_TEMPLATE_SECRET", templateSecret) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "IdpManagerConfig": { + "ClientConfig": { + "ClientSecret": "{{ .MANAGEMENT_TEMPLATE_SECRET }}" + } + } +}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + require.NoError(t, EnsureEncryptionKey(context.Background(), configPath, cfg)) + + persistedConfig, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.True(t, bytes.Contains(persistedConfig, []byte(templateSecret)), + "Template-expanded values were historically persisted during key generation") +} + +func TestLoadMgmtConfigPreservesLegacyDataDirPrecedence(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "Datadir": "/stale-config-directory", + "DataStoreEncryptionKey": "configured-key", + "HttpConfig": { + "AuthAudience": "test-audience" + } +}`), 0o600)) + + cfg, err := LoadMgmtConfig(context.Background(), configPath, unchangedManagementFlags()) + require.NoError(t, err) + assert.Equal(t, defaultMgmtDataDir, cfg.Datadir, + "The historical command default should continue to override a stale config value") +} + +func TestApplyCommandLineOverridesPreservesLegacyEmptyValues(t *testing.T) { + oldDatadir := mgmtDataDir + oldLetsencryptDomain := mgmtLetsencryptDomain + oldCertFile := certFile + oldCertKey := certKey + t.Cleanup(func() { + mgmtDataDir = oldDatadir + mgmtLetsencryptDomain = oldLetsencryptDomain + certFile = oldCertFile + certKey = oldCertKey + }) + + tests := []struct { + name string + configure func(*testing.T, *pflag.FlagSet) + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "empty datadir", + configure: func(t *testing.T, flags *pflag.FlagSet) { + mgmtDataDir = "" + require.NoError(t, flags.Set("datadir", "")) + }, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, "/file-data", cfg.Datadir, "An empty datadir flag should not clear the file value") + }, + }, + { + name: "empty letsencrypt domain", + configure: func(t *testing.T, flags *pflag.FlagSet) { + mgmtLetsencryptDomain = "" + require.NoError(t, flags.Set("letsencrypt-domain", "")) + }, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, "file.example.com", cfg.HttpConfig.LetsEncryptDomain, + "An empty letsencrypt flag should not clear the file value") + }, + }, + { + name: "empty certificate pair", + configure: func(t *testing.T, flags *pflag.FlagSet) { + certFile = "" + certKey = "" + require.NoError(t, flags.Set("cert-file", "")) + require.NoError(t, flags.Set("cert-key", "")) + }, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, "/file/tls.crt", cfg.HttpConfig.CertFile, + "An empty certificate pair should not clear the file certificate") + assert.Equal(t, "/file/tls.key", cfg.HttpConfig.CertKey, + "An empty certificate pair should not clear the file key") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mgmtDataDir = defaultMgmtDataDir + mgmtLetsencryptDomain = "" + certFile = "" + certKey = "" + flags := unchangedManagementFlags() + test.configure(t, flags) + cfg := &nbconfig.Config{ + Datadir: "/file-data", + HttpConfig: &nbconfig.HttpServerConfig{ + LetsEncryptDomain: "file.example.com", + CertFile: "/file/tls.crt", + CertKey: "/file/tls.key", + }, + } + + ApplyCommandLineOverrides(cfg, flags) + test.validate(t, cfg) + }) + } +} + +func TestManagementPortDefaultsRemainCompatible(t *testing.T) { + tests := []struct { + name string + httpConfig string + flags map[string]string + expected int + }{ + { + name: "no TLS", + httpConfig: `{"AuthAudience":"test-audience"}`, + expected: 80, + }, + { + name: "file-only letsencrypt", + httpConfig: `{"AuthAudience":"test-audience","LetsEncryptDomain":"management.example.com"}`, + expected: 80, + }, + { + name: "file certificate pair", + httpConfig: `{"AuthAudience":"test-audience","CertFile":"/tls.crt","CertKey":"/tls.key"}`, + expected: 443, + }, + { + name: "letsencrypt flag", + httpConfig: `{"AuthAudience":"test-audience"}`, + flags: map[string]string{"letsencrypt-domain": "management.example.com"}, + expected: 443, + }, + { + name: "explicit zero port", + httpConfig: `{"AuthAudience":"test-audience"}`, + flags: map[string]string{"port": "0"}, + expected: 0, + }, + { + name: "explicit nonzero port", + httpConfig: `{"AuthAudience":"test-audience"}`, + flags: map[string]string{"port": "10002"}, + expected: 10002, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := runManagementPreRun(t, test.httpConfig, test.flags) + assert.Equal(t, test.expected, actual, "Management implicit port selection should retain legacy behavior") + }) + } +} + +func TestLegacyManagementFlagsRemainRegistered(t *testing.T) { + for _, name := range []string{ + "port", + "disable-legacy-port", + "metrics-port", + "datadir", + "config", + "letsencrypt-domain", + "single-account-mode-domain", + "disable-single-account-mode", + "cert-file", + "cert-key", + "disable-anonymous-metrics", + "dns-domain", + idpSignKeyRefreshEnabledFlagName, + "user-delete-from-idp", + "disable-geolite-update", + } { + assert.NotNil(t, mgmtCmd.Flags().Lookup(name), "Legacy Management flag %s should remain registered", name) + } + for _, name := range []string{"log-level", "log-file"} { + assert.NotNil(t, rootCmd.PersistentFlags().Lookup(name), + "Legacy persistent Management flag %s should remain registered", name) + } +} + func TestLoadManagementConfigSources(t *testing.T) { + clearManagementConfigEnvironment(t) t.Setenv("MANAGEMENT_DATA_DIR", "/template-data") t.Setenv("MANAGEMENT_ENCRYPTION_KEY", "template-key") t.Setenv("NB_DATADIR", "/environment-data") @@ -57,3 +411,1443 @@ func TestLoadManagementConfigSources(t *testing.T) { ApplyCommandLineOverrides(cfg, mgmtCmd.Flags()) assert.Equal(t, "/flag-data", cfg.Datadir, "Flags should override environment values") } + +// management-01: environment variables were never read directly by the legacy loader. +func TestLoadManagementConfigPreservesLegacyFileValuesOverEnvironment(t *testing.T) { + const templateAudience = "template-audience" + + clearManagementConfigEnvironment(t) + t.Setenv("MANAGEMENT_TEMPLATE_AUDIENCE", templateAudience) + + tests := []struct { + name string + environment map[string]string + contents string + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "http audience", + environment: map[string]string{"NB_HTTPCONFIG_AUTHAUDIENCE": "env-aud"}, + contents: `{"HttpConfig":{"AuthAudience":"file-aud"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.HttpConfig, "HTTP configuration should be decoded") + assert.Equal(t, "file-aud", cfg.HttpConfig.AuthAudience, + "The legacy loader never read NB_ environment variables; the file value must win") + }, + }, + { + name: "relay secret", + environment: map[string]string{"NB_RELAY_SECRET": "env-secret"}, + contents: `{"Relay":{"Addresses":["rel://a"],"Secret":"file-secret"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.Relay, "Relay configuration should be decoded") + assert.Equal(t, "file-secret", cfg.Relay.Secret, + "The legacy loader never read NB_ environment variables; the file value must win") + }, + }, + { + name: "turn secret", + environment: map[string]string{"NB_TURNCONFIG_SECRET": "env-secret"}, + contents: `{"TURNConfig":{"Secret":"file-secret"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.TURNConfig, "TURN configuration should be decoded") + assert.Equal(t, "file-secret", cfg.TURNConfig.Secret, + "The legacy loader never read NB_ environment variables; the file value must win") + }, + }, + { + name: "idp client secret", + environment: map[string]string{"NB_IDPMANAGERCONFIG_CLIENTCONFIG_CLIENTSECRET": "env-secret"}, + contents: `{"IdpManagerConfig":{"ClientConfig":{"ClientSecret":"file-secret"}}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.IdpManagerConfig, "IDP configuration should be decoded") + require.NotNil(t, cfg.IdpManagerConfig.ClientConfig, "IDP client configuration should be decoded") + assert.Equal(t, "file-secret", cfg.IdpManagerConfig.ClientConfig.ClientSecret, + "The legacy loader never read NB_ environment variables; the file value must win") + }, + }, + { + name: "sign key refresh", + environment: map[string]string{"NB_HTTPCONFIG_IDPSIGNKEYREFRESHENABLED": "true"}, + contents: `{"HttpConfig":{"IdpSignKeyRefreshEnabled":false}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.HttpConfig, "HTTP configuration should be decoded") + assert.False(t, cfg.HttpConfig.IdpSignKeyRefreshEnabled, + "Only the --idp-sign-key-refresh-enabled flag could toggle the refresh setting outside the file") + }, + }, + { + name: "embedded idp storage dsn", + environment: map[string]string{"NB_EMBEDDEDIDP_STORAGE_CONFIG_DSN": "env-dsn"}, + contents: `{"EmbeddedIdP":{"Enabled":true,"Storage":{"Type":"postgres","Config":{"DSN":"file-dsn"}}}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.EmbeddedIdP, "Embedded IdP configuration should be decoded") + assert.Equal(t, "file-dsn", cfg.EmbeddedIdP.Storage.Config.DSN, + "The legacy loader never read NB_ environment variables; the file value must win") + }, + }, + { + name: "template expanded value", + environment: map[string]string{"NB_HTTPCONFIG_AUTHAUDIENCE": "env-aud"}, + contents: `{"HttpConfig":{"AuthAudience":"{{ .MANAGEMENT_TEMPLATE_AUDIENCE }}"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.HttpConfig, "HTTP configuration should be decoded") + assert.Equal(t, templateAudience, cfg.HttpConfig.AuthAudience, + "Template references were the only supported environment input and must win over NB_ variables") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for name, value := range test.environment { + t.Setenv(name, value) + } + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +// management-02 / management-03: the encryption key came only from the file and the file +// was only rewritten when it contained no key at all. +func TestLoadManagementConfigPreservesLegacyFileEncryptionKey(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentKey string + }{ + {name: "empty environment key", environmentKey: ""}, + {name: "different environment key", environmentKey: "environment-key"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv("NB_DATASTOREENCRYPTIONKEY", test.environmentKey) + configPath := filepath.Join(t.TempDir(), "management.json") + contents := []byte(`{"DataStoreEncryptionKey":"operator-key","HttpConfig":{"AuthAudience":"file-aud"}}`) + require.NoError(t, os.WriteFile(configPath, contents, 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + assert.Equal(t, "operator-key", cfg.DataStoreEncryptionKey, + "NB_DATASTOREENCRYPTIONKEY was never read; the file key must be used") + + require.NoError(t, EnsureEncryptionKey(context.Background(), configPath, cfg)) + assert.Equal(t, "operator-key", cfg.DataStoreEncryptionKey, + "EnsureEncryptionKey must not replace an operator-provided key") + persistedConfig, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.Equal(t, string(contents), string(persistedConfig), + "The config file must not be rewritten when it already contains an encryption key") + }) + } +} + +// management-04 / management-05: implicit TLS port selection only considered the +// --letsencrypt-domain flag and the certificate pair from the file. +func TestManagementPortDefaultsIgnoreEnvironmentTLS(t *testing.T) { + tests := []struct { + name string + environment map[string]string + expected int + }{ + { + name: "letsencrypt domain from environment", + environment: map[string]string{"NB_HTTPCONFIG_LETSENCRYPTDOMAIN": "management.example.com"}, + expected: 80, + }, + { + name: "certificate pair from environment", + environment: map[string]string{ + "NB_HTTPCONFIG_CERTFILE": "/tls.crt", + "NB_HTTPCONFIG_CERTKEY": "/tls.key", + }, + expected: 80, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := runManagementPreRunWithEnvironment(t, `{"AuthAudience":"test-audience"}`, nil, test.environment) + assert.Equal(t, test.expected, actual, + "Environment variables never enabled TLS, so the implicit port must remain the plain HTTP default") + }) + } +} + +// management-06: an empty-but-set environment variable was ignored and never cleared file values. +func TestLoadManagementConfigPreservesLegacyFileValuesOverEmptyEnvironment(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentName string + contents string + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "http audience", + environmentName: "NB_HTTPCONFIG_AUTHAUDIENCE", + contents: `{"HttpConfig":{"AuthAudience":"file-aud"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.HttpConfig, "HTTP configuration should be decoded") + assert.Equal(t, "file-aud", cfg.HttpConfig.AuthAudience, + "An empty NB_ variable was ignored and must not clear the file value") + }, + }, + { + name: "relay addresses", + environmentName: "NB_RELAY_ADDRESSES", + contents: `{"Relay":{"Addresses":["rel://a"]}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.Relay, "Relay configuration should be decoded") + assert.Equal(t, []string{"rel://a"}, cfg.Relay.Addresses, + "An empty NB_ variable was ignored and must not empty the relay list") + }, + }, + { + name: "stuns", + environmentName: "NB_STUNS", + contents: `{"Stuns":[{"Proto":"udp","URI":"stun:stun.example.com:3478"}]}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Len(t, cfg.Stuns, 1, "An empty NB_ variable was ignored and must not empty the STUN list") + }, + }, + { + name: "trusted proxies", + environmentName: "NB_REVERSEPROXY_TRUSTEDHTTPPROXIES", + contents: `{"ReverseProxy":{"TrustedHTTPProxies":["10.0.0.0/8"]}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Len(t, cfg.ReverseProxy.TrustedHTTPProxies, 1, + "An empty NB_ variable was ignored and must not empty the trusted proxy list") + }, + }, + { + name: "datadir", + environmentName: "NB_DATADIR", + contents: `{"Datadir":"/file-data"}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, "/file-data", cfg.Datadir, + "An empty NB_ variable was ignored and must not discard the file data directory") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.environmentName, "") + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +// management-07: NB_DATADIR had no effect on either the server or the admin commands. +func TestLoadMgmtConfigIgnoresEnvironmentDataDir(t *testing.T) { + clearManagementConfigEnvironment(t) + t.Setenv("NB_DATADIR", "/env-data") + + oldDatadir := mgmtDataDir + mgmtDataDir = defaultMgmtDataDir + t.Cleanup(func() { mgmtDataDir = oldDatadir }) + + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "DataStoreEncryptionKey": "configured-key", + "HttpConfig": { + "AuthAudience": "test-audience" + } +}`), 0o600)) + + cfg, err := LoadMgmtConfig(context.Background(), configPath, unchangedManagementFlags()) + require.NoError(t, err) + assert.Equal(t, defaultMgmtDataDir, cfg.Datadir, + "The server data directory came from the --datadir flag default; NB_DATADIR was never read") +} + +func TestLoadAdminMgmtConfigIgnoresEnvironmentDataDir(t *testing.T) { + clearManagementConfigEnvironment(t) + t.Setenv("NB_DATADIR", "/env-data") + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{"Datadir":"/file-data"}`), 0o600)) + setAdminConfigPath(t, configPath) + + cfg, datadir, err := loadAdminMgmtConfig(context.Background(), false) + require.NoError(t, err) + assert.Equal(t, "/file-data", cfg.Datadir, "Admin commands read the data directory from the file only") + assert.Equal(t, "/file-data", datadir, "Admin commands read the effective data directory from the file only") +} + +// management-08: unparsable environment values were ignored instead of aborting startup. +func TestLoadManagementConfigIgnoresInvalidScalarEnvironmentValues(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentName string + value string + }{ + {name: "invalid bool", environmentName: "NB_DISABLEDEFAULTPOLICY", value: "maybe"}, + {name: "padded bool", environmentName: "NB_DISABLEDEFAULTPOLICY", value: " true"}, + {name: "invalid int", environmentName: "NB_HIGHESTSUPPORTEDSYNCMESSAGEVERSION", value: "abc"}, + {name: "float int", environmentName: "NB_HIGHESTSUPPORTEDSYNCMESSAGEVERSION", value: "1.0"}, + {name: "negative uint", environmentName: "NB_REVERSEPROXY_TRUSTEDHTTPPROXIESCOUNT", value: "-1"}, + {name: "empty duration", environmentName: "NB_TURNCONFIG_CREDENTIALSTTL", value: ""}, + {name: "invalid duration", environmentName: "NB_TURNCONFIG_CREDENTIALSTTL", value: "abc"}, + {name: "numeric duration", environmentName: "NB_TURNCONFIG_CREDENTIALSTTL", value: "3600000000000"}, + {name: "invalid prefix", environmentName: "NB_REVERSEPROXY_TRUSTEDHTTPPROXIES", value: "bad"}, + {name: "address instead of prefix", environmentName: "NB_REVERSEPROXY_TRUSTEDHTTPPROXIES", value: "10.0.0.1"}, + {name: "overflowing login flag", environmentName: "NB_PKCEAUTHORIZATIONFLOW_PROVIDERCONFIG_LOGINFLAG", value: "300"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.environmentName, test.value) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "Datadir": "/file-data", + "TURNConfig": {"CredentialsTTL": "1h"}, + "PKCEAuthorizationFlow": {"ProviderConfig": {"LoginFlag": 1}} +}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err, "Environment variables were never read, so an unparsable value must not fail startup") + assert.Equal(t, "/file-data", cfg.Datadir, "File values should be used when the environment is ignored") + }) + } +} + +// management-09: a variable named after a configuration section had no effect. +func TestLoadManagementConfigIgnoresSectionEnvironmentVariables(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentName string + value string + contents string + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "http config", + environmentName: "NB_HTTPCONFIG", + value: "x", + contents: `{"HttpConfig":{"AuthAudience":"file-aud"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.HttpConfig, "A section-named variable was ignored and must not drop the HTTP section") + assert.Equal(t, "file-aud", cfg.HttpConfig.AuthAudience, "The HTTP section should be loaded from the file") + }, + }, + { + name: "relay", + environmentName: "NB_RELAY", + value: "x", + contents: `{"Relay":{"Addresses":["rel://a"]}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.Relay, "A section-named variable was ignored and must not drop the relay section") + assert.Equal(t, []string{"rel://a"}, cfg.Relay.Addresses, "The relay section should be loaded from the file") + }, + }, + { + name: "empty idp manager config", + environmentName: "NB_IDPMANAGERCONFIG", + value: "", + contents: `{"IdpManagerConfig":{"ManagerType":"none"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.IdpManagerConfig, "A section-named variable was ignored and must not drop the IDP section") + assert.Equal(t, "none", cfg.IdpManagerConfig.ManagerType, "The IDP section should be loaded from the file") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.environmentName, test.value) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +// management-10: the optional embedded IdP section existed only when present in the file. +func TestLoadManagementConfigDoesNotMaterializeEmbeddedIdPFromEnvironment(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentName string + value string + }{ + {name: "enabled", environmentName: "NB_EMBEDDEDIDP_ENABLED", value: "true"}, + {name: "empty issuer", environmentName: "NB_EMBEDDEDIDP_ISSUER", value: ""}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.environmentName, test.value) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{"HttpConfig":{"AuthAudience":"file-aud"}}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + assert.Nil(t, cfg.EmbeddedIdP, + "Without an EmbeddedIdP object in the file the section stayed nil; environment variables were never read") + }) + } +} + +// management-11: optional pointer sections stayed nil unless present in the file. +func TestLoadManagementConfigDoesNotMaterializePointersFromEnvironment(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentName string + value string + contents string + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "missing http config", + environmentName: "NB_HTTPCONFIG_AUTHAUDIENCE", + value: "env-aud", + contents: `{}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Nil(t, cfg.HttpConfig, "A missing HTTP section stayed nil; environment variables were never read") + }, + }, + { + name: "null http config", + environmentName: "NB_HTTPCONFIG_AUTHAUDIENCE", + value: "", + contents: `{"HttpConfig":null}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Nil(t, cfg.HttpConfig, "A null HTTP section stayed nil; environment variables were never read") + }, + }, + { + name: "signal", + environmentName: "NB_SIGNAL_URI", + value: "sig:x", + contents: `{}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Nil(t, cfg.Signal, "A missing signal section stayed nil; environment variables were never read") + }, + }, + { + name: "relay", + environmentName: "NB_RELAY_SECRET", + value: "s", + contents: `{}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Nil(t, cfg.Relay, "A missing relay section stayed nil; environment variables were never read") + }, + }, + { + name: "embedded idp owner", + environmentName: "NB_EMBEDDEDIDP_OWNER_EMAIL", + value: "owner@example.com", + contents: `{"EmbeddedIdP":{"Enabled":true,"Issuer":"https://management.example.com/oauth2"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.EmbeddedIdP, "Embedded IdP configuration should be decoded") + assert.Nil(t, cfg.EmbeddedIdP.Owner, "A missing owner stayed nil; environment variables were never read") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.environmentName, test.value) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +// management-12: variables named after struct slices or maps were ignored instead of aborting startup. +func TestLoadManagementConfigIgnoresCollectionEnvironmentValues(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentName string + value string + }{ + {name: "stuns", environmentName: "NB_STUNS", value: "stun:stun.example.com:3478"}, + {name: "stuns json", environmentName: "NB_STUNS", value: `[{"Proto":"udp","URI":"stun:x"}]`}, + {name: "turns", environmentName: "NB_TURNCONFIG_TURNS", value: "turn:x"}, + {name: "static connectors", environmentName: "NB_EMBEDDEDIDP_STATICCONNECTORS", value: "x"}, + {name: "empty per account versions", environmentName: "NB_PERACCOUNTHIGHESTSUPPORTEDSYNCMESSAGEVERSION", value: ""}, + {name: "per account versions", environmentName: "NB_PERACCOUNTHIGHESTSUPPORTEDSYNCMESSAGEVERSION", value: "acc1=1"}, + {name: "empty extra config", environmentName: "NB_IDPMANAGERCONFIG_EXTRACONFIG", value: ""}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.environmentName, test.value) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "Stuns": [{"Proto":"udp","URI":"stun:file.example.com:3478"}], + "TURNConfig": {"Turns": [{"Proto":"udp","URI":"turn:file.example.com:3478"}]}, + "EmbeddedIdP": {"Enabled": true, "Issuer": "https://management.example.com/oauth2"}, + "PerAccountHighestSupportedSyncMessageVersion": {"acc1": 1}, + "IdpManagerConfig": {"ExtraConfig": {"CustomerId": "customer-id"}} +}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err, "Environment variables were never read, so a collection-named variable must not fail startup") + assert.Len(t, cfg.Stuns, 1, "File STUN servers should be retained") + require.NotNil(t, cfg.TURNConfig, "TURN configuration should be decoded") + assert.Len(t, cfg.TURNConfig.Turns, 1, "File TURN servers should be retained") + assert.Equal(t, map[string]int{"acc1": 1}, cfg.PerAccountHighestSupportedSyncMessageVersion, + "File per-account versions should be retained") + require.NotNil(t, cfg.IdpManagerConfig, "IDP configuration should be decoded") + assert.Equal(t, map[string]string{"CustomerId": "customer-id"}, map[string]string(cfg.IdpManagerConfig.ExtraConfig), + "File extra IDP configuration should be retained") + }) + } +} + +// management-13: list-valued environment variables did not exist; lists came only from JSON arrays. +func TestLoadManagementConfigPreservesLegacyFileListsOverEnvironment(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentName string + value string + contents string + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "relay addresses with spaces", + environmentName: "NB_RELAY_ADDRESSES", + value: "rel://a, rel://b", + contents: `{"Relay":{"Addresses":["rel://file"]}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.Relay, "Relay configuration should be decoded") + assert.Equal(t, []string{"rel://file"}, cfg.Relay.Addresses, + "Relay addresses came only from the JSON array; environment lists were never parsed") + }, + }, + { + name: "relay addresses trailing comma", + environmentName: "NB_RELAY_ADDRESSES", + value: "rel://a,rel://b,", + contents: `{"Relay":{"Addresses":["rel://file"]}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.Relay, "Relay configuration should be decoded") + assert.Equal(t, []string{"rel://file"}, cfg.Relay.Addresses, + "Relay addresses came only from the JSON array; environment lists were never parsed") + }, + }, + { + name: "relay addresses json", + environmentName: "NB_RELAY_ADDRESSES", + value: `["rel://a","rel://b"]`, + contents: `{"Relay":{"Addresses":["rel://file"]}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.Relay, "Relay configuration should be decoded") + assert.Equal(t, []string{"rel://file"}, cfg.Relay.Addresses, + "Relay addresses came only from the JSON array; environment lists were never parsed") + }, + }, + { + name: "grant types", + environmentName: "NB_EMBEDDEDIDP_GRANTTYPES", + value: "authorization_code,refresh_token", + contents: `{"EmbeddedIdP":{"Enabled":true,"GrantTypes":["device_code"]}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.EmbeddedIdP, "Embedded IdP configuration should be decoded") + assert.Equal(t, []string{"device_code"}, cfg.EmbeddedIdP.GrantTypes, + "Grant types came only from the JSON array; environment lists were never parsed") + }, + }, + { + name: "trusted proxies with spaces", + environmentName: "NB_REVERSEPROXY_TRUSTEDHTTPPROXIES", + value: "10.0.0.0/8, 192.168.0.0/16", + contents: `{"ReverseProxy":{"TrustedHTTPProxies":["172.16.0.0/12"]}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.Len(t, cfg.ReverseProxy.TrustedHTTPProxies, 1, + "Trusted proxies came only from the JSON array; environment lists were never parsed") + assert.Equal(t, "172.16.0.0/12", cfg.ReverseProxy.TrustedHTTPProxies[0].String(), + "Trusted proxies came only from the JSON array; environment lists were never parsed") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.environmentName, test.value) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err, "Environment lists were never parsed, so they must not fail startup") + test.validate(t, cfg) + }) + } +} + +// management-14: boolean spellings in the environment were never interpreted. +func TestLoadManagementConfigIgnoresBooleanEnvironmentSpellings(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + value string + contents string + expected bool + }{ + {name: "one", value: "1", contents: `{"DisableDefaultPolicy":false}`, expected: false}, + {name: "true", value: "TRUE", contents: `{"DisableDefaultPolicy":false}`, expected: false}, + {name: "yes", value: "yes", contents: `{"DisableDefaultPolicy":false}`, expected: false}, + {name: "on", value: "on", contents: `{"DisableDefaultPolicy":false}`, expected: false}, + {name: "zero", value: "0", contents: `{"DisableDefaultPolicy":true}`, expected: true}, + {name: "no", value: "no", contents: `{"DisableDefaultPolicy":true}`, expected: true}, + {name: "off", value: "off", contents: `{"DisableDefaultPolicy":true}`, expected: true}, + {name: "empty", value: "", contents: `{"DisableDefaultPolicy":true}`, expected: true}, + {name: "invalid", value: "maybe", contents: `{"DisableDefaultPolicy":true}`, expected: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv("NB_DISABLEDEFAULTPOLICY", test.value) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err, "Boolean environment spellings were never interpreted, so they must not fail startup") + assert.Equal(t, test.expected, cfg.DisableDefaultPolicy, + "NB_DISABLEDEFAULTPOLICY was never read; the file value must be used") + }) + } +} + +// management-15: an empty environment value never zeroed numeric fields. +func TestLoadManagementConfigPreservesLegacyNumericFileValuesOverEmptyEnvironment(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environmentName string + contents string + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "missing sync version", + environmentName: "NB_HIGHESTSUPPORTEDSYNCMESSAGEVERSION", + contents: `{}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Nil(t, cfg.HighestSupportedSyncMessageVersion, + "An empty NB_ variable was ignored, so a missing sync version must stay nil") + }, + }, + { + name: "configured sync version", + environmentName: "NB_HIGHESTSUPPORTEDSYNCMESSAGEVERSION", + contents: `{"HighestSupportedSyncMessageVersion":1}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.HighestSupportedSyncMessageVersion, "Sync version should be decoded") + assert.Equal(t, 1, *cfg.HighestSupportedSyncMessageVersion, + "An empty NB_ variable was ignored, so the file sync version must be retained") + }, + }, + { + name: "trusted proxies count", + environmentName: "NB_REVERSEPROXY_TRUSTEDHTTPPROXIESCOUNT", + contents: `{"ReverseProxy":{"TrustedHTTPProxiesCount":2}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, uint(2), cfg.ReverseProxy.TrustedHTTPProxiesCount, + "An empty NB_ variable was ignored, so the file proxy count must be retained") + }, + }, + { + name: "access log retention", + environmentName: "NB_REVERSEPROXY_ACCESSLOGRETENTIONDAYS", + contents: `{"ReverseProxy":{"AccessLogRetentionDays":30}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, 30, cfg.ReverseProxy.AccessLogRetentionDays, + "An empty NB_ variable was ignored, so the file retention must be retained") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(test.environmentName, "") + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +// management-16: admin and legacy token commands operated strictly on the file's view of the deployment. +func TestLoadAdminMgmtConfigIgnoresEnvironment(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + environment map[string]string + contents string + applyIDPDefaults bool + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "store engine", + environment: map[string]string{"NB_STORECONFIG_ENGINE": "postgres"}, + contents: `{"StoreConfig":{"Engine":"sqlite"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, "sqlite", string(cfg.StoreConfig.Engine), + "Admin commands read the store engine from the file only") + }, + }, + { + name: "encryption key", + environment: map[string]string{"NB_DATASTOREENCRYPTIONKEY": "env-key"}, + contents: `{"DataStoreEncryptionKey":"file-key"}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Equal(t, "file-key", cfg.DataStoreEncryptionKey, + "Admin commands read the encryption key from the file only") + }, + }, + { + name: "embedded idp enabled", + environment: map[string]string{"NB_EMBEDDEDIDP_ENABLED": "true"}, + contents: `{"HttpConfig":{"AuthAudience":"file-aud"}}`, + applyIDPDefaults: true, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.Nil(t, cfg.EmbeddedIdP, "Admin commands never enabled the embedded IdP from the environment") + require.NotNil(t, cfg.HttpConfig, "HTTP configuration should be decoded") + assert.Equal(t, "file-aud", cfg.HttpConfig.AuthAudience, + "HTTP configuration must not be rewritten by an environment-enabled embedded IdP") + }, + }, + { + name: "invalid value", + environment: map[string]string{"NB_DISABLEDEFAULTPOLICY": "maybe"}, + contents: `{"DisableDefaultPolicy":true}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + assert.True(t, cfg.DisableDefaultPolicy, "Admin commands read the policy flag from the file only") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for name, value := range test.environment { + t.Setenv(name, value) + } + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + setAdminConfigPath(t, configPath) + + cfg, _, err := loadAdminMgmtConfig(context.Background(), test.applyIDPDefaults) + require.NoError(t, err, "Admin commands never read the environment, so it must not fail loading") + test.validate(t, cfg) + }) + } +} + +// management-17: the store engine in the config came from the file only; NETBIRD_STORE_ENGINE +// was consulted later by the store package. +func TestLoadManagementConfigIgnoresEnvironmentStoreEngine(t *testing.T) { + clearManagementConfigEnvironment(t) + t.Setenv("NETBIRD_STORE_ENGINE", "sqlite") + + for _, value := range []string{"postgres", "Postgres"} { + t.Run(value, func(t *testing.T) { + t.Setenv("NB_STORECONFIG_ENGINE", value) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{"StoreConfig":{"Engine":""}}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + assert.Empty(t, string(cfg.StoreConfig.Engine), + "NB_STORECONFIG_ENGINE was never read; an empty file engine left NETBIRD_STORE_ENGINE in charge") + }) + } +} + +// management-20: the certificate override only fired when both flag values were non-empty. +func TestApplyCommandLineOverridesPreservesLegacyMixedCertificatePair(t *testing.T) { + oldCertFile := certFile + oldCertKey := certKey + t.Cleanup(func() { + certFile = oldCertFile + certKey = oldCertKey + }) + + tests := []struct { + name string + certFile string + certKey string + }{ + {name: "empty key", certFile: "/new.crt", certKey: ""}, + {name: "empty file", certFile: "", certKey: "/new.key"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + certFile = test.certFile + certKey = test.certKey + flags := unchangedManagementFlags() + require.NoError(t, flags.Set("cert-file", test.certFile)) + require.NoError(t, flags.Set("cert-key", test.certKey)) + cfg := &nbconfig.Config{ + HttpConfig: &nbconfig.HttpServerConfig{ + CertFile: "/file/tls.crt", + CertKey: "/file/tls.key", + }, + } + + ApplyCommandLineOverrides(cfg, flags) + assert.Equal(t, "/file/tls.crt", cfg.HttpConfig.CertFile, + "A partially empty certificate pair never overrode the file certificate") + assert.Equal(t, "/file/tls.key", cfg.HttpConfig.CertKey, + "A partially empty certificate pair never overrode the file key") + }) + } +} + +// management-21: TLS flags with a missing HttpConfig section crashed instead of continuing. +func TestApplyCommandLineOverridesPreservesLegacyMissingHTTPConfigPanic(t *testing.T) { + oldLetsencryptDomain := mgmtLetsencryptDomain + oldCertFile := certFile + oldCertKey := certKey + t.Cleanup(func() { + mgmtLetsencryptDomain = oldLetsencryptDomain + certFile = oldCertFile + certKey = oldCertKey + }) + + tests := []struct { + name string + configure func(*testing.T, *pflag.FlagSet) + }{ + { + name: "letsencrypt domain", + configure: func(t *testing.T, flags *pflag.FlagSet) { + mgmtLetsencryptDomain = "example.com" + require.NoError(t, flags.Set("letsencrypt-domain", "example.com")) + }, + }, + { + name: "certificate pair", + configure: func(t *testing.T, flags *pflag.FlagSet) { + certFile = "/tls.crt" + certKey = "/tls.key" + require.NoError(t, flags.Set("cert-file", "/tls.crt")) + require.NoError(t, flags.Set("cert-key", "/tls.key")) + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mgmtLetsencryptDomain = "" + certFile = "" + certKey = "" + flags := unchangedManagementFlags() + test.configure(t, flags) + cfg := &nbconfig.Config{Datadir: "/d", DataStoreEncryptionKey: "k"} + + assert.Panics(t, func() { ApplyCommandLineOverrides(cfg, flags) }, + "TLS flags with no HttpConfig section historically dereferenced a nil pointer and crashed startup") + }) + } +} + +// management-23: the config file was always parsed as JSON regardless of its extension. +func TestLoadManagementConfigParsesJSONRegardlessOfExtension(t *testing.T) { + clearManagementConfigEnvironment(t) + + for _, extension := range []string{".toml", ".ini", ".env", ".dotenv", ".properties", ".props", ".prop", ".hcl", ".tfvars"} { + t.Run(extension, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management"+extension) + require.NoError(t, os.WriteFile(configPath, []byte(`{"Datadir":"/file-data"}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err, "JSON content was always accepted regardless of the file extension") + assert.Equal(t, "/file-data", cfg.Datadir, "JSON content should be decoded regardless of the file extension") + }) + } +} + +// management-24: account IDs used as map keys kept their case. +func TestLoadManagementConfigPreservesPerAccountKeyCase(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{"PerAccountHighestSupportedSyncMessageVersion":{"AbC123":1}}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + assert.Equal(t, map[string]int{"AbC123": 1}, cfg.PerAccountHighestSupportedSyncMessageVersion, + "Per-account sync version keys are account IDs and must retain their case") +} + +// management-25: connector configuration keys inside slices kept their case. +func TestLoadManagementConfigPreservesStaticConnectorConfigKeyCase(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "EmbeddedIdP": { + "Enabled": true, + "StaticConnectors": [ + {"type": "oidc", "id": "x", "config": {"clientID": "a", "redirectURI": "b", "insecureEnableGroups": false}} + ] + } +}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + require.NotNil(t, cfg.EmbeddedIdP, "Embedded IdP configuration should be decoded") + require.Len(t, cfg.EmbeddedIdP.StaticConnectors, 1, "Static connectors should be decoded") + assert.Equal(t, map[string]any{ + "clientID": "a", + "redirectURI": "b", + "insecureEnableGroups": false, + }, cfg.EmbeddedIdP.StaticConnectors[0].Config, "Dex connector option keys are case-sensitive and must retain their case") +} + +// management-26: mistyped scalars in the file were rejected instead of coerced. +func TestLoadManagementConfigRejectsWeaklyTypedFileValues(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + contents string + }{ + {name: "string sync version", contents: `{"HighestSupportedSyncMessageVersion":"1"}`}, + {name: "string per account version", contents: `{"PerAccountHighestSupportedSyncMessageVersion":{"a":"1"}}`}, + {name: "numeric datadir", contents: `{"Datadir":123}`}, + {name: "numeric engine", contents: `{"StoreConfig":{"Engine":1}}`}, + {name: "numeric proto", contents: `{"Signal":{"Proto":1}}`}, + {name: "string bool", contents: `{"DisableDefaultPolicy":"true"}`}, + {name: "upper string bool", contents: `{"DisableDefaultPolicy":"TRUE"}`}, + {name: "yes bool", contents: `{"DisableDefaultPolicy":"yes"}`}, + {name: "on bool", contents: `{"DisableDefaultPolicy":"on"}`}, + {name: "numeric bool", contents: `{"DisableDefaultPolicy":1}`}, + {name: "string login flag", contents: `{"PKCEAuthorizationFlow":{"ProviderConfig":{"LoginFlag":"1"}}}`}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + _, err := loadManagementConfig(configPath) + assert.Error(t, err, "encoding/json rejected mistyped scalars, so loading must fail instead of coercing the value") + }) + } +} + +// management-27: numeric edge cases were rejected or decoded exactly rather than silently mangled. +func TestLoadManagementConfigPreservesLegacyNumericDecoding(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + contents string + validate func(*testing.T, *nbconfig.Config, error) + }{ + { + name: "fractional sync version", + contents: `{"HighestSupportedSyncMessageVersion":1.7}`, + validate: func(t *testing.T, _ *nbconfig.Config, err error) { + assert.Error(t, err, "encoding/json rejected a fractional number for an int field") + }, + }, + { + name: "whole float sync version", + contents: `{"HighestSupportedSyncMessageVersion":1.0}`, + validate: func(t *testing.T, _ *nbconfig.Config, err error) { + assert.Error(t, err, "encoding/json rejected a float literal for an int field") + }, + }, + { + name: "fractional retention days", + contents: `{"ReverseProxy":{"AccessLogRetentionDays":7.9}}`, + validate: func(t *testing.T, _ *nbconfig.Config, err error) { + assert.Error(t, err, "encoding/json rejected a fractional number for an int field") + }, + }, + { + name: "negative proxy count", + contents: `{"ReverseProxy":{"TrustedHTTPProxiesCount":-1}}`, + validate: func(t *testing.T, _ *nbconfig.Config, err error) { + assert.Error(t, err, "encoding/json rejected a negative number for a uint field") + }, + }, + { + name: "overflowing login flag", + contents: `{"PKCEAuthorizationFlow":{"ProviderConfig":{"LoginFlag":300}}}`, + validate: func(t *testing.T, _ *nbconfig.Config, err error) { + assert.Error(t, err, "encoding/json rejected a number overflowing a uint8 field") + }, + }, + { + name: "large sync version", + contents: `{"HighestSupportedSyncMessageVersion":9007199254740993}`, + validate: func(t *testing.T, cfg *nbconfig.Config, err error) { + require.NoError(t, err) + require.NotNil(t, cfg.HighestSupportedSyncMessageVersion, "Sync version should be decoded") + assert.Equal(t, 9007199254740993, *cfg.HighestSupportedSyncMessageVersion, + "encoding/json decoded large integers exactly without float64 precision loss") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + test.validate(t, cfg, err) + }) + } +} + +// management-28: the rewritten config reproduced the decoded struct shape. +func TestEnsureEncryptionKeyPersistsLegacyConfigShape(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{ + "HttpConfig": {}, + "Relay": {}, + "IdpManagerConfig": { + "ExtraConfig": { + "ServiceAccountKey": "service-account-key" + } + }, + "PerAccountHighestSupportedSyncMessageVersion": {"AbC123": 1} +}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + require.NoError(t, EnsureEncryptionKey(context.Background(), configPath, cfg)) + + persistedConfig, err := os.ReadFile(configPath) + require.NoError(t, err) + var persisted map[string]any + require.NoError(t, json.Unmarshal(persistedConfig, &persisted)) + + _, httpConfigIsObject := persisted["HttpConfig"].(map[string]any) + assert.True(t, httpConfigIsObject, "An explicitly configured empty HttpConfig was persisted as an object, not null") + _, relayIsObject := persisted["Relay"].(map[string]any) + assert.True(t, relayIsObject, "An explicitly configured empty Relay section was persisted as an object, not null") + + idpConfig, _ := persisted["IdpManagerConfig"].(map[string]any) + extraConfig, _ := idpConfig["ExtraConfig"].(map[string]any) + assert.Contains(t, extraConfig, "ServiceAccountKey", "Persisted IDP-specific keys retained their case") + + perAccount, _ := persisted["PerAccountHighestSupportedSyncMessageVersion"].(map[string]any) + assert.Contains(t, perAccount, "AbC123", "Persisted per-account keys retained their case") +} + +// management-29: an empty config file failed to load instead of silently producing defaults. +func TestLoadManagementConfigRejectsEmptyFile(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, nil, 0o600)) + + _, err := loadManagementConfig(configPath) + assert.Error(t, err, "json.Unmarshal rejected a zero-byte file with 'unexpected end of JSON input'") +} + +// management-30: duplicate object keys differing only in case were merged into the same struct. +func TestLoadManagementConfigMergesCaseVariantDuplicateObjects(t *testing.T) { + clearManagementConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{"Signal":{"URI":"signal.example.com:10000"},"signal":{"Proto":"udp"}}`), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + require.NotNil(t, cfg.Signal, "Signal configuration should be decoded") + assert.Equal(t, "signal.example.com:10000", cfg.Signal.URI, + "encoding/json decoded case-variant duplicate objects into the same struct, keeping both fields") + assert.Equal(t, nbconfig.UDP, cfg.Signal.Proto, + "encoding/json decoded case-variant duplicate objects into the same struct, keeping both fields") +} + +// management-31: a scalar string for a slice field in the file was rejected. +func TestLoadManagementConfigRejectsScalarStringsForSlices(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + contents string + }{ + {name: "relay addresses", contents: `{"Relay":{"Addresses":"rel://a,rel://b"}}`}, + {name: "grant types", contents: `{"EmbeddedIdP":{"GrantTypes":"authorization_code"}}`}, + {name: "trusted proxies", contents: `{"ReverseProxy":{"TrustedHTTPProxies":"10.0.0.0/8,192.168.0.0/16"}}`}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + _, err := loadManagementConfig(configPath) + assert.Error(t, err, "encoding/json rejected a string for a slice field instead of splitting it on commas") + }) + } +} + +// management-32: a null duration in the file was rejected. +func TestLoadManagementConfigRejectsNullDurations(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + contents string + }{ + {name: "turn credentials ttl", contents: `{"TURNConfig":{"CredentialsTTL":null}}`}, + {name: "relay credentials ttl", contents: `{"Relay":{"CredentialsTTL":null}}`}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + _, err := loadManagementConfig(configPath) + assert.Error(t, err, "util.Duration rejected JSON null with 'invalid duration' instead of defaulting to 0s") + }) + } +} + +// management-critic-1: duplicate same-case object keys were decoded into the same struct, merging both blocks. +func TestLoadManagementConfigMergesSameCaseDuplicateObjects(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + contents string + validate func(*testing.T, *nbconfig.Config) + }{ + { + name: "signal", + contents: `{"Signal":{"URI":"signal.example.com:10000"},"Signal":{"Proto":"udp"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.Signal, "Signal configuration should be decoded") + assert.Equal(t, "signal.example.com:10000", cfg.Signal.URI, + "encoding/json decoded duplicate Signal objects into the same struct, keeping the first block's fields") + assert.Equal(t, nbconfig.UDP, cfg.Signal.Proto, + "encoding/json decoded duplicate Signal objects into the same struct, keeping the second block's fields") + }, + }, + { + name: "http config", + contents: `{"HttpConfig":{"AuthAudience":"a"},"HttpConfig":{"AuthIssuer":"i"}}`, + validate: func(t *testing.T, cfg *nbconfig.Config) { + require.NotNil(t, cfg.HttpConfig, "HTTP configuration should be decoded") + assert.Equal(t, "a", cfg.HttpConfig.AuthAudience, + "encoding/json decoded duplicate HttpConfig objects into the same struct, keeping the first block's fields") + assert.Equal(t, "i", cfg.HttpConfig.AuthIssuer, + "encoding/json decoded duplicate HttpConfig objects into the same struct, keeping the second block's fields") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +// management-critic-2: case-variant duplicate scalar keys were applied in document order, so the last one won. +func TestLoadManagementConfigPreservesLegacyDocumentOrderForCaseVariantScalars(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + contents string + expected string + }{ + { + name: "uppercase first", + contents: `{"Datadir":"/first-data","datadir":"/second-data"}`, + expected: "/second-data", + }, + { + name: "lowercase first", + contents: `{"datadir":"/first-data","Datadir":"/second-data"}`, + expected: "/second-data", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + assert.Equal(t, test.expected, cfg.Datadir, + "encoding/json applied case-variant duplicate scalar keys in document order, so the last key in the file won") + }) + } +} + +// management-critic-3: NB_DATASTOREENCRYPTIONKEY was never read, so a file without a key always got a generated one persisted. +func TestEnsureEncryptionKeyIgnoresEnvironmentKeyWhenFileHasNone(t *testing.T) { + const environmentKey = "environment-key" + + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + contents string + }{ + {name: "missing key", contents: `{"HttpConfig":{"AuthAudience":"file-aud"}}`}, + {name: "empty key", contents: `{"DataStoreEncryptionKey":"","HttpConfig":{"AuthAudience":"file-aud"}}`}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv("NB_DATASTOREENCRYPTIONKEY", environmentKey) + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + cfg, err := loadManagementConfig(configPath) + require.NoError(t, err) + assert.Empty(t, cfg.DataStoreEncryptionKey, + "NB_DATASTOREENCRYPTIONKEY was never read; a file without a key loaded with an empty key") + + require.NoError(t, EnsureEncryptionKey(context.Background(), configPath, cfg)) + assert.NotEmpty(t, cfg.DataStoreEncryptionKey, + "EnsureEncryptionKey generated a fresh key when the file had none") + assert.NotEqual(t, environmentKey, cfg.DataStoreEncryptionKey, + "EnsureEncryptionKey generated a random key instead of adopting the environment value") + + persistedConfig, err := os.ReadFile(configPath) + require.NoError(t, err) + var persisted map[string]any + require.NoError(t, json.Unmarshal(persistedConfig, &persisted)) + persistedKey, _ := persisted["DataStoreEncryptionKey"].(string) + assert.NotEmpty(t, persistedKey, + "The generated key was written back to management.json so later starts reuse it") + assert.NotEqual(t, environmentKey, persistedKey, + "The persisted key was the generated one, never the environment value") + }) + } +} + +// management-critic-4: a single JSON object where an array of hosts was expected was rejected. +func TestLoadManagementConfigRejectsObjectsForHostSlices(t *testing.T) { + clearManagementConfigEnvironment(t) + + tests := []struct { + name string + contents string + }{ + {name: "stuns", contents: `{"Stuns":{"Proto":"udp","URI":"stun:x"}}`}, + {name: "turns", contents: `{"TURNConfig":{"Turns":{"Proto":"udp","URI":"turn:x"}}}`}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "management.json") + require.NoError(t, os.WriteFile(configPath, []byte(test.contents), 0o600)) + + _, err := loadManagementConfig(configPath) + assert.Error(t, err, + "encoding/json rejected an object for a []*Host field ('cannot unmarshal object into Go struct field') instead of wrapping it into a one-element slice") + }) + } +} + +func setAdminConfigPath(t *testing.T, configPath string) { + t.Helper() + + oldConfigPath := nbconfig.MgmtConfigPath + oldAdminDatadir := adminDatadir + nbconfig.MgmtConfigPath = configPath + adminDatadir = "" + t.Cleanup(func() { + nbconfig.MgmtConfigPath = oldConfigPath + adminDatadir = oldAdminDatadir + }) +} + +func clearManagementConfigEnvironment(t *testing.T) { + t.Helper() + clearManagementEnvironmentType(t, reflect.TypeOf(nbconfig.Config{}), "", make(map[reflect.Type]bool)) +} + +func clearManagementEnvironmentType(t *testing.T, configType reflect.Type, prefix string, visiting map[reflect.Type]bool) { + t.Helper() + + for configType.Kind() == reflect.Pointer { + configType = configType.Elem() + } + if configType.Kind() != reflect.Struct || visiting[configType] { + return + } + visiting[configType] = true + defer delete(visiting, configType) + + for i := range configType.NumField() { + field := configType.Field(i) + if !field.IsExported() { + continue + } + key := strings.Split(field.Tag.Get("json"), ",")[0] + if key == "-" { + continue + } + if key == "" { + key = field.Name + } + if prefix != "" { + key = prefix + "." + key + } + environmentName := "NB_" + strings.ToUpper(strings.NewReplacer(".", "_", "-", "_").Replace(key)) + t.Setenv(environmentName, "") + require.NoError(t, os.Unsetenv(environmentName)) + clearManagementEnvironmentType(t, field.Type, key, visiting) + } +} + +func runManagementPreRun(t *testing.T, httpConfig string, changedFlags map[string]string) int { + t.Helper() + return runManagementPreRunWithEnvironment(t, httpConfig, changedFlags, nil) +} + +func runManagementPreRunWithEnvironment(t *testing.T, httpConfig string, changedFlags map[string]string, environment map[string]string) int { + t.Helper() + + for _, name := range []string{ + "NB_DATADIR", + "NB_DATASTOREENCRYPTIONKEY", + "NB_HTTPCONFIG_LETSENCRYPTDOMAIN", + "NB_HTTPCONFIG_CERTFILE", + "NB_HTTPCONFIG_CERTKEY", + } { + t.Setenv(name, "") + require.NoError(t, os.Unsetenv(name)) + } + for name, value := range environment { + t.Setenv(name, value) + } + + configPath := filepath.Join(t.TempDir(), "management.json") + contents := `{"DataStoreEncryptionKey":"configured-key","HttpConfig":` + httpConfig + `}` + require.NoError(t, os.WriteFile(configPath, []byte(contents), 0o600)) + + flags := map[string]*pflag.Flag{ + "port": mgmtCmd.Flags().Lookup("port"), + "datadir": mgmtCmd.Flags().Lookup("datadir"), + "letsencrypt-domain": mgmtCmd.Flags().Lookup("letsencrypt-domain"), + "cert-file": mgmtCmd.Flags().Lookup("cert-file"), + "cert-key": mgmtCmd.Flags().Lookup("cert-key"), + } + for name, flag := range flags { + require.NotNil(t, flag, "Management compatibility flag %s should be registered", name) + } + + oldConfigPath := nbconfig.MgmtConfigPath + oldCommandContext := mgmtCmd.Context() + oldMgmtPort := mgmtPort + oldMgmtDataDir := mgmtDataDir + oldLogLevel := logLevel + oldLogFile := logFile + oldDNSDomain := dnsDomain + oldLetsencryptDomain := mgmtLetsencryptDomain + oldCertFile := certFile + oldCertKey := certKey + oldConfig := config + oldFlagStates := make(map[*pflag.Flag]struct { + value string + changed bool + }, len(flags)) + for _, flag := range flags { + oldFlagStates[flag] = struct { + value string + changed bool + }{value: flag.Value.String(), changed: flag.Changed} + } + t.Cleanup(func() { + for flag, state := range oldFlagStates { + require.NoError(t, flag.Value.Set(state.value)) + flag.Changed = state.changed + } + nbconfig.MgmtConfigPath = oldConfigPath + mgmtCmd.SetContext(oldCommandContext) + mgmtPort = oldMgmtPort + mgmtDataDir = oldMgmtDataDir + logLevel = oldLogLevel + logFile = oldLogFile + dnsDomain = oldDNSDomain + mgmtLetsencryptDomain = oldLetsencryptDomain + certFile = oldCertFile + certKey = oldCertKey + config = oldConfig + }) + + baseline := map[string]string{ + "port": "80", + "datadir": defaultMgmtDataDir, + "letsencrypt-domain": "", + "cert-file": "", + "cert-key": "", + } + for name, value := range baseline { + require.NoError(t, flags[name].Value.Set(value)) + flags[name].Changed = false + } + for name, value := range changedFlags { + flag, ok := flags[name] + require.True(t, ok, "Compatibility scenario should reference a known flag") + require.NoError(t, flag.Value.Set(value)) + flag.Changed = true + } + + nbconfig.MgmtConfigPath = configPath + mgmtCmd.SetContext(context.Background()) + logLevel = "info" + logFile = "console" + dnsDomain = defaultSingleAccModeDomain + require.NoError(t, mgmtCmd.PreRunE(mgmtCmd, nil)) + return mgmtPort +} + +func unchangedManagementFlags() *pflag.FlagSet { + flags := pflag.NewFlagSet("management", pflag.ContinueOnError) + flags.String("datadir", defaultMgmtDataDir, "") + flags.String("letsencrypt-domain", "", "") + flags.String("cert-key", "", "") + flags.String("cert-file", "", "") + return flags +} diff --git a/proxy/cmd/proxy/cmd/config_test.go b/proxy/cmd/proxy/cmd/config_test.go index cf4812de5..858d3f0b8 100644 --- a/proxy/cmd/proxy/cmd/config_test.go +++ b/proxy/cmd/proxy/cmd/config_test.go @@ -1,23 +1,299 @@ package cmd import ( + "io" "os" "path/filepath" "testing" "time" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/trustedproxy" ) func TestExampleConfig(t *testing.T) { - require.NoError(t, rootCmd.ParseFlags(nil)) - cfg, err := loadConfig(rootCmd, filepath.Join("..", "..", "..", "config.example.yaml")) + clearProxyConfigEnvironment(t) + cmd := newLegacyProxyCommand(t) + cfg, err := loadConfig(cmd, filepath.Join("..", "..", "..", "config.example.yaml")) require.NoError(t, err) assert.Equal(t, "proxy.example.com", cfg.ProxyURL, "Example config should load") } +func TestLoadConfigPreservesLegacyDefaults(t *testing.T) { + clearProxyConfigEnvironment(t) + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + require.NoError(t, err) + assert.Equal(t, legacyProxyDefaults(t), cfg, "Proxy defaults should remain unchanged") +} + +func TestLoadConfigPreservesLegacyEnvironmentBindings(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv("NB_PROXY_LOG_LEVEL", "debug") + t.Setenv("NB_PROXY_ADDRESS", ":8443") + t.Setenv("NB_PROXY_MANAGEMENT_ADDRESS", "https://management.example.com:443") + t.Setenv("NB_PROXY_DOMAIN", "proxy.example.com") + t.Setenv("NB_PROXY_TOKEN", "proxy-token") + t.Setenv("NB_PROXY_CERTIFICATE_DIRECTORY", "/var/lib/proxy/certs") + t.Setenv("NB_PROXY_CERTIFICATE_FILE", "proxy.crt") + t.Setenv("NB_PROXY_CERTIFICATE_KEY_FILE", "proxy.key") + t.Setenv("NB_PROXY_ACME_CERTIFICATES", "true") + t.Setenv("NB_PROXY_ACME_ADDRESS", ":8080") + t.Setenv("NB_PROXY_ACME_DIRECTORY", "https://acme.example.com/directory") + t.Setenv("NB_PROXY_ACME_EAB_KID", "eab-kid") + t.Setenv("NB_PROXY_ACME_EAB_HMAC_KEY", "eab-hmac") + t.Setenv("NB_PROXY_ACME_CHALLENGE_TYPE", "http-01") + t.Setenv("NB_PROXY_CERT_LOCK_METHOD", "flock") + t.Setenv("NB_PROXY_WILDCARD_CERT_DIR", "/var/lib/proxy/wildcards") + t.Setenv("NB_PROXY_DEBUG_ENDPOINT", "true") + t.Setenv("NB_PROXY_DEBUG_ENDPOINT_ADDRESS", "localhost:9444") + t.Setenv("NB_PROXY_HEALTH_ADDRESS", "localhost:9080") + t.Setenv("NB_PROXY_FORWARDED_PROTO", "https") + t.Setenv("NB_PROXY_TRUSTED_PROXIES", "192.0.2.0/24") + t.Setenv("NB_PROXY_WG_PORT", "51820") + t.Setenv("NB_PROXY_PROXY_PROTOCOL", "true") + t.Setenv("NB_PROXY_PRESHARED_KEY", "pre-shared-key") + t.Setenv("NB_PROXY_SUPPORTS_CUSTOM_PORTS", "false") + t.Setenv("NB_PROXY_REQUIRE_SUBDOMAIN", "true") + t.Setenv("NB_PROXY_PRIVATE", "true") + t.Setenv("NB_PROXY_MAX_DIAL_TIMEOUT", "5s") + t.Setenv("NB_PROXY_MAX_SESSION_IDLE_TIMEOUT", "10m") + t.Setenv("NB_PROXY_MAPPING_BATCH_WATCHDOG", "30s") + t.Setenv("NB_PROXY_GEO_DATA_DIR", "/var/lib/proxy/geo") + t.Setenv("NB_PROXY_CROWDSEC_API_URL", "https://crowdsec.example.com") + t.Setenv("NB_PROXY_CROWDSEC_API_KEY", "crowdsec-key") + t.Setenv("NB_PROXY_PREALLOCATED_BUFFERS", "1024") + t.Setenv("NB_PROXY_MAX_BATCH_SIZE", "64") + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + require.NoError(t, err) + assert.Equal(t, "debug", cfg.LogLevel, "Legacy log-level environment binding should remain supported") + assert.Equal(t, ":8443", cfg.ListenAddr, "Legacy address environment binding should remain supported") + assert.Equal(t, "https://management.example.com:443", cfg.ManagementAddress, + "Legacy management environment binding should remain supported") + assert.Equal(t, "proxy.example.com", cfg.ProxyURL, "Legacy domain environment binding should remain supported") + assert.Equal(t, "proxy-token", cfg.ProxyToken, "Legacy token environment binding should remain supported") + assert.Equal(t, "/var/lib/proxy/certs", cfg.CertificateDirectory, "Legacy certificate directory should remain supported") + assert.Equal(t, "proxy.crt", cfg.CertificateFile, "Legacy certificate file should remain supported") + assert.Equal(t, "proxy.key", cfg.CertificateKeyFile, "Legacy certificate key should remain supported") + assert.True(t, cfg.GenerateACMECertificates, "Legacy ACME toggle should remain supported") + assert.Equal(t, ":8080", cfg.ACMEChallengeAddress, "Legacy ACME address should remain supported") + assert.Equal(t, "https://acme.example.com/directory", cfg.ACMEDirectory, "Legacy ACME directory should remain supported") + assert.Equal(t, "eab-kid", cfg.ACMEEABKID, "Legacy EAB KID should remain supported") + assert.Equal(t, "eab-hmac", cfg.ACMEEABHMACKey, "Legacy EAB HMAC key should remain supported") + assert.Equal(t, "http-01", cfg.ACMEChallengeType, "Legacy ACME challenge type should remain supported") + assert.Equal(t, "flock", string(cfg.CertLockMethod), "Legacy certificate lock method should remain supported") + assert.Equal(t, "/var/lib/proxy/wildcards", cfg.WildcardCertDir, "Legacy wildcard certificate directory should remain supported") + assert.True(t, cfg.DebugEndpointEnabled, "Legacy debug endpoint toggle should remain supported") + assert.Equal(t, "localhost:9444", cfg.DebugEndpointAddress, "Legacy debug endpoint address should remain supported") + assert.Equal(t, "localhost:9080", cfg.HealthAddr, "Legacy health address should remain supported") + assert.Equal(t, "https", cfg.ForwardedProto, "Legacy forwarded-proto environment binding should remain supported") + require.NotNil(t, cfg.TrustedProxies, "Legacy trusted proxy environment binding should remain supported") + assert.False(t, cfg.TrustedProxies.Empty(), "Legacy trusted proxy environment binding should remain populated") + assert.Equal(t, uint16(51820), cfg.WireguardPort, "Legacy tunnel port should remain supported") + assert.True(t, cfg.ProxyProtocol, "Legacy PROXY protocol toggle should remain supported") + assert.Equal(t, "pre-shared-key", cfg.PreSharedKey, "Legacy pre-shared key should remain supported") + assert.False(t, cfg.SupportsCustomPorts, "Legacy custom-port toggle should remain supported") + assert.True(t, cfg.RequireSubdomain, "Legacy subdomain toggle should remain supported") + assert.True(t, cfg.Private, "Legacy private toggle should remain supported") + assert.Equal(t, 5*time.Second, cfg.MaxDialTimeout, "Legacy dial timeout should remain supported") + assert.Equal(t, 10*time.Minute, cfg.MaxSessionIdleTimeout, "Legacy idle timeout should remain supported") + assert.Equal(t, 30*time.Second, cfg.MappingBatchWatchdog, "Legacy mapping watchdog should remain supported") + assert.Equal(t, "/var/lib/proxy/geo", cfg.GeoDataDir, "Legacy geodata directory should remain supported") + assert.Equal(t, "https://crowdsec.example.com", cfg.CrowdSecAPIURL, "Legacy CrowdSec URL should remain supported") + assert.Equal(t, "crowdsec-key", cfg.CrowdSecAPIKey, "Legacy CrowdSec key should remain supported") + require.NotNil(t, cfg.PreallocatedBuffers, "Legacy preallocated buffer environment binding should remain supported") + assert.Equal(t, uint32(1024), *cfg.PreallocatedBuffers, "Legacy preallocated buffer value should remain supported") + require.NotNil(t, cfg.MaxBatchSize, "Legacy maximum batch environment binding should remain supported") + assert.Equal(t, uint32(64), *cfg.MaxBatchSize, "Legacy maximum batch value should remain supported") +} + +func TestLoadConfigPreservesLegacyInvalidEnvironmentBehavior(t *testing.T) { + tests := []struct { + name string + envName string + value string + wantErr bool + validate func(*testing.T, *commandConfig) + }{ + { + name: "invalid boolean uses default", + envName: "NB_PROXY_SUPPORTS_CUSTOM_PORTS", + value: "invalid", + validate: func(t *testing.T, cfg *commandConfig) { + assert.True(t, cfg.SupportsCustomPorts, "Invalid legacy booleans should retain their default") + }, + }, + { + name: "invalid uint16 uses default", + envName: "NB_PROXY_WG_PORT", + value: "invalid", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.WireguardPort, "Invalid legacy uint16 values should retain their default") + }, + }, + { + name: "invalid duration uses default", + envName: "NB_PROXY_MAX_DIAL_TIMEOUT", + value: "invalid", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.MaxDialTimeout, "Invalid legacy durations should retain their default") + }, + }, + { + name: "invalid watchdog uses default", + envName: "NB_PROXY_MAPPING_BATCH_WATCHDOG", + value: "invalid", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.MappingBatchWatchdog, "Invalid legacy watchdog values should retain their default") + }, + }, + { + name: "empty performance value remains absent", + envName: "NB_PROXY_PREALLOCATED_BUFFERS", + value: "", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Nil(t, cfg.PreallocatedBuffers, "Empty legacy performance values should remain absent") + }, + }, + { + name: "invalid performance value remains fatal", + envName: "NB_PROXY_PREALLOCATED_BUFFERS", + value: "invalid", + wantErr: true, + }, + { + name: "empty maximum batch remains absent", + envName: "NB_PROXY_MAX_BATCH_SIZE", + value: "", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Nil(t, cfg.MaxBatchSize, "Empty legacy maximum batch values should remain absent") + }, + }, + { + name: "invalid maximum batch remains fatal", + envName: "NB_PROXY_MAX_BATCH_SIZE", + value: "invalid", + wantErr: true, + }, + { + name: "empty string clears default", + envName: "NB_PROXY_MANAGEMENT_ADDRESS", + value: "", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Empty(t, cfg.ManagementAddress, "Empty legacy strings should continue to clear their default") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv(test.envName, test.value) + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + if test.wantErr { + assert.Error(t, err, "Legacy-fatal environment input should remain fatal") + return + } + if !assert.NoError(t, err, "Legacy fallback parsing should not abort Proxy startup") { + return + } + test.validate(t, cfg) + }) + } +} + +func TestLegacyProxyFlagsRemainRegistered(t *testing.T) { + for _, name := range []string{ + "mgmt", + "addr", + "domain", + "cert-dir", + "acme-certs", + "acme-addr", + "acme-dir", + "acme-eab-kid", + "acme-eab-hmac-key", + "acme-challenge-type", + "debug-endpoint", + "debug-endpoint-addr", + "health-addr", + "forwarded-proto", + "trusted-proxies", + "cert-file", + "cert-key-file", + "cert-lock-method", + "wildcard-cert-dir", + "wg-port", + "proxy-protocol", + "preshared-key", + "supports-custom-ports", + "require-subdomain", + "private", + "max-dial-timeout", + "max-session-idle-timeout", + "geo-data-dir", + "crowdsec-api-url", + "crowdsec-api-key", + } { + assert.NotNil(t, rootCmd.Flags().Lookup(name), "Legacy flag %s should remain registered", name) + } + for _, name := range []string{"log-level", "debug"} { + assert.NotNil(t, rootCmd.PersistentFlags().Lookup(name), "Legacy persistent flag %s should remain registered", name) + } +} + +func TestLoadConfigIgnoresEmptyBooleanEnvironment(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv("NB_PROXY_SUPPORTS_CUSTOM_PORTS", "") + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + require.NoError(t, err) + assert.True(t, cfg.SupportsCustomPorts, + "An empty boolean environment variable should retain the previous default") +} + +func TestDeprecatedDebugEnvironmentOverridesLogLevelFlag(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv("NB_PROXY_TOKEN", "test-token") + t.Setenv("NB_PROXY_DOMAIN", "invalid domain") + t.Setenv("NB_PROXY_DEBUG_LOGS", "true") + cmd := newLegacyProxyCommand(t) + + logLevelFlag := cmd.Flags().Lookup("log-level") + require.NotNil(t, logLevelFlag, "Log level flag should be registered") + oldDebugLogs := debugLogs + oldConfigPath := configPath + t.Cleanup(func() { + debugLogs = oldDebugLogs + configPath = oldConfigPath + }) + + require.NoError(t, logLevelFlag.Value.Set("trace")) + logLevelFlag.Changed = true + debugLogs = envBoolOrDefault("NB_PROXY_DEBUG_LOGS", false) + configPath = "" + require.True(t, debugLogs, "Deprecated debug environment variable should be enabled") + + var runErr error + output := captureStderr(t, func() { + runErr = runServer(cmd, nil) + }) + require.Error(t, runErr) + assert.Contains(t, output, "configured log level: debug", + "The deprecated debug environment variable should retain its previous precedence") +} + func TestLoadConfigPrecedence(t *testing.T) { + clearProxyConfigEnvironment(t) configPath := filepath.Join(t.TempDir(), "proxy.yaml") require.NoError(t, os.WriteFile(configPath, []byte(` domain: file.example.com @@ -25,19 +301,13 @@ trustedProxies: 192.0.2.0/24 maxDialTimeout: 5s `), 0o600)) t.Setenv("NB_PROXY_TOKEN", "environment-token") - require.NoError(t, rootCmd.ParseFlags(nil)) + cmd := newLegacyProxyCommand(t) - domainFlag := rootCmd.Flags().Lookup("domain") - oldDomain := domainFlag.Value.String() - oldChanged := domainFlag.Changed - t.Cleanup(func() { - require.NoError(t, domainFlag.Value.Set(oldDomain)) - domainFlag.Changed = oldChanged - }) + domainFlag := cmd.Flags().Lookup("domain") require.NoError(t, domainFlag.Value.Set("flag.example.com")) domainFlag.Changed = true - cfg, err := loadConfig(rootCmd, configPath) + cfg, err := loadConfig(cmd, configPath) require.NoError(t, err) assert.Equal(t, "flag.example.com", cfg.ProxyURL, "Flags should override the configuration file") assert.Equal(t, "environment-token", cfg.ProxyToken, "Environment should populate secrets") @@ -45,3 +315,765 @@ maxDialTimeout: 5s require.NotNil(t, cfg.TrustedProxies, "Trusted proxies should be decoded") assert.False(t, cfg.TrustedProxies.Empty(), "Configured trusted proxies should not be empty") } + +// proxy-01: legacy read only the explicit NB_PROXY_* names; NB_ style +// variables were never consulted. +func TestLoadConfigIgnoresAutomaticEnvironmentAliases(t *testing.T) { + tests := []struct { + name string + alias string + value string + legacy map[string]string + validate func(*testing.T, *commandConfig) + }{ + { + name: "listen address alias is ignored", + alias: "NB_LISTENADDRESS", + value: ":9999", + legacy: map[string]string{"NB_PROXY_ADDRESS": ":8443"}, + validate: func(t *testing.T, cfg *commandConfig) { + assert.Equal(t, ":8443", cfg.ListenAddr, + "Legacy only read NB_PROXY_ADDRESS; NB_LISTENADDRESS must not override it") + }, + }, + { + name: "log level alias is ignored", + alias: "NB_LOGLEVEL", + value: "trace", + legacy: map[string]string{"NB_PROXY_LOG_LEVEL": "warn"}, + validate: func(t *testing.T, cfg *commandConfig) { + assert.Equal(t, "warn", cfg.LogLevel, + "Legacy only read NB_PROXY_LOG_LEVEL; NB_LOGLEVEL must not override it") + }, + }, + { + name: "token alias is ignored", + alias: "NB_PROXYTOKEN", + value: "alias-token", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Empty(t, cfg.ProxyToken, + "Legacy only read NB_PROXY_TOKEN; NB_PROXYTOKEN must not satisfy the token requirement") + }, + }, + { + name: "private alias is ignored", + alias: "NB_PRIVATE", + value: "true", + validate: func(t *testing.T, cfg *commandConfig) { + assert.False(t, cfg.Private, + "Legacy only read NB_PROXY_PRIVATE; NB_PRIVATE must not enable private mode") + }, + }, + { + name: "id alias is ignored", + alias: "NB_ID", + value: "alias-id", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Empty(t, cfg.ID, "Legacy never read NB_ID; the proxy ID must stay empty") + }, + }, + { + name: "management address alias is ignored", + alias: "NB_MANAGEMENTADDRESS", + value: "https://alias.example.com:443", + legacy: map[string]string{"NB_PROXY_MANAGEMENT_ADDRESS": "https://management.example.com:443"}, + validate: func(t *testing.T, cfg *commandConfig) { + assert.Equal(t, "https://management.example.com:443", cfg.ManagementAddress, + "Legacy only read NB_PROXY_MANAGEMENT_ADDRESS; NB_MANAGEMENTADDRESS must not override it") + }, + }, + { + name: "wireguard port alias is ignored", + alias: "NB_WIREGUARDPORT", + value: "51820", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.WireguardPort, + "Legacy only read NB_PROXY_WG_PORT; NB_WIREGUARDPORT must not set the tunnel port") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + for name, value := range test.legacy { + t.Setenv(name, value) + } + t.Setenv(test.alias, test.value) + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +// proxy-02: NB_DOMAIN is a common self-hosted variable; legacy ignored it and +// only honoured NB_PROXY_DOMAIN / --domain. +func TestLoadConfigIgnoresSharedDomainEnvironment(t *testing.T) { + tests := []struct { + name string + domain string + }{ + {name: "valid shared domain is ignored", domain: "netbird.example.org"}, + {name: "invalid shared domain is ignored", domain: "bad domain"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv("NB_PROXY_DOMAIN", "proxy.example.com") + t.Setenv("NB_DOMAIN", test.domain) + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + require.NoError(t, err) + assert.Equal(t, "proxy.example.com", cfg.ProxyURL, + "Legacy only read NB_PROXY_DOMAIN; the shared NB_DOMAIN variable must not override it") + }) + } +} + +// proxy-03: legacy had no NB_PROXY_ID binding; the ID was always generated at +// Server.Start time. +func TestLoadConfigIgnoresProxyIDEnvironment(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv("NB_PROXY_ID", "my-proxy") + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + require.NoError(t, err) + assert.Empty(t, cfg.ID, + "Legacy never read NB_PROXY_ID; the proxy ID must stay empty so Server.Start generates it") +} + +// proxy-04: legacy strconv.ParseBool rejected yes/no/on/off/y/n, logging a +// warning and keeping the flag default. +func TestLoadConfigPreservesLegacyBooleanVocabulary(t *testing.T) { + tests := []struct { + name string + envName string + value string + validate func(*testing.T, *commandConfig) + }{ + { + name: "no keeps custom ports enabled", + envName: "NB_PROXY_SUPPORTS_CUSTOM_PORTS", + value: "no", + validate: func(t *testing.T, cfg *commandConfig) { + assert.True(t, cfg.SupportsCustomPorts, "Legacy ParseBool rejected \"no\" and kept the default true") + }, + }, + { + name: "off keeps custom ports enabled", + envName: "NB_PROXY_SUPPORTS_CUSTOM_PORTS", + value: "off", + validate: func(t *testing.T, cfg *commandConfig) { + assert.True(t, cfg.SupportsCustomPorts, "Legacy ParseBool rejected \"off\" and kept the default true") + }, + }, + { + name: "n keeps custom ports enabled", + envName: "NB_PROXY_SUPPORTS_CUSTOM_PORTS", + value: "n", + validate: func(t *testing.T, cfg *commandConfig) { + assert.True(t, cfg.SupportsCustomPorts, "Legacy ParseBool rejected \"n\" and kept the default true") + }, + }, + { + name: "uppercase OFF keeps custom ports enabled", + envName: "NB_PROXY_SUPPORTS_CUSTOM_PORTS", + value: "OFF", + validate: func(t *testing.T, cfg *commandConfig) { + assert.True(t, cfg.SupportsCustomPorts, "Legacy ParseBool rejected \"OFF\" and kept the default true") + }, + }, + { + name: "yes keeps ACME disabled", + envName: "NB_PROXY_ACME_CERTIFICATES", + value: "yes", + validate: func(t *testing.T, cfg *commandConfig) { + assert.False(t, cfg.GenerateACMECertificates, "Legacy ParseBool rejected \"yes\" and kept the default false") + }, + }, + { + name: "uppercase ON keeps ACME disabled", + envName: "NB_PROXY_ACME_CERTIFICATES", + value: "ON", + validate: func(t *testing.T, cfg *commandConfig) { + assert.False(t, cfg.GenerateACMECertificates, "Legacy ParseBool rejected \"ON\" and kept the default false") + }, + }, + { + name: "y keeps ACME disabled", + envName: "NB_PROXY_ACME_CERTIFICATES", + value: "y", + validate: func(t *testing.T, cfg *commandConfig) { + assert.False(t, cfg.GenerateACMECertificates, "Legacy ParseBool rejected \"y\" and kept the default false") + }, + }, + { + name: "on keeps PROXY protocol disabled", + envName: "NB_PROXY_PROXY_PROTOCOL", + value: "on", + validate: func(t *testing.T, cfg *commandConfig) { + assert.False(t, cfg.ProxyProtocol, "Legacy ParseBool rejected \"on\" and kept the default false") + }, + }, + { + name: "Y keeps subdomain requirement disabled", + envName: "NB_PROXY_REQUIRE_SUBDOMAIN", + value: "Y", + validate: func(t *testing.T, cfg *commandConfig) { + assert.False(t, cfg.RequireSubdomain, "Legacy ParseBool rejected \"Y\" and kept the default false") + }, + }, + { + name: "on keeps private mode disabled", + envName: "NB_PROXY_PRIVATE", + value: "on", + validate: func(t *testing.T, cfg *commandConfig) { + assert.False(t, cfg.Private, "Legacy ParseBool rejected \"on\" and kept the default false") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv(test.envName, test.value) + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + if !assert.NoError(t, err, "Legacy boolean fallback parsing should not abort Proxy startup") { + return + } + test.validate(t, cfg) + }) + } +} + +// proxy-05: legacy parsed NB_PROXY_WG_PORT with strconv.ParseUint(v, 10, 16). +func TestLoadConfigPreservesLegacyWireguardPortParsing(t *testing.T) { + tests := []struct { + name string + value string + want uint16 + }{ + {name: "leading zero is decimal", value: "010", want: 10}, + {name: "hex prefix falls back to default", value: "0x1F", want: 0}, + {name: "octal prefix falls back to default", value: "0o17", want: 0}, + {name: "underscore separator falls back to default", value: "1_000", want: 0}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv("NB_PROXY_WG_PORT", test.value) + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + if !assert.NoError(t, err, "Legacy uint16 fallback parsing should not abort Proxy startup") { + return + } + assert.Equal(t, test.want, cfg.WireguardPort, + "Legacy parsed NB_PROXY_WG_PORT=%q in base 10 (parse errors fell back to 0)", test.value) + }) + } +} + +// proxy-07: legacy time.ParseDuration("") failed, logging a warning and +// keeping the default of 0; startup continued. +func TestLoadConfigPreservesLegacyEmptyDurationEnvironment(t *testing.T) { + tests := []struct { + name string + envName string + validate func(*testing.T, *commandConfig) + }{ + { + name: "empty dial timeout uses default", + envName: "NB_PROXY_MAX_DIAL_TIMEOUT", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.MaxDialTimeout, "Legacy treated an empty dial timeout as the default 0") + }, + }, + { + name: "empty idle timeout uses default", + envName: "NB_PROXY_MAX_SESSION_IDLE_TIMEOUT", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.MaxSessionIdleTimeout, "Legacy treated an empty idle timeout as the default 0") + }, + }, + { + name: "empty watchdog uses default", + envName: "NB_PROXY_MAPPING_BATCH_WATCHDOG", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.MappingBatchWatchdog, "Legacy treated an empty watchdog as the default 0") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv(test.envName, "") + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + if !assert.NoError(t, err, "Legacy treated an empty duration environment value as a warning, not a fatal error") { + return + } + test.validate(t, cfg) + }) + } +} + +// proxy-08: legacy time.ParseDuration errors (missing unit / invalid) logged a +// warning and kept the default of 0; startup continued. +func TestLoadConfigPreservesLegacyDurationFallbacks(t *testing.T) { + tests := []struct { + name string + envName string + value string + validate func(*testing.T, *commandConfig) + }{ + { + name: "unit-less dial timeout uses default", + envName: "NB_PROXY_MAX_DIAL_TIMEOUT", + value: "5", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.MaxDialTimeout, "Legacy treated a unit-less dial timeout as the default 0") + }, + }, + { + name: "unit-less idle timeout uses default", + envName: "NB_PROXY_MAX_SESSION_IDLE_TIMEOUT", + value: "5", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.MaxSessionIdleTimeout, "Legacy treated a unit-less idle timeout as the default 0") + }, + }, + { + name: "invalid idle timeout uses default", + envName: "NB_PROXY_MAX_SESSION_IDLE_TIMEOUT", + value: "invalid", + validate: func(t *testing.T, cfg *commandConfig) { + assert.Zero(t, cfg.MaxSessionIdleTimeout, "Legacy treated an invalid idle timeout as the default 0") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv(test.envName, test.value) + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + if !assert.NoError(t, err, "Legacy treated an unparseable duration environment value as a warning, not a fatal error") { + return + } + test.validate(t, cfg) + }) + } +} + +// proxy-09: legacy parsed the performance variables with +// strconv.ParseUint(raw, 10, 32) and aborted startup on any parse error. +func TestLoadConfigPreservesLegacyPerformanceParsing(t *testing.T) { + tests := []struct { + name string + envName string + value string + want uint32 + wantErr bool + }{ + {name: "preallocated buffers leading zero is decimal", envName: "NB_PROXY_PREALLOCATED_BUFFERS", value: "010", want: 10}, + {name: "preallocated buffers two leading zeros is decimal", envName: "NB_PROXY_PREALLOCATED_BUFFERS", value: "0100", want: 100}, + {name: "preallocated buffers hex remains fatal", envName: "NB_PROXY_PREALLOCATED_BUFFERS", value: "0x10", wantErr: true}, + {name: "preallocated buffers long hex remains fatal", envName: "NB_PROXY_PREALLOCATED_BUFFERS", value: "0x100", wantErr: true}, + {name: "preallocated buffers underscore remains fatal", envName: "NB_PROXY_PREALLOCATED_BUFFERS", value: "1_000", wantErr: true}, + {name: "maximum batch leading zero is decimal", envName: "NB_PROXY_MAX_BATCH_SIZE", value: "010", want: 10}, + {name: "maximum batch two leading zeros is decimal", envName: "NB_PROXY_MAX_BATCH_SIZE", value: "0100", want: 100}, + {name: "maximum batch hex remains fatal", envName: "NB_PROXY_MAX_BATCH_SIZE", value: "0x10", wantErr: true}, + {name: "maximum batch long hex remains fatal", envName: "NB_PROXY_MAX_BATCH_SIZE", value: "0x100", wantErr: true}, + {name: "maximum batch underscore remains fatal", envName: "NB_PROXY_MAX_BATCH_SIZE", value: "1_000", wantErr: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv(test.envName, test.value) + cmd := newLegacyProxyCommand(t) + + cfg, err := loadConfig(cmd, "") + if test.wantErr { + assert.Error(t, err, + "Legacy strconv.ParseUint(%q, 10, 32) failed and aborted Proxy startup", test.value) + return + } + if !assert.NoError(t, err, "Legacy base-10 performance values should not abort Proxy startup") { + return + } + got := cfg.PreallocatedBuffers + if test.envName == "NB_PROXY_MAX_BATCH_SIZE" { + got = cfg.MaxBatchSize + } + require.NotNil(t, got, "Legacy performance values should be present when set") + assert.Equal(t, test.want, *got, + "Legacy parsed %s=%q in base 10", test.envName, test.value) + }) + } +} + +// proxy-10 / proxy-12: legacy read the token only from NB_PROXY_TOKEN, checked +// it before anything else, and reported a fixed message when it was missing. +func TestRunServerPreservesLegacyTokenRequirement(t *testing.T) { + const legacyTokenError = "proxy token is required: set NB_PROXY_TOKEN environment variable" + + tests := []struct { + name string + env map[string]string + fileConfig string + }{ + { + name: "missing token without other sources", + }, + { + name: "file token does not satisfy the requirement", + fileConfig: "proxyToken: file-token\ndomain: \"invalid domain\"\n", + }, + { + name: "token alias does not satisfy the requirement", + env: map[string]string{"NB_PROXYTOKEN": "alias-token", "NB_PROXY_DOMAIN": "invalid domain"}, + }, + { + name: "empty token with file token remains fatal", + env: map[string]string{"NB_PROXY_TOKEN": ""}, + fileConfig: "proxyToken: file-token\ndomain: \"invalid domain\"\n", + }, + { + name: "missing token is reported before performance parsing", + env: map[string]string{"NB_PROXY_PREALLOCATED_BUFFERS": "invalid"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + for name, value := range test.env { + t.Setenv(name, value) + } + cmd := newLegacyProxyCommand(t) + + oldDebugLogs := debugLogs + oldConfigPath := configPath + t.Cleanup(func() { + debugLogs = oldDebugLogs + configPath = oldConfigPath + }) + debugLogs = false + configPath = "" + if test.fileConfig != "" { + configPath = filepath.Join(t.TempDir(), "proxy.yaml") + require.NoError(t, os.WriteFile(configPath, []byte(test.fileConfig), 0o600)) + } + + var runErr error + captureStderr(t, func() { + runErr = runServer(cmd, nil) + }) + require.Error(t, runErr) + assert.EqualError(t, runErr, legacyTokenError, + "Legacy accepted the token only from NB_PROXY_TOKEN and reported it before any other startup step") + }) + } +} + +// proxy-12: with a valid token, legacy initialised the logger before parsing +// the performance variables, so the log-level line preceded the parse error. +func TestRunServerPreservesLegacyPerformanceErrorOrdering(t *testing.T) { + clearProxyConfigEnvironment(t) + t.Setenv("NB_PROXY_TOKEN", "test-token") + t.Setenv("NB_PROXY_PREALLOCATED_BUFFERS", "invalid") + cmd := newLegacyProxyCommand(t) + + oldDebugLogs := debugLogs + oldConfigPath := configPath + t.Cleanup(func() { + debugLogs = oldDebugLogs + configPath = oldConfigPath + }) + debugLogs = false + configPath = "" + + var runErr error + output := captureStderr(t, func() { + runErr = runServer(cmd, nil) + }) + require.Error(t, runErr) + assert.Contains(t, output, "configured log level: info", + "Legacy initialised the logger before parsing the performance environment variables") + assert.EqualError(t, runErr, `invalid NB_PROXY_PREALLOCATED_BUFFERS "invalid": strconv.ParseUint: parsing "invalid": invalid syntax`, + "Legacy reported the performance parse error with the variable name after logger initialisation") +} + +// proxy-13: legacy had no --config flag; cobra rejected it as unknown. +func TestLegacyProxyRejectsConfigFlag(t *testing.T) { + oldConfigPath := configPath + t.Cleanup(func() { + configPath = oldConfigPath + if flag := rootCmd.Flags().Lookup("config"); flag != nil { + require.NoError(t, flag.Value.Set(oldConfigPath)) + flag.Changed = false + } + }) + + err := rootCmd.ParseFlags([]string{"--config", filepath.Join(t.TempDir(), "proxy.yaml")}) + require.Error(t, err, "Legacy proxy command did not register a --config flag") + assert.EqualError(t, err, "unknown flag: --config", + "Legacy cobra rejected --config as an unknown flag") +} + +// proxy-critic-1: legacy parsed --trusted-proxies / NB_PROXY_TRUSTED_PROXIES +// last in runServer, after the token check, logger initialisation and the +// forwarded-proto and domain validation, and reported it as +// `invalid --trusted-proxies: ...`. +func TestRunServerPreservesLegacyTrustedProxiesValidationOrdering(t *testing.T) { + const legacyTokenError = "proxy token is required: set NB_PROXY_TOKEN environment variable" + + tests := []struct { + name string + env map[string]string + flagValue string + wantErr string + wantErrPart string + wantLogLine bool + }{ + { + name: "missing token is reported before trusted proxy parsing", + env: map[string]string{"NB_PROXY_TRUSTED_PROXIES": "not-an-ip"}, + wantErr: legacyTokenError, + }, + { + name: "invalid forwarded-proto is reported before trusted proxy parsing", + env: map[string]string{ + "NB_PROXY_TOKEN": "test-token", + "NB_PROXY_FORWARDED_PROTO": "bogus", + "NB_PROXY_TRUSTED_PROXIES": "not-an-ip", + }, + wantErr: `invalid --forwarded-proto value "bogus": must be auto, http, or https`, + wantLogLine: true, + }, + { + name: "invalid domain is reported before trusted proxy parsing", + env: map[string]string{ + "NB_PROXY_TOKEN": "test-token", + "NB_PROXY_DOMAIN": "invalid domain", + "NB_PROXY_TRUSTED_PROXIES": "not-an-ip", + }, + wantErrPart: `invalid domain value "invalid domain"`, + wantLogLine: true, + }, + { + name: "invalid trusted proxy environment is reported after logger initialisation", + env: map[string]string{ + "NB_PROXY_TOKEN": "test-token", + "NB_PROXY_DOMAIN": "proxy.example.com", + "NB_PROXY_TRUSTED_PROXIES": "not-an-ip", + }, + wantErr: `invalid --trusted-proxies: parse trusted proxy "not-an-ip": not a valid CIDR or IP: ParseAddr("not-an-ip"): unable to parse IP`, + wantLogLine: true, + }, + { + name: "invalid trusted proxy flag is reported after logger initialisation", + env: map[string]string{ + "NB_PROXY_TOKEN": "test-token", + "NB_PROXY_DOMAIN": "proxy.example.com", + }, + flagValue: "10.0.0.0/8,garbage", + wantErr: `invalid --trusted-proxies: parse trusted proxy "garbage": not a valid CIDR or IP: ParseAddr("garbage"): unable to parse IP`, + wantLogLine: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyConfigEnvironment(t) + for name, value := range test.env { + t.Setenv(name, value) + } + cmd := newLegacyProxyCommand(t) + if test.flagValue != "" { + trustedProxiesFlag := cmd.Flags().Lookup("trusted-proxies") + require.NotNil(t, trustedProxiesFlag, "Trusted proxies flag should be registered") + require.NoError(t, trustedProxiesFlag.Value.Set(test.flagValue)) + trustedProxiesFlag.Changed = true + } + + oldDebugLogs := debugLogs + oldConfigPath := configPath + t.Cleanup(func() { + debugLogs = oldDebugLogs + configPath = oldConfigPath + }) + debugLogs = false + configPath = "" + + var runErr error + output := captureStderr(t, func() { + runErr = runServer(cmd, nil) + }) + require.Error(t, runErr) + if test.wantLogLine { + assert.Contains(t, output, "configured log level: info", + "Legacy initialised the logger before validating the trusted proxy list") + } else { + assert.NotContains(t, output, "configured log level", + "Legacy checked the token before initialising the logger") + } + if test.wantErr != "" { + assert.EqualError(t, runErr, test.wantErr, + "Legacy parsed the trusted proxy list last in runServer and reported it as invalid --trusted-proxies") + } + if test.wantErrPart != "" { + assert.ErrorContains(t, runErr, test.wantErrPart, + "Legacy validated the domain before parsing the trusted proxy list") + assert.NotContains(t, runErr.Error(), "trusted prox", + "Legacy never mentioned trusted proxies when an earlier validation step failed") + } + }) + } +} + +func newLegacyProxyCommand(t *testing.T) *cobra.Command { + t.Helper() + + defaults := legacyProxyDefaults(t) + cmd := &cobra.Command{Use: "proxy-test"} + flags := cmd.Flags() + flags.String("log-level", defaults.LogLevel, "") + flags.Bool("debug", false, "") + flags.String("mgmt", defaults.ManagementAddress, "") + flags.String("addr", defaults.ListenAddr, "") + flags.String("domain", defaults.ProxyURL, "") + flags.String("cert-dir", defaults.CertificateDirectory, "") + flags.Bool("acme-certs", defaults.GenerateACMECertificates, "") + flags.String("acme-addr", defaults.ACMEChallengeAddress, "") + flags.String("acme-dir", defaults.ACMEDirectory, "") + flags.String("acme-eab-kid", defaults.ACMEEABKID, "") + flags.String("acme-eab-hmac-key", defaults.ACMEEABHMACKey, "") + flags.String("acme-challenge-type", defaults.ACMEChallengeType, "") + flags.Bool("debug-endpoint", defaults.DebugEndpointEnabled, "") + flags.String("debug-endpoint-addr", defaults.DebugEndpointAddress, "") + flags.String("health-addr", defaults.HealthAddr, "") + flags.String("forwarded-proto", defaults.ForwardedProto, "") + flags.String("trusted-proxies", "", "") + flags.String("cert-file", defaults.CertificateFile, "") + flags.String("cert-key-file", defaults.CertificateKeyFile, "") + flags.String("cert-lock-method", string(defaults.CertLockMethod), "") + flags.String("wildcard-cert-dir", defaults.WildcardCertDir, "") + flags.Uint16("wg-port", defaults.WireguardPort, "") + flags.Bool("proxy-protocol", defaults.ProxyProtocol, "") + flags.String("preshared-key", defaults.PreSharedKey, "") + flags.Bool("supports-custom-ports", defaults.SupportsCustomPorts, "") + flags.Bool("require-subdomain", defaults.RequireSubdomain, "") + flags.Bool("private", defaults.Private, "") + flags.Duration("max-dial-timeout", defaults.MaxDialTimeout, "") + flags.Duration("max-session-idle-timeout", defaults.MaxSessionIdleTimeout, "") + flags.String("geo-data-dir", defaults.GeoDataDir, "") + flags.String("crowdsec-api-url", defaults.CrowdSecAPIURL, "") + flags.String("crowdsec-api-key", defaults.CrowdSecAPIKey, "") + return cmd +} + +func legacyProxyDefaults(t *testing.T) *commandConfig { + t.Helper() + + trustedProxies, err := trustedproxy.Parse("") + require.NoError(t, err) + cfg := &commandConfig{LogLevel: "info"} + cfg.ListenAddr = ":443" + cfg.ManagementAddress = DefaultManagementURL + cfg.CertificateDirectory = "./certs" + cfg.CertificateFile = "tls.crt" + cfg.CertificateKeyFile = "tls.key" + cfg.ACMEChallengeAddress = ":80" + cfg.ACMEDirectory = "https://acme-v02.api.letsencrypt.org/directory" + cfg.ACMEChallengeType = "tls-alpn-01" + cfg.CertLockMethod = "auto" + cfg.DebugEndpointAddress = "localhost:8444" + cfg.HealthAddr = "localhost:8080" + cfg.ForwardedProto = "auto" + cfg.TrustedProxies = trustedProxies + cfg.SupportsCustomPorts = true + cfg.GeoDataDir = "/var/lib/netbird/geolocation" + return cfg +} + +func clearProxyConfigEnvironment(t *testing.T) { + t.Helper() + + for _, name := range []string{ + "NB_PROXY_LOG_LEVEL", + "NB_PROXY_DEBUG_LOGS", + "NB_PROXY_MANAGEMENT_ADDRESS", + "NB_PROXY_ADDRESS", + "NB_PROXY_DOMAIN", + "NB_PROXY_TOKEN", + "NB_PROXY_CERTIFICATE_DIRECTORY", + "NB_PROXY_CERTIFICATE_FILE", + "NB_PROXY_CERTIFICATE_KEY_FILE", + "NB_PROXY_ACME_CERTIFICATES", + "NB_PROXY_ACME_ADDRESS", + "NB_PROXY_ACME_DIRECTORY", + "NB_PROXY_ACME_EAB_KID", + "NB_PROXY_ACME_EAB_HMAC_KEY", + "NB_PROXY_ACME_CHALLENGE_TYPE", + "NB_PROXY_CERT_LOCK_METHOD", + "NB_PROXY_WILDCARD_CERT_DIR", + "NB_PROXY_DEBUG_ENDPOINT", + "NB_PROXY_DEBUG_ENDPOINT_ADDRESS", + "NB_PROXY_HEALTH_ADDRESS", + "NB_PROXY_FORWARDED_PROTO", + "NB_PROXY_TRUSTED_PROXIES", + "NB_PROXY_WG_PORT", + "NB_PROXY_PROXY_PROTOCOL", + "NB_PROXY_PRESHARED_KEY", + "NB_PROXY_SUPPORTS_CUSTOM_PORTS", + "NB_PROXY_REQUIRE_SUBDOMAIN", + "NB_PROXY_PRIVATE", + "NB_PROXY_MAX_DIAL_TIMEOUT", + "NB_PROXY_MAX_SESSION_IDLE_TIMEOUT", + "NB_PROXY_MAPPING_BATCH_WATCHDOG", + "NB_PROXY_GEO_DATA_DIR", + "NB_PROXY_CROWDSEC_API_URL", + "NB_PROXY_CROWDSEC_API_KEY", + "NB_PROXY_PREALLOCATED_BUFFERS", + "NB_PROXY_MAX_BATCH_SIZE", + } { + t.Setenv(name, "") + require.NoError(t, os.Unsetenv(name)) + } +} + +func captureStderr(t *testing.T, fn func()) string { + t.Helper() + + reader, writer, err := os.Pipe() + require.NoError(t, err) + oldStderr := os.Stderr + os.Stderr = writer + defer func() { + os.Stderr = oldStderr + }() + fn() + os.Stderr = oldStderr + require.NoError(t, writer.Close()) + + output, err := io.ReadAll(reader) + require.NoError(t, err) + require.NoError(t, reader.Close()) + return string(output) +} diff --git a/relay/cmd/config_test.go b/relay/cmd/config_test.go index d93b12be6..d037a66fb 100644 --- a/relay/cmd/config_test.go +++ b/relay/cmd/config_test.go @@ -1,15 +1,20 @@ package cmd import ( + "bytes" + "math" "os" "path/filepath" "testing" + log "github.com/sirupsen/logrus" + "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestExampleConfig(t *testing.T) { + clearRelayConfigEnvironment(t) require.NoError(t, rootCmd.ParseFlags(nil)) oldConfigPath := configPath configPath = filepath.Join("..", "config.example.yaml") @@ -23,7 +28,168 @@ func TestExampleConfig(t *testing.T) { assert.True(t, cfg.EnableSTUN, "Example config should enable STUN") } +func TestLoadConfigPreservesLegacyEffectiveDefaults(t *testing.T) { + clearRelayConfigEnvironment(t) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + cfg.LetsencryptDomains = nil + assert.Equal(t, defaultConfig(), cfg, "Relay effective defaults should remain unchanged") +} + +func TestLoadConfigPreservesLegacyEnvironmentBindings(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_LISTEN_ADDRESS", ":7443") + t.Setenv("NB_EXPOSED_ADDRESS", "rels://relay.example.com:443") + t.Setenv("NB_METRICS_PORT", "9191") + t.Setenv("NB_LETSENCRYPT_EMAIL", "admin@example.com") + t.Setenv("NB_LETSENCRYPT_DATA_DIR", "/var/lib/relay/certs") + t.Setenv("NB_LETSENCRYPT_DOMAINS", "relay.example.com,relay-alt.example.com") + t.Setenv("NB_LETSENCRYPT_AWS_ROUTE53", "true") + t.Setenv("NB_TLS_CERT_FILE", "/etc/relay/tls.crt") + t.Setenv("NB_TLS_KEY_FILE", "/etc/relay/tls.key") + t.Setenv("NB_AUTH_SECRET", "relay-secret") + t.Setenv("NB_LOG_LEVEL", "debug") + t.Setenv("NB_LOG_FILE", "/var/log/relay.log") + t.Setenv("NB_HEALTH_LISTEN_ADDRESS", ":9001") + t.Setenv("NB_TRUSTED_PROXIES", "192.0.2.0/24") + t.Setenv("NB_ENABLE_STUN", "true") + t.Setenv("NB_STUN_PORTS", "3479,3480") + t.Setenv("NB_STUN_LOG_LEVEL", "trace") + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Equal(t, &Config{ + ListenAddress: ":7443", + ExposedAddress: "rels://relay.example.com:443", + MetricsPort: 9191, + LetsencryptEmail: "admin@example.com", + LetsencryptDataDir: "/var/lib/relay/certs", + LetsencryptDomains: []string{"relay.example.com", "relay-alt.example.com"}, + LetsencryptAWSRoute53: true, + TlsCertFile: "/etc/relay/tls.crt", + TlsKeyFile: "/etc/relay/tls.key", + AuthSecret: "relay-secret", + LogLevel: "debug", + LogFile: "/var/log/relay.log", + HealthcheckListenAddress: ":9001", + TrustedProxies: "192.0.2.0/24", + EnableSTUN: true, + STUNPorts: []int{3479, 3480}, + STUNLogLevel: "trace", + }, cfg, "Every legacy Relay environment binding should remain supported") +} + +func TestLoadConfigPreservesLegacyEnvironmentParsing(t *testing.T) { + tests := []struct { + name string + envName string + value string + validate func(*testing.T, *Config) + }{ + { + name: "invalid integer becomes zero", + envName: "NB_METRICS_PORT", + value: "invalid", + validate: func(t *testing.T, cfg *Config) { + assert.Zero(t, cfg.MetricsPort, "Invalid legacy integer input should retain its parsed zero value") + }, + }, + { + name: "legacy boolean spelling remains false", + envName: "NB_ENABLE_STUN", + value: "yes", + validate: func(t *testing.T, cfg *Config) { + assert.False(t, cfg.EnableSTUN, "Unsupported legacy boolean spelling should remain disabled") + }, + }, + { + name: "invalid boolean remains false", + envName: "NB_ENABLE_STUN", + value: "invalid", + validate: func(t *testing.T, cfg *Config) { + assert.False(t, cfg.EnableSTUN, "Invalid legacy boolean input should retain its default") + }, + }, + { + name: "empty integer list retains default", + envName: "NB_STUN_PORTS", + value: "", + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, []int{3478}, cfg.STUNPorts, "Empty legacy integer lists should retain their default") + }, + }, + { + name: "trailing comma integer list retains default", + envName: "NB_STUN_PORTS", + value: "3479,", + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, []int{3478}, cfg.STUNPorts, "Invalid legacy integer lists should retain their default") + }, + }, + { + name: "quoted comma in string list", + envName: "NB_LETSENCRYPT_DOMAINS", + value: `"relay,one.example.com",relay-two.example.com`, + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, []string{"relay,one.example.com", "relay-two.example.com"}, cfg.LetsencryptDomains, + "Legacy string lists should retain CSV quoting") + }, + }, + { + name: "malformed quoted string list is ignored", + envName: "NB_LETSENCRYPT_DOMAINS", + value: `"relay.example.com`, + validate: func(t *testing.T, cfg *Config) { + assert.Nil(t, cfg.LetsencryptDomains, "Malformed legacy string lists should retain their default") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv(test.envName, test.value) + + cfg, err := loadRelayConfigWithoutFile(t) + if !assert.NoError(t, err, "Legacy environment parse errors should not abort Relay startup") { + return + } + test.validate(t, cfg) + }) + } +} + +func TestLegacyRelayFlagsRemainRegistered(t *testing.T) { + expected := map[string]string{ + "listen-address": "l", + "exposed-address": "e", + "metrics-port": "", + "letsencrypt-data-dir": "d", + "letsencrypt-domains": "a", + "letsencrypt-email": "", + "letsencrypt-aws-route53": "", + "tls-cert-file": "c", + "tls-key-file": "k", + "auth-secret": "s", + "log-level": "", + "log-file": "", + "health-listen-address": "H", + "trusted-proxies": "", + "enable-stun": "", + "stun-ports": "", + "stun-log-level": "", + } + + for name, shorthand := range expected { + flag := rootCmd.Flags().Lookup(name) + require.NotNil(t, flag, "Legacy flag %s should remain registered", name) + assert.Equal(t, shorthand, flag.Shorthand, "Legacy shorthand for %s should remain unchanged", name) + } +} + func TestLoadConfigPrecedence(t *testing.T) { + clearRelayConfigEnvironment(t) path := filepath.Join(t.TempDir(), "relay.yaml") require.NoError(t, os.WriteFile(path, []byte(` listenAddress: ":8443" @@ -60,3 +226,476 @@ stunPorts: [3478, 3479] assert.True(t, cfg.EnableSTUN, "STUN should be configurable from the file") assert.Equal(t, []int{3478, 3479}, cfg.STUNPorts, "STUN ports should load from the file") } + +func loadRelayConfigWithoutFile(t *testing.T) (*Config, error) { + t.Helper() + + require.NoError(t, rootCmd.ParseFlags(nil)) + oldConfigPath := configPath + configPath = "" + t.Cleanup(func() { + configPath = oldConfigPath + }) + return loadConfig(rootCmd) +} + +func clearRelayConfigEnvironment(t *testing.T) { + t.Helper() + + for _, name := range []string{ + "NB_LISTEN_ADDRESS", + "NB_EXPOSED_ADDRESS", + "NB_METRICS_PORT", + "NB_LETSENCRYPT_EMAIL", + "NB_LETSENCRYPT_DATA_DIR", + "NB_LETSENCRYPT_DOMAINS", + "NB_LETSENCRYPT_AWS_ROUTE53", + "NB_TLS_CERT_FILE", + "NB_TLS_KEY_FILE", + "NB_AUTH_SECRET", + "NB_LOG_LEVEL", + "NB_LOG_FILE", + "NB_HEALTH_LISTEN_ADDRESS", + "NB_TRUSTED_PROXIES", + "NB_ENABLE_STUN", + "NB_STUN_PORTS", + "NB_STUN_LOG_LEVEL", + } { + t.Setenv(name, "") + require.NoError(t, os.Unsetenv(name)) + } +} + +func TestLoadConfigPreservesLegacyIntegerOverflowParsing(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_METRICS_PORT", "99999999999999999999") + + cfg, err := loadRelayConfigWithoutFile(t) + if !assert.NoError(t, err, "Legacy pflag stored the clamped integer and only logged the range error, so loading continued") { + return + } + assert.Equal(t, math.MaxInt64, cfg.MetricsPort, + "Legacy strconv.ParseInt range errors left math.MaxInt64 in the metrics port instead of aborting startup") +} + +func TestLoadConfigPreservesLegacyRoute53BooleanSpelling(t *testing.T) { + for _, value := range []string{"yes", "y", "on", "YES", "On"} { + t.Run(value, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_LETSENCRYPT_AWS_ROUTE53", value) + + cfg, err := loadRelayConfigWithoutFile(t) + if !assert.NoError(t, err, "Legacy boolean parse errors should not abort Relay startup") { + return + } + assert.False(t, cfg.LetsencryptAWSRoute53, + "Legacy strconv.ParseBool rejected %q, leaving Route 53 disabled so the Route 53 TLS branch was never taken", value) + }) + } +} + +func TestLoadConfigPreservesLegacyRoute53InvalidBoolean(t *testing.T) { + tests := []struct { + name string + value string + }{ + {name: "invalid word", value: "invalid"}, + {name: "numeric two", value: "2"}, + {name: "mixed case true", value: "tRuE"}, + {name: "trailing space", value: "true "}, + {name: "enabled", value: "enabled"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_LETSENCRYPT_AWS_ROUTE53", test.value) + + cfg, err := loadRelayConfigWithoutFile(t) + if !assert.NoError(t, err, "Legacy invalid boolean input was logged at Info and ignored, not fatal") { + return + } + assert.False(t, cfg.LetsencryptAWSRoute53, + "Legacy strconv.ParseBool rejected %q and pflag stored false, so Relay started with Route 53 disabled", test.value) + }) + } +} + +func TestLoadConfigPreservesLegacySTUNPortListRejection(t *testing.T) { + tests := []struct { + name string + value string + }{ + {name: "non-numeric trailing element", value: "3479,abc"}, + {name: "non-numeric single element", value: "abc"}, + {name: "space after comma", value: "3479, 3480"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_STUN_PORTS", test.value) + + cfg, err := loadRelayConfigWithoutFile(t) + if !assert.NoError(t, err, "Legacy integer list parse errors were logged at Info and ignored, not fatal") { + return + } + assert.Equal(t, []int{3478}, cfg.STUNPorts, + "Legacy pflag intSlice ran strconv.Atoi on every element and rejected the whole value %q, retaining the default", test.value) + }) + } +} + +func TestLoadConfigPreservesLegacySTUNPortDecimalParsing(t *testing.T) { + tests := []struct { + name string + value string + expected []int + }{ + {name: "leading zero", value: "010", expected: []int{10}}, + {name: "double leading zero", value: "0010", expected: []int{10}}, + {name: "leading zero element", value: "3478,010", expected: []int{3478, 10}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_STUN_PORTS", test.value) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Equal(t, test.expected, cfg.STUNPorts, + "Legacy pflag intSlice used strconv.Atoi (base 10 only), so leading zeros were decimal and never octal") + }) + } +} + +func TestLoadConfigPreservesLegacySTUNPortStrictDecimalSyntax(t *testing.T) { + tests := []struct { + name string + value string + }{ + {name: "hex prefix", value: "0x10"}, + {name: "underscore separator", value: "3_479"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_STUN_PORTS", test.value) + + cfg, err := loadRelayConfigWithoutFile(t) + if !assert.NoError(t, err, "Legacy integer list parse errors were logged at Info and ignored, not fatal") { + return + } + assert.Equal(t, []int{3478}, cfg.STUNPorts, + "Legacy strconv.Atoi rejected %q (no hex prefix or underscores), so the default STUN port list was retained", test.value) + }) + } +} + +func TestLoadConfigPreservesLegacyNewlineInDomainList(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_LETSENCRYPT_DOMAINS", "a.example.com\nb.example.com") + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Equal(t, []string{"a.example.com"}, cfg.LetsencryptDomains, + "Legacy pflag readAsCSV performed a single csv.Reader.Read, so only the first line of a newline separated value was used") +} + +func TestLoadConfigPreservesLegacyEnvAndFlagDomainAppend(t *testing.T) { + tests := []struct { + name string + flagValues []string + expected []string + }{ + { + name: "single flag value", + flagValues: []string{"b.example.com"}, + expected: []string{"a.example.com", "b.example.com"}, + }, + { + name: "repeated flag values", + flagValues: []string{"b.example.com", "c.example.com"}, + expected: []string{"a.example.com", "b.example.com", "c.example.com"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_LETSENCRYPT_DOMAINS", "a.example.com") + setRelaySliceFlag(t, "letsencrypt-domains", test.flagValues) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Equal(t, test.expected, cfg.LetsencryptDomains, + "Legacy applied NB_LETSENCRYPT_DOMAINS via flags.Set before argv parsing, so command line domains were appended to the environment list") + }) + } +} + +func TestLoadConfigPreservesLegacyEnvAndFlagSTUNPortAppend(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_STUN_PORTS", "3479") + setRelaySliceFlag(t, "stun-ports", []string{"3480"}) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Equal(t, []int{3479, 3480}, cfg.STUNPorts, + "Legacy applied NB_STUN_PORTS via flags.Set before argv parsing, so --stun-ports appended to the environment list and both UDP ports were bound") +} + +func TestLoadConfigPreservesLegacyDuplicateSTUNPortFromEnvAndFlag(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_EXPOSED_ADDRESS", "rels://relay.example.com:443") + t.Setenv("NB_AUTH_SECRET", "relay-secret") + t.Setenv("NB_ENABLE_STUN", "true") + t.Setenv("NB_STUN_PORTS", "3478") + setRelaySliceFlag(t, "stun-ports", []string{"3478"}) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Equal(t, []int{3478, 3478}, cfg.STUNPorts, + "Legacy appended the --stun-ports value to the NB_STUN_PORTS value, producing a duplicated port list") + + err = cfg.Validate() + if assert.Error(t, err, "Legacy Validate rejected the duplicated STUN port list and Relay exited") { + assert.Contains(t, err.Error(), "duplicate STUN port 3478", "Legacy reported the duplicate STUN port") + } +} + +func TestLoadConfigPreservesLegacyEnvironmentNameMatching(t *testing.T) { + tests := []struct { + name string + env map[string]string + validate func(*testing.T, *Config) + }{ + { + name: "listen address alias does not outrank legacy name", + env: map[string]string{"NB_LISTENADDRESS": ":1", "NB_LISTEN_ADDRESS": ":2"}, + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, ":2", cfg.ListenAddress, + "Legacy only read NB_LISTEN_ADDRESS; the un-underscored NB_LISTENADDRESS was ignored") + }, + }, + { + name: "healthcheck alias is ignored", + env: map[string]string{"NB_HEALTHCHECKLISTENADDRESS": ":1"}, + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, ":9000", cfg.HealthcheckListenAddress, + "Legacy only read NB_HEALTH_LISTEN_ADDRESS; NB_HEALTHCHECKLISTENADDRESS was ignored and the default retained") + }, + }, + { + name: "stun ports alias is ignored", + env: map[string]string{"NB_STUNPORTS": "1111"}, + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, []int{3478}, cfg.STUNPorts, + "Legacy only read NB_STUN_PORTS; NB_STUNPORTS was ignored and the default retained") + }, + }, + { + name: "auth secret alias is ignored", + env: map[string]string{"NB_AUTHSECRET": "alias-secret"}, + validate: func(t *testing.T, cfg *Config) { + assert.Empty(t, cfg.AuthSecret, + "Legacy only read NB_AUTH_SECRET; NB_AUTHSECRET was ignored") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + for name, value := range test.env { + t.Setenv(name, value) + } + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +func TestLoadConfigPreservesLegacyValuesWithEmptyEnvironmentAliases(t *testing.T) { + tests := []struct { + name string + env map[string]string + validate func(*testing.T, *Config) + }{ + { + name: "empty log level alias keeps default", + env: map[string]string{"NB_LOGLEVEL": ""}, + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, "info", cfg.LogLevel, + "Legacy never read NB_LOGLEVEL, so an empty alias could not blank the log level and break InitLog") + }, + }, + { + name: "empty listen address alias keeps legacy name", + env: map[string]string{"NB_LISTENADDRESS": "", "NB_LISTEN_ADDRESS": ":2"}, + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, ":2", cfg.ListenAddress, + "Legacy never read NB_LISTENADDRESS, so NB_LISTEN_ADDRESS remained effective") + }, + }, + { + name: "empty healthcheck alias keeps legacy name", + env: map[string]string{"NB_HEALTHCHECKLISTENADDRESS": "", "NB_HEALTH_LISTEN_ADDRESS": ":1"}, + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, ":1", cfg.HealthcheckListenAddress, + "Legacy never read NB_HEALTHCHECKLISTENADDRESS, so NB_HEALTH_LISTEN_ADDRESS remained effective") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + for name, value := range test.env { + t.Setenv(name, value) + } + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + test.validate(t, cfg) + }) + } +} + +func TestLegacyRelayHasNoConfigFlag(t *testing.T) { + require.NoError(t, rootCmd.ParseFlags(nil)) + + configFlag := rootCmd.Flags().Lookup("config") + assert.Nil(t, configFlag, "Legacy Relay registered no --config flag, so --help did not list it") + + if configFlag != nil { + oldValue := configFlag.Value.String() + oldChanged := configFlag.Changed + oldConfigPath := configPath + t.Cleanup(func() { + require.NoError(t, configFlag.Value.Set(oldValue)) + configFlag.Changed = oldChanged + configPath = oldConfigPath + }) + } + + err := rootCmd.ParseFlags([]string{"--config", filepath.Join(t.TempDir(), "relay.yaml")}) + if assert.Error(t, err, "Legacy Relay rejected --config on the command line and exited with 'failed to execute command'") { + assert.Contains(t, err.Error(), "unknown flag: --config", "Legacy cobra reported --config as an unknown flag") + } +} + +func TestLoadConfigPreservesLegacyNBConfigIgnored(t *testing.T) { + clearRelayConfigEnvironment(t) + path := filepath.Join(t.TempDir(), "relay.yaml") + require.NoError(t, os.WriteFile(path, []byte("authSecret: file-secret\n"), 0o600)) + t.Setenv("NB_CONFIG", path) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Empty(t, cfg.AuthSecret, "Legacy Relay never read NB_CONFIG, so no configuration file was loaded from it") +} + +func TestLoadConfigPreservesLegacyNilDomainsDefault(t *testing.T) { + clearRelayConfigEnvironment(t) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Nil(t, cfg.LetsencryptDomains, + "Legacy left LetsencryptDomains nil when neither NB_LETSENCRYPT_DOMAINS nor --letsencrypt-domains was given, and Route53TLS.Domains received nil") +} + +func TestLoadConfigPreservesLegacyEnvironmentParseDiagnostics(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_METRICS_PORT", "abc") + + logger := log.StandardLogger() + oldOutput := logger.Out + oldLevel := logger.GetLevel() + var logs bytes.Buffer + logger.SetOutput(&logs) + logger.SetLevel(log.TraceLevel) + t.Cleanup(func() { + logger.SetOutput(oldOutput) + logger.SetLevel(oldLevel) + }) + + _, err := loadRelayConfigWithoutFile(t) + assert.NoError(t, err, "Legacy rejected environment values were logged and ignored; startup continued") + assert.Contains(t, logs.String(), "unable to configure flag metrics-port using variable NB_METRICS_PORT", + "Legacy emitted an Info diagnostic naming the flag and the NB_ variable when an environment value was rejected") +} + +func setRelaySliceFlag(t *testing.T, name string, values []string) { + t.Helper() + + require.NoError(t, rootCmd.ParseFlags(nil)) + flag := rootCmd.Flags().Lookup(name) + require.NotNil(t, flag, "flag %s should be registered", name) + sliceValue, ok := flag.Value.(pflag.SliceValue) + require.True(t, ok, "flag %s should be a slice flag", name) + + oldValues := sliceValue.GetSlice() + oldChanged := flag.Changed + t.Cleanup(func() { + require.NoError(t, sliceValue.Replace(oldValues)) + flag.Changed = oldChanged + }) + require.NoError(t, sliceValue.Replace(values)) + flag.Changed = true +} + +func TestLoadConfigPreservesLegacyQuotedDomainListElements(t *testing.T) { + tests := []struct { + name string + value string + expected []string + }{ + {name: "single quoted element", value: `"relay.example.com"`, expected: []string{"relay.example.com"}}, + {name: "quoted trailing element", value: `a.example.com,"b.example.com"`, expected: []string{"a.example.com", "b.example.com"}}, + {name: "csv escaped double quote", value: `"a""b"`, expected: []string{`a"b`}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_LETSENCRYPT_DATA_DIR", "/var/lib/relay/certs") + t.Setenv("NB_LETSENCRYPT_DOMAINS", test.value) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Equal(t, test.expected, cfg.LetsencryptDomains, + "Legacy pflag readAsCSV used encoding/csv, which stripped surrounding quotes and unescaped doubled quotes in %q, so the Let's Encrypt host whitelist held the clean hostname", test.value) + assert.True(t, cfg.HasLetsEncrypt(), "Legacy still entered the Let's Encrypt path for a quoted domain list") + }) + } +} + +func TestLoadConfigPreservesLegacyTrailingNewlineInDomainList(t *testing.T) { + tests := []struct { + name string + value string + expected []string + }{ + {name: "single domain", value: "relay.example.com\n", expected: []string{"relay.example.com"}}, + {name: "multiple domains", value: "a.example.com,b.example.com\n", expected: []string{"a.example.com", "b.example.com"}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearRelayConfigEnvironment(t) + t.Setenv("NB_LETSENCRYPT_DATA_DIR", "/var/lib/relay/certs") + t.Setenv("NB_LETSENCRYPT_DOMAINS", test.value) + + cfg, err := loadRelayConfigWithoutFile(t) + require.NoError(t, err) + assert.Equal(t, test.expected, cfg.LetsencryptDomains, + "Legacy pflag readAsCSV let csv.Reader.Read consume the trailing newline in %q as the record terminator, so the Let's Encrypt host whitelist held the clean hostname without a newline", test.value) + assert.True(t, cfg.HasLetsEncrypt(), "Legacy still entered the Let's Encrypt path for a newline terminated domain list") + }) + } +} diff --git a/signal/cmd/config_test.go b/signal/cmd/config_test.go index 163ce1693..f5fb78556 100644 --- a/signal/cmd/config_test.go +++ b/signal/cmd/config_test.go @@ -1,22 +1,186 @@ package cmd import ( + "bytes" "os" + "os/exec" "path/filepath" + "strings" "testing" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestExampleConfig(t *testing.T) { + clearSignalConfigEnvironment(t) require.NoError(t, runCmd.ParseFlags(nil)) cfg, err := loadConfig(runCmd, filepath.Join("..", "config.example.yaml")) require.NoError(t, err) assert.Equal(t, "/etc/netbird/tls.crt", cfg.CertFile, "Example config should load") } +func TestLoadConfigPreservesLegacyDefaults(t *testing.T) { + clearSignalConfigEnvironment(t) + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, defaultConfig(), cfg, "Signal defaults should remain unchanged") +} + +func TestLoadConfigPreservesLegacyEnvironmentBindings(t *testing.T) { + clearSignalConfigEnvironment(t) + t.Setenv("NB_PORT", "10001") + t.Setenv("NB_LETSENCRYPT_DOMAIN", "signal.example.com") + t.Setenv("NB_LETSENCRYPT_EMAIL", "admin@example.com") + t.Setenv("NB_LETSENCRYPT_DATA_DIR", "/var/lib/signal/certs") + t.Setenv("NB_CERT_FILE", "/etc/signal/tls.crt") + t.Setenv("NB_CERT_KEY", "/etc/signal/tls.key") + t.Setenv("NB_PPROF_ADDR", "localhost:6060") + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, &Config{ + Port: 10001, + MetricsPort: 9090, + LetsencryptDomain: "signal.example.com", + LetsencryptEmail: "admin@example.com", + LetsencryptDataDir: "/var/lib/signal/certs", + CertFile: "/etc/signal/tls.crt", + CertKey: "/etc/signal/tls.key", + LogLevel: "info", + LogFile: defaultConfig().LogFile, + PprofAddress: "localhost:6060", + }, cfg, "Every legacy Signal environment binding should remain supported") +} + +func TestLoadConfigIgnoresPreviouslyUnboundEmptyEnvironment(t *testing.T) { + clearSignalConfigEnvironment(t) + t.Setenv("NB_METRICS_PORT", "") + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, defaultConfig().MetricsPort, cfg.MetricsPort, + "An environment variable that was previously unbound should not alter the default when empty") +} + +func TestLoadConfigPreservesLegacyEnvironmentAliasPrecedence(t *testing.T) { + clearSignalConfigEnvironment(t) + t.Setenv("NB_LETSENCRYPT_DATA_DIR", "/preferred-certs") + t.Setenv("NB_SSL_DIR", "/legacy-certs") + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, "/legacy-certs", cfg.LetsencryptDataDir, + "The legacy alias should retain its previous precedence when both variables are set") +} + +func TestSignalPortDefaultsRemainCompatible(t *testing.T) { + tests := []struct { + name string + env map[string]string + expected int + }{ + {name: "no TLS", expected: 80}, + {name: "letsencrypt", env: map[string]string{"NB_LETSENCRYPT_DOMAIN": "signal.example.com"}, expected: 443}, + {name: "certificate pair", env: map[string]string{"NB_CERT_FILE": "/tls.crt", "NB_CERT_KEY": "/tls.key"}, expected: 443}, + {name: "explicit nonzero port", env: map[string]string{"NB_PORT": "10002"}, expected: 10002}, + {name: "explicit zero port", env: map[string]string{"NB_PORT": "0"}, expected: 0}, + {name: "invalid port falls back", env: map[string]string{"NB_PORT": "invalid"}, expected: 80}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := runSignalPreRun(t, test.env) + assert.Equal(t, test.expected, actual, "Signal implicit port selection should retain legacy behavior") + }) + } +} + +func TestSignalFlagAliasesRetainArgumentOrder(t *testing.T) { + tests := []struct { + name string + first string + second string + expected string + }{ + { + name: "legacy alias last", + first: "letsencrypt-data-dir", + second: "ssl-dir", + expected: "/ssl-dir", + }, + { + name: "preferred alias last", + first: "ssl-dir", + second: "letsencrypt-data-dir", + expected: "/letsencrypt-data-dir", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearSignalConfigEnvironment(t) + require.NoError(t, runCmd.ParseFlags(nil)) + firstFlag := runCmd.PersistentFlags().Lookup(test.first) + secondFlag := runCmd.PersistentFlags().Lookup(test.second) + require.NotNil(t, firstFlag, "First alias should be registered") + require.NotNil(t, secondFlag, "Second alias should be registered") + oldDataDir := signalLetsencryptDataDir + oldFirstValue, oldFirstChanged := firstFlag.Value.String(), firstFlag.Changed + oldSecondValue, oldSecondChanged := secondFlag.Value.String(), secondFlag.Changed + t.Cleanup(func() { + require.NoError(t, firstFlag.Value.Set(oldFirstValue)) + firstFlag.Changed = oldFirstChanged + require.NoError(t, secondFlag.Value.Set(oldSecondValue)) + secondFlag.Changed = oldSecondChanged + signalLetsencryptDataDir = oldDataDir + }) + + require.NoError(t, firstFlag.Value.Set("/"+test.first)) + firstFlag.Changed = true + require.NoError(t, secondFlag.Value.Set("/"+test.second)) + secondFlag.Changed = true + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, test.expected, cfg.LetsencryptDataDir, + "When both CLI aliases are supplied, the last value should retain precedence") + }) + } +} + +func TestLegacySignalFlagsRemainRegistered(t *testing.T) { + for _, name := range []string{ + "port", + "letsencrypt-data-dir", + "ssl-dir", + "letsencrypt-domain", + "letsencrypt-email", + "cert-file", + "cert-key", + } { + flag := runCmd.PersistentFlags().Lookup(name) + require.NotNil(t, flag, "Legacy persistent flag %s should remain registered", name) + assert.Empty(t, flag.Shorthand, "Legacy Signal flag %s should remain without a shorthand", name) + } + metricsFlag := runCmd.Flags().Lookup("metrics-port") + require.NotNil(t, metricsFlag, "Legacy metrics flag should remain registered") + assert.Empty(t, metricsFlag.Shorthand, "Legacy metrics flag should remain without a shorthand") + for _, name := range []string{"log-level", "log-file"} { + flag := rootCmd.PersistentFlags().Lookup(name) + require.NotNil(t, flag, "Legacy root flag %s should remain registered", name) + assert.Empty(t, flag.Shorthand, "Legacy Signal flag %s should remain without a shorthand", name) + } +} + func TestLoadConfigPrecedence(t *testing.T) { + clearSignalConfigEnvironment(t) configPath := filepath.Join(t.TempDir(), "signal.yaml") require.NoError(t, os.WriteFile(configPath, []byte(` port: 10001 @@ -28,7 +192,7 @@ pprofAddress: localhost:6060 t.Setenv("NB_SSL_DIR", "/legacy-certs") require.NoError(t, runCmd.ParseFlags(nil)) - portFlag := runCmd.Flags().Lookup("port") + portFlag := runCmd.PersistentFlags().Lookup("port") oldPort := portFlag.Value.String() oldChanged := portFlag.Changed t.Cleanup(func() { @@ -46,3 +210,392 @@ pprofAddress: localhost:6060 assert.Equal(t, "/legacy-certs", cfg.LetsencryptDataDir, "Legacy environment aliases should remain supported") assert.Equal(t, "localhost:6060", cfg.PprofAddress, "Environment-only settings should load from the file") } + +func runSignalPreRun(t *testing.T, environment map[string]string) int { + t.Helper() + + clearSignalConfigEnvironment(t) + for name, value := range environment { + t.Setenv(name, value) + } + t.Setenv("NB_LOG_FILE", "console") + require.NoError(t, runCmd.ParseFlags(nil)) + require.NoError(t, executeSignalPreRun(t, "0", false)) + return signalPort +} + +// executeSignalPreRun snapshots the Signal runtime globals, the port flag and the standard logger, +// forces the port flag into the requested state, runs runCmd.PreRunE against the current +// environment and returns its error. Callers prepare the environment and call ParseFlags first. +func executeSignalPreRun(t *testing.T, portValue string, portChanged bool) error { + t.Helper() + + oldSignalPort := signalPort + oldMetricsPort := metricsPort + oldLetsencryptDomain := signalLetsencryptDomain + oldLetsencryptEmail := signalLetsencryptEmail + oldLetsencryptDataDir := signalLetsencryptDataDir + oldCertFile := signalCertFile + oldCertKey := signalCertKey + oldLogLevel := logLevel + oldLogFile := logFile + oldPprofAddress := signalPprofAddress + oldConfigPath := signalConfigPath + portFlag := runCmd.PersistentFlags().Lookup("port") + require.NotNil(t, portFlag, "Signal port flag should be registered") + oldPortValue := portFlag.Value.String() + oldPortChanged := portFlag.Changed + logger := log.StandardLogger() + oldLoggerLevel, oldLoggerOut, oldLoggerFormatter := logger.GetLevel(), logger.Out, logger.Formatter + t.Cleanup(func() { + logger.SetLevel(oldLoggerLevel) + logger.SetOutput(oldLoggerOut) + logger.SetFormatter(oldLoggerFormatter) + require.NoError(t, portFlag.Value.Set(oldPortValue)) + portFlag.Changed = oldPortChanged + signalPort = oldSignalPort + metricsPort = oldMetricsPort + signalLetsencryptDomain = oldLetsencryptDomain + signalLetsencryptEmail = oldLetsencryptEmail + signalLetsencryptDataDir = oldLetsencryptDataDir + signalCertFile = oldCertFile + signalCertKey = oldCertKey + logLevel = oldLogLevel + logFile = oldLogFile + signalPprofAddress = oldPprofAddress + signalConfigPath = oldConfigPath + }) + + signalConfigPath = "" + require.NoError(t, portFlag.Value.Set(portValue)) + portFlag.Changed = portChanged + return runCmd.PreRunE(runCmd, nil) +} + +func clearSignalConfigEnvironment(t *testing.T) { + t.Helper() + + for _, name := range []string{ + "NB_PORT", + "NB_METRICS_PORT", + "NB_LETSENCRYPT_DOMAIN", + "NB_LETSENCRYPT_EMAIL", + "NB_LETSENCRYPT_DATA_DIR", + "NB_SSL_DIR", + "NB_CERT_FILE", + "NB_CERT_KEY", + "NB_LOG_LEVEL", + "NB_LOG_FILE", + "NB_PPROF_ADDR", + // Collapsed camelCase spellings that the shared loader derives automatically; they never + // existed in the legacy NB_ mapping, so the suite must not inherit them either. + "NB_METRICSPORT", + "NB_LETSENCRYPTDOMAIN", + "NB_LETSENCRYPTEMAIL", + "NB_LETSENCRYPTDATADIR", + "NB_CERTFILE", + "NB_CERTKEY", + "NB_LOGLEVEL", + "NB_LOGFILE", + "NB_PPROFADDRESS", + } { + t.Setenv(name, "") + require.NoError(t, os.Unsetenv(name)) + } +} + +func TestLoadConfigIgnoresPreviouslyUnboundMetricsPortEnvironment(t *testing.T) { + tests := []struct { + name string + value string + }{ + {name: "valid value", value: "9191"}, + {name: "invalid value", value: "abc"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearSignalConfigEnvironment(t) + t.Setenv("NB_METRICS_PORT", test.value) + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + assert.NoError(t, err, + "metrics-port was a non-persistent flag that the legacy NB_ mapping never visited, so NB_METRICS_PORT could not fail startup") + if err != nil { + return + } + assert.Equal(t, defaultConfig().MetricsPort, cfg.MetricsPort, + "metrics-port was a non-persistent flag that the legacy NB_ mapping never visited, so NB_METRICS_PORT must not move the metrics endpoint") + }) + } +} + +func TestLoadConfigPreservesLegacyEnvironmentAliasPrecedenceWithEmptyValues(t *testing.T) { + tests := []struct { + name string + env map[string]string + expected string + }{ + { + name: "empty preferred alias keeps legacy alias", + env: map[string]string{"NB_LETSENCRYPT_DATA_DIR": "", "NB_SSL_DIR": "/legacy-certs"}, + expected: "/legacy-certs", + }, + { + name: "empty legacy alias clears preferred alias", + env: map[string]string{"NB_LETSENCRYPT_DATA_DIR": "/preferred-certs", "NB_SSL_DIR": ""}, + expected: "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearSignalConfigEnvironment(t) + for name, value := range test.env { + t.Setenv(name, value) + } + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, test.expected, cfg.LetsencryptDataDir, + "Legacy applied every present NB_ variable in flag order (letsencrypt-data-dir, then ssl-dir) into the same variable, so the last present alias won even when it was empty") + }) + } +} + +func TestLoadConfigIgnoresPreviouslyUnboundLogEnvironment(t *testing.T) { + tests := []struct { + name string + env map[string]string + field string + }{ + {name: "valid log level", env: map[string]string{"NB_LOG_LEVEL": "debug"}}, + {name: "invalid log level", env: map[string]string{"NB_LOG_LEVEL": "verbose"}}, + {name: "empty log level", env: map[string]string{"NB_LOG_LEVEL": ""}}, + {name: "console log file", env: map[string]string{"NB_LOG_FILE": "console"}}, + {name: "syslog log file", env: map[string]string{"NB_LOG_FILE": "syslog"}}, + {name: "writable log file", env: map[string]string{"NB_LOG_FILE": filepath.Join(t.TempDir(), "signal.log")}}, + {name: "unwritable log file", env: map[string]string{"NB_LOG_FILE": filepath.Join(t.TempDir(), "missing", "dir", "signal.log")}}, + {name: "empty log file", env: map[string]string{"NB_LOG_FILE": ""}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearSignalConfigEnvironment(t) + for name, value := range test.env { + t.Setenv(name, value) + } + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, defaultConfig().LogLevel, cfg.LogLevel, + "log-level lived on the root command; the legacy NB_ mapping only visited run persistent flags, so NB_LOG_LEVEL was ignored and the level stayed info") + assert.Equal(t, defaultConfig().LogFile, cfg.LogFile, + "log-file lived on the root command; the legacy NB_ mapping only visited run persistent flags, so NB_LOG_FILE was ignored and logs went to the default file") + }) + } +} + +func TestSignalPreRunIgnoresPreviouslyUnboundLogLevelEnvironment(t *testing.T) { + tests := []struct { + name string + value string + }{ + {name: "valid log level", value: "debug"}, + {name: "invalid log level", value: "verbose"}, + {name: "empty log level", value: ""}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearSignalConfigEnvironment(t) + t.Setenv("NB_LOG_LEVEL", test.value) + t.Setenv("NB_LOG_FILE", "console") + require.NoError(t, runCmd.ParseFlags(nil)) + + err := executeSignalPreRun(t, "0", false) + assert.NoError(t, err, + "NB_LOG_LEVEL was never applied by the legacy NB_ mapping, so any value left the service starting at level info") + assert.Equal(t, "info", logLevel, + "NB_LOG_LEVEL was never applied by the legacy NB_ mapping, so the effective level must remain info") + if err == nil { + assert.Equal(t, log.InfoLevel, log.StandardLogger().GetLevel(), + "Legacy initialized the logger with the flag-only level info regardless of NB_LOG_LEVEL") + } + }) + } +} + +func TestLoadConfigIgnoresCollapsedEnvironmentNames(t *testing.T) { + clearSignalConfigEnvironment(t) + t.Setenv("NB_METRICSPORT", "1234") + t.Setenv("NB_LETSENCRYPTDOMAIN", "auto.example.com") + t.Setenv("NB_LETSENCRYPTEMAIL", "auto@example.com") + t.Setenv("NB_LETSENCRYPTDATADIR", "/auto-certs") + t.Setenv("NB_CERTFILE", "/auto/tls.crt") + t.Setenv("NB_CERTKEY", "/auto/tls.key") + t.Setenv("NB_LOGLEVEL", "trace") + t.Setenv("NB_LOGFILE", "console") + t.Setenv("NB_PPROFADDRESS", "localhost:1") + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, defaultConfig(), cfg, + "Legacy only mapped NB_ with underscores between words; collapsed camelCase spellings such as NB_METRICSPORT did not exist and were ignored") +} + +func TestLoadConfigPreservesDocumentedEnvironmentNamesOverCollapsedNames(t *testing.T) { + tests := []struct { + name string + env map[string]string + actual func(*Config) any + expected any + }{ + { + name: "pprof address", + env: map[string]string{"NB_PPROFADDRESS": "auto:2", "NB_PPROF_ADDR": "explicit:1"}, + actual: func(cfg *Config) any { return cfg.PprofAddress }, + expected: "explicit:1", + }, + { + name: "metrics port", + env: map[string]string{"NB_METRICSPORT": "7777", "NB_METRICS_PORT": "8888"}, + actual: func(cfg *Config) any { return cfg.MetricsPort }, + expected: defaultConfig().MetricsPort, + }, + { + name: "letsencrypt data dir", + env: map[string]string{"NB_LETSENCRYPTDATADIR": "/auto-certs", "NB_LETSENCRYPT_DATA_DIR": "/preferred-certs", "NB_SSL_DIR": "/legacy-certs"}, + actual: func(cfg *Config) any { return cfg.LetsencryptDataDir }, + expected: "/legacy-certs", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearSignalConfigEnvironment(t) + for name, value := range test.env { + t.Setenv(name, value) + } + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, "") + require.NoError(t, err) + assert.Equal(t, test.expected, test.actual(cfg), + "Legacy read only the documented NB_ names (NB_PPROF_ADDR directly, NB_ for persistent flags); an undocumented collapsed spelling must not override them") + }) + } +} + +func TestSignalExplicitZeroPortFlagRemainsCompatible(t *testing.T) { + tests := []struct { + name string + env map[string]string + }{ + {name: "no TLS"}, + {name: "letsencrypt", env: map[string]string{"NB_LETSENCRYPT_DOMAIN": "signal.example.com"}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearSignalConfigEnvironment(t) + for name, value := range test.env { + t.Setenv(name, value) + } + t.Setenv("NB_LOG_FILE", "console") + require.NoError(t, runCmd.ParseFlags(nil)) + + require.NoError(t, executeSignalPreRun(t, "0", true)) + assert.Equal(t, 0, signalPort, + "Legacy skipped the 80/443 default heuristic whenever the port flag was Changed, so `--port 0` kept 0 and bound an ephemeral port") + }) + } +} + +func TestLegacySignalRunCommandHadNoConfigFlag(t *testing.T) { + clearSignalConfigEnvironment(t) + require.NoError(t, runCmd.ParseFlags(nil)) + + configFlag := runCmd.PersistentFlags().Lookup("config") + assert.Nil(t, configFlag, "Legacy Signal had no --config flag; cobra rejected it as an unknown flag") + + if configFlag != nil { + oldValue, oldChanged := configFlag.Value.String(), configFlag.Changed + oldConfigPath := signalConfigPath + t.Cleanup(func() { + require.NoError(t, configFlag.Value.Set(oldValue)) + configFlag.Changed = oldChanged + signalConfigPath = oldConfigPath + }) + } + + err := runCmd.ParseFlags([]string{"--config", filepath.Join(t.TempDir(), "signal.yaml")}) + assert.Error(t, err, "Legacy Signal rejected `run --config ` with `unknown flag: --config` and exited with a usage error") +} + +func TestSignalPortFlagHelpRetainsLegacyDefault(t *testing.T) { + portFlag := runCmd.PersistentFlags().Lookup("port") + require.NotNil(t, portFlag, "Signal port flag should be registered") + + assert.Equal(t, "80", portFlag.DefValue, + "Legacy registered --port with default 80, so `run --help` rendered a `(default 80)` suffix") + assert.Regexp(t, `--port int\s+Server port to listen on .*\(default 80\)`, runCmd.PersistentFlags().FlagUsages(), + "Legacy `run --help` printed `(default 80)` after the --port description") +} + +func TestLegacySignalInformationalCommandsAppliedEnvironmentAtStartup(t *testing.T) { + tests := []struct { + name string + args []string + }{ + {name: "version", args: []string{"--version"}}, + {name: "run help", args: []string{"run", "--help"}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearSignalConfigEnvironment(t) + + cmd := exec.Command(os.Args[0], "-test.run=^TestSignalHelperProcess$") + cmd.Env = append(os.Environ(), + "NB_SIGNAL_TEST_HELPER_PROCESS=1", + "NB_SIGNAL_TEST_HELPER_ARGS="+strings.Join(test.args, " "), + "NB_PORT=invalid", + ) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + require.NoError(t, cmd.Run(), "helper process should exit cleanly, stdout: %s stderr: %s", stdout.String(), stderr.String()) + + assert.Contains(t, stderr.String(), "unable to configure flag port using variable NB_PORT", + "Legacy applied NB_ variables to flags in init() for every invocation, so an invalid NB_PORT logged a warning even for informational commands") + }) + } +} + +// TestSignalHelperProcess executes the Signal root command inside a child test process. It is only +// active when spawned by TestLegacySignalInformationalCommandsAppliedEnvironmentAtStartup. +func TestSignalHelperProcess(t *testing.T) { + if os.Getenv("NB_SIGNAL_TEST_HELPER_PROCESS") != "1" { + t.Skip("helper process for subprocess-based tests") + } + + rootCmd.SetArgs(strings.Fields(os.Getenv("NB_SIGNAL_TEST_HELPER_ARGS"))) + require.NoError(t, rootCmd.Execute()) +} + +func TestLoadConfigFileCannotEnablePprofAsLegacy(t *testing.T) { + clearSignalConfigEnvironment(t) + configPath := filepath.Join(t.TempDir(), "signal.yaml") + require.NoError(t, os.WriteFile(configPath, []byte("pprofAddress: localhost:6060\n"), 0o600)) + require.NoError(t, runCmd.ParseFlags(nil)) + + cfg, err := loadConfig(runCmd, configPath) + require.NoError(t, err) + assert.Equal(t, "", cfg.PprofAddress, + "Legacy enabled pprof only from os.Getenv(\"NB_PPROF_ADDR\") at run time; no file source could turn it on") +}