Merge branch 'main' into feat/network-map-serial

client/internal/engine_test.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/golang/mock/gomock"
 	"github.com/google/uuid"
-	"github.com/pion/transport/v3/stdnet"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -25,8 +24,19 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/keepalive"
 
+	"github.com/netbirdio/netbird/client/internal/stdnet"
+
 	"github.com/netbirdio/management-integrations/integrations"
 
+	"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller"
+	"github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel"
+	"github.com/netbirdio/netbird/management/internals/modules/peers"
+	"github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager"
+	nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"
+
+	"github.com/netbirdio/netbird/management/internals/server/config"
+	"github.com/netbirdio/netbird/management/server/groups"
+
 	"github.com/netbirdio/netbird/client/iface"
 	"github.com/netbirdio/netbird/client/iface/configurer"
 	"github.com/netbirdio/netbird/client/iface/device"
@@ -39,15 +49,12 @@ import (
 	icemaker "github.com/netbirdio/netbird/client/internal/peer/ice"
 	"github.com/netbirdio/netbird/client/internal/profilemanager"
 	"github.com/netbirdio/netbird/client/internal/routemanager"
-	"github.com/netbirdio/netbird/client/ssh"
+	nbssh "github.com/netbirdio/netbird/client/ssh"
 	"github.com/netbirdio/netbird/client/system"
 	nbdns "github.com/netbirdio/netbird/dns"
-	"github.com/netbirdio/netbird/management/internals/server/config"
 	"github.com/netbirdio/netbird/management/server"
 	"github.com/netbirdio/netbird/management/server/activity"
-	"github.com/netbirdio/netbird/management/server/groups"
 	"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
-	"github.com/netbirdio/netbird/management/server/peers"
 	"github.com/netbirdio/netbird/management/server/permissions"
 	"github.com/netbirdio/netbird/management/server/settings"
 	"github.com/netbirdio/netbird/management/server/store"
@@ -103,6 +110,14 @@ type MockWGIface struct {
 	LastActivitiesFunc         func() map[string]monotime.Time
 }
 
+func (m *MockWGIface) RenewTun(_ int) error {
+	return nil
+}
+
+func (m *MockWGIface) RemoveEndpointAddress(_ string) error {
+	return nil
+}
+
 func (m *MockWGIface) FullStats() (*configurer.Stats, error) {
 	return nil, fmt.Errorf("not implemented")
 }
@@ -205,11 +220,13 @@ func TestMain(m *testing.M) {
 }
 
 func TestEngine_SSH(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("skipping TestEngine_SSH")
+	key, err := wgtypes.GeneratePrivateKey()
+	if err != nil {
+		t.Fatal(err)
+		return
 	}
 
-	key, err := wgtypes.GeneratePrivateKey()
+	sshKey, err := nbssh.GeneratePrivateKey(nbssh.ED25519)
 	if err != nil {
 		t.Fatal(err)
 		return
@@ -231,6 +248,7 @@ func TestEngine_SSH(t *testing.T) {
 			WgPort:           33100,
 			ServerSSHAllowed: true,
 			MTU:              iface.DefaultMTU,
+			SSHKey:           sshKey,
 		},
 		MobileDependency{},
 		peer.NewRecorder("https://mgm"),
@@ -241,35 +259,8 @@ func TestEngine_SSH(t *testing.T) {
 		UpdateDNSServerFunc: func(serial uint64, update nbdns.Config) error { return nil },
 	}
 
-	var sshKeysAdded []string
-	var sshPeersRemoved []string
-
-	sshCtx, cancel := context.WithCancel(context.Background())
-
-	engine.sshServerFunc = func(hostKeyPEM []byte, addr string) (ssh.Server, error) {
-		return &ssh.MockServer{
-			Ctx: sshCtx,
-			StopFunc: func() error {
-				cancel()
-				return nil
-			},
-			StartFunc: func() error {
-				<-ctx.Done()
-				return ctx.Err()
-			},
-			AddAuthorizedKeyFunc: func(peer, newKey string) error {
-				sshKeysAdded = append(sshKeysAdded, newKey)
-				return nil
-			},
-			RemoveAuthorizedKeyFunc: func(peer string) {
-				sshPeersRemoved = append(sshPeersRemoved, peer)
-			},
-		}, nil
-	}
 	err = engine.Start(nil, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	defer func() {
 		err := engine.Stop()
@@ -295,9 +286,7 @@ func TestEngine_SSH(t *testing.T) {
 	}
 
 	err = engine.updateNetworkMap(networkMap)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	assert.Nil(t, engine.sshServer)
 
@@ -305,19 +294,24 @@ func TestEngine_SSH(t *testing.T) {
 	networkMap = &mgmtProto.NetworkMap{
 		Serial: 7,
 		PeerConfig: &mgmtProto.PeerConfig{Address: "100.64.0.1/24",
-			SshConfig: &mgmtProto.SSHConfig{SshEnabled: true}},
+			SshConfig: &mgmtProto.SSHConfig{
+				SshEnabled: true,
+				JwtConfig: &mgmtProto.JWTConfig{
+					Issuer:       "test-issuer",
+					Audience:     "test-audience",
+					KeysLocation: "test-keys",
+					MaxTokenAge:  3600,
+				},
+			}},
 		RemotePeers:        []*mgmtProto.RemotePeerConfig{peerWithSSH},
 		RemotePeersIsEmpty: false,
 	}
 
 	err = engine.updateNetworkMap(networkMap)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	time.Sleep(250 * time.Millisecond)
 	assert.NotNil(t, engine.sshServer)
-	assert.Contains(t, sshKeysAdded, "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFATYCqaQw/9id1Qkq3n16JYhDhXraI6Pc1fgB8ynEfQ")
 
 	// now remove peer
 	networkMap = &mgmtProto.NetworkMap{
@@ -327,13 +321,10 @@ func TestEngine_SSH(t *testing.T) {
 	}
 
 	err = engine.updateNetworkMap(networkMap)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	// time.Sleep(250 * time.Millisecond)
 	assert.NotNil(t, engine.sshServer)
-	assert.Contains(t, sshPeersRemoved, "MNHf3Ma6z6mdLbriAJbqhX7+nM/B71lgw2+91q3LfhU=")
 
 	// now disable SSH server
 	networkMap = &mgmtProto.NetworkMap{
@@ -345,12 +336,70 @@ func TestEngine_SSH(t *testing.T) {
 	}
 
 	err = engine.updateNetworkMap(networkMap)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	assert.Nil(t, engine.sshServer)
+}
 
+func TestEngine_SSHUpdateLogic(t *testing.T) {
+	// Test that SSH server start/stop logic works based on config
+	engine := &Engine{
+		config: &EngineConfig{
+			ServerSSHAllowed: false, // Start with SSH disabled
+		},
+		syncMsgMux: &sync.Mutex{},
+	}
+
+	// Test SSH disabled config
+	sshConfig := &mgmtProto.SSHConfig{SshEnabled: false}
+	err := engine.updateSSH(sshConfig)
+	assert.NoError(t, err)
+	assert.Nil(t, engine.sshServer)
+
+	// Test inbound blocked
+	engine.config.BlockInbound = true
+	err = engine.updateSSH(&mgmtProto.SSHConfig{SshEnabled: true})
+	assert.NoError(t, err)
+	assert.Nil(t, engine.sshServer)
+	engine.config.BlockInbound = false
+
+	// Test with server SSH not allowed
+	err = engine.updateSSH(&mgmtProto.SSHConfig{SshEnabled: true})
+	assert.NoError(t, err)
+	assert.Nil(t, engine.sshServer)
+}
+
+func TestEngine_SSHServerConsistency(t *testing.T) {
+
+	t.Run("server set only on successful creation", func(t *testing.T) {
+		engine := &Engine{
+			config: &EngineConfig{
+				ServerSSHAllowed: true,
+				SSHKey:           []byte("test-key"),
+			},
+			syncMsgMux: &sync.Mutex{},
+		}
+
+		engine.wgInterface = nil
+
+		err := engine.updateSSH(&mgmtProto.SSHConfig{SshEnabled: true})
+
+		assert.Error(t, err)
+		assert.Nil(t, engine.sshServer)
+	})
+
+	t.Run("cleanup handles nil gracefully", func(t *testing.T) {
+		engine := &Engine{
+			config: &EngineConfig{
+				ServerSSHAllowed: false,
+			},
+			syncMsgMux: &sync.Mutex{},
+		}
+
+		err := engine.stopSSHServer()
+		assert.NoError(t, err)
+		assert.Nil(t, engine.sshServer)
+	})
 }
 
 func TestEngine_UpdateNetworkMap(t *testing.T) {
@@ -582,7 +631,7 @@ func TestEngine_Sync(t *testing.T) {
 	// feed updates to Engine via mocked Management client
 	updates := make(chan *mgmtProto.SyncResponse)
 	defer close(updates)
-	syncFunc := func(ctx context.Context, info *system.Info, msgHandler func(msg *mgmtProto.SyncResponse) error) error {
+	syncFunc := func(ctx context.Context, info *system.Info, networkSerial uint64, msgHandler func(msg *mgmtProto.SyncResponse) error) error {
 		for msg := range updates {
 			err := msgHandler(msg)
 			if err != nil {
@@ -765,7 +814,7 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
 				MTU:          iface.DefaultMTU,
 			}, MobileDependency{}, peer.NewRecorder("https://mgm"), nil)
 			engine.ctx = ctx
-			newNet, err := stdnet.NewNet()
+			newNet, err := stdnet.NewNet(context.Background(), nil)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -968,7 +1017,7 @@ func TestEngine_UpdateNetworkMapWithDNSUpdate(t *testing.T) {
 			}, MobileDependency{}, peer.NewRecorder("https://mgm"), nil)
 			engine.ctx = ctx
 
-			newNet, err := stdnet.NewNet()
+			newNet, err := stdnet.NewNet(context.Background(), nil)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1550,7 +1599,6 @@ func startManagement(t *testing.T, dataDir, testFile string) (*grpc.Server, stri
 	}
 	t.Cleanup(cleanUp)
 
-	peersUpdateManager := server.NewPeersUpdateManager(nil)
 	eventStore := &activity.InMemoryEventStore{}
 	if err != nil {
 		return nil, "", err
@@ -1578,13 +1626,19 @@ func startManagement(t *testing.T, dataDir, testFile string) (*grpc.Server, stri
 
 	groupsManager := groups.NewManagerMock()
 
-	accountManager, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "netbird.selfhosted", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManager, false)
+	updateManager := update_channel.NewPeersUpdateManager(metrics)
+	requestBuffer := server.NewAccountRequestBuffer(context.Background(), store)
+	networkMapController := controller.NewController(context.Background(), store, metrics, updateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock(), manager.NewEphemeralManager(store, peersManager), config)
+	accountManager, err := server.BuildManager(context.Background(), config, store, networkMapController, nil, "", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManager, false)
 	if err != nil {
 		return nil, "", err
 	}
 
-	secretsManager := server.NewTimeBasedAuthSecretsManager(peersUpdateManager, config.TURNConfig, config.Relay, settingsMockManager, groupsManager)
-	mgmtServer, err := server.NewServer(context.Background(), config, accountManager, settingsMockManager, peersUpdateManager, secretsManager, nil, nil, nil, &server.MockIntegratedValidator{})
+	secretsManager, err := nbgrpc.NewTimeBasedAuthSecretsManager(updateManager, config.TURNConfig, config.Relay, settingsMockManager, groupsManager)
+	if err != nil {
+		return nil, "", err
+	}
+	mgmtServer, err := nbgrpc.NewServer(config, accountManager, settingsMockManager, secretsManager, nil, nil, &server.MockIntegratedValidator{}, networkMapController)
 	if err != nil {
 		return nil, "", err
 	}