From f675e72d41b36481e38e8a1266926ba1f7fc516e Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 29 Oct 2024 22:45:25 +0100 Subject: [PATCH 1/7] Improve state safety --- client/firewall/iptables/rulestore_linux.go | 10 ++++++++ .../routemanager/refcounter/refcounter.go | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/client/firewall/iptables/rulestore_linux.go b/client/firewall/iptables/rulestore_linux.go index bfd08bee27d..004c512a4b4 100644 --- a/client/firewall/iptables/rulestore_linux.go +++ b/client/firewall/iptables/rulestore_linux.go @@ -37,6 +37,11 @@ func (s *ipList) UnmarshalJSON(data []byte) error { return err } s.ips = temp.IPs + + if temp.IPs == nil { + temp.IPs = make(map[string]struct{}) + } + return nil } @@ -89,5 +94,10 @@ func (s *ipsetStore) UnmarshalJSON(data []byte) error { return err } s.ipsets = temp.IPSets + + if temp.IPSets == nil { + temp.IPSets = make(map[string]*ipList) + } + return nil } diff --git a/client/internal/routemanager/refcounter/refcounter.go b/client/internal/routemanager/refcounter/refcounter.go index c121b7d774b..0ad6032ff2e 100644 --- a/client/internal/routemanager/refcounter/refcounter.go +++ b/client/internal/routemanager/refcounter/refcounter.go @@ -72,9 +72,15 @@ func New[Key comparable, I, O any](add AddFunc[Key, I, O], remove RemoveFunc[Key } // LoadData loads the data from the existing counter +// The passed counter should not be used any longer after calling this function. func (rm *Counter[Key, I, O]) LoadData( existingCounter *Counter[Key, I, O], ) { + existingCounter.refCountMu.Lock() + defer existingCounter.refCountMu.Unlock() + existingCounter.idMu.Lock() + defer existingCounter.idMu.Unlock() + rm.refCountMu.Lock() defer rm.refCountMu.Unlock() rm.idMu.Lock() @@ -217,6 +223,11 @@ func (rm *Counter[Key, I, O]) Clear() { // MarshalJSON implements the json.Marshaler interface for Counter. func (rm *Counter[Key, I, O]) MarshalJSON() ([]byte, error) { + rm.refCountMu.Lock() + defer rm.refCountMu.Unlock() + rm.idMu.Lock() + defer rm.idMu.Unlock() + return json.Marshal(struct { RefCountMap map[Key]Ref[O] `json:"refCountMap"` IDMap map[string][]Key `json:"idMap"` @@ -228,6 +239,11 @@ func (rm *Counter[Key, I, O]) MarshalJSON() ([]byte, error) { // UnmarshalJSON implements the json.Unmarshaler interface for Counter. func (rm *Counter[Key, I, O]) UnmarshalJSON(data []byte) error { + rm.refCountMu.Lock() + defer rm.refCountMu.Unlock() + rm.idMu.Lock() + defer rm.idMu.Unlock() + var temp struct { RefCountMap map[Key]Ref[O] `json:"refCountMap"` IDMap map[string][]Key `json:"idMap"` @@ -238,6 +254,13 @@ func (rm *Counter[Key, I, O]) UnmarshalJSON(data []byte) error { rm.refCountMap = temp.RefCountMap rm.idMap = temp.IDMap + if temp.RefCountMap == nil { + temp.RefCountMap = map[Key]Ref[O]{} + } + if temp.IDMap == nil { + temp.IDMap = map[string][]Key{} + } + return nil } From b1d2408ba6f2121f915006c4607bbff1f0f8798f Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 30 Oct 2024 00:41:22 +0100 Subject: [PATCH 2/7] Persist route selector --- client/internal/engine.go | 16 ++- client/internal/engine_test.go | 2 +- client/internal/routemanager/manager.go | 38 ++++++- client/internal/routemanager/manager_test.go | 4 +- client/internal/routemanager/mock.go | 2 +- client/internal/routemanager/state.go | 23 +++++ .../internal/routeselector/routeselector.go | 67 +++++++++++++ client/internal/statemanager/manager.go | 98 +++++++++++++++---- 8 files changed, 219 insertions(+), 31 deletions(-) create mode 100644 client/internal/routemanager/state.go diff --git a/client/internal/engine.go b/client/internal/engine.go index 190d795cdbe..a4630a4d0c5 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -38,7 +38,6 @@ import ( "github.com/netbirdio/netbird/client/internal/routemanager/systemops" "github.com/netbirdio/netbird/client/internal/statemanager" - nbssh "github.com/netbirdio/netbird/client/ssh" "github.com/netbirdio/netbird/client/system" nbdns "github.com/netbirdio/netbird/dns" @@ -171,7 +170,7 @@ type Engine struct { relayManager *relayClient.Manager stateManager *statemanager.Manager - srWatcher *guard.SRWatcher + srWatcher *guard.SRWatcher } // Peer is an instance of the Connection Peer @@ -349,8 +348,17 @@ func (e *Engine) Start() error { } e.dnsServer = dnsServer - e.routeManager = routemanager.NewManager(e.ctx, e.config.WgPrivateKey.PublicKey().String(), e.config.DNSRouteInterval, e.wgInterface, e.statusRecorder, e.relayManager, initialRoutes) - beforePeerHook, afterPeerHook, err := e.routeManager.Init(e.stateManager) + e.routeManager = routemanager.NewManager( + e.ctx, + e.config.WgPrivateKey.PublicKey().String(), + e.config.DNSRouteInterval, + e.wgInterface, + e.statusRecorder, + e.relayManager, + initialRoutes, + e.stateManager, + ) + beforePeerHook, afterPeerHook, err := e.routeManager.Init() if err != nil { log.Errorf("Failed to initialize route manager: %s", err) } else { diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 0018af6df8f..2df0a50e06e 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -250,7 +250,7 @@ func TestEngine_UpdateNetworkMap(t *testing.T) { }, } engine.wgInterface = wgIface - engine.routeManager = routemanager.NewManager(ctx, key.PublicKey().String(), time.Minute, engine.wgInterface, engine.statusRecorder, relayMgr, nil) + engine.routeManager = routemanager.NewManager(ctx, key.PublicKey().String(), time.Minute, engine.wgInterface, engine.statusRecorder, relayMgr, nil, nil) engine.dnsServer = &dns.MockServer{ UpdateDNSServerFunc: func(serial uint64, update nbdns.Config) error { return nil }, } diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go index 0a1c7dc56b8..f1c4ae5ef75 100644 --- a/client/internal/routemanager/manager.go +++ b/client/internal/routemanager/manager.go @@ -32,7 +32,7 @@ import ( // Manager is a route manager interface type Manager interface { - Init(*statemanager.Manager) (nbnet.AddHookFunc, nbnet.RemoveHookFunc, error) + Init() (nbnet.AddHookFunc, nbnet.RemoveHookFunc, error) UpdateRoutes(updateSerial uint64, newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap, error) TriggerSelection(route.HAMap) GetRouteSelector() *routeselector.RouteSelector @@ -59,6 +59,7 @@ type DefaultManager struct { routeRefCounter *refcounter.RouteRefCounter allowedIPsRefCounter *refcounter.AllowedIPsRefCounter dnsRouteInterval time.Duration + stateManager *statemanager.Manager } func NewManager( @@ -69,6 +70,7 @@ func NewManager( statusRecorder *peer.Status, relayMgr *relayClient.Manager, initialRoutes []*route.Route, + stateManager *statemanager.Manager, ) *DefaultManager { mCTX, cancel := context.WithCancel(ctx) notifier := notifier.NewNotifier() @@ -80,12 +82,12 @@ func NewManager( dnsRouteInterval: dnsRouteInterval, clientNetworks: make(map[route.HAUniqueID]*clientNetwork), relayMgr: relayMgr, - routeSelector: routeselector.NewRouteSelector(), sysOps: sysOps, statusRecorder: statusRecorder, wgInterface: wgInterface, pubKey: pubKey, notifier: notifier, + stateManager: stateManager, } dm.routeRefCounter = refcounter.New( @@ -121,7 +123,7 @@ func NewManager( } // Init sets up the routing -func (m *DefaultManager) Init(stateManager *statemanager.Manager) (nbnet.AddHookFunc, nbnet.RemoveHookFunc, error) { +func (m *DefaultManager) Init() (nbnet.AddHookFunc, nbnet.RemoveHookFunc, error) { if nbnet.CustomRoutingDisabled() { return nil, nil, nil } @@ -137,14 +139,38 @@ func (m *DefaultManager) Init(stateManager *statemanager.Manager) (nbnet.AddHook ips := resolveURLsToIPs(initialAddresses) - beforePeerHook, afterPeerHook, err := m.sysOps.SetupRouting(ips, stateManager) + beforePeerHook, afterPeerHook, err := m.sysOps.SetupRouting(ips, m.stateManager) if err != nil { return nil, nil, fmt.Errorf("setup routing: %w", err) } + + m.routeSelector = m.initSelector() + log.Info("Routing setup complete") return beforePeerHook, afterPeerHook, nil } +func (m *DefaultManager) initSelector() *routeselector.RouteSelector { + var state *SelectorState + m.stateManager.RegisterState(state) + + // restore selector state if it exists + if err := m.stateManager.LoadState(state); err != nil { + log.Warnf("failed to load state: %v", err) + return routeselector.NewRouteSelector() + } + + if state := m.stateManager.GetState(state); state != nil { + if selector, ok := state.(*SelectorState); ok { + return (*routeselector.RouteSelector)(selector) + } + + log.Warnf("failed to convert state with type %T to SelectorState", state) + } + + return routeselector.NewRouteSelector() +} + func (m *DefaultManager) EnableServerRouter(firewall firewall.Manager) error { var err error m.serverRouter, err = newServerRouter(m.ctx, m.wgInterface, firewall, m.statusRecorder) @@ -252,6 +278,10 @@ func (m *DefaultManager) TriggerSelection(networks route.HAMap) { go clientNetworkWatcher.peersStateAndUpdateWatcher() clientNetworkWatcher.sendUpdateToClientNetworkWatcher(routesUpdate{routes: routes}) } + + if err := m.stateManager.UpdateState((*SelectorState)(m.routeSelector)); err != nil { + log.Errorf("failed to update state: %v", err) + } } // stopObsoleteClients stops the client network watcher for the networks that are not in the new list diff --git a/client/internal/routemanager/manager_test.go b/client/internal/routemanager/manager_test.go index e669bc44a08..07dac21b819 100644 --- a/client/internal/routemanager/manager_test.go +++ b/client/internal/routemanager/manager_test.go @@ -424,9 +424,9 @@ func TestManagerUpdateRoutes(t *testing.T) { statusRecorder := peer.NewRecorder("https://mgm") ctx := context.TODO() - routeManager := NewManager(ctx, localPeerKey, 0, wgInterface, statusRecorder, nil, nil) + routeManager := NewManager(ctx, localPeerKey, 0, wgInterface, statusRecorder, nil, nil, nil) - _, _, err = routeManager.Init(nil) + _, _, err = routeManager.Init() require.NoError(t, err, "should init route manager") defer routeManager.Stop(nil) diff --git a/client/internal/routemanager/mock.go b/client/internal/routemanager/mock.go index 503185f0311..556a6235138 100644 --- a/client/internal/routemanager/mock.go +++ b/client/internal/routemanager/mock.go @@ -21,7 +21,7 @@ type MockManager struct { StopFunc func(manager *statemanager.Manager) } -func (m *MockManager) Init(*statemanager.Manager) (net.AddHookFunc, net.RemoveHookFunc, error) { +func (m *MockManager) Init() (net.AddHookFunc, net.RemoveHookFunc, error) { return nil, nil, nil } diff --git a/client/internal/routemanager/state.go b/client/internal/routemanager/state.go new file mode 100644 index 00000000000..069a0570d53 --- /dev/null +++ b/client/internal/routemanager/state.go @@ -0,0 +1,23 @@ +package routemanager + +import ( + "github.com/netbirdio/netbird/client/internal/routeselector" +) + +type SelectorState routeselector.RouteSelector + +func (s *SelectorState) Name() string { + return "routeselector_state" +} + +func (s *SelectorState) Cleanup() error { + return nil +} + +func (s *SelectorState) MarshalJSON() ([]byte, error) { + return (*routeselector.RouteSelector)(s).MarshalJSON() +} + +func (s *SelectorState) UnmarshalJSON(data []byte) error { + return (*routeselector.RouteSelector)(s).UnmarshalJSON(data) +} diff --git a/client/internal/routeselector/routeselector.go b/client/internal/routeselector/routeselector.go index 00128a27b03..2874604fdd3 100644 --- a/client/internal/routeselector/routeselector.go +++ b/client/internal/routeselector/routeselector.go @@ -1,8 +1,10 @@ package routeselector import ( + "encoding/json" "fmt" "slices" + "sync" "github.com/hashicorp/go-multierror" "golang.org/x/exp/maps" @@ -12,6 +14,7 @@ import ( ) type RouteSelector struct { + mu sync.RWMutex selectedRoutes map[route.NetID]struct{} selectAll bool } @@ -26,6 +29,9 @@ func NewRouteSelector() *RouteSelector { // SelectRoutes updates the selected routes based on the provided route IDs. func (rs *RouteSelector) SelectRoutes(routes []route.NetID, appendRoute bool, allRoutes []route.NetID) error { + rs.mu.Lock() + defer rs.mu.Unlock() + if !appendRoute { rs.selectedRoutes = map[route.NetID]struct{}{} } @@ -46,6 +52,9 @@ func (rs *RouteSelector) SelectRoutes(routes []route.NetID, appendRoute bool, al // SelectAllRoutes sets the selector to select all routes. func (rs *RouteSelector) SelectAllRoutes() { + rs.mu.Lock() + defer rs.mu.Unlock() + rs.selectAll = true rs.selectedRoutes = map[route.NetID]struct{}{} } @@ -53,6 +62,9 @@ func (rs *RouteSelector) SelectAllRoutes() { // DeselectRoutes removes specific routes from the selection. // If the selector is in "select all" mode, it will transition to "select specific" mode. func (rs *RouteSelector) DeselectRoutes(routes []route.NetID, allRoutes []route.NetID) error { + rs.mu.Lock() + defer rs.mu.Unlock() + if rs.selectAll { rs.selectAll = false rs.selectedRoutes = map[route.NetID]struct{}{} @@ -76,12 +88,18 @@ func (rs *RouteSelector) DeselectRoutes(routes []route.NetID, allRoutes []route. // DeselectAllRoutes deselects all routes, effectively disabling route selection. func (rs *RouteSelector) DeselectAllRoutes() { + rs.mu.Lock() + defer rs.mu.Unlock() + rs.selectAll = false rs.selectedRoutes = map[route.NetID]struct{}{} } // IsSelected checks if a specific route is selected. func (rs *RouteSelector) IsSelected(routeID route.NetID) bool { + rs.mu.RLock() + defer rs.mu.RUnlock() + if rs.selectAll { return true } @@ -91,6 +109,9 @@ func (rs *RouteSelector) IsSelected(routeID route.NetID) bool { // FilterSelected removes unselected routes from the provided map. func (rs *RouteSelector) FilterSelected(routes route.HAMap) route.HAMap { + rs.mu.RLock() + defer rs.mu.RUnlock() + if rs.selectAll { return maps.Clone(routes) } @@ -103,3 +124,49 @@ func (rs *RouteSelector) FilterSelected(routes route.HAMap) route.HAMap { } return filtered } + +// MarshalJSON implements the json.Marshaler interface +func (rs *RouteSelector) MarshalJSON() ([]byte, error) { + rs.mu.RLock() + defer rs.mu.RUnlock() + + return json.Marshal(struct { + SelectedRoutes map[route.NetID]struct{} `json:"selected_routes"` + SelectAll bool `json:"select_all"` + }{ + SelectAll: rs.selectAll, + SelectedRoutes: rs.selectedRoutes, + }) +} + +// UnmarshalJSON implements the json.Unmarshaler interface +// If the JSON is empty or null, it will initialize like a NewRouteSelector. +func (rs *RouteSelector) UnmarshalJSON(data []byte) error { + rs.mu.Lock() + defer rs.mu.Unlock() + + // Check for null or empty JSON + if len(data) == 0 || string(data) == "null" { + rs.selectedRoutes = map[route.NetID]struct{}{} + rs.selectAll = true + return nil + } + + var temp struct { + SelectedRoutes map[route.NetID]struct{} `json:"selected_routes"` + SelectAll bool `json:"select_all"` + } + + if err := json.Unmarshal(data, &temp); err != nil { + return err + } + + rs.selectedRoutes = temp.SelectedRoutes + rs.selectAll = temp.SelectAll + + if rs.selectedRoutes == nil { + rs.selectedRoutes = map[route.NetID]struct{}{} + } + + return nil +} diff --git a/client/internal/statemanager/manager.go b/client/internal/statemanager/manager.go index a5a14f807a2..043d624cb53 100644 --- a/client/internal/statemanager/manager.go +++ b/client/internal/statemanager/manager.go @@ -215,15 +215,15 @@ func (m *Manager) PersistState(ctx context.Context) error { return nil } -// loadState loads the existing state from the state file -func (m *Manager) loadState() error { +// loadStateFile reads and unmarshals the state file into a map of raw JSON messages +func (m *Manager) loadStateFile() (map[string]json.RawMessage, error) { data, err := os.ReadFile(m.filePath) if err != nil { if errors.Is(err, fs.ErrNotExist) { log.Debug("state file does not exist") - return nil + return nil, nil } - return fmt.Errorf("read state file: %w", err) + return nil, fmt.Errorf("read state file: %w", err) } var rawStates map[string]json.RawMessage @@ -234,30 +234,90 @@ func (m *Manager) loadState() error { } else { log.Info("State file deleted") } - return fmt.Errorf("unmarshal states: %w", err) + return nil, fmt.Errorf("unmarshal states: %w", err) + } + + return rawStates, nil +} + +// loadSingleRawState unmarshals a raw state into a concrete state object +func (m *Manager) loadSingleRawState(name string, rawState json.RawMessage) (State, error) { + stateType, ok := m.stateTypes[name] + if !ok { + return nil, fmt.Errorf("state %s not registered", name) + } + + if string(rawState) == "null" { + return nil, nil //nolint:nilnil + } + + statePtr := reflect.New(stateType).Interface().(State) + if err := json.Unmarshal(rawState, statePtr); err != nil { + return nil, fmt.Errorf("unmarshal state %s: %w", name, err) + } + + return statePtr, nil +} + +// LoadState loads a specific state from the state file +func (m *Manager) LoadState(state State) error { + if m == nil { + return nil + } + + m.mu.Lock() + defer m.mu.Unlock() + + rawStates, err := m.loadStateFile() + if err != nil { + return err + } + if rawStates == nil { + return nil + } + + name := state.Name() + rawState, exists := rawStates[name] + if !exists { + return nil + } + + loadedState, err := m.loadSingleRawState(name, rawState) + if err != nil { + return err + } + + m.states[name] = loadedState + if loadedState != nil { + log.Debugf("loaded state: %s", name) + } + + return nil +} + +// loadState loads all registered states from the state file +func (m *Manager) loadState() error { + rawStates, err := m.loadStateFile() + if err != nil { + return err + } + if rawStates == nil { + return nil } var merr *multierror.Error for name, rawState := range rawStates { - stateType, ok := m.stateTypes[name] - if !ok { - merr = multierror.Append(merr, fmt.Errorf("unknown state type: %s", name)) - continue - } - - if string(rawState) == "null" { + loadedState, err := m.loadSingleRawState(name, rawState) + if err != nil { + merr = multierror.Append(merr, err) continue } - statePtr := reflect.New(stateType).Interface().(State) - if err := json.Unmarshal(rawState, statePtr); err != nil { - merr = multierror.Append(merr, fmt.Errorf("unmarshal state %s: %w", name, err)) - continue + m.states[name] = loadedState + if loadedState != nil { + log.Debugf("loaded state: %s", name) } - - m.states[name] = statePtr - log.Debugf("loaded state: %s", name) } return nberrors.FormatErrorOrNil(merr) From 9d22cf79af8f5108f7fc88ab59ceac3af7568969 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 30 Oct 2024 12:02:58 +0100 Subject: [PATCH 3/7] Improve state behavior for states that don't need cleanup --- client/internal/routemanager/state.go | 4 -- client/internal/statemanager/manager.go | 66 ++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/client/internal/routemanager/state.go b/client/internal/routemanager/state.go index 069a0570d53..a45c32b506d 100644 --- a/client/internal/routemanager/state.go +++ b/client/internal/routemanager/state.go @@ -10,10 +10,6 @@ func (s *SelectorState) Name() string { return "routeselector_state" } -func (s *SelectorState) Cleanup() error { - return nil -} - func (s *SelectorState) MarshalJSON() ([]byte, error) { return (*routeselector.RouteSelector)(s).MarshalJSON() } diff --git a/client/internal/statemanager/manager.go b/client/internal/statemanager/manager.go index 043d624cb53..59f758c3545 100644 --- a/client/internal/statemanager/manager.go +++ b/client/internal/statemanager/manager.go @@ -21,9 +21,28 @@ import ( // State interface defines the methods that all state types must implement type State interface { Name() string +} + +// CleanableState interface extends State with cleanup capability +type CleanableState interface { + State Cleanup() error } +// RawState wraps raw JSON data for unregistered states +type RawState struct { + data json.RawMessage +} + +func (r *RawState) Name() string { + return "" // This is a placeholder implementation +} + +// MarshalJSON implements json.Marshaler to preserve the original JSON +func (r *RawState) MarshalJSON() ([]byte, error) { + return r.data, nil +} + // Manager handles the persistence and management of various states type Manager struct { mu sync.Mutex @@ -323,8 +342,8 @@ func (m *Manager) loadState() error { return nberrors.FormatErrorOrNil(merr) } -// PerformCleanup retrieves all states from the state file for the registered states and calls Cleanup on them. -// If the cleanup is successful, the state is marked for deletion. +// PerformCleanup retrieves all states from the state file and calls Cleanup on registered states that support it. +// Unregistered states are preserved in their original state. func (m *Manager) PerformCleanup() error { if m == nil { return nil @@ -333,22 +352,53 @@ func (m *Manager) PerformCleanup() error { m.mu.Lock() defer m.mu.Unlock() - if err := m.loadState(); err != nil { + // Load raw states from file + rawStates, err := m.loadStateFile() + if err != nil { log.Warnf("Failed to load state during cleanup: %v", err) + return err + } + if rawStates == nil { + return nil } var merr *multierror.Error - for name, state := range m.states { - if state == nil { - // If no state was found in the state file, we don't mark the state dirty nor return an error + + // Process each state in the file + for name, rawState := range rawStates { + // For unregistered states, preserve the raw JSON + if _, registered := m.stateTypes[name]; !registered { + m.states[name] = &RawState{data: rawState} + continue + } + + // Load the registered state + loadedState, err := m.loadSingleRawState(name, rawState) + if err != nil { + merr = multierror.Append(merr, err) + continue + } + + if loadedState == nil { + continue + } + + // Check if state supports cleanup + cleanableState, isCleanable := loadedState.(CleanableState) + if !isCleanable { + // If it doesn't support cleanup, keep it as-is + m.states[name] = loadedState continue } + // Perform cleanup for cleanable states log.Infof("client was not shut down properly, cleaning up %s", name) - if err := state.Cleanup(); err != nil { + if err := cleanableState.Cleanup(); err != nil { merr = multierror.Append(merr, fmt.Errorf("cleanup state for %s: %w", name, err)) + // On cleanup error, preserve the state + m.states[name] = loadedState } else { - // mark for deletion on cleanup success + // Successfully cleaned up - mark for deletion m.states[name] = nil m.dirty[name] = struct{}{} } From 67f8ab988675a4634855924a5b45c633ddc0b262 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 30 Oct 2024 15:24:49 +0100 Subject: [PATCH 4/7] Fix test --- client/internal/engine_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 2df0a50e06e..ddfae0a2f1f 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -251,6 +251,8 @@ func TestEngine_UpdateNetworkMap(t *testing.T) { } engine.wgInterface = wgIface engine.routeManager = routemanager.NewManager(ctx, key.PublicKey().String(), time.Minute, engine.wgInterface, engine.statusRecorder, relayMgr, nil, nil) + _, _, err = engine.routeManager.Init() + require.NoError(t, err) engine.dnsServer = &dns.MockServer{ UpdateDNSServerFunc: func(serial uint64, update nbdns.Config) error { return nil }, } From 2c100fc9ea7f2641464532895f52aa5ae1fcfd2e Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 30 Oct 2024 15:27:40 +0100 Subject: [PATCH 5/7] Remove unused code --- client/internal/statemanager/manager.go | 28 ------------------------- 1 file changed, 28 deletions(-) diff --git a/client/internal/statemanager/manager.go b/client/internal/statemanager/manager.go index 59f758c3545..8d112b3efeb 100644 --- a/client/internal/statemanager/manager.go +++ b/client/internal/statemanager/manager.go @@ -314,34 +314,6 @@ func (m *Manager) LoadState(state State) error { return nil } -// loadState loads all registered states from the state file -func (m *Manager) loadState() error { - rawStates, err := m.loadStateFile() - if err != nil { - return err - } - if rawStates == nil { - return nil - } - - var merr *multierror.Error - - for name, rawState := range rawStates { - loadedState, err := m.loadSingleRawState(name, rawState) - if err != nil { - merr = multierror.Append(merr, err) - continue - } - - m.states[name] = loadedState - if loadedState != nil { - log.Debugf("loaded state: %s", name) - } - } - - return nberrors.FormatErrorOrNil(merr) -} - // PerformCleanup retrieves all states from the state file and calls Cleanup on registered states that support it. // Unregistered states are preserved in their original state. func (m *Manager) PerformCleanup() error { From b0bc0c797c19421f6b37559bfaa87a23a62d9121 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 30 Oct 2024 15:28:19 +0100 Subject: [PATCH 6/7] Ignore lint --- client/internal/statemanager/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/internal/statemanager/manager.go b/client/internal/statemanager/manager.go index 8d112b3efeb..448ff899204 100644 --- a/client/internal/statemanager/manager.go +++ b/client/internal/statemanager/manager.go @@ -240,7 +240,7 @@ func (m *Manager) loadStateFile() (map[string]json.RawMessage, error) { if err != nil { if errors.Is(err, fs.ErrNotExist) { log.Debug("state file does not exist") - return nil, nil + return nil, nil // nolint:nilnil } return nil, fmt.Errorf("read state file: %w", err) } From b78fd1bc1bffc23062359e239a5cc5a00698dcb2 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 30 Oct 2024 19:14:32 +0100 Subject: [PATCH 7/7] Fix test --- client/internal/engine_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index ddfae0a2f1f..5a6f05adb2c 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -245,6 +245,7 @@ func TestEngine_UpdateNetworkMap(t *testing.T) { nil) wgIface := &iface.MockWGIface{ + NameFunc: func() string { return "utun102" }, RemovePeerFunc: func(peerKey string) error { return nil },