Merge remote-tracking branch 'origin/main' into refactor/permissions-manager

shared/management/client/client_test.go

@@ -2,24 +2,26 @@ package client
 
 import (
 	"context"
+	"fmt"
 	"net"
 	"os"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
-	"github.com/golang/mock/gomock"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/mock/gomock"
 	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
-	"github.com/netbirdio/management-integrations/integrations"
-
 	ephemeral_manager "github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral/manager"
+	"github.com/netbirdio/netbird/management/server/integrations/integrated_validator/validator"
+
 	"github.com/netbirdio/netbird/management/internals/modules/permissions"
 
 	"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller"
@@ -90,7 +92,7 @@ func startManagement(t *testing.T) (*grpc.Server, net.Listener) {
 			gomock.Any(),
 			gomock.Any(),
 		).
-		Return(true, nil).
+		Return(true, context.Background(), nil).
 		AnyTimes()
 
 	peersManger := peers.NewManager(store)
@@ -104,7 +106,7 @@ func startManagement(t *testing.T) (*grpc.Server, net.Listener) {
 		t.Fatal(err)
 	}
 
-	ia, _ := integrations.NewIntegratedValidator(ctx, peersManger, settingsManagerMock, eventStore, cacheStore)
+	ia, _ := validator.NewIntegratedValidator(ctx, peersManger, settingsManagerMock, eventStore, cacheStore)
 
 	metrics, err := telemetry.NewDefaultAppMetrics(ctx)
 	require.NoError(t, err)
@@ -127,7 +129,7 @@ func startManagement(t *testing.T) (*grpc.Server, net.Listener) {
 
 	updateManager := update_channel.NewPeersUpdateManager(metrics)
 	requestBuffer := mgmt.NewAccountRequestBuffer(ctx, store)
-	networkMapController := controller.NewController(ctx, store, metrics, updateManager, requestBuffer, mgmt.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock(), ephemeral_manager.NewEphemeralManager(store, peersManger), config)
+	networkMapController := controller.NewController(ctx, store, metrics, updateManager, requestBuffer, mgmt.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock(), ephemeral_manager.NewEphemeralManager(store, peersManger), config, nil)
 	accountManager, err := mgmt.BuildManager(context.Background(), config, store, networkMapController, jobManager, nil, "", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock, false, cacheStore)
 	if err != nil {
 		t.Fatal(err)
@@ -139,7 +141,7 @@ func startManagement(t *testing.T) (*grpc.Server, net.Listener) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	mgmtServer, err := nbgrpc.NewServer(config, accountManager, settingsMockManager, jobManager, secretsManager, nil, nil, mgmt.MockIntegratedValidator{}, networkMapController, nil)
+	mgmtServer, err := nbgrpc.NewServer(config, accountManager, settingsMockManager, jobManager, secretsManager, nil, nil, mgmt.MockIntegratedValidator{}, networkMapController, nil, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -306,7 +308,7 @@ func TestClient_Sync(t *testing.T) {
 	defer cancel()
 
 	go func() {
-		err = client.Sync(ctx, info, func(msg *mgmtProto.SyncResponse) error {
+		err = client.Sync(ctx, func(context.Context) *system.Info { return info }, func(msg *mgmtProto.SyncResponse) error {
 			ch <- msg
 			return nil
 		})
@@ -317,27 +319,156 @@ func TestClient_Sync(t *testing.T) {
 
 	select {
 	case resp := <-ch:
-		if resp.GetPeerConfig() == nil {
+		if resp.GetPeerConfig() == nil && resp.GetNetworkMap().GetPeerConfig() == nil {
 			t.Error("expecting non nil PeerConfig got nil")
 		}
 		if resp.GetNetbirdConfig() == nil {
 			t.Error("expecting non nil NetbirdConfig got nil")
 		}
-		if len(resp.GetRemotePeers()) != 1 {
-			t.Errorf("expecting RemotePeers size %d got %d", 1, len(resp.GetRemotePeers()))
+		// Top-level RemotePeers is deprecated and must stay empty for
+		// v0.29.3+ (and dev) clients — the field rides inside NetworkMap
+		// (legacy) or the NetworkMapEnvelope (components) instead.
+		if len(resp.GetRemotePeers()) != 0 {
+			t.Error("expecting top-level RemotePeers to be empty for v0.29.3+ clients")
+		}
+		// Component-capable clients receive a NetworkMapEnvelope; the
+		// remote-peers list is encoded inside it. Decode it and check the
+		// envelope's peers slice. Legacy peers populate NetworkMap.RemotePeers;
+		// both shapes must surface exactly one remote peer.
+		remotePeerKeys := remotePeerKeysFromSync(resp, testKey.PublicKey().String())
+		if len(remotePeerKeys) != 1 {
+			t.Errorf("expecting RemotePeers size %d got %d", 1, len(remotePeerKeys))
 			return
 		}
-		if resp.GetRemotePeersIsEmpty() == true {
+		if resp.GetNetworkMap() != nil && resp.GetNetworkMap().GetRemotePeersIsEmpty() {
 			t.Error("expecting RemotePeers property to be false, got true")
 		}
-		if resp.GetRemotePeers()[0].GetWgPubKey() != remoteKey.PublicKey().String() {
-			t.Errorf("expecting RemotePeer public key %s got %s", remoteKey.PublicKey().String(), resp.GetRemotePeers()[0].GetWgPubKey())
+		if remotePeerKeys[0] != remoteKey.PublicKey().String() {
+			t.Errorf("expecting RemotePeer public key %s got %s", remoteKey.PublicKey().String(), remotePeerKeys[0])
 		}
 	case <-time.After(3 * time.Second):
 		t.Error("timeout waiting for test to finish")
 	}
 }
 
+// remotePeerKeysFromSync extracts the remote-peer WG keys from either the
+// legacy NetworkMap.RemotePeers list or the components NetworkMapEnvelope's
+// inner peers slice (filtering out the local receiving peer identified by
+// localKey, since the envelope's peers list is index-addressed and includes
+// the local peer alongside remotes).
+func remotePeerKeysFromSync(resp *mgmtProto.SyncResponse, localKey string) []string {
+	if rp := resp.GetRemotePeers(); len(rp) > 0 {
+		out := make([]string, 0, len(rp))
+		for _, p := range rp {
+			out = append(out, p.GetWgPubKey())
+		}
+		return out
+	}
+	if rp := resp.GetNetworkMap().GetRemotePeers(); len(rp) > 0 {
+		out := make([]string, 0, len(rp))
+		for _, p := range rp {
+			out = append(out, p.GetWgPubKey())
+		}
+		return out
+	}
+	env := resp.GetNetworkMapEnvelope().GetFull()
+	if env == nil {
+		return nil
+	}
+	out := make([]string, 0, len(env.GetPeers()))
+	for _, p := range env.GetPeers() {
+		key := wgKeyFromBytes(p.GetWgPubKey())
+		if key == "" || key == localKey {
+			continue
+		}
+		out = append(out, key)
+	}
+	return out
+}
+
+// wgKeyFromBytes mirrors the client-side decoder: the envelope ships raw 32
+// bytes; reconstruct the standard base64 key the test compares against.
+func wgKeyFromBytes(raw []byte) string {
+	if len(raw) == 0 {
+		return ""
+	}
+	var k wgtypes.Key
+	if len(raw) != len(k) {
+		return ""
+	}
+	copy(k[:], raw)
+	return k.String()
+}
+
+func TestClient_SyncGathersInfoOnEveryConnect(t *testing.T) {
+	s, lis, mgmtMockServer, serverKey := startMockManagement(t)
+	defer s.GracefulStop()
+
+	testKey, err := wgtypes.GenerateKey()
+	require.NoError(t, err)
+
+	hostnames := make(chan string, 2)
+	mgmtMockServer.SyncFunc = func(msg *mgmtProto.EncryptedMessage, _ mgmtProto.ManagementService_SyncServer) error {
+		peerKey, err := wgtypes.ParseKey(msg.GetWgPubKey())
+		if err != nil {
+			t.Errorf("invalid peer key: %v", err)
+			return status.Error(codes.InvalidArgument, err.Error())
+		}
+		syncReq := &mgmtProto.SyncRequest{}
+		if err := encryption.DecryptMessage(peerKey, serverKey, msg.Body, syncReq); err != nil {
+			t.Errorf("decrypt sync request: %v", err)
+			return status.Error(codes.InvalidArgument, err.Error())
+		}
+		select {
+		case hostnames <- syncReq.GetMeta().GetHostname():
+		default:
+		}
+		// Returning closes the stream, so the client reconnects and gathers again.
+		return nil
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	client, err := NewClient(ctx, lis.Addr().String(), testKey, false)
+	require.NoError(t, err)
+
+	var gathers atomic.Int32
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		_ = client.Sync(ctx, func(ctx context.Context) *system.Info {
+			info := system.GetInfo(ctx)
+			info.Hostname = fmt.Sprintf("host-%d", gathers.Add(1))
+			return info
+		}, func(*mgmtProto.SyncResponse) error { return nil })
+	}()
+
+	// A connect attempt can fail before it reaches the server, so the sequence
+	// numbers seen here may skip. What matters is that the reconnect carries a
+	// newly gathered info instead of the one sent on the previous stream.
+	var seen []int
+	for len(seen) < 2 {
+		select {
+		case got := <-hostnames:
+			var n int
+			_, err := fmt.Sscanf(got, "host-%d", &n)
+			require.NoError(t, err, "hostname should carry the gather sequence number")
+			seen = append(seen, n)
+		case <-time.After(10 * time.Second):
+			t.Fatalf("timeout waiting for the second sync request, got %v", seen)
+		}
+	}
+	assert.Greater(t, seen[1], seen[0], "the reconnect should carry a newly gathered info")
+
+	cancel()
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatal("timeout waiting for Sync to return after cancel")
+	}
+}
+
 func Test_SystemMetaDataFromClient(t *testing.T) {
 	s, lis, mgmtMockServer, serverKey := startMockManagement(t)
 	defer s.GracefulStop()
@@ -532,7 +663,7 @@ func Test_GetPKCEAuthorizationFlow(t *testing.T) {
 	expectedFlowInfo := &mgmtProto.PKCEAuthorizationFlow{
 		ProviderConfig: &mgmtProto.ProviderConfig{
 			ClientID:     "client",
-			ClientSecret: "secret",
+			ClientSecret: "secret", //nolint:staticcheck
 		},
 	}