sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-09-12 17:59:06 +02:00
Code Issues Packages Projects Releases Wiki Activity

[client] Name the two config readers for what they do

Browse Source 
ReadConfig and GetConfig differed in one thing — what happens when the file is
absent — and neither name said which was which:

- ReadConfig      -> ReadOrGenerateConfig  (reads it, or generates one in memory)
- GetConfig       -> GetExistingConfig     (reads it, or fails)

Three comments went with them:

- GetConfig's said "return with Config and if it was created. Errors out if it
  does not exist", which described a bool it does not return and a creation it
  never performs.
- ReadConfig's explained that it does not write, which is what a reader is
  supposed to do anyway.
- Server.getConfig's said it "errors out if it does not exist", which it does
  not — it resolves a default config, and now provisions the identity too.
This commit is contained in:
riccardom
2026-09-02 15:21:35 +02:00
parent 7a1f095eb5
commit 844bf24a6f
14 changed files with 51 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/android/preferences.go
+16 -16
View File
@@ -23,7 +23,7 @@ func (p *Preferences) GetManagementURL() (string, error) {
return p.configInput.ManagementURL, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return "", err
}
@@ -41,7 +41,7 @@ func (p *Preferences) GetAdminURL() (string, error) {
return p.configInput.AdminURL, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return "", err
}
@@ -59,7 +59,7 @@ func (p *Preferences) GetPreSharedKey() (string, error) {
return *p.configInput.PreSharedKey, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return "", err
}
@@ -82,7 +82,7 @@ func (p *Preferences) GetRosenpassEnabled() (bool, error) {
return *p.configInput.RosenpassEnabled, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -100,7 +100,7 @@ func (p *Preferences) GetRosenpassPermissive() (bool, error) {
return *p.configInput.RosenpassPermissive, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -113,7 +113,7 @@ func (p *Preferences) GetDisableClientRoutes() (bool, error) {
return *p.configInput.DisableClientRoutes, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -131,7 +131,7 @@ func (p *Preferences) GetDisableServerRoutes() (bool, error) {
return *p.configInput.DisableServerRoutes, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -149,7 +149,7 @@ func (p *Preferences) GetDisableDNS() (bool, error) {
return *p.configInput.DisableDNS, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -167,7 +167,7 @@ func (p *Preferences) GetDisableFirewall() (bool, error) {
return *p.configInput.DisableFirewall, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -185,7 +185,7 @@ func (p *Preferences) GetServerSSHAllowed() (bool, error) {
return *p.configInput.ServerSSHAllowed, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -207,7 +207,7 @@ func (p *Preferences) GetEnableSSHRoot() (bool, error) {
return *p.configInput.EnableSSHRoot, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -229,7 +229,7 @@ func (p *Preferences) GetEnableSSHSFTP() (bool, error) {
return *p.configInput.EnableSSHSFTP, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -251,7 +251,7 @@ func (p *Preferences) GetEnableSSHLocalPortForwarding() (bool, error) {
return *p.configInput.EnableSSHLocalPortForwarding, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -273,7 +273,7 @@ func (p *Preferences) GetEnableSSHRemotePortForwarding() (bool, error) {
return *p.configInput.EnableSSHRemotePortForwarding, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -295,7 +295,7 @@ func (p *Preferences) GetBlockInbound() (bool, error) {
return *p.configInput.BlockInbound, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -313,7 +313,7 @@ func (p *Preferences) GetDisableIPv6() (bool, error) {
return *p.configInput.DisableIPv6, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
client/cmd/login.go
+1 -1
View File
@@ -326,7 +326,7 @@ func doForegroundLogin(ctx context.Context, cmd *cobra.Command, setupKey string,
}
config, err := profilemanager.ReadConfig(configFilePath)
config, err := profilemanager.ReadOrGenerateConfig(configFilePath)
if err != nil {
return fmt.Errorf("read config file %s: %v", configFilePath, err)
}
client/internal/profilemanager/config.go
+5 -9
View File
@@ -1130,8 +1130,8 @@ func update(input ConfigInput) (*Config, error) {
return config, nil
}
// GetConfig read config file and return with Config and if it was created. Errors out if it does not exist
func GetConfig(configPath string) (*Config, error) {
// GetExistingConfig reads and returns the config if it exists on disk. Fails otherwise.
func GetExistingConfig(configPath string) (*Config, error) {
return readConfig(configPath, false)
}
@@ -1219,13 +1219,9 @@ func CreateInMemoryConfig(input ConfigInput) (*Config, error) {
return createNewConfig(input)
}
// ReadConfig reads the profile config at configPath, resolving a default config
// in memory when the file does not exist.
//
// It never writes. A caller that wants what it got back to be on disk calls
// WriteOutConfig itself, and one that resolved a config for a peer to run with
// calls EnsureIdentity first — see Server.getConfig for that pair.
func ReadConfig(configPath string) (*Config, error) {
// ReadOrGenerateConfig reads the profile config at configPath if it exists, or
// generates one in memory from the defaults.
func ReadOrGenerateConfig(configPath string) (*Config, error) {
return readConfig(configPath, true)
}
client/internal/profilemanager/config_test.go
+1 -1
View File
@@ -196,7 +196,7 @@ func TestWireguardPortZeroExplicit(t *testing.T) {
assert.Equal(t, 0, config.WgPort, "WgPort should be 0 when explicitly set by user")
// Verify it persists
readConfig, err := GetConfig(configPath)
readConfig, err := GetExistingConfig(configPath)
require.NoError(t, err)
assert.Equal(t, 0, readConfig.WgPort, "WgPort should remain 0 after reading from file")
}
client/internal/profilemanager/config_would_change_test.go
+6 -6
View File
@@ -108,8 +108,8 @@ func TestReadsDoNotWriteTheConfigBack(t *testing.T) {
denormalized := []byte(`{"WgIface":"wt0"}`)
for name, read := range map[string]func(string) (*Config, error){
"GetConfig": GetConfig,
"ReadConfig": ReadConfig,
"GetExistingConfig": GetExistingConfig,
"ReadOrGenerateConfig": ReadOrGenerateConfig,
} {
t.Run(name, func(t *testing.T) {
path := filepath.Join(t.TempDir(), "profile.json")
@@ -132,7 +132,7 @@ func TestReadsDoNotWriteTheConfigBack(t *testing.T) {
func TestReadConfigDoesNotCreateTheFile(t *testing.T) {
path := filepath.Join(t.TempDir(), "absent.json")
cfg, err := ReadConfig(path)
cfg, err := ReadOrGenerateConfig(path)
require.NoError(t, err)
require.Equal(t, DefaultManagementURL, cfg.ManagementURL.String())
@@ -198,7 +198,7 @@ func TestWouldChangeIgnoresURLSpelling(t *testing.T) {
})
require.NoError(t, err)
cfg, err := GetConfig(path)
cfg, err := GetExistingConfig(path)
require.NoError(t, err)
for _, spelling := range []string{
@@ -246,7 +246,7 @@ func TestUpdateConfigProvisionsAMissingIdentity(t *testing.T) {
require.NoError(t, err)
// Stand in for the logout, which zeroes the keys and writes the config out.
loggedOut, err := GetConfig(path)
loggedOut, err := GetExistingConfig(path)
require.NoError(t, err)
loggedOut.PrivateKey = ""
loggedOut.SSHKey = ""
@@ -257,7 +257,7 @@ func TestUpdateConfigProvisionsAMissingIdentity(t *testing.T) {
require.NotEmpty(t, cfg.PrivateKey, "the write path did not provision an identity")
require.NotEmpty(t, cfg.SSHKey)
persisted, err := GetConfig(path)
persisted, err := GetExistingConfig(path)
require.NoError(t, err)
require.Equal(t, cfg.PrivateKey, persisted.PrivateKey, "the provisioned identity was not persisted")
}
client/ios/NetBirdSDK/preferences.go
+6 -6
View File
@@ -26,7 +26,7 @@ func (p *Preferences) GetManagementURL() (string, error) {
return p.configInput.ManagementURL, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return "", err
}
@@ -44,7 +44,7 @@ func (p *Preferences) GetAdminURL() (string, error) {
return p.configInput.AdminURL, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return "", err
}
@@ -62,7 +62,7 @@ func (p *Preferences) GetPreSharedKey() (string, error) {
return *p.configInput.PreSharedKey, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return "", err
}
@@ -85,7 +85,7 @@ func (p *Preferences) GetRosenpassEnabled() (bool, error) {
return *p.configInput.RosenpassEnabled, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -103,7 +103,7 @@ func (p *Preferences) GetRosenpassPermissive() (bool, error) {
return *p.configInput.RosenpassPermissive, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
@@ -116,7 +116,7 @@ func (p *Preferences) GetDisableIPv6() (bool, error) {
return *p.configInput.DisableIPv6, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath)
if err != nil {
return false, err
}
client/mobile/profile_manager.go
+1 -1
View File
@@ -174,7 +174,7 @@ func (pm *ProfileManager) LogoutProfile(id string) error {
return fmt.Errorf("profile %q does not exist", id)
}
config, err := profilemanager.ReadConfig(configPath)
config, err := profilemanager.ReadOrGenerateConfig(configPath)
if err != nil {
return fmt.Errorf("read profile config: %w", err)
}
client/server/login_gate_test.go
+1 -1
View File
@@ -93,7 +93,7 @@ func TestLogin_ChangeThatBecomesPrivilegedMidRequestHasNoSideEffects(t *testing.
require.NoError(t, err)
require.Equal(t, profilemanager.ID(activeProfile), active.ID, "the refused login switched the active profile anyway")
stored, err := profilemanager.ReadConfig(targetPath)
stored, err := profilemanager.ReadOrGenerateConfig(targetPath)
require.NoError(t, err)
require.Equal(t, "https://api.netbird.io:443", stored.ManagementURL.String(), "the refused login moved the management URL")
}
client/server/login_overrides_test.go
+1 -1
View File
@@ -87,7 +87,7 @@ func TestPersistLoginOverrides(t *testing.T) {
})
require.NoError(t, err, "persistLoginOverrides")
cfg, err := profilemanager.ReadConfig(profilemanager.DefaultConfigPath)
cfg, err := profilemanager.ReadOrGenerateConfig(profilemanager.DefaultConfigPath)
require.NoError(t, err, "read back config")
require.Equal(t, tt.wantMgmtURL, cfg.ManagementURL.String(), "management URL")
client/server/logout_gate_test.go
+1 -1
View File
@@ -129,7 +129,7 @@ func TestLogout_ForeignUserProfileDoesNotUseTheRunningConfig(t *testing.T) {
// refused with PermissionDenied. The namesake profile does not, so the
// correct path gets as far as dialing its own unreachable management URL.
enableSSHOnProfile(t, cfgPath)
running, err := profilemanager.GetConfig(cfgPath)
running, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
s.config = running
s.connectClient = newDummyConnectClient(context.Background())
client/server/server.go
+6 -5
View File
@@ -1173,7 +1173,7 @@ func (s *Server) storedConfigAtPath(path string) (*profilemanager.Config, error)
return nil, fmt.Errorf("stat profile config: %w", err)
}
cfg, err := profilemanager.GetConfig(path)
cfg, err := profilemanager.GetExistingConfig(path)
if err != nil {
return nil, fmt.Errorf("read profile config: %w", err)
}
@@ -1472,7 +1472,8 @@ func (s *Server) handleActiveProfileLogout(ctx context.Context) (*proto.LogoutRe
return &proto.LogoutResponse{}, nil
}
// getConfig reads config file and returns Config and whether the config file already existed. Errors out if it does not exist
// getConfig resolves the active profile's config, provisions its identity and
// reports whether the config file already existed.
func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*profilemanager.Config, bool, error) {
cfgPath, err := activeProf.FilePath()
if err != nil {
@@ -1484,7 +1485,7 @@ func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*prof
log.Infof("active profile config existed: %t, err %v", configExisted, err)
config, err := profilemanager.ReadConfig(cfgPath)
config, err := profilemanager.ReadOrGenerateConfig(cfgPath)
if err != nil {
return nil, false, fmt.Errorf("failed to get config: %w", err)
}
@@ -1557,7 +1558,7 @@ func (s *Server) logoutFromProfile(ctx context.Context, profile *profilemanager.
cfgPath = profilemanager.DefaultConfigPath
}
config, err := profilemanager.GetConfig(cfgPath)
config, err := profilemanager.GetExistingConfig(cfgPath)
if err != nil {
return fmt.Errorf("profile '%s' not found", profile.ID)
}
@@ -2159,7 +2160,7 @@ func (s *Server) GetConfig(ctx context.Context, req *proto.GetConfigRequest) (*p
cfgPath = profilemanager.DefaultConfigPath
}
cfg, err := profilemanager.GetConfig(cfgPath)
cfg, err := profilemanager.GetExistingConfig(cfgPath)
if err != nil {
log.Errorf("failed to get active profile config: %v", err)
return nil, fmt.Errorf("failed to get active profile config: %w", err)
client/server/setconfig_mdm_test.go
+1 -1
View File
@@ -205,7 +205,7 @@ func TestSetConfig_MDMReject_AllOrNothing(t *testing.T) {
// Confirm RosenpassEnabled was NOT applied even though it was not
// in the conflict list: the request was rejected as a whole.
reloaded, err := profilemanager.GetConfig(cfgPath)
reloaded, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
assert.False(t, reloaded.RosenpassEnabled, "non-conflicting field must not be applied when request is rejected")
}
client/server/setconfig_test.go
+1 -1
View File
@@ -125,7 +125,7 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) {
cfgPath, err := profState.FilePath()
require.NoError(t, err)
cfg, err := profilemanager.GetConfig(cfgPath)
cfg, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
require.Equal(t, "https://new-api.netbird.io:443", cfg.ManagementURL.String())
client/server/update_settings_gate_test.go
+4 -4
View File
@@ -64,7 +64,7 @@ func TestSetConfig_ChangingASettingIsRefused(t *testing.T) {
require.Error(t, err, "moving the management URL is a settings change")
require.Equal(t, codes.Unavailable, gstatus.Code(err), "want the update-settings refusal, got %v", err)
cfg, err := profilemanager.GetConfig(cfgPath)
cfg, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
require.Equal(t, storedManagementURL, cfg.ManagementURL.String(), "the refused request changed the config anyway")
}
@@ -98,7 +98,7 @@ func TestSetConfig_ChangeAllowedWhenTheSwitchIsOff(t *testing.T) {
})
require.NoError(t, err)
cfg, err := profilemanager.GetConfig(cfgPath)
cfg, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
require.Equal(t, "https://mgmt.elsewhere.example:443", cfg.ManagementURL.String())
}
@@ -139,7 +139,7 @@ func seedProfileConfig(t *testing.T, managementURL, preSharedKey string) string
// keeps a re-login, or a container restart carrying NB_MANAGEMENT_URL, working
// with the kill switch on.
func TestLoginGateDecision(t *testing.T) {
stored, err := profilemanager.GetConfig(seedProfileConfig(t, storedManagementURL, "stored-key"))
stored, err := profilemanager.GetExistingConfig(seedProfileConfig(t, storedManagementURL, "stored-key"))
require.NoError(t, err)
redacted := preSharedKeyRedactedSentinel
@@ -336,7 +336,7 @@ func TestLogin_ChangeThatAppearsMidRequestIsRefused(t *testing.T) {
require.Equal(t, codes.Unavailable, gstatus.Code(err), "want the update-settings refusal, got %v", err)
require.False(t, cancelled, "the refused login cancelled the login already in progress")
stored, err := profilemanager.GetConfig(targetPath)
stored, err := profilemanager.GetExistingConfig(targetPath)
require.NoError(t, err)
require.Equal(t, "https://mgmt.elsewhere.example:443", stored.ManagementURL.String(),
"the refused login wrote the management URL it was asked for")