diff --git a/management/cmd/admin.go b/management/cmd/admin.go index 8de7e70aa..e5c0f6ac9 100644 --- a/management/cmd/admin.go +++ b/management/cmd/admin.go @@ -114,8 +114,8 @@ func withAdminConfig(cmd *cobra.Command, applyIDPDefaults bool, fn func(ctx cont } func loadAdminMgmtConfig(ctx context.Context, applyIDPDefaults bool) (*nbconfig.Config, string, error) { - config, err := decodeManagementConfig(nbconfig.MgmtConfigPath, &nbconfig.Config{}) - if err != nil { + config := &nbconfig.Config{} + if _, err := util.ReadJsonWithEnvSub(nbconfig.MgmtConfigPath, config); err != nil { return nil, "", err } diff --git a/management/cmd/config.go b/management/cmd/config.go deleted file mode 100644 index 4a7c76725..000000000 --- a/management/cmd/config.go +++ /dev/null @@ -1,25 +0,0 @@ -package cmd - -import ( - nbconfig "github.com/netbirdio/netbird/management/internals/server/config" - configloader "github.com/netbirdio/netbird/util/config" - "github.com/netbirdio/netbird/util/envtemplate" -) - -func loadManagementConfig(configPath string) (*nbconfig.Config, error) { - cfg, err := decodeManagementConfig(configPath, &nbconfig.Config{Datadir: defaultMgmtDataDir}) - if err != nil { - return nil, err - } - if cfg.Datadir == "" { - cfg.Datadir = defaultMgmtDataDir - } - return cfg, nil -} - -func decodeManagementConfig(configPath string, defaults *nbconfig.Config) (*nbconfig.Config, error) { - return configloader.Load(configPath, defaults, configloader.Options{ - TagName: "json", - Transform: envtemplate.Expand, - }) -} diff --git a/management/cmd/config_test.go b/management/cmd/config_test.go deleted file mode 100644 index b8e2cb974..000000000 --- a/management/cmd/config_test.go +++ /dev/null @@ -1,1764 +0,0 @@ -package cmd - -import ( - "bytes" - "context" - "encoding/json" - "os" - "path/filepath" - "reflect" - "strings" - "testing" - - "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)) - - cfg, err := loadManagementConfig(configPath) - require.NoError(t, err) - 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) - } -} - -// 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-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/management/cmd/management.go b/management/cmd/management.go index ba0decc7c..147985314 100644 --- a/management/cmd/management.go +++ b/management/cmd/management.go @@ -19,7 +19,6 @@ import ( log "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "github.com/spf13/pflag" "github.com/netbirdio/netbird/management/server/types" @@ -61,7 +60,7 @@ var ( // detect whether user specified a port userPort := cmd.Flag("port").Changed - config, err = LoadMgmtConfig(ctx, nbconfig.MgmtConfigPath, cmd.Flags()) + config, err = LoadMgmtConfig(ctx, nbconfig.MgmtConfigPath) if err != nil { return fmt.Errorf("failed reading provided config file: %s: %v", nbconfig.MgmtConfigPath, err) } @@ -172,15 +171,15 @@ var ( } ) -func LoadMgmtConfig(ctx context.Context, mgmtConfigPath string, flags *pflag.FlagSet) (*nbconfig.Config, error) { - loadedConfig, err := loadManagementConfig(mgmtConfigPath) - if err != nil { +func LoadMgmtConfig(ctx context.Context, mgmtConfigPath string) (*nbconfig.Config, error) { + loadedConfig := &nbconfig.Config{} + if _, err := util.ReadJsonWithEnvSub(mgmtConfigPath, loadedConfig); err != nil { return nil, err } - ApplyCommandLineOverrides(loadedConfig, flags) + ApplyCommandLineOverrides(loadedConfig) - err = grpc.ValidateSyncMessageVersion(loadedConfig.HighestSupportedSyncMessageVersion) + err := grpc.ValidateSyncMessageVersion(loadedConfig.HighestSupportedSyncMessageVersion) if err != nil { return nil, err } @@ -212,18 +211,14 @@ func LoadMgmtConfig(ctx context.Context, mgmtConfigPath string, flags *pflag.Fla } // ApplyCommandLineOverrides applies command-line flag overrides to the config -func ApplyCommandLineOverrides(cfg *nbconfig.Config, _ *pflag.FlagSet) { - hasCertOverride := certKey != "" && certFile != "" - if (mgmtLetsencryptDomain != "" || hasCertOverride) && cfg.HttpConfig == nil { - cfg.HttpConfig = &nbconfig.HttpServerConfig{} - } +func ApplyCommandLineOverrides(cfg *nbconfig.Config) { if mgmtLetsencryptDomain != "" { cfg.HttpConfig.LetsEncryptDomain = mgmtLetsencryptDomain } if mgmtDataDir != "" { cfg.Datadir = mgmtDataDir } - if hasCertOverride { + if certKey != "" && certFile != "" { cfg.HttpConfig.CertFile = certFile cfg.HttpConfig.CertKey = certKey } @@ -378,17 +373,11 @@ func EnsureEncryptionKey(ctx context.Context, configPath string, cfg *nbconfig.C if err != nil { return fmt.Errorf("failed to generate datastore encryption key: %v", err) } + cfg.DataStoreEncryptionKey = key - fileConfig, err := decodeManagementConfig(configPath, &nbconfig.Config{}) - if err != nil { - return fmt.Errorf("reload config before saving encryption key: %w", err) - } - fileConfig.DataStoreEncryptionKey = key - if err := util.DirectWriteJson(ctx, configPath, fileConfig); err != nil { + if err := util.DirectWriteJson(ctx, configPath, cfg); err != nil { return fmt.Errorf("failed to save config with new encryption key: %v", err) } - - cfg.DataStoreEncryptionKey = key log.WithContext(ctx).Infof("DataStoreEncryptionKey generated and saved to config") return nil } diff --git a/management/cmd/management_test.go b/management/cmd/management_test.go index c95986ce2..2c3481213 100644 --- a/management/cmd/management_test.go +++ b/management/cmd/management_test.go @@ -36,7 +36,7 @@ func Test_LoadMgmtConfig(t *testing.T) { tmpFile, err := createConfig(exampleConfig) assert.NoError(t, err) - cfg, err := LoadMgmtConfig(context.Background(), tmpFile, mgmtCmd.Flags()) + cfg, err := LoadMgmtConfig(context.Background(), tmpFile) assert.NoError(t, err) assert.NotEmpty(t, cfg.Relay) assert.NotEmpty(t, cfg.Relay.Addresses) @@ -54,7 +54,7 @@ func Test_LoadMgmtConfig_Empty(t *testing.T) { }`) assert.NoError(t, err) - cfg, err := LoadMgmtConfig(context.Background(), tmpFile, mgmtCmd.Flags()) + cfg, err := LoadMgmtConfig(context.Background(), tmpFile) assert.NoError(t, err) assert.Nil(t, cfg.HighestSupportedSyncMessageVersion) assert.Nil(t, cfg.PerAccountHighestSupportedSyncMessageVersion) diff --git a/proxy/cmd/proxy/cmd/config.go b/proxy/cmd/proxy/cmd/config.go deleted file mode 100644 index fe90537da..000000000 --- a/proxy/cmd/proxy/cmd/config.go +++ /dev/null @@ -1,84 +0,0 @@ -package cmd - -import ( - "fmt" - "os" - "strconv" - - "github.com/spf13/cobra" - "golang.org/x/crypto/acme" - - "github.com/netbirdio/netbird/proxy" - "github.com/netbirdio/netbird/trustedproxy" - configloader "github.com/netbirdio/netbird/util/config" -) - -type commandConfig struct { - proxy.Config `yaml:",inline"` - - LogLevel string `yaml:"logLevel" env:"NB_PROXY_LOG_LEVEL" flag:"log-level"` - PreallocatedBuffers *uint32 `yaml:"preallocatedBuffers" env:"-"` - MaxBatchSize *uint32 `yaml:"maxBatchSize" env:"-"` -} - -func defaultConfig() *commandConfig { - return &commandConfig{ - Config: proxy.Config{ - ListenAddr: ":443", - ManagementAddress: DefaultManagementURL, - CertificateDirectory: "./certs", - CertificateFile: "tls.crt", - CertificateKeyFile: "tls.key", - ACMEChallengeAddress: ":80", - ACMEDirectory: acme.LetsEncryptURL, - ACMEChallengeType: "tls-alpn-01", - CertLockMethod: "auto", - DebugEndpointAddress: "localhost:8444", - HealthAddr: "localhost:8080", - TrustedProxies: trustedproxy.FromPrefixes(nil), - ForwardedProto: "auto", - SupportsCustomPorts: true, - GeoDataDir: "/var/lib/netbird/geolocation", - }, - LogLevel: "info", - } -} - -func loadConfig(cmd *cobra.Command, configPath string) (*commandConfig, error) { - cfg, err := configloader.Load(configPath, defaultConfig(), configloader.Options{ - TagName: "yaml", - AllowMissing: configPath == "", - FlagSet: cmd.Flags(), - Strict: true, - InvalidEnvironment: configloader.InvalidEnvironmentIgnore, - }) - if err != nil { - return nil, err - } - if err := applyPerformanceEnvironment(cfg); err != nil { - return nil, err - } - return cfg, nil -} - -func applyPerformanceEnvironment(cfg *commandConfig) error { - for _, setting := range []struct { - name string - target **uint32 - }{ - {name: envPreallocatedBuffers, target: &cfg.PreallocatedBuffers}, - {name: envMaxBatchSize, target: &cfg.MaxBatchSize}, - } { - raw := os.Getenv(setting.name) - if raw == "" { - continue - } - parsed, err := strconv.ParseUint(raw, 10, 32) - if err != nil { - return fmt.Errorf("invalid %s %q: %w", setting.name, raw, err) - } - value := uint32(parsed) - *setting.target = &value - } - return nil -} diff --git a/proxy/cmd/proxy/cmd/config_test.go b/proxy/cmd/proxy/cmd/config_test.go deleted file mode 100644 index 20c93756b..000000000 --- a/proxy/cmd/proxy/cmd/config_test.go +++ /dev/null @@ -1,923 +0,0 @@ -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) { - 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 -trustedProxies: 192.0.2.0/24 -maxDialTimeout: 5s -`), 0o600)) - t.Setenv("NB_PROXY_TOKEN", "environment-token") - cmd := newLegacyProxyCommand(t) - - domainFlag := cmd.Flags().Lookup("domain") - require.NoError(t, domainFlag.Value.Set("flag.example.com")) - domainFlag.Changed = true - - 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") - assert.Equal(t, 5*time.Second, cfg.MaxDialTimeout, "Durations should be decoded from the file") - 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") - }) - } -} - -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/proxy/cmd/proxy/cmd/root.go b/proxy/cmd/proxy/cmd/root.go index 65fc586fc..9b180a5c4 100644 --- a/proxy/cmd/proxy/cmd/root.go +++ b/proxy/cmd/proxy/cmd/root.go @@ -15,7 +15,10 @@ import ( "github.com/netbirdio/netbird/shared/management/domain" + "github.com/netbirdio/netbird/client/embed" "github.com/netbirdio/netbird/proxy" + nbacme "github.com/netbirdio/netbird/proxy/internal/acme" + "github.com/netbirdio/netbird/trustedproxy" "github.com/netbirdio/netbird/util" ) @@ -23,7 +26,10 @@ const ( // envPreallocatedBuffers caps the per-tunnel buffer pool. Zero (unset) // keeps the upstream uncapped default. envPreallocatedBuffers = "NB_PROXY_PREALLOCATED_BUFFERS" - envMaxBatchSize = "NB_PROXY_MAX_BATCH_SIZE" + // envMaxBatchSize overrides the per-tunnel batch size, which controls + // how many buffers each receive/TUN worker eagerly allocates. Zero + // (unset) keeps the platform default. + envMaxBatchSize = "NB_PROXY_MAX_BATCH_SIZE" ) const DefaultManagementURL = "https://api.netbird.io:443" @@ -73,7 +79,6 @@ var ( geoDataDir string crowdsecAPIURL string crowdsecAPIKey string - configPath string ) var rootCmd = &cobra.Command{ @@ -86,7 +91,6 @@ var rootCmd = &cobra.Command{ } func init() { - rootCmd.Flags().StringVar(&configPath, "config", "", "path to configuration file") rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", envStringOrDefault("NB_PROXY_LOG_LEVEL", "info"), "Log level: panic, fatal, error, warn, info, debug, trace") rootCmd.PersistentFlags().BoolVar(&debugLogs, "debug", envBoolOrDefault("NB_PROXY_DEBUG_LOGS", false), "Enable debug logs") _ = rootCmd.PersistentFlags().MarkDeprecated("debug", "use --log-level instead") @@ -146,66 +150,113 @@ func runServer(cmd *cobra.Command, args []string) error { return fmt.Errorf("proxy token is required: set %s environment variable", envProxyToken) } - cfg, err := loadConfig(cmd, configPath) - if err != nil { - return fmt.Errorf("load config: %w", err) - } - cfg.ProxyToken = proxyToken - - level := cfg.LogLevel + level := logLevel if debugLogs { level = "debug" } logger := log.New() + _ = util.InitLogger(logger, level, util.LogConsole) + logger.Infof("configured log level: %s", level) - proxyConfig := cfg.Config - proxyConfig.Logger = logger - proxyConfig.Version = Version - applyPerformanceConfig(&proxyConfig, cfg, logger) - - switch proxyConfig.ForwardedProto { - case "auto", "http", "https": - default: - return fmt.Errorf("invalid --forwarded-proto value %q: must be auto, http, or https", proxyConfig.ForwardedProto) + var wgPool, wgBatch uint64 + var perf embed.Performance + if raw := os.Getenv(envPreallocatedBuffers); raw != "" { + n, err := strconv.ParseUint(raw, 10, 32) + if err != nil { + return fmt.Errorf("invalid %s %q: %w", envPreallocatedBuffers, raw, err) + } + wgPool = n + v := uint32(n) + perf.PreallocatedBuffersPerPool = &v + logger.Infof("tunnel preallocated buffers per pool: %d", n) + } + if raw := os.Getenv(envMaxBatchSize); raw != "" { + n, err := strconv.ParseUint(raw, 10, 32) + if err != nil { + return fmt.Errorf("invalid %s %q: %w", envMaxBatchSize, raw, err) + } + wgBatch = n + v := uint32(n) + perf.MaxBatchSize = &v + logger.Infof("tunnel max batch size override: %d", n) + } + if wgPool > 0 { + // Each bind recv goroutine (IPv4 + IPv6 + ICE relay) plus + // RoutineReadFromTUN eagerly reserves `batch` message buffers for + // the lifetime of the Device. A pool cap below that floor blocks + // the receive pipeline at startup. + batch := wgBatch + if batch == 0 { + batch = 128 + } + const recvGoroutines = 4 + floor := batch * recvGoroutines + if wgPool < floor { + logger.Warnf("%s=%d is below the eager-allocation floor (~%d for batch=%d); startup may deadlock", + envPreallocatedBuffers, wgPool, floor, batch) + } } - if _, err := domain.ValidateDomains([]string{proxyConfig.ProxyURL}); err != nil { - return fmt.Errorf("invalid domain value %q: %w", proxyConfig.ProxyURL, err) + switch forwardedProto { + case "auto", "http", "https": + default: + return fmt.Errorf("invalid --forwarded-proto value %q: must be auto, http, or https", forwardedProto) + } + + _, err := domain.ValidateDomains([]string{proxyDomain}) + if err != nil { + return fmt.Errorf("invalid domain value %q: %w", proxyDomain, err) + } + + parsedTrustedProxies, err := trustedproxy.Parse(trustedProxies) + if err != nil { + return fmt.Errorf("invalid --trusted-proxies: %w", err) } ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT) defer stop() - srv := proxy.New(ctx, proxyConfig) - return srv.ListenAndServe(ctx, proxyConfig.ListenAddr) -} + srv := proxy.New(ctx, proxy.Config{ + ListenAddr: addr, + Logger: logger, + Version: Version, + ManagementAddress: mgmtAddr, + ProxyURL: proxyDomain, + ProxyToken: proxyToken, + CertificateDirectory: certDir, + CertificateFile: certFile, + CertificateKeyFile: certKeyFile, + GenerateACMECertificates: acmeCerts, + ACMEChallengeAddress: acmeAddr, + ACMEDirectory: acmeDir, + ACMEEABKID: acmeEABKID, + ACMEEABHMACKey: acmeEABHMACKey, + ACMEChallengeType: acmeChallengeType, + DebugEndpointEnabled: debugEndpoint, + DebugEndpointAddress: debugEndpointAddr, + HealthAddr: healthAddr, + ForwardedProto: forwardedProto, + TrustedProxies: parsedTrustedProxies, + CertLockMethod: nbacme.CertLockMethod(certLockMethod), + WildcardCertDir: wildcardCertDir, + WireguardPort: wgPort, + Performance: perf, + ProxyProtocol: proxyProtocol, + PreSharedKey: preSharedKey, + SupportsCustomPorts: supportsCustomPorts, + RequireSubdomain: requireSubdomain, + Private: private, + MaxDialTimeout: maxDialTimeout, + MaxSessionIdleTimeout: maxSessionIdleTimeout, + MappingBatchWatchdog: envDurationOrDefault("NB_PROXY_MAPPING_BATCH_WATCHDOG", 0), + GeoDataDir: geoDataDir, + CrowdSecAPIURL: crowdsecAPIURL, + CrowdSecAPIKey: crowdsecAPIKey, + }) -func applyPerformanceConfig(proxyConfig *proxy.Config, cfg *commandConfig, logger *log.Logger) { - if cfg.PreallocatedBuffers != nil { - proxyConfig.Performance.PreallocatedBuffersPerPool = cfg.PreallocatedBuffers - logger.Infof("tunnel preallocated buffers per pool: %d", *cfg.PreallocatedBuffers) - } - if cfg.MaxBatchSize != nil { - proxyConfig.Performance.MaxBatchSize = cfg.MaxBatchSize - logger.Infof("tunnel max batch size override: %d", *cfg.MaxBatchSize) - } - if cfg.PreallocatedBuffers == nil || *cfg.PreallocatedBuffers == 0 { - return - } - - batch := uint64(128) - if cfg.MaxBatchSize != nil && *cfg.MaxBatchSize > 0 { - batch = uint64(*cfg.MaxBatchSize) - } - const recvGoroutines = 4 - floor := batch * recvGoroutines - pool := uint64(*cfg.PreallocatedBuffers) - if pool < floor { - logger.Warnf("%s=%d is below the eager-allocation floor (~%d for batch=%d); startup may deadlock", - envPreallocatedBuffers, pool, floor, batch) - } + return srv.ListenAndServe(ctx, addr) } func envBoolOrDefault(key string, def bool) bool { diff --git a/proxy/config.example.yaml b/proxy/config.example.yaml deleted file mode 100644 index f461af56d..000000000 --- a/proxy/config.example.yaml +++ /dev/null @@ -1,41 +0,0 @@ -# Command-line flags override environment variables, which override this file. -listenAddress: ":443" -id: "proxy-1" -domain: "proxy.example.com" -managementAddress: "https://api.netbird.io:443" -proxyToken: "replace-with-a-proxy-token" - -certificateDirectory: "/var/lib/netbird/certs" -certificateFile: "tls.crt" -certificateKeyFile: "tls.key" -generateACMECertificates: true -acmeChallengeAddress: ":80" -acmeDirectory: "https://acme-v02.api.letsencrypt.org/directory" -acmeEABKID: "" -acmeEABHMACKey: "" -acmeChallengeType: "tls-alpn-01" -certLockMethod: "auto" -wildcardCertDir: "" - -debugEndpointEnabled: false -debugEndpointAddress: "localhost:8444" -healthAddress: "localhost:8080" -forwardedProto: "auto" -trustedProxies: "" -wireguardPort: 0 -proxyProtocol: false -preSharedKey: "" -supportsCustomPorts: true -requireSubdomain: false -private: false -maxDialTimeout: "0s" -maxSessionIdleTimeout: "0s" -mappingBatchWatchdog: "0s" -geoDataDir: "/var/lib/netbird/geolocation" -crowdSecAPIURL: "" -crowdSecAPIKey: "" -logLevel: "info" - -# Optional tunnel memory and throughput tuning. -# preallocatedBuffers: 4096 -# maxBatchSize: 128 diff --git a/proxy/lifecycle.go b/proxy/lifecycle.go index 8b6660d81..f8c74d8b5 100644 --- a/proxy/lifecycle.go +++ b/proxy/lifecycle.go @@ -20,112 +20,112 @@ import ( // adding fields here must not change the zero-value behaviour of Server. type Config struct { // ListenAddr is the TCP address the main listener binds. Required. - ListenAddr string `yaml:"listenAddress" env:"NB_PROXY_ADDRESS" flag:"addr"` + ListenAddr string // ID identifies this proxy instance to management. Empty values are // replaced with a timestamped default at Server.Start time (see // initDefaults), not in New. - ID string `yaml:"id" env:"-"` + ID string // Logger is the logrus logger used everywhere. Empty values fall // back to log.StandardLogger() at Server.Start time (see // initDefaults), not in New. - Logger *log.Logger `yaml:"-" env:"-" flag:"-"` + Logger *log.Logger // Version is the build version string reported to management. Empty // values are replaced with "dev" at Server.Start time (see // initDefaults), not in New. - Version string `yaml:"-" env:"-" flag:"-"` + Version string // ProxyURL is the public address operators use to reach this proxy. - ProxyURL string `yaml:"domain" env:"NB_PROXY_DOMAIN" flag:"domain"` + ProxyURL string // ManagementAddress is the gRPC URL of the management server. - ManagementAddress string `yaml:"managementAddress" env:"NB_PROXY_MANAGEMENT_ADDRESS" flag:"mgmt"` + ManagementAddress string // ProxyToken authenticates this proxy with the management server. - ProxyToken string `yaml:"proxyToken" env:"NB_PROXY_TOKEN"` + ProxyToken string // CertificateDirectory is the directory holding TLS certificate // material (static or ACME-provisioned). - CertificateDirectory string `yaml:"certificateDirectory" env:"NB_PROXY_CERTIFICATE_DIRECTORY" flag:"cert-dir"` + CertificateDirectory string // CertificateFile is the certificate filename within // CertificateDirectory. - CertificateFile string `yaml:"certificateFile" env:"NB_PROXY_CERTIFICATE_FILE" flag:"cert-file"` + CertificateFile string // CertificateKeyFile is the private key filename within // CertificateDirectory. - CertificateKeyFile string `yaml:"certificateKeyFile" env:"NB_PROXY_CERTIFICATE_KEY_FILE" flag:"cert-key-file"` + CertificateKeyFile string // GenerateACMECertificates toggles ACME certificate provisioning. - GenerateACMECertificates bool `yaml:"generateACMECertificates" env:"NB_PROXY_ACME_CERTIFICATES" flag:"acme-certs"` + GenerateACMECertificates bool // ACMEChallengeAddress is the listen address for HTTP-01 challenges. - ACMEChallengeAddress string `yaml:"acmeChallengeAddress" env:"NB_PROXY_ACME_ADDRESS" flag:"acme-addr"` + ACMEChallengeAddress string // ACMEDirectory is the ACME directory URL (Let's Encrypt by default). - ACMEDirectory string `yaml:"acmeDirectory" env:"NB_PROXY_ACME_DIRECTORY" flag:"acme-dir"` + ACMEDirectory string // ACMEEABKID is the External Account Binding Key ID for CAs that // require EAB (e.g. ZeroSSL). - ACMEEABKID string `yaml:"acmeEABKID" env:"NB_PROXY_ACME_EAB_KID" flag:"acme-eab-kid"` + ACMEEABKID string // ACMEEABHMACKey is the External Account Binding HMAC key for CAs // that require EAB. - ACMEEABHMACKey string `yaml:"acmeEABHMACKey" env:"NB_PROXY_ACME_EAB_HMAC_KEY" flag:"acme-eab-hmac-key"` + ACMEEABHMACKey string // ACMEChallengeType is the ACME challenge type ("tls-alpn-01" or // "http-01"). Empty defaults to "tls-alpn-01". - ACMEChallengeType string `yaml:"acmeChallengeType" env:"NB_PROXY_ACME_CHALLENGE_TYPE" flag:"acme-challenge-type"` + ACMEChallengeType string // CertLockMethod controls how ACME certificate locks are coordinated // across replicas. - CertLockMethod acme.CertLockMethod `yaml:"certLockMethod" env:"NB_PROXY_CERT_LOCK_METHOD" flag:"cert-lock-method"` + CertLockMethod acme.CertLockMethod // WildcardCertDir is an optional directory containing static wildcard // certificates that override ACME for matching domains. - WildcardCertDir string `yaml:"wildcardCertDir" env:"NB_PROXY_WILDCARD_CERT_DIR" flag:"wildcard-cert-dir"` + WildcardCertDir string // DebugEndpointEnabled toggles the debug HTTP endpoint. - DebugEndpointEnabled bool `yaml:"debugEndpointEnabled" env:"NB_PROXY_DEBUG_ENDPOINT" flag:"debug-endpoint"` + DebugEndpointEnabled bool // DebugEndpointAddress is the bind address for the debug endpoint. - DebugEndpointAddress string `yaml:"debugEndpointAddress" env:"NB_PROXY_DEBUG_ENDPOINT_ADDRESS" flag:"debug-endpoint-addr"` + DebugEndpointAddress string // HealthAddr is the bind address for the health probe and metrics // surface. Empty disables the health probe entirely (library callers // can attach their own). - HealthAddr string `yaml:"healthAddress" env:"NB_PROXY_HEALTH_ADDRESS" flag:"health-addr"` + HealthAddr string // ForwardedProto overrides the X-Forwarded-Proto value sent to // backends. Valid values: "auto", "http", "https". - ForwardedProto string `yaml:"forwardedProto" env:"NB_PROXY_FORWARDED_PROTO" flag:"forwarded-proto"` + ForwardedProto string // TrustedProxies is the set of trusted upstream proxies that may set // forwarding headers. - TrustedProxies *trustedproxy.List `yaml:"trustedProxies" env:"NB_PROXY_TRUSTED_PROXIES" flag:"trusted-proxies"` + TrustedProxies *trustedproxy.List // WireguardPort is the UDP port for the embedded NetBird tunnel. // Zero asks the OS for a random port. - WireguardPort uint16 `yaml:"wireguardPort" env:"NB_PROXY_WG_PORT" flag:"wg-port"` + WireguardPort uint16 // ProxyProtocol enables PROXY protocol (v1/v2) on TCP listeners. - ProxyProtocol bool `yaml:"proxyProtocol" env:"NB_PROXY_PROXY_PROTOCOL" flag:"proxy-protocol"` + ProxyProtocol bool // PreSharedKey is the WireGuard pre-shared key used between the // proxy's embedded clients and peers. - PreSharedKey string `yaml:"preSharedKey" env:"NB_PROXY_PRESHARED_KEY" flag:"preshared-key"` + PreSharedKey string // Performance configures the tunnel pool/batch sizes for every // embedded client this proxy creates. Zero values fall back to // upstream defaults. - Performance embed.Performance `yaml:"performance" env:"-" flag:"-"` + Performance embed.Performance // SupportsCustomPorts indicates whether the proxy can bind arbitrary // ports for TCP/UDP/TLS services. - SupportsCustomPorts bool `yaml:"supportsCustomPorts" env:"NB_PROXY_SUPPORTS_CUSTOM_PORTS" flag:"supports-custom-ports"` + SupportsCustomPorts bool // RequireSubdomain forces accounts to use a subdomain in front of // the proxy's cluster domain. - RequireSubdomain bool `yaml:"requireSubdomain" env:"NB_PROXY_REQUIRE_SUBDOMAIN" flag:"require-subdomain"` + RequireSubdomain bool // Private flags this proxy as embedded in a netbird client and // serving exclusively over the WireGuard tunnel. Also enables // per-account inbound listeners on each embedded client's netstack. - Private bool `yaml:"private" env:"NB_PROXY_PRIVATE" flag:"private"` + Private bool // MaxDialTimeout caps the per-service backend dial timeout. - MaxDialTimeout time.Duration `yaml:"maxDialTimeout" env:"NB_PROXY_MAX_DIAL_TIMEOUT" flag:"max-dial-timeout"` + MaxDialTimeout time.Duration // MaxSessionIdleTimeout caps the per-service session idle timeout. - MaxSessionIdleTimeout time.Duration `yaml:"maxSessionIdleTimeout" env:"NB_PROXY_MAX_SESSION_IDLE_TIMEOUT" flag:"max-session-idle-timeout"` + MaxSessionIdleTimeout time.Duration // MappingBatchWatchdog bounds how long a single mapping batch may spend // being applied before the receive loop reconnects to resync. Zero falls // back to the internal default. - MappingBatchWatchdog time.Duration `yaml:"mappingBatchWatchdog" env:"NB_PROXY_MAPPING_BATCH_WATCHDOG"` + MappingBatchWatchdog time.Duration // GeoDataDir is the directory containing GeoLite2 MMDB files. - GeoDataDir string `yaml:"geoDataDir" env:"NB_PROXY_GEO_DATA_DIR" flag:"geo-data-dir"` + GeoDataDir string // CrowdSecAPIURL is the CrowdSec LAPI URL. Empty disables CrowdSec. - CrowdSecAPIURL string `yaml:"crowdSecAPIURL" env:"NB_PROXY_CROWDSEC_API_URL" flag:"crowdsec-api-url"` + CrowdSecAPIURL string // CrowdSecAPIKey is the CrowdSec bouncer API key. Empty disables // CrowdSec. - CrowdSecAPIKey string `yaml:"crowdSecAPIKey" env:"NB_PROXY_CROWDSEC_API_KEY" flag:"crowdsec-api-key"` + CrowdSecAPIKey string } // New builds a Server from cfg without performing any I/O. No goroutines diff --git a/relay/cmd/config_test.go b/relay/cmd/config_test.go deleted file mode 100644 index c047eac90..000000000 --- a/relay/cmd/config_test.go +++ /dev/null @@ -1,655 +0,0 @@ -package cmd - -import ( - "math" - "os" - "path/filepath" - "testing" - - "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") - t.Cleanup(func() { - configPath = oldConfigPath - }) - - cfg, err := loadConfig(rootCmd) - require.NoError(t, err) - assert.Equal(t, "rels://relay.example.com:443", cfg.ExposedAddress, "Example config should load") - 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" -exposedAddress: "relay.example.com:443" -authSecret: file-secret -metricsPort: 9091 -enableSTUN: true -stunPorts: [3478, 3479] -`), 0o600)) - t.Setenv("NB_METRICS_PORT", "9191") - require.NoError(t, rootCmd.ParseFlags(nil)) - - listenFlag := rootCmd.Flags().Lookup("listen-address") - oldListen := listenFlag.Value.String() - oldChanged := listenFlag.Changed - t.Cleanup(func() { - require.NoError(t, listenFlag.Value.Set(oldListen)) - listenFlag.Changed = oldChanged - }) - require.NoError(t, listenFlag.Value.Set(":7443")) - listenFlag.Changed = true - - oldConfigPath := configPath - configPath = path - t.Cleanup(func() { - configPath = oldConfigPath - }) - - cfg, err := loadConfig(rootCmd) - require.NoError(t, err) - assert.Equal(t, ":7443", cfg.ListenAddress, "Flags should override the configuration file") - assert.Equal(t, 9191, cfg.MetricsPort, "Environment should override the configuration file") - assert.Equal(t, "relay.example.com:443", cfg.ExposedAddress, "File values should override defaults") - 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 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 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/relay/cmd/env.go b/relay/cmd/env.go new file mode 100644 index 000000000..3c15ebe1f --- /dev/null +++ b/relay/cmd/env.go @@ -0,0 +1,35 @@ +package cmd + +import ( + "os" + "strings" + + log "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// setFlagsFromEnvVars reads and updates flag values from environment variables with prefix NB_ +func setFlagsFromEnvVars(cmd *cobra.Command) { + flags := cmd.PersistentFlags() + flags.VisitAll(func(f *pflag.Flag) { + newEnvVar := flagNameToEnvVar(f.Name, "NB_") + value, present := os.LookupEnv(newEnvVar) + if !present { + return + } + + err := flags.Set(f.Name, value) + if err != nil { + log.Infof("unable to configure flag %s using variable %s, err: %v", f.Name, newEnvVar, err) + } + }) +} + +// flagNameToEnvVar converts flag name to environment var name adding a prefix, +// replacing dashes and making all uppercase (e.g. setup-keys is converted to NB_SETUP_KEYS according to the input prefix) +func flagNameToEnvVar(cmdFlag string, prefix string) string { + parsed := strings.ReplaceAll(cmdFlag, "-", "_") + upper := strings.ToUpper(parsed) + return prefix + upper +} diff --git a/relay/cmd/root.go b/relay/cmd/root.go index d14a4e5cf..a64812d4d 100644 --- a/relay/cmd/root.go +++ b/relay/cmd/root.go @@ -26,47 +26,33 @@ import ( "github.com/netbirdio/netbird/stun" "github.com/netbirdio/netbird/trustedproxy" "github.com/netbirdio/netbird/util" - configloader "github.com/netbirdio/netbird/util/config" ) -// Config contains relay service startup configuration. type Config struct { - ListenAddress string `yaml:"listenAddress" env:"NB_LISTEN_ADDRESS" flag:"listen-address"` + ListenAddress string // in HA every peer connect to a common domain, the instance domain has been distributed during the p2p connection // it is a domain:port or ip:port - ExposedAddress string `yaml:"exposedAddress" env:"NB_EXPOSED_ADDRESS" flag:"exposed-address"` - MetricsPort int `yaml:"metricsPort" env:"NB_METRICS_PORT" flag:"metrics-port"` - LetsencryptEmail string `yaml:"letsencryptEmail" env:"NB_LETSENCRYPT_EMAIL" flag:"letsencrypt-email"` - LetsencryptDataDir string `yaml:"letsencryptDataDir" env:"NB_LETSENCRYPT_DATA_DIR" flag:"letsencrypt-data-dir"` - LetsencryptDomains []string `yaml:"letsencryptDomains" env:"NB_LETSENCRYPT_DOMAINS" flag:"letsencrypt-domains"` + ExposedAddress string + MetricsPort int + LetsencryptEmail string + LetsencryptDataDir string + LetsencryptDomains []string // in case of using Route 53 for DNS challenge the credentials should be provided in the environment variables or // in the AWS credentials file - LetsencryptAWSRoute53 bool `yaml:"letsencryptAWSRoute53" env:"NB_LETSENCRYPT_AWS_ROUTE53" flag:"letsencrypt-aws-route53"` - TlsCertFile string `yaml:"tlsCertFile" env:"NB_TLS_CERT_FILE" flag:"tls-cert-file"` - TlsKeyFile string `yaml:"tlsKeyFile" env:"NB_TLS_KEY_FILE" flag:"tls-key-file"` - AuthSecret string `yaml:"authSecret" env:"NB_AUTH_SECRET" flag:"auth-secret"` - LogLevel string `yaml:"logLevel" env:"NB_LOG_LEVEL" flag:"log-level"` - LogFile string `yaml:"logFile" env:"NB_LOG_FILE" flag:"log-file"` - HealthcheckListenAddress string `yaml:"healthcheckListenAddress" env:"NB_HEALTH_LISTEN_ADDRESS" flag:"health-listen-address"` + LetsencryptAWSRoute53 bool + TlsCertFile string + TlsKeyFile string + AuthSecret string + LogLevel string + LogFile string + HealthcheckListenAddress string // TrustedProxies is a comma-separated list of upstream proxy CIDRs/IPs whose // X-Real-Ip/X-Real-Port headers are trusted. Empty means never trust these headers. - TrustedProxies string `yaml:"trustedProxies" env:"NB_TRUSTED_PROXIES" flag:"trusted-proxies"` + TrustedProxies string // STUN server configuration - EnableSTUN bool `yaml:"enableSTUN" env:"NB_ENABLE_STUN" flag:"enable-stun"` - STUNPorts []int `yaml:"stunPorts" env:"NB_STUN_PORTS" flag:"stun-ports"` - STUNLogLevel string `yaml:"stunLogLevel" env:"NB_STUN_LOG_LEVEL" flag:"stun-log-level"` -} - -func defaultConfig() *Config { - return &Config{ - ListenAddress: ":443", - MetricsPort: 9090, - LogLevel: "info", - LogFile: "console", - HealthcheckListenAddress: ":9000", - STUNPorts: []int{3478}, - STUNLogLevel: "info", - } + EnableSTUN bool + STUNPorts []int + STUNLogLevel string } func (c Config) Validate() error { @@ -107,7 +93,6 @@ func (c Config) HasLetsEncrypt() bool { } var ( - configPath string cobraConfig *Config rootCmd = &cobra.Command{ Use: "relay", @@ -121,11 +106,10 @@ var ( func init() { _ = util.InitLog("trace", util.LogConsole) - cobraConfig = defaultConfig() - rootCmd.PersistentFlags().StringVar(&configPath, "config", "", "path to configuration file") - rootCmd.PersistentFlags().StringVarP(&cobraConfig.ListenAddress, "listen-address", "l", cobraConfig.ListenAddress, "listen address") + cobraConfig = &Config{} + rootCmd.PersistentFlags().StringVarP(&cobraConfig.ListenAddress, "listen-address", "l", ":443", "listen address") rootCmd.PersistentFlags().StringVarP(&cobraConfig.ExposedAddress, "exposed-address", "e", "", "instance domain address (or ip) and port, it will be distributes between peers") - rootCmd.PersistentFlags().IntVar(&cobraConfig.MetricsPort, "metrics-port", cobraConfig.MetricsPort, "metrics endpoint http port. Metrics are accessible under host:metrics-port/metrics") + rootCmd.PersistentFlags().IntVar(&cobraConfig.MetricsPort, "metrics-port", 9090, "metrics endpoint http port. Metrics are accessible under host:metrics-port/metrics") rootCmd.PersistentFlags().StringVarP(&cobraConfig.LetsencryptDataDir, "letsencrypt-data-dir", "d", "", "a directory to store Let's Encrypt data. Required if Let's Encrypt is enabled.") rootCmd.PersistentFlags().StringSliceVarP(&cobraConfig.LetsencryptDomains, "letsencrypt-domains", "a", nil, "list of domains to issue Let's Encrypt certificate for. Enables TLS using Let's Encrypt. Will fetch and renew certificate, and run the server with TLS") rootCmd.PersistentFlags().StringVar(&cobraConfig.LetsencryptEmail, "letsencrypt-email", "", "email address to use for Let's Encrypt certificate registration") @@ -133,13 +117,15 @@ func init() { rootCmd.PersistentFlags().StringVarP(&cobraConfig.TlsCertFile, "tls-cert-file", "c", "", "") rootCmd.PersistentFlags().StringVarP(&cobraConfig.TlsKeyFile, "tls-key-file", "k", "", "") rootCmd.PersistentFlags().StringVarP(&cobraConfig.AuthSecret, "auth-secret", "s", "", "auth secret") - rootCmd.PersistentFlags().StringVar(&cobraConfig.LogLevel, "log-level", cobraConfig.LogLevel, "log level") - rootCmd.PersistentFlags().StringVar(&cobraConfig.LogFile, "log-file", cobraConfig.LogFile, "log file") - rootCmd.PersistentFlags().StringVarP(&cobraConfig.HealthcheckListenAddress, "health-listen-address", "H", cobraConfig.HealthcheckListenAddress, "listen address of healthcheck server") + rootCmd.PersistentFlags().StringVar(&cobraConfig.LogLevel, "log-level", "info", "log level") + rootCmd.PersistentFlags().StringVar(&cobraConfig.LogFile, "log-file", "console", "log file") + rootCmd.PersistentFlags().StringVarP(&cobraConfig.HealthcheckListenAddress, "health-listen-address", "H", ":9000", "listen address of healthcheck server") rootCmd.PersistentFlags().StringVar(&cobraConfig.TrustedProxies, "trusted-proxies", "", "comma-separated list of upstream proxy CIDRs or IPs whose X-Real-Ip/X-Real-Port headers are trusted; leave empty to always use the direct connection address") rootCmd.PersistentFlags().BoolVar(&cobraConfig.EnableSTUN, "enable-stun", false, "enable embedded STUN server") - rootCmd.PersistentFlags().IntSliceVar(&cobraConfig.STUNPorts, "stun-ports", cobraConfig.STUNPorts, "ports for the embedded STUN server (can be specified multiple times or comma-separated)") - rootCmd.PersistentFlags().StringVar(&cobraConfig.STUNLogLevel, "stun-log-level", cobraConfig.STUNLogLevel, "log level for STUN server (panic, fatal, error, warn, info, debug, trace)") + rootCmd.PersistentFlags().IntSliceVar(&cobraConfig.STUNPorts, "stun-ports", []int{3478}, "ports for the embedded STUN server (can be specified multiple times or comma-separated)") + rootCmd.PersistentFlags().StringVar(&cobraConfig.STUNLogLevel, "stun-log-level", "info", "log level for STUN server (panic, fatal, error, warn, info, debug, trace)") + + setFlagsFromEnvVars(rootCmd) } func Execute() error { @@ -153,14 +139,8 @@ func waitForExitSignal() { } func execute(cmd *cobra.Command, args []string) error { - loadedConfig, err := loadConfig(cmd) - if err != nil { - return fmt.Errorf("load config: %w", err) - } - cobraConfig = loadedConfig - wg := sync.WaitGroup{} - err = cobraConfig.Validate() + err := cobraConfig.Validate() if err != nil { log.Debugf("invalid config: %s", err) return fmt.Errorf("invalid config: %s", err) @@ -248,16 +228,6 @@ func execute(cmd *cobra.Command, args []string) error { return err } -func loadConfig(cmd *cobra.Command) (*Config, error) { - return configloader.Load(configPath, defaultConfig(), configloader.Options{ - TagName: "yaml", - AllowMissing: configPath == "", - FlagSet: cmd.Flags(), - Strict: true, - InvalidEnvironment: configloader.InvalidEnvironmentUsePartial, - }) -} - func startServers(wg *sync.WaitGroup, metricsServer *metrics.Metrics, srv *server.Server, srvListenerCfg server.ListenerConfig, httpHealthcheck *healthcheck.Server, stunServer *stun.Server) { wg.Add(1) go func() { diff --git a/relay/config.example.yaml b/relay/config.example.yaml deleted file mode 100644 index cb3a56054..000000000 --- a/relay/config.example.yaml +++ /dev/null @@ -1,22 +0,0 @@ -# Command-line flags override environment variables, which override this file. -listenAddress: ":443" -exposedAddress: "rels://relay.example.com:443" -metricsPort: 9090 - -# Use either static certificates or Let's Encrypt. -letsencryptEmail: "" -letsencryptDataDir: "" -letsencryptDomains: [] -letsencryptAWSRoute53: false -tlsCertFile: "/etc/netbird/tls.crt" -tlsKeyFile: "/etc/netbird/tls.key" - -authSecret: "replace-with-a-strong-secret" -logLevel: "info" -logFile: "console" -healthcheckListenAddress: ":9000" -trustedProxies: "" - -enableSTUN: true -stunPorts: [3478] -stunLogLevel: "info" diff --git a/signal/cmd/config.go b/signal/cmd/config.go deleted file mode 100644 index 066a9a99e..000000000 --- a/signal/cmd/config.go +++ /dev/null @@ -1,46 +0,0 @@ -package cmd - -import ( - "os" - "runtime" - - "github.com/spf13/cobra" - - configloader "github.com/netbirdio/netbird/util/config" -) - -// Config contains Signal service startup configuration. -type Config struct { - Port int `yaml:"port" env:"NB_PORT" flag:"port"` - MetricsPort int `yaml:"metricsPort" env:"-" flag:"metrics-port"` - LetsencryptDomain string `yaml:"letsencryptDomain" env:"NB_LETSENCRYPT_DOMAIN" flag:"letsencrypt-domain"` - LetsencryptEmail string `yaml:"letsencryptEmail" env:"NB_LETSENCRYPT_EMAIL" flag:"letsencrypt-email"` - LetsencryptDataDir string `yaml:"letsencryptDataDir" env:"NB_SSL_DIR,NB_LETSENCRYPT_DATA_DIR" flag:"letsencrypt-data-dir,ssl-dir"` - CertFile string `yaml:"certFile" env:"NB_CERT_FILE" flag:"cert-file"` - CertKey string `yaml:"certKey" env:"NB_CERT_KEY" flag:"cert-key"` - LogLevel string `yaml:"logLevel" env:"-" flag:"log-level"` - LogFile string `yaml:"logFile" env:"-" flag:"log-file"` - PprofAddress string `yaml:"pprofAddress" env:"NB_PPROF_ADDR"` -} - -func defaultConfig() *Config { - logFile := "/var/log/netbird/signal.log" - if runtime.GOOS == "windows" { - logFile = os.Getenv("PROGRAMDATA") + "\\Netbird\\signal.log" - } - return &Config{ - MetricsPort: 9090, - LogLevel: "info", - LogFile: logFile, - } -} - -func loadConfig(cmd *cobra.Command, configPath string) (*Config, error) { - return configloader.Load(configPath, defaultConfig(), configloader.Options{ - TagName: "yaml", - AllowMissing: configPath == "", - FlagSet: cmd.Flags(), - Strict: true, - InvalidEnvironment: configloader.InvalidEnvironmentUsePartial, - }) -} diff --git a/signal/cmd/config_test.go b/signal/cmd/config_test.go deleted file mode 100644 index 624d79618..000000000 --- a/signal/cmd/config_test.go +++ /dev/null @@ -1,482 +0,0 @@ -package cmd - -import ( - "os" - "path/filepath" - "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 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") - }) - } -} diff --git a/signal/cmd/env.go b/signal/cmd/env.go new file mode 100644 index 000000000..3c15ebe1f --- /dev/null +++ b/signal/cmd/env.go @@ -0,0 +1,35 @@ +package cmd + +import ( + "os" + "strings" + + log "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// setFlagsFromEnvVars reads and updates flag values from environment variables with prefix NB_ +func setFlagsFromEnvVars(cmd *cobra.Command) { + flags := cmd.PersistentFlags() + flags.VisitAll(func(f *pflag.Flag) { + newEnvVar := flagNameToEnvVar(f.Name, "NB_") + value, present := os.LookupEnv(newEnvVar) + if !present { + return + } + + err := flags.Set(f.Name, value) + if err != nil { + log.Infof("unable to configure flag %s using variable %s, err: %v", f.Name, newEnvVar, err) + } + }) +} + +// flagNameToEnvVar converts flag name to environment var name adding a prefix, +// replacing dashes and making all uppercase (e.g. setup-keys is converted to NB_SETUP_KEYS according to the input prefix) +func flagNameToEnvVar(cmdFlag string, prefix string) string { + parsed := strings.ReplaceAll(cmdFlag, "-", "_") + upper := strings.ToUpper(parsed) + return prefix + upper +} diff --git a/signal/cmd/root.go b/signal/cmd/root.go index 11757c145..155790482 100644 --- a/signal/cmd/root.go +++ b/signal/cmd/root.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "os/signal" + "runtime" "github.com/spf13/cobra" @@ -16,8 +17,9 @@ const ( ) var ( - logLevel string - logFile string + logLevel string + defaultLogFile string + logFile string rootCmd = &cobra.Command{ Use: "netbird-signal", @@ -37,9 +39,14 @@ func Execute() error { func init() { stopCh = make(chan int) - defaults := defaultConfig() - rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", defaults.LogLevel, "") - rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaults.LogFile, "sets Netbird log path. If console is specified the log will be output to stdout") + defaultLogFile = "/var/log/netbird/signal.log" + + if runtime.GOOS == "windows" { + defaultLogFile = os.Getenv("PROGRAMDATA") + "\\Netbird\\" + "signal.log" + } + + rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", "info", "") + rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaultLogFile, "sets Netbird log path. If console is specified the log will be output to stdout") rootCmd.AddCommand(runCmd) } diff --git a/signal/cmd/run.go b/signal/cmd/run.go index 435376656..a36623c6b 100644 --- a/signal/cmd/run.go +++ b/signal/cmd/run.go @@ -8,10 +8,9 @@ import ( "fmt" "net" "net/http" - "os" - "strconv" // nolint:gosec _ "net/http/pprof" + "os" "time" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" @@ -47,8 +46,6 @@ var ( signalLetsencryptDataDir string signalCertFile string signalCertKey string - signalConfigPath string - signalPprofAddress string signalKaep = grpc.KeepaliveEnforcementPolicy(keepalive.EnforcementPolicy{ MinTime: 5 * time.Second, @@ -67,26 +64,30 @@ var ( Short: "start NetBird Signal Server daemon", SilenceUsage: true, PreRunE: func(cmd *cobra.Command, args []string) error { - userPort := signalPortConfigured(cmd) - cfg, err := loadConfig(cmd, signalConfigPath) + err := util.InitLog(logLevel, logFile) if err != nil { - return fmt.Errorf("load config: %w", err) + return fmt.Errorf("failed initializing log: %w", err) } - applyConfig(cfg) - if !userPort && signalPort == 0 { - if signalLetsencryptDomain != "" || (signalCertFile != "" && signalCertKey != "") { + flag.Parse() + + // detect whether user specified a port + userPort := cmd.Flag("port").Changed + + var tlsEnabled bool + if signalLetsencryptDomain != "" || (signalCertFile != "" && signalCertKey != "") { + tlsEnabled = true + } + + if !userPort { + // different defaults for signalPort + if tlsEnabled { signalPort = 443 } else { signalPort = 80 } } - if err := util.InitLog(logLevel, logFile); err != nil { - return fmt.Errorf("initialize log: %w", err) - } - - flag.Parse() return nil }, RunE: func(cmd *cobra.Command, args []string) error { @@ -195,10 +196,10 @@ var ( ) func startPprof() { - if signalPprofAddress != "" { - log.Infof("pprof enabled, listening on: %s", signalPprofAddress) + if pprofAddr := os.Getenv("NB_PPROF_ADDR"); pprofAddr != "" { + log.Infof("pprof enabled, listening on: %s", pprofAddr) go func() { - if err := http.ListenAndServe(signalPprofAddress, nil); err != nil { + if err := http.ListenAndServe(pprofAddr, nil); err != nil { log.Fatalf("pprof server failed: %v", err) } }() @@ -325,40 +326,15 @@ func loadTLSConfig(certFile string, certKey string) (*tls.Config, error) { return config, nil } -func signalPortConfigured(cmd *cobra.Command) bool { - if cmd.Flag("port").Changed { - return true - } - raw, present := os.LookupEnv("NB_PORT") - if !present { - return false - } - _, err := strconv.ParseInt(raw, 0, 64) - return err == nil -} func init() { - defaults := defaultConfig() - runCmd.PersistentFlags().StringVar(&signalConfigPath, "config", "", "path to configuration file") - runCmd.PersistentFlags().IntVar(&signalPort, "port", defaults.Port, "Server port to listen on (defaults to 443 if TLS is enabled, 80 otherwise)") - runCmd.Flags().IntVar(&metricsPort, "metrics-port", defaults.MetricsPort, "metrics endpoint http port. Metrics are accessible under host:metrics-port/metrics") + runCmd.PersistentFlags().IntVar(&signalPort, "port", 80, "Server port to listen on (defaults to 443 if TLS is enabled, 80 otherwise") + runCmd.Flags().IntVar(&metricsPort, "metrics-port", 9090, "metrics endpoint http port. Metrics are accessible under host:metrics-port/metrics") runCmd.PersistentFlags().StringVar(&signalLetsencryptDataDir, "letsencrypt-data-dir", "", "a directory to store Let's Encrypt data. Required if Let's Encrypt is enabled.") runCmd.PersistentFlags().StringVar(&signalLetsencryptDataDir, "ssl-dir", "", "server ssl directory location. *Required only for Let's Encrypt certificates. Deprecated: use --letsencrypt-data-dir") runCmd.PersistentFlags().StringVar(&signalLetsencryptDomain, "letsencrypt-domain", "", "a domain to issue Let's Encrypt certificate for. Enables TLS using Let's Encrypt. Will fetch and renew certificate, and run the server with TLS") runCmd.PersistentFlags().StringVar(&signalLetsencryptEmail, "letsencrypt-email", "", "email address to use for Let's Encrypt certificate registration") runCmd.PersistentFlags().StringVar(&signalCertFile, "cert-file", "", "Location of your SSL certificate. Can be used when you have an existing certificate and don't want a new certificate be generated automatically. If letsencrypt-domain is specified this property has no effect") runCmd.PersistentFlags().StringVar(&signalCertKey, "cert-key", "", "Location of your SSL certificate private key. Can be used when you have an existing certificate and don't want a new certificate be generated automatically. If letsencrypt-domain is specified this property has no effect") -} - -func applyConfig(cfg *Config) { - signalPort = cfg.Port - metricsPort = cfg.MetricsPort - signalLetsencryptDomain = cfg.LetsencryptDomain - signalLetsencryptEmail = cfg.LetsencryptEmail - signalLetsencryptDataDir = cfg.LetsencryptDataDir - signalCertFile = cfg.CertFile - signalCertKey = cfg.CertKey - logLevel = cfg.LogLevel - logFile = cfg.LogFile - signalPprofAddress = cfg.PprofAddress + setFlagsFromEnvVars(runCmd) } diff --git a/signal/config.example.yaml b/signal/config.example.yaml deleted file mode 100644 index 04e175d51..000000000 --- a/signal/config.example.yaml +++ /dev/null @@ -1,15 +0,0 @@ -# Command-line flags override environment variables, which override this file. -# Set port to 0 to select 443 with TLS or 80 without TLS. -port: 0 -metricsPort: 9090 - -# Use either static certificates or Let's Encrypt. -letsencryptDomain: "" -letsencryptEmail: "" -letsencryptDataDir: "" -certFile: "/etc/netbird/tls.crt" -certKey: "/etc/netbird/tls.key" - -logLevel: "info" -logFile: "console" -pprofAddress: "" diff --git a/trustedproxy/trustedproxy.go b/trustedproxy/trustedproxy.go index 3db9e1ce6..70df01d92 100644 --- a/trustedproxy/trustedproxy.go +++ b/trustedproxy/trustedproxy.go @@ -49,16 +49,6 @@ func Parse(raw string) (*List, error) { return &List{prefixes: prefixes}, nil } -// UnmarshalText parses trusted proxy prefixes from text configuration. -func (l *List) UnmarshalText(text []byte) error { - parsed, err := Parse(string(text)) - if err != nil { - return err - } - *l = *parsed - return nil -} - // FromPrefixes wraps an already-parsed set of prefixes in a List. func FromPrefixes(prefixes []netip.Prefix) *List { return &List{prefixes: prefixes} diff --git a/util/config/loader_test.go b/util/config/loader_test.go index 6d3580edd..0ea458f0d 100644 --- a/util/config/loader_test.go +++ b/util/config/loader_test.go @@ -1,6 +1,7 @@ package config import ( + "bytes" "encoding/json" "net/netip" "os" @@ -12,8 +13,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" - - "github.com/netbirdio/netbird/util/envtemplate" ) type testConfig struct { @@ -137,15 +136,16 @@ server: } func TestLoadTransformsConfig(t *testing.T) { - t.Setenv("CONFIG_ADDRESS", ":8443") configPath := writeConfigFile(t, "config.yaml", ` server: - address: "{{ .CONFIG_ADDRESS }}" + address: "PLACEHOLDER" `) cfg, err := Load(configPath, defaultTestConfig(), Options{ - TagName: "yaml", - Transform: envtemplate.Expand, + TagName: "yaml", + Transform: func(data []byte) ([]byte, error) { + return bytes.ReplaceAll(data, []byte("PLACEHOLDER"), []byte(":8443")), nil + }, }) require.NoError(t, err) assert.Equal(t, ":8443", cfg.Server.Address, "The transform should run before decoding") diff --git a/util/envtemplate/template.go b/util/envtemplate/template.go deleted file mode 100644 index 753b89a09..000000000 --- a/util/envtemplate/template.go +++ /dev/null @@ -1,35 +0,0 @@ -// Package envtemplate expands Go templates with environment variables. -package envtemplate - -import ( - "bytes" - "fmt" - "os" - "strings" - "text/template" -) - -// Expand substitutes Go-template references with environment values. -func Expand(data []byte) ([]byte, error) { - tmpl, err := template.New("config").Parse(string(data)) - if err != nil { - return nil, fmt.Errorf("parse environment template: %w", err) - } - - var output bytes.Buffer - if err := tmpl.Execute(&output, environment()); err != nil { - return nil, fmt.Errorf("execute environment template: %w", err) - } - return output.Bytes(), nil -} - -func environment() map[string]string { - values := make(map[string]string) - for _, entry := range os.Environ() { - key, value, ok := strings.Cut(entry, "=") - if ok { - values[key] = value - } - } - return values -} diff --git a/util/file.go b/util/file.go index 8254368a3..926904f9f 100644 --- a/util/file.go +++ b/util/file.go @@ -1,6 +1,7 @@ package util import ( + "bytes" "context" "encoding/json" "errors" @@ -9,10 +10,10 @@ import ( "os" "path/filepath" "sort" + "strings" + "text/template" log "github.com/sirupsen/logrus" - - "github.com/netbirdio/netbird/util/envtemplate" ) func WriteBytesWithRestrictedPermission(ctx context.Context, file string, bs []byte) error { @@ -232,6 +233,8 @@ func ListFiles(dir, pattern string) ([]string, error) { // ReadJsonWithEnvSub reads JSON config file and maps to a provided interface with environment variable substitution func ReadJsonWithEnvSub(file string, res interface{}) (interface{}, error) { + envVars := getEnvMap() + f, err := os.Open(file) if err != nil { return nil, err @@ -243,12 +246,19 @@ func ReadJsonWithEnvSub(file string, res interface{}) (interface{}, error) { return nil, err } - output, err := envtemplate.Expand(bs) + t, err := template.New("").Parse(string(bs)) if err != nil { - return nil, err + return nil, fmt.Errorf("error parsing template: %v", err) } - err = json.Unmarshal(output, &res) + var output bytes.Buffer + // Execute the template, substituting environment variables + err = t.Execute(&output, envVars) + if err != nil { + return nil, fmt.Errorf("error executing template: %v", err) + } + + err = json.Unmarshal(output.Bytes(), &res) if err != nil { return nil, fmt.Errorf("failed parsing Json file after template was executed, err: %v", err) } @@ -256,6 +266,20 @@ func ReadJsonWithEnvSub(file string, res interface{}) (interface{}, error) { return res, nil } +// getEnvMap Convert the output of os.Environ() to a map +func getEnvMap() map[string]string { + envMap := make(map[string]string) + + for _, env := range os.Environ() { + parts := strings.SplitN(env, "=", 2) + if len(parts) == 2 { + envMap[parts[0]] = parts[1] + } + } + + return envMap +} + // CopyFileContents copies contents of the given src file to the dst file func CopyFileContents(src, dst string) (err error) { in, err := os.Open(src)