Allow other states for windows neighbor network monitor

* Fix route retrieval condition

* Make error messages take domains into account

client/internal/networkmonitor/monitor_windows.go

@@ -33,12 +33,8 @@ func checkChange(ctx context.Context, nexthopv4, nexthopv6 systemops.Nexthop, ca
 			return fmt.Errorf("get neighbors: %w", err)
 		}
 
-		if n, ok := initialNeighbors[nexthopv4.IP]; ok {
-			neighborv4 = &n
-		}
-		if n, ok := initialNeighbors[nexthopv6.IP]; ok {
-			neighborv6 = &n
-		}
+		neighborv4 = assignNeighbor(nexthopv4, initialNeighbors)
+		neighborv6 = assignNeighbor(nexthopv6, initialNeighbors)
 	}
 	log.Debugf("Network monitor: initial IPv4 neighbor: %v, IPv6 neighbor: %v", neighborv4, neighborv6)
 
@@ -58,6 +54,16 @@ func checkChange(ctx context.Context, nexthopv4, nexthopv6 systemops.Nexthop, ca
 	}
 }
 
+func assignNeighbor(nexthop systemops.Nexthop, initialNeighbors map[netip.Addr]systemops.Neighbor) *systemops.Neighbor {
+	if n, ok := initialNeighbors[nexthop.IP]; ok &&
+		n.State != unreachable &&
+		n.State != incomplete &&
+		n.State != tbd {
+		return &n
+	}
+	return nil
+}
+
 func changed(
 	nexthopv4 systemops.Nexthop,
 	neighborv4 *systemops.Neighbor,
@@ -101,11 +107,14 @@ func routeChanged(nexthop systemops.Nexthop, intf *net.Interface, routes map[net
 
 	if r, ok := routes[unspec]; ok {
 		if r.Nexthop != nexthop.IP || compareIntf(r.Interface, intf) != 0 {
-			intf := "<nil>"
-			if r.Interface != nil {
-				intf = r.Interface.Name
+			oldIntf, newIntf := "<nil>", "<nil>"
+			if intf != nil {
+				oldIntf = intf.Name
 			}
-			log.Infof("network monitor: default route changed: %s via %s (%s)", r.Destination, r.Nexthop, intf)
+			if r.Interface != nil {
+				newIntf = r.Interface.Name
+			}
+			log.Infof("network monitor: default route changed: %s from %s (%s) to %s (%s)", r.Destination, nexthop.IP, oldIntf, r.Nexthop, newIntf)
 			return true
 		}
 	} else {
@@ -124,7 +133,7 @@ func neighborChanged(nexthop systemops.Nexthop, neighbor *systemops.Neighbor, ne
 
 	// TODO: consider non-local nexthops, e.g. on point-to-point interfaces
 	if n, ok := neighbors[nexthop.IP]; ok {
-		if n.State != reachable && n.State != permanent {
+		if n.State == unreachable || n.State == incomplete {
 			log.Infof("network monitor: neighbor %s (%s) is not reachable: %s", neighbor.IPAddress, neighbor.LinkLayerAddress, stateFromInt(n.State))
 			return true
 		} else if n.InterfaceIndex != neighbor.InterfaceIndex {

client/internal/routemanager/dynamic/route.go

@@ -23,7 +23,8 @@ import (
 const (
 	DefaultInterval = time.Minute
 
-	minInterval = 2 * time.Second
+	minInterval     = 2 * time.Second
+	failureInterval = 5 * time.Second
 
 	addAllowedIP = "add allowed IP %s: %w"
 )
@@ -160,7 +161,12 @@ func (r *Route) startResolver(ctx context.Context) {
 	ticker := time.NewTicker(interval)
 	defer ticker.Stop()
 
-	r.update(ctx)
+	if err := r.update(ctx); err != nil {
+		log.Errorf("Failed to resolve domains for route [%v]: %v", r, err)
+		if interval > failureInterval {
+			ticker.Reset(failureInterval)
+		}
+	}
 
 	for {
 		select {
@@ -168,17 +174,28 @@ func (r *Route) startResolver(ctx context.Context) {
 			log.Debugf("Stopping dynamic route resolver for domains [%v]", r)
 			return
 		case <-ticker.C:
-			r.update(ctx)
+			if err := r.update(ctx); err != nil {
+				log.Errorf("Failed to resolve domains for route [%v]: %v", r, err)
+				// Use a lower ticker interval if the update fails
+				if interval > failureInterval {
+					ticker.Reset(failureInterval)
+				}
+			} else if interval > failureInterval {
+				// Reset to the original interval if the update succeeds
+				ticker.Reset(interval)
+			}
 		}
 	}
 }
 
-func (r *Route) update(ctx context.Context) {
+func (r *Route) update(ctx context.Context) error {
 	if resolved, err := r.resolveDomains(); err != nil {
-		log.Errorf("Failed to resolve domains for route [%v]: %v", r, err)
+		return fmt.Errorf("resolve domains: %w", err)
 	} else if err := r.updateDynamicRoutes(ctx, resolved); err != nil {
-		log.Errorf("Failed to update dynamic routes for [%v]: %v", r, err)
+		return fmt.Errorf("update dynamic routes: %w", err)
 	}
+
+	return nil
 }
 
 func (r *Route) resolveDomains() (domainMap, error) {

client/internal/routemanager/systemops/systemops_bsd.go

@@ -92,7 +92,7 @@ func toNetIP(a route.Addr) netip.Addr {
 	case *route.Inet6Addr:
 		ip := netip.AddrFrom16(t.IP)
 		if t.ZoneID != 0 {
-			ip.WithZone(strconv.Itoa(t.ZoneID))
+			ip = ip.WithZone(strconv.Itoa(t.ZoneID))
 		}
 		return ip
 	default:

client/internal/routemanager/systemops/systemops_generic.go

@@ -356,7 +356,7 @@ func GetNextHop(ip netip.Addr) (Nexthop, error) {
 			return Nexthop{}, fmt.Errorf("convert preferred source to address: %w", err)
 		}
 		return Nexthop{
-			IP:   addr.Unmap(),
+			IP:   addr,
 			Intf: intf,
 		}, nil
 	}
@@ -380,12 +380,12 @@ func ipToAddr(ip net.IP, intf *net.Interface) (netip.Addr, error) {
 	}
 
 	if intf != nil && (addr.IsLinkLocalMulticast() || addr.IsLinkLocalUnicast()) {
-		log.Tracef("Adding zone %s to address %s", intf.Name, addr)
+		zone := intf.Name
 		if runtime.GOOS == "windows" {
-			addr = addr.WithZone(strconv.Itoa(intf.Index))
-		} else {
-			addr = addr.WithZone(intf.Name)
+			zone = strconv.Itoa(intf.Index)
 		}
+		log.Tracef("Adding zone %s to address %s", zone, addr)
+		addr = addr.WithZone(zone)
 	}
 
 	return addr.Unmap(), nil

client/internal/routemanager/systemops/systemops_windows.go

@@ -71,7 +71,6 @@ func (r *SysOps) addToRouteTable(prefix netip.Prefix, nexthop Nexthop) error {
 			return fmt.Errorf("invalid zone: %w", err)
 		}
 		nexthop.Intf = &net.Interface{Index: zone}
-		nexthop.IP.WithZone("")
 	}
 
 	return addRouteCmd(prefix, nexthop)
@@ -80,8 +79,8 @@ func (r *SysOps) addToRouteTable(prefix netip.Prefix, nexthop Nexthop) error {
 func (r *SysOps) removeFromRouteTable(prefix netip.Prefix, nexthop Nexthop) error {
 	args := []string{"delete", prefix.String()}
 	if nexthop.IP.IsValid() {
-		nexthop.IP.WithZone("")
-		args = append(args, nexthop.IP.Unmap().String())
+		ip := nexthop.IP.WithZone("")
+		args = append(args, ip.Unmap().String())
 	}
 
 	routeCmd := uspfilter.GetSystem32Command("route")
@@ -146,6 +145,10 @@ func GetRoutes() ([]Route, error) {
 				Index: int(entry.InterfaceIndex),
 				Name:  entry.InterfaceAlias,
 			}
+
+			if nexthop.Is6() && (nexthop.IsLinkLocalUnicast() || nexthop.IsLinkLocalMulticast()) {
+				nexthop = nexthop.WithZone(strconv.Itoa(int(entry.InterfaceIndex)))
+			}
 		}
 
 		routes = append(routes, Route{
@@ -189,7 +192,8 @@ func addRouteCmd(prefix netip.Prefix, nexthop Nexthop) error {
 	args := []string{"add", prefix.String()}
 
 	if nexthop.IP.IsValid() {
-		args = append(args, nexthop.IP.Unmap().String())
+		ip := nexthop.IP.WithZone("")
+		args = append(args, ip.Unmap().String())
 	} else {
 		addr := "0.0.0.0"
 		if prefix.Addr().Is6() {

management/server/account.go

@@ -18,6 +18,11 @@ import (
 
 	"github.com/eko/gocache/v3/cache"
 	cacheStore "github.com/eko/gocache/v3/store"
+	gocache "github.com/patrickmn/go-cache"
+	"github.com/rs/xid"
+	log "github.com/sirupsen/logrus"
+	"golang.org/x/exp/maps"
+
 	"github.com/netbirdio/netbird/base62"
 	nbdns "github.com/netbirdio/netbird/dns"
 	"github.com/netbirdio/netbird/management/domain"
@@ -33,10 +38,6 @@ import (
 	"github.com/netbirdio/netbird/management/server/posture"
 	"github.com/netbirdio/netbird/management/server/status"
 	"github.com/netbirdio/netbird/route"
-	gocache "github.com/patrickmn/go-cache"
-	"github.com/rs/xid"
-	log "github.com/sirupsen/logrus"
-	"golang.org/x/exp/maps"
 )
 
 const (
@@ -383,9 +384,9 @@ func (a *Account) getRoutingPeerRoutes(peerID string) (enabledRoutes []*route.Ro
 func (a *Account) GetRoutesByPrefixOrDomains(prefix netip.Prefix, domains domain.List) []*route.Route {
 	var routes []*route.Route
 	for _, r := range a.Routes {
-		if r.IsDynamic() && r.Domains.PunycodeString() == domains.PunycodeString() {
-			routes = append(routes, r)
-		} else if r.Network.String() == prefix.String() {
+		dynamic := r.IsDynamic()
+		if dynamic && r.Domains.PunycodeString() == domains.PunycodeString() ||
+			!dynamic && r.Network.String() == prefix.String() {
 			routes = append(routes, r)
 		}
 	}

management/server/route.go

@@ -1,6 +1,7 @@
 package server
 
 import (
+	"fmt"
 	"net/netip"
 	"unicode/utf8"
 
@@ -51,7 +52,7 @@ func (am *DefaultAccountManager) checkRoutePrefixOrDomainsExistForPeers(account
 
 	for _, prefixRoute := range routesWithPrefix {
 		// we skip route(s) with the same network ID as we want to allow updating of the existing route
-		// when create a new route routeID is newly generated so nothing will be skipped
+		// when creating a new route routeID is newly generated so nothing will be skipped
 		if routeID == prefixRoute.ID {
 			continue
 		}
@@ -65,8 +66,9 @@ func (am *DefaultAccountManager) checkRoutePrefixOrDomainsExistForPeers(account
 			group := account.GetGroup(groupID)
 			if group == nil {
 				return status.Errorf(
-					status.InvalidArgument, "failed to add route with prefix %s - peer group %s doesn't exist",
-					prefix.String(), groupID)
+					status.InvalidArgument, "failed to add route with %s - peer group %s doesn't exist",
+					getRouteDescriptor(prefix, domains), groupID,
+				)
 			}
 
 			for _, pID := range group.Peers {
@@ -83,18 +85,18 @@ func (am *DefaultAccountManager) checkRoutePrefixOrDomainsExistForPeers(account
 		}
 		if _, ok := seenPeers[peerID]; ok {
 			return status.Errorf(status.AlreadyExists,
-				"failed to add route with prefix %s - peer %s already has this route", prefix.String(), peerID)
+				"failed to add route with %s - peer %s already has this route", getRouteDescriptor(prefix, domains), peerID)
 		}
 	}
 
 	// check that peerGroupIDs are not in any route peerGroups list
 	for _, groupID := range peerGroupIDs {
-		group := account.GetGroup(groupID) // we validated the group existent before entering this function, o need to check again.
+		group := account.GetGroup(groupID) // we validated the group existence before entering this function, no need to check again.
 
 		if _, ok := seenPeerGroups[groupID]; ok {
 			return status.Errorf(
-				status.AlreadyExists, "failed to add route with prefix %s - peer group %s already has this route",
-				prefix.String(), group.Name)
+				status.AlreadyExists, "failed to add route with %s - peer group %s already has this route",
+				getRouteDescriptor(prefix, domains), group.Name)
 		}
 
 		// check that the peers from peerGroupIDs groups are not the same peers we saw in routesWithPrefix
@@ -105,8 +107,8 @@ func (am *DefaultAccountManager) checkRoutePrefixOrDomainsExistForPeers(account
 					return status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID)
 				}
 				return status.Errorf(status.AlreadyExists,
-					"failed to add route with prefix %s - peer %s from the group %s already has this route",
-					prefix.String(), peer.Name, group.Name)
+					"failed to add route with %s - peer %s from the group %s already has this route",
+					getRouteDescriptor(prefix, domains), peer.Name, group.Name)
 			}
 		}
 	}
@@ -114,6 +116,13 @@ func (am *DefaultAccountManager) checkRoutePrefixOrDomainsExistForPeers(account
 	return nil
 }
 
+func getRouteDescriptor(prefix netip.Prefix, domains domain.List) string {
+	if len(domains) > 0 {
+		return fmt.Sprintf("domains [%s]", domains.SafeString())
+	}
+	return fmt.Sprintf("prefix %s", prefix.String())
+}
+
 // CreateRoute creates and saves a new route
 func (am *DefaultAccountManager) CreateRoute(accountID string, prefix netip.Prefix, networkType route.NetworkType, domains domain.List, peerID string, peerGroupIDs []string, description string, netID route.NetID, masquerade bool, metric int, groups []string, enabled bool, userID string, keepRoute bool) (*route.Route, error) {
 	unlock := am.Store.AcquireAccountWriteLock(accountID)

management/server/store.go

@@ -86,20 +86,38 @@ func getStoreEngineFromEnv() StoreEngine {
 	return SqliteStoreEngine
 }
 
-// getStoreEngine determines the store engine to use
-func getStoreEngine(kind StoreEngine) StoreEngine {
+// getStoreEngine determines the store engine to use.
+// If no engine is specified, it attempts to retrieve it from the environment.
+// If still not specified, it defaults to using SQLite.
+// Additionally, it handles the migration from a JSON store file to SQLite if applicable.
+func getStoreEngine(dataDir string, kind StoreEngine) StoreEngine {
 	if kind == "" {
 		kind = getStoreEngineFromEnv()
 		if kind == "" {
 			kind = SqliteStoreEngine
+
+			// Migrate if it is the first run with a JSON file existing and no SQLite file present
+			jsonStoreFile := filepath.Join(dataDir, storeFileName)
+			sqliteStoreFile := filepath.Join(dataDir, storeSqliteFileName)
+
+			if util.FileExists(jsonStoreFile) && !util.FileExists(sqliteStoreFile) {
+				log.Warnf("unsupported store engine specified, but found %s. Automatically migrating to SQLite.", jsonStoreFile)
+
+				// Attempt to migrate from JSON store to SQLite
+				if err := MigrateFileStoreToSqlite(dataDir); err != nil {
+					log.Errorf("failed to migrate filestore to SQLite: %v", err)
+					kind = FileStoreEngine
+				}
+			}
 		}
 	}
+
 	return kind
 }
 
 // NewStore creates a new store based on the provided engine type, data directory, and telemetry metrics
 func NewStore(kind StoreEngine, dataDir string, metrics telemetry.AppMetrics) (Store, error) {
-	kind = getStoreEngine(kind)
+	kind = getStoreEngine(dataDir, kind)
 
 	if err := checkFileStoreEngine(kind, dataDir); err != nil {
 		return nil, err
@@ -113,7 +131,7 @@ func NewStore(kind StoreEngine, dataDir string, metrics telemetry.AppMetrics) (S
 		log.Info("using Postgres store engine")
 		return newPostgresStore(metrics)
 	default:
-		return handleUnsupportedStoreEngine(kind, dataDir, metrics)
+		return nil, fmt.Errorf("unsupported kind of store: %s", kind)
 	}
 }
 
@@ -128,25 +146,6 @@ func checkFileStoreEngine(kind StoreEngine, dataDir string) error {
 	return nil
 }
 
-// handleUnsupportedStoreEngine handles cases where the store engine is unsupported
-func handleUnsupportedStoreEngine(kind StoreEngine, dataDir string, metrics telemetry.AppMetrics) (Store, error) {
-	jsonStoreFile := filepath.Join(dataDir, storeFileName)
-	sqliteStoreFile := filepath.Join(dataDir, storeSqliteFileName)
-
-	if util.FileExists(jsonStoreFile) && !util.FileExists(sqliteStoreFile) {
-		log.Warnf("unsupported store engine, but found %s. Automatically migrating to SQLite.", jsonStoreFile)
-
-		if err := MigrateFileStoreToSqlite(dataDir); err != nil {
-			return nil, fmt.Errorf("failed to migrate data to SQLite store: %w", err)
-		}
-
-		log.Info("using SQLite store engine")
-		return NewSqliteStore(dataDir, metrics)
-	}
-
-	return nil, fmt.Errorf("unsupported kind of store: %s", kind)
-}
-
 // migrate migrates the SQLite database to the latest schema
 func migrate(db *gorm.DB) error {
 	migrations := getMigrations()