Merge branch 'peers-get-account-refactoring' into refactor/optimize-peer-expiration

# Conflicts:
#	management/server/peer.go

.github/workflows/golang-test-freebsd.yml

@@ -24,7 +24,7 @@ jobs:
           copyback: false
           release: "14.1"
           prepare: |
-            pkg install -y go pkgconf xorg
+            pkg install -y go
 
           # -x - to print all executed commands
           # -e - to faile on first error

client/internal/peer/worker_ice.go

@@ -255,10 +255,6 @@ func (w *WorkerICE) closeAgent(cancel context.CancelFunc) {
 	defer w.muxAgent.Unlock()
 
 	cancel()
-	if w.agent == nil {
-		return
-	}
-
 	if err := w.agent.Close(); err != nil {
 		w.log.Warnf("failed to close ICE agent: %s", err)
 	}

client/ui/client_ui.go

@@ -1,4 +1,4 @@
-//go:build !(linux && 386)
+//go:build !(linux && 386) && !freebsd
 
 package main
 
@@ -876,7 +876,7 @@ func openURL(url string) error {
 		err = exec.Command("rundll32", "url.dll,FileProtocolHandler", url).Start()
 	case "darwin":
 		err = exec.Command("open", url).Start()
-	case "linux", "freebsd":
+	case "linux":
 		err = exec.Command("xdg-open", url).Start()
 	default:
 		err = fmt.Errorf("unsupported platform")

client/ui/font_bsd.go

@@ -1,4 +1,4 @@
-//go:build freebsd || openbsd || netbsd || dragonfly
+//go:build darwin
 
 package main
 
@@ -9,22 +9,18 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
+const defaultFontPath = "/Library/Fonts/Arial Unicode.ttf"
+
 func (s *serviceClient) setDefaultFonts() {
-	paths := []string{
-		"/usr/local/share/fonts/TTF/DejaVuSans.ttf",
-		"/usr/local/share/fonts/dejavu/DejaVuSans.ttf",
-		"/usr/local/share/noto/NotoSans-Regular.ttf",
-		"/usr/local/share/fonts/noto/NotoSans-Regular.ttf",
-		"/usr/local/share/fonts/liberation-fonts-ttf/LiberationSans-Regular.ttf",
+	// TODO: add other bsd paths
+	if runtime.GOOS != "darwin" {
+		return
 	}
 
-	for _, fontPath := range paths {
-		if _, err := os.Stat(fontPath); err == nil {
-			os.Setenv("FYNE_FONT", fontPath)
-			log.Debugf("Using font: %s", fontPath)
-			return
-		}
+	if _, err := os.Stat(defaultFontPath); err != nil {
+		log.Errorf("Failed to find default font file: %v", err)
+		return
 	}
 
-	log.Errorf("Failed to find any suitable font files for %s", runtime.GOOS)
+	os.Setenv("FYNE_FONT", defaultFontPath)
 }

client/ui/font_darwin.go

@@ -1,18 +0,0 @@
-package main
-
-import (
-	"os"
-
-	log "github.com/sirupsen/logrus"
-)
-
-const defaultFontPath = "/Library/Fonts/Arial Unicode.ttf"
-
-func (s *serviceClient) setDefaultFonts() {
-	if _, err := os.Stat(defaultFontPath); err != nil {
-		log.Errorf("Failed to find default font file: %v", err)
-		return
-	}
-
-	os.Setenv("FYNE_FONT", defaultFontPath)
-}

client/ui/network.go

@@ -1,4 +1,4 @@
-//go:build !(linux && 386)
+//go:build !(linux && 386) && !freebsd
 
 package main
 

management/client/client_test.go

@@ -256,10 +256,12 @@ func TestClient_Sync(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	ch := make(chan *mgmtProto.SyncResponse, 1)
+	ch := make(chan *mgmtProto.SyncResponse, 5)
 
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
 	go func() {
-		err = client.Sync(context.Background(), info, func(msg *mgmtProto.SyncResponse) error {
+		err = client.Sync(ctx, info, func(msg *mgmtProto.SyncResponse) error {
 			ch <- msg
 			return nil
 		})
@@ -289,6 +291,7 @@ func TestClient_Sync(t *testing.T) {
 	case <-time.After(3 * time.Second):
 		t.Error("timeout waiting for test to finish")
 	}
+
 }
 
 func Test_SystemMetaDataFromClient(t *testing.T) {

management/server/account.go

@@ -473,12 +473,7 @@ func (am *DefaultAccountManager) peerLoginExpirationJob(ctx context.Context, acc
 			return peerSchedulerRetryInterval, true
 		}
 
-		var peerIDs []string
-		for _, peer := range expiredPeers {
-			peerIDs = append(peerIDs, peer.ID)
-		}
-
-		log.WithContext(ctx).Debugf("discovered %d peers to expire for account %s", len(peerIDs), accountID)
+		log.WithContext(ctx).Debugf("discovered %d peers to expire for account %s", len(expiredPeers), accountID)
 
 		if err := am.expireAndUpdatePeers(ctx, accountID, expiredPeers); err != nil {
 			log.WithContext(ctx).Errorf("failed updating account peers while expiring peers for account %s", accountID)
@@ -490,6 +485,9 @@ func (am *DefaultAccountManager) peerLoginExpirationJob(ctx context.Context, acc
 }
 
 func (am *DefaultAccountManager) checkAndSchedulePeerLoginExpiration(ctx context.Context, accountID string) {
+	if am.peerLoginExpiry.IsJobRunning(accountID) {
+		return
+	}
 	am.peerLoginExpiry.Cancel(ctx, []string{accountID})
 	if nextRun, ok := am.getNextPeerExpiration(ctx, accountID); ok {
 		go am.peerLoginExpiry.Schedule(ctx, nextRun, accountID, am.peerLoginExpirationJob(ctx, accountID))
@@ -526,6 +524,9 @@ func (am *DefaultAccountManager) peerInactivityExpirationJob(ctx context.Context
 
 // checkAndSchedulePeerInactivityExpiration periodically checks for inactive peers to end their sessions
 func (am *DefaultAccountManager) checkAndSchedulePeerInactivityExpiration(ctx context.Context, accountID string) {
+	if am.peerInactivityExpiry.IsJobRunning(accountID) {
+		return
+	}
 	am.peerInactivityExpiry.Cancel(ctx, []string{accountID})
 	if nextRun, ok := am.getNextInactivePeerExpiration(ctx, accountID); ok {
 		go am.peerInactivityExpiry.Schedule(ctx, nextRun, accountID, am.peerInactivityExpirationJob(ctx, accountID))

management/server/integrated_validator.go

@@ -89,17 +89,17 @@ func (am *DefaultAccountManager) GetValidatedPeers(ctx context.Context, accountI
 		}
 
 		peers, err = transaction.GetAccountPeers(ctx, store.LockingStrengthShare, accountID)
+		if err != nil {
+			return err
+		}
+
+		settings, err = transaction.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
 		return err
 	})
 	if err != nil {
 		return nil, err
 	}
 
-	settings, err = am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
-	if err != nil {
-		return nil, err
-	}
-
 	return am.integratedPeerValidator.GetValidatedPeers(accountID, groups, peers, settings.Extra)
 }
 

management/server/management_proto_test.go

@@ -254,13 +254,13 @@ func Test_SyncProtocol(t *testing.T) {
 	}
 
 	// expired peers come separately.
-	if len(networkMap.GetOfflinePeers()) != 1 {
-		t.Fatal("expecting SyncResponse to have NetworkMap with 1 offline peer")
+	if len(networkMap.GetOfflinePeers()) != 2 {
+		t.Fatal("expecting SyncResponse to have NetworkMap with 2 offline peer")
 	}
 
 	expiredPeerPubKey := "RlSy2vzoG2HyMBTUImXOiVhCBiiBa5qD5xzMxkiFDW4="
-	if networkMap.GetOfflinePeers()[0].WgPubKey != expiredPeerPubKey {
-		t.Fatalf("expecting SyncResponse to have NetworkMap with 1 offline peer with a key %s", expiredPeerPubKey)
+	if !(networkMap.GetOfflinePeers()[0].WgPubKey == expiredPeerPubKey || networkMap.GetOfflinePeers()[1].WgPubKey == expiredPeerPubKey) {
+		t.Fatalf("expecting SyncResponse to have NetworkMap with 2 offline peer with a key %s", expiredPeerPubKey)
 	}
 
 	if networkMap.GetPeerConfig() == nil {

management/server/management_suite_test.go

@@ -1,13 +1,13 @@
 package server_test
 
-import (
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
-
-	"testing"
-)
-
-func TestManagement(t *testing.T) {
-	RegisterFailHandler(Fail)
-	RunSpecs(t, "Management Service Suite")
-}
+// import (
+// 	. "github.com/onsi/ginkgo"
+// 	. "github.com/onsi/gomega"
+//
+// 	"testing"
+// )
+//
+// func TestManagement(t *testing.T) {
+// 	RegisterFailHandler(Fail)
+// 	RunSpecs(t, "Management Service Suite")
+// }

management/server/peer.go

@@ -126,8 +126,8 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK
 	}()
 
 	var peer *nbpeer.Peer
-	var settings *types.Settings
 	var expired bool
+	var settings *types.Settings
 	var err error
 
 	err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
@@ -136,6 +136,11 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK
 			return err
 		}
 
+		settings, err = transaction.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
+		if err != nil {
+			return err
+		}
+
 		expired, err = updatePeerStatusAndLocation(ctx, am.geo, transaction, peer, connected, realIP, accountID)
 		return err
 	})
@@ -144,11 +149,6 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK
 	}
 
 	if peer.AddedWithSSOLogin() {
-		settings, err = am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
-		if err != nil {
-			return err
-		}
-
 		if peer.LoginExpirationEnabled && settings.PeerLoginExpirationEnabled {
 			am.checkAndSchedulePeerLoginExpiration(ctx, accountID)
 		}
@@ -676,11 +676,6 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync PeerSync, ac
 	var err error
 	var postureChecks []*posture.Checks
 
-	settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
-	if err != nil {
-		return nil, nil, nil, err
-	}
-
 	err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
 		peer, err = transaction.GetPeerByPeerPubKey(ctx, store.LockingStrengthUpdate, sync.WireGuardPubKey)
 		if err != nil {
@@ -698,6 +693,11 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync PeerSync, ac
 			}
 		}
 
+		settings, err := transaction.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
+		if err != nil {
+			return err
+		}
+
 		if peerLoginExpired(ctx, peer, settings) {
 			return status.NewPeerLoginExpiredError()
 		}
@@ -790,17 +790,16 @@ func (am *DefaultAccountManager) LoginPeer(ctx context.Context, login PeerLogin)
 	var isPeerUpdated bool
 	var postureChecks []*posture.Checks
 
-	settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
-	if err != nil {
-		return nil, nil, nil, err
-	}
-
 	err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
 		peer, err = transaction.GetPeerByPeerPubKey(ctx, store.LockingStrengthUpdate, login.WireGuardPubKey)
 		if err != nil {
 			return err
 		}
 
+		settings, err := transaction.GetAccountSettings(ctx, store.LockingStrengthShare, accountID)
+		if err != nil {
+			return err
+		}
 		// this flag prevents unnecessary calls to the persistent store.
 		shouldStorePeer := false
 
@@ -1228,8 +1227,7 @@ func (am *DefaultAccountManager) getNextPeerExpiration(ctx context.Context, acco
 
 	var nextExpiry *time.Duration
 	for _, peer := range peersWithExpiry {
-		// consider only connected peers because others will require login on connecting to the management server
-		if peer.Status.LoginExpired || !peer.Status.Connected {
+		if peer.Status.LoginExpired {
 			continue
 		}
 		_, duration := peer.LoginExpired(settings.PeerLoginExpiration)

management/server/scheduler.go

@@ -12,6 +12,7 @@ import (
 type Scheduler interface {
 	Cancel(ctx context.Context, IDs []string)
 	Schedule(ctx context.Context, in time.Duration, ID string, job func() (nextRunIn time.Duration, reschedule bool))
+	IsJobRunning(ID string) bool
 }
 
 // MockScheduler is a mock implementation of  Scheduler
@@ -38,6 +39,10 @@ func (mock *MockScheduler) Schedule(ctx context.Context, in time.Duration, ID st
 	log.WithContext(ctx).Errorf("MockScheduler doesn't have Schedule function defined")
 }
 
+func (mock *MockScheduler) IsJobRunning(_ string) bool {
+	return false
+}
+
 // DefaultScheduler is a generic structure that allows to schedule jobs (functions) to run in the future and cancel them.
 type DefaultScheduler struct {
 	// jobs map holds cancellation channels indexed by the job ID
@@ -124,3 +129,7 @@ func (wm *DefaultScheduler) Schedule(ctx context.Context, in time.Duration, ID s
 
 	}()
 }
+
+func (wm *DefaultScheduler) IsJobRunning(ID string) bool {
+	return wm.jobs[ID] != nil
+}

management/server/store/sql_store.go

@@ -1307,7 +1307,7 @@ func (s *SqlStore) GetPeersByIDs(ctx context.Context, lockStrength LockingStreng
 func (s *SqlStore) GetAccountPeersWithExpiration(ctx context.Context, lockStrength LockingStrength, accountID string) ([]*nbpeer.Peer, error) {
 	var peers []*nbpeer.Peer
 	result := s.db.Clauses(clause.Locking{Strength: string(lockStrength)}).
-		Where("login_expiration_enabled = ? AND user_id IS NOT NULL AND user_id != ''", true).
+		Where("login_expiration_enabled = ? AND peer_status_login_expired = ? AND user_id IS NOT NULL AND user_id != ''", true, false).
 		Find(&peers, accountIDCondition, accountID)
 	if err := result.Error; err != nil {
 		log.WithContext(ctx).Errorf("failed to get peers with expiration from the store: %s", result.Error)

management/server/store/sql_store_test.go

@@ -14,12 +14,13 @@ import (
 	"time"
 
 	"github.com/google/uuid"
-	"github.com/netbirdio/netbird/management/server/util"
 	"github.com/rs/xid"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/netbirdio/netbird/management/server/util"
+
 	nbdns "github.com/netbirdio/netbird/dns"
 	resourceTypes "github.com/netbirdio/netbird/management/server/networks/resources/types"
 	routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types"
@@ -2669,7 +2670,7 @@ func TestSqlStore_GetAccountPeers(t *testing.T) {
 		{
 			name:          "should retrieve peers for an existing account ID",
 			accountID:     "bf1c8084-ba50-4ce7-9439-34653001fc3b",
-			expectedCount: 4,
+			expectedCount: 5,
 		},
 		{
 			name:          "should return no peers for a non-existing account ID",
@@ -2809,7 +2810,7 @@ func TestSqlStore_GetUserPeers(t *testing.T) {
 			name:          "should retrieve peers for another valid account ID and user ID",
 			accountID:     "bf1c8084-ba50-4ce7-9439-34653001fc3b",
 			userID:        "edafee4e-63fb-11ec-90d6-0242ac120003",
-			expectedCount: 2,
+			expectedCount: 3,
 		},
 		{
 			name:          "should return no peers for existing account ID with empty user ID",

management/server/store/store_test.go

@@ -21,6 +21,7 @@ var newSqlite = func(b *testing.B) Store {
 }
 
 func BenchmarkTest_StoreWrite(b *testing.B) {
+	b.Skipf("skipping benchmark")
 	cases := []benchCase{
 		{name: "SqliteStore_Write", storeFn: newSqlite, size: 100},
 		{name: "SqliteStore_Write", storeFn: newSqlite, size: 500},
@@ -50,6 +51,7 @@ func BenchmarkTest_StoreWrite(b *testing.B) {
 }
 
 func BenchmarkTest_StoreRead(b *testing.B) {
+	b.Skipf("skipping benchmark")
 	cases := []benchCase{
 		{name: "SqliteStore_Read", storeFn: newSqlite, size: 100},
 		{name: "SqliteStore_Read", storeFn: newSqlite, size: 500},

management/server/testdata/store_with_expired_peers.sql

@@ -29,6 +29,7 @@ INSERT INTO accounts VALUES('bf1c8084-ba50-4ce7-9439-34653001fc3b','','2024-10-0
 INSERT INTO setup_keys VALUES('','bf1c8084-ba50-4ce7-9439-34653001fc3b','A2C8E62B-38F5-4553-B31E-DD66C696CEBB','Default key','reusable','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,0,NULL,'[]',0,0);
 INSERT INTO peers VALUES('cfvprsrlo1hqoo49ohog','bf1c8084-ba50-4ce7-9439-34653001fc3b','5rvhvriKJZ3S9oxYToVj5TzDM9u9y8cxg7htIMWlYAg=','72546A29-6BC8-4311-BCFC-9CDBF33F1A48','"100.64.114.31"','f2a34f6a4731','linux','Linux','11','unknown','Debian GNU/Linux','','0.12.0','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'f2a34f6a4731','f2a34f6a4731','2023-03-02 09:21:02.189035775+01:00',0,0,0,'','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILzUUSYG/LGnV8zarb2SGN+tib/PZ+M7cL4WtTzUrTpk',0,1,0,'2023-03-01 19:48:19.817799698+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0);
 INSERT INTO peers VALUES('cg05lnblo1hkg2j514p0','bf1c8084-ba50-4ce7-9439-34653001fc3b','RlSy2vzoG2HyMBTUImXOiVhCBiiBa5qD5xzMxkiFDW4=','','"100.64.39.54"','expiredhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'expiredhost','expiredhost','2023-03-02 09:19:57.276717255+01:00',0,1,0,'edafee4e-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMbK5ZXJsGOOWoBT4OmkPtgdPZe2Q7bDuS/zjn2CZxhK',0,1,0,'2023-03-02 09:14:21.791679181+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0);
+INSERT INTO peers VALUES('cg05lnblo1hkg2j514p1','bf1c8084-ba50-4ce7-9439-34653001fc3b','RlSy2vzoG2HyMBTUImXOiVhCBiiBa5qD5xzMxkiFDW5=','','"100.64.39.54"','tobeexpiredhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'tobeexpiredhost','tobeexpiredhost','2023-03-02 09:19:57.276717255+01:00',0,0,0,'edafee4e-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMbK5ZXJsGOOWoBT4OmkPtgdPZe2Q7bDuS/zjn2CZxhK',0,1,0,'2023-03-02 09:14:21.791679181+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0);
 INSERT INTO peers VALUES('cg3161rlo1hs9cq94gdg','bf1c8084-ba50-4ce7-9439-34653001fc3b','mVABSKj28gv+JRsf7e0NEGKgSOGTfU/nPB2cpuG56HU=','','"100.64.117.96"','testhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'testhost','testhost','2023-03-06 18:21:27.252010027+01:00',0,0,0,'edafee4e-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINWvvUkFFcrj48CWTkNUb/do/n52i1L5dH4DhGu+4ZuM',0,0,0,'2023-03-07 09:02:47.442857106+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0);
 INSERT INTO peers VALUES('csrnkiq7qv9d8aitqd50','bf1c8084-ba50-4ce7-9439-34653001fc3b','mVABSKj28gv+JRsf7e0NEGKgSOGTfU/nPB2cpuG56HU=','','"100.64.117.96"','testhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'testhost','testhost','2023-03-06 18:21:27.252010027+01:00',0,0,0,'f4f6d672-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINWvvUkFFcrj48CWTkNUb/do/n52i1L5dH4DhGu+4ZuM',0,0,1,'2023-03-07 09:02:47.442857106+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0);
 INSERT INTO users VALUES('f4f6d672-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','user',0,0,'','[]',0,NULL,'2024-10-02 17:00:32.528196+02:00','api',0,'');

management/server/token_mgr_test.go

@@ -71,7 +71,7 @@ func TestTimeBasedAuthSecretsManager_SetupRefresh(t *testing.T) {
 	ttl := util.Duration{Duration: 2 * time.Second}
 	secret := "some_secret"
 	peersManager := NewPeersUpdateManager(nil)
-	peer := "some_peer"
+	peer := "some_peer_SetupRefresh"
 	updateChannel := peersManager.CreateChannel(context.Background(), peer)
 
 	rc := &Relay{
@@ -169,7 +169,7 @@ func TestTimeBasedAuthSecretsManager_CancelRefresh(t *testing.T) {
 	ttl := util.Duration{Duration: time.Hour}
 	secret := "some_secret"
 	peersManager := NewPeersUpdateManager(nil)
-	peer := "some_peer"
+	peer := "some_peer_CancelRefresh"
 
 	rc := &Relay{
 		Addresses:      []string{"localhost:0"},