mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-12 17:59:06 +02:00
[client] Normalize the config before diffing it in WouldChange
apply() reports two different things through one bool: an input that changed a value, and a field it had to fill in because the config carried none. The update-settings gate reads that bool as "the caller asked for a change", so any config still missing a default answered a request that asks for nothing with a refusal. Readers already hand out normalized configs — readConfig applies an empty input for exactly this reason — which is why the gate got away with it. But a handler that refuses a request must not depend on where its caller obtained the config, and it must not start reading "this profile predates a field" as "the caller asked for a change" the day someone adds one with a default. WouldChange now runs the filling-in as a pass of its own and discards its verdict, so the pass that answers the caller measures only what the input did.
This commit is contained in:
@@ -1090,6 +1090,22 @@ func (config *Config) WouldChange(input ConfigInput) (bool, error) {
|
||||
probe = baseline
|
||||
}
|
||||
|
||||
// Normalize before measuring. apply() reports two different things through
|
||||
// one bool: an input that changed a value, and a field it had to fill in
|
||||
// because the config carried none. Only the first is a settings change, so
|
||||
// the filling-in gets a pass of its own whose verdict is discarded, and the
|
||||
// pass that answers the caller runs against a config with nothing left to
|
||||
// fill in.
|
||||
//
|
||||
// Readers already hand out normalized configs — readConfig applies an empty
|
||||
// input for this very reason — so this is normally a no-op. But a gate that
|
||||
// refuses a request must not depend on where its caller got the config
|
||||
// from, and it must not start reading "this profile predates a field" as
|
||||
// "the caller asked for a change" the day someone adds one.
|
||||
if _, err := probe.apply(ConfigInput{ConfigPath: input.ConfigPath}); err != nil {
|
||||
return true, fmt.Errorf("normalize the config to diff against: %w", err)
|
||||
}
|
||||
|
||||
if isPreSharedKeyHidden(input.PreSharedKey) {
|
||||
input.PreSharedKey = nil
|
||||
}
|
||||
|
||||
@@ -448,6 +448,35 @@ func TestWouldChangeIgnoresRestatedDefaultsOfUnsetFields(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The verdict must not depend on where the caller got the config from. Readers
|
||||
// normalize what they hand out, but apply() signals "I filled in a default"
|
||||
// through the same bool as "the input changed something", so a config that
|
||||
// never passed through a read would otherwise report a change for an input
|
||||
// that asks for nothing.
|
||||
func TestWouldChangeNormalizesBeforeMeasuring(t *testing.T) {
|
||||
rawConfig := func(t *testing.T) *Config {
|
||||
t.Helper()
|
||||
|
||||
cfg := &Config{WgIface: iface.WgInterfaceDefault}
|
||||
require.Nil(t, cfg.ServerSSHAllowed, "the fixture is only useful while the config is not normalized")
|
||||
require.Nil(t, cfg.EnableSSHRoot)
|
||||
require.Empty(t, cfg.IFaceBlackList)
|
||||
return cfg
|
||||
}
|
||||
|
||||
changed, err := rawConfig(t).WouldChange(ConfigInput{})
|
||||
require.NoError(t, err)
|
||||
require.False(t, changed, "an input carrying nothing cannot change anything")
|
||||
|
||||
changed, err = rawConfig(t).WouldChange(ConfigInput{EnableSSHRoot: boolPtr(false)})
|
||||
require.NoError(t, err)
|
||||
require.False(t, changed, "the default of a field the config never held is not a change")
|
||||
|
||||
changed, err = rawConfig(t).WouldChange(ConfigInput{EnableSSHRoot: boolPtr(true)})
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed, "a non-default value is still a change")
|
||||
}
|
||||
|
||||
// A zero-padded port addresses the same port.
|
||||
func TestServiceURLPortIsNormalizedNumerically(t *testing.T) {
|
||||
padded, err := ParseServiceURL("padded", "https://mgmt.example.com:0443")
|
||||
|
||||
Reference in New Issue
Block a user