From 2885e3771d277baaf210ea54f1b02dc5ea1967d2 Mon Sep 17 00:00:00 2001 From: riccardom Date: Fri, 25 Sep 2026 13:21:15 +0200 Subject: [PATCH] [client] Name the resolving reader for what it does, not what it makes ReadOrGenerateConfig reads the profile config and falls back to the defaults in memory when there is no file. "Generate" reads as "produces and stores", which is the opposite of the property the rename it came from was meant to advertise: the read is pure, writes nothing and mints no identity. ReadConfigOrDefault says the same without the side effect, and pairs with GetExistingConfig, which fails where this one falls back. Its doc comment now states the absence of a write rather than only the fallback. Pure rename; the two remaining mentions of the pre-branch name ReadConfig in the tests go with it. Reported by pappz in review. --- client/android/preferences.go | 34 +++++++++---------- client/cmd/login.go | 2 +- client/internal/profilemanager/config.go | 10 +++--- .../config_would_change_test.go | 16 ++++----- client/ios/NetBirdSDK/preferences.go | 14 ++++---- client/server/login_overrides_test.go | 2 +- client/server/server.go | 2 +- 7 files changed, 41 insertions(+), 39 deletions(-) diff --git a/client/android/preferences.go b/client/android/preferences.go index 88626c096..3623de23f 100644 --- a/client/android/preferences.go +++ b/client/android/preferences.go @@ -46,7 +46,7 @@ func (p *Preferences) GetManagementURL() (string, error) { return p.configInput.ManagementURL, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return "", err } @@ -64,7 +64,7 @@ func (p *Preferences) GetAdminURL() (string, error) { return p.configInput.AdminURL, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return "", err } @@ -86,7 +86,7 @@ func (p *Preferences) HasPreSharedKey() (bool, error) { return *p.configInput.PreSharedKey != "", nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -112,7 +112,7 @@ func (p *Preferences) GetRosenpassEnabled() (bool, error) { return *p.configInput.RosenpassEnabled, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -133,7 +133,7 @@ func (p *Preferences) GetRosenpassPermissive() (bool, error) { return *p.configInput.RosenpassPermissive, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -149,7 +149,7 @@ func (p *Preferences) GetDisableClientRoutes() (bool, error) { return *p.configInput.DisableClientRoutes, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -170,7 +170,7 @@ func (p *Preferences) GetDisableServerRoutes() (bool, error) { return *p.configInput.DisableServerRoutes, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -188,7 +188,7 @@ func (p *Preferences) GetDisableDNS() (bool, error) { return *p.configInput.DisableDNS, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -206,7 +206,7 @@ func (p *Preferences) GetDisableFirewall() (bool, error) { return *p.configInput.DisableFirewall, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -227,7 +227,7 @@ func (p *Preferences) GetServerSSHAllowed() (bool, error) { return *p.configInput.ServerSSHAllowed, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -249,7 +249,7 @@ func (p *Preferences) GetEnableSSHRoot() (bool, error) { return *p.configInput.EnableSSHRoot, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -271,7 +271,7 @@ func (p *Preferences) GetEnableSSHSFTP() (bool, error) { return *p.configInput.EnableSSHSFTP, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -293,7 +293,7 @@ func (p *Preferences) GetEnableSSHLocalPortForwarding() (bool, error) { return *p.configInput.EnableSSHLocalPortForwarding, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -315,7 +315,7 @@ func (p *Preferences) GetEnableSSHRemotePortForwarding() (bool, error) { return *p.configInput.EnableSSHRemotePortForwarding, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -340,7 +340,7 @@ func (p *Preferences) GetBlockInbound() (bool, error) { return *p.configInput.BlockInbound, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -358,7 +358,7 @@ func (p *Preferences) GetDisableIPv6() (bool, error) { return *p.configInput.DisableIPv6, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -377,7 +377,7 @@ func (p *Preferences) GetRemoteJobsAllowed() (bool, error) { return *p.configInput.RemoteJobsAllowed, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } diff --git a/client/cmd/login.go b/client/cmd/login.go index 428864375..1dc2d0d09 100644 --- a/client/cmd/login.go +++ b/client/cmd/login.go @@ -322,7 +322,7 @@ func doForegroundLogin(ctx context.Context, cmd *cobra.Command, setupKey string, } - config, err := profilemanager.ReadOrGenerateConfig(configFilePath) + config, err := profilemanager.ReadConfigOrDefault(configFilePath) if err != nil { return fmt.Errorf("read config file %s: %v", configFilePath, err) } diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index a6b46971c..9218a3bfc 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -1330,9 +1330,11 @@ func CreateInMemoryConfig(input ConfigInput) (*Config, error) { return createProvisionedConfig(input) } -// ReadOrGenerateConfig reads the profile config at configPath if it exists, or -// generates one in memory from the defaults. -func ReadOrGenerateConfig(configPath string) (*Config, error) { +// ReadConfigOrDefault reads the profile config at configPath, or resolves the +// default config in memory when the file does not exist. It never writes, and +// never mints an identity — EnsureIdentity is where that happens, so the +// caller that provisions is also the one that persists. +func ReadConfigOrDefault(configPath string) (*Config, error) { return readConfig(configPath, true) } @@ -1457,7 +1459,7 @@ func ConfigToJSON(config *Config) (string, error) { // // The peer identity is deliberately none of its business, in either direction. // It does not generate one: a read cannot hand back keys that nothing will -// write down (see ReadOrGenerateConfig). Nor does it refuse a document that +// write down (see ReadConfigOrDefault). Nor does it refuse a document that // carries none, because a config legitimately has no identity between a logout // and the next login — mobile logout clears both keys in place — and this is // also the deserializer the iOS SDK copies a config through. Whoever goes on diff --git a/client/internal/profilemanager/config_would_change_test.go b/client/internal/profilemanager/config_would_change_test.go index 2387cf1b5..7a83dc2fe 100644 --- a/client/internal/profilemanager/config_would_change_test.go +++ b/client/internal/profilemanager/config_would_change_test.go @@ -112,8 +112,8 @@ func TestReadsDoNotWriteTheConfigBack(t *testing.T) { denormalized := []byte(`{"WgIface":"wt0"}`) for name, read := range map[string]func(string) (*Config, error){ - "GetExistingConfig": GetExistingConfig, - "ReadOrGenerateConfig": ReadOrGenerateConfig, + "GetExistingConfig": GetExistingConfig, + "ReadConfigOrDefault": ReadConfigOrDefault, } { t.Run(name, func(t *testing.T) { path := filepath.Join(t.TempDir(), "profile.json") @@ -131,17 +131,17 @@ func TestReadsDoNotWriteTheConfigBack(t *testing.T) { } } -// ReadConfig resolves a default config for a profile that has no file yet, and -// that must not create the file either. +// ReadConfigOrDefault resolves a default config for a profile that has no file +// yet, and that must not create the file either. func TestReadConfigDoesNotCreateTheFile(t *testing.T) { path := filepath.Join(t.TempDir(), "absent.json") - cfg, err := ReadOrGenerateConfig(path) + cfg, err := ReadConfigOrDefault(path) require.NoError(t, err) require.Equal(t, DefaultManagementURL, cfg.ManagementURL.String()) _, err = os.Stat(path) - require.True(t, os.IsNotExist(err), "ReadConfig created the config file") + require.True(t, os.IsNotExist(err), "ReadConfigOrDefault created the config file") } // The identity is the one thing a read cannot recompute, so it is provisioned @@ -310,8 +310,8 @@ func TestWouldChangeIgnoresRestatedCertificatePaths(t *testing.T) { // A read that lands on a missing file must not hand back keys: nothing would // write them down, so the caller would connect with an identity that changes on // the next run and registers a second peer. -func TestReadOrGenerateConfigCarriesNoIdentity(t *testing.T) { - cfg, err := ReadOrGenerateConfig(filepath.Join(t.TempDir(), "absent.json")) +func TestReadConfigOrDefaultCarriesNoIdentity(t *testing.T) { + cfg, err := ReadConfigOrDefault(filepath.Join(t.TempDir(), "absent.json")) require.NoError(t, err) require.Empty(t, cfg.PrivateKey, "a read minted a WireGuard key") diff --git a/client/ios/NetBirdSDK/preferences.go b/client/ios/NetBirdSDK/preferences.go index db1306df3..642f9e160 100644 --- a/client/ios/NetBirdSDK/preferences.go +++ b/client/ios/NetBirdSDK/preferences.go @@ -49,7 +49,7 @@ func (p *Preferences) GetManagementURL() (string, error) { return p.configInput.ManagementURL, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return "", err } @@ -67,7 +67,7 @@ func (p *Preferences) GetAdminURL() (string, error) { return p.configInput.AdminURL, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return "", err } @@ -89,7 +89,7 @@ func (p *Preferences) HasPreSharedKey() (bool, error) { return *p.configInput.PreSharedKey != "", nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -115,7 +115,7 @@ func (p *Preferences) GetRosenpassEnabled() (bool, error) { return *p.configInput.RosenpassEnabled, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -136,7 +136,7 @@ func (p *Preferences) GetRosenpassPermissive() (bool, error) { return *p.configInput.RosenpassPermissive, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -149,7 +149,7 @@ func (p *Preferences) GetDisableIPv6() (bool, error) { return *p.configInput.DisableIPv6, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } @@ -168,7 +168,7 @@ func (p *Preferences) GetRemoteJobsAllowed() (bool, error) { return *p.configInput.RemoteJobsAllowed, nil } - cfg, err := profilemanager.ReadOrGenerateConfig(p.configInput.ConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(p.configInput.ConfigPath) if err != nil { return false, err } diff --git a/client/server/login_overrides_test.go b/client/server/login_overrides_test.go index b455f23a4..f858c6059 100644 --- a/client/server/login_overrides_test.go +++ b/client/server/login_overrides_test.go @@ -87,7 +87,7 @@ func TestPersistLoginOverrides(t *testing.T) { }) require.NoError(t, err, "persistLoginOverrides") - cfg, err := profilemanager.ReadOrGenerateConfig(profilemanager.DefaultConfigPath) + cfg, err := profilemanager.ReadConfigOrDefault(profilemanager.DefaultConfigPath) require.NoError(t, err, "read back config") require.Equal(t, tt.wantMgmtURL, cfg.ManagementURL.String(), "management URL") diff --git a/client/server/server.go b/client/server/server.go index c57bbc6a3..f7f81b688 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -1524,7 +1524,7 @@ func provisionProfileIdentity(activeProf *profilemanager.ActiveProfileState) (*p log.Infof("active profile config existed: %t, err %v", configExisted, err) - config, err := profilemanager.ReadOrGenerateConfig(cfgPath) + config, err := profilemanager.ReadConfigOrDefault(cfgPath) if err != nil { return nil, false, fmt.Errorf("failed to get config: %w", err) }