Merge remote-tracking branch 'origin/main' into feature/affected-peers

management/internals/shared/grpc/proxy.go

@@ -9,6 +9,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
+	"io"
 	"net"
 	"net/http"
 	"net/url"
@@ -136,9 +137,12 @@ type proxyConnection struct {
 	tokenID      string
 	capabilities *proto.ProxyCapabilities
 	stream       proto.ProxyService_GetMappingUpdateServer
-	sendChan     chan *proto.GetMappingUpdateResponse
-	ctx          context.Context
-	cancel       context.CancelFunc
+	// syncStream is set when the proxy connected via SyncMappings.
+	// When non-nil, the sender goroutine uses this instead of stream.
+	syncStream proto.ProxyService_SyncMappingsServer
+	sendChan   chan *proto.GetMappingUpdateResponse
+	ctx        context.Context
+	cancel     context.CancelFunc
 }
 
 func enforceAccountScope(ctx context.Context, requestAccountID string) error {
@@ -206,145 +210,322 @@ func (s *ProxyServiceServer) SetProxyController(proxyController proxy.Controller
 	s.proxyController = proxyController
 }
 
+// proxyConnectParams holds the validated parameters extracted from either
+// a GetMappingUpdateRequest or a SyncMappingsInit message.
+type proxyConnectParams struct {
+	proxyID      string
+	address      string
+	capabilities *proto.ProxyCapabilities
+}
+
 // GetMappingUpdate handles the control stream with proxy clients
 func (s *ProxyServiceServer) GetMappingUpdate(req *proto.GetMappingUpdateRequest, stream proto.ProxyService_GetMappingUpdateServer) error {
-	ctx := stream.Context()
+	params, err := s.validateProxyConnect(req.GetProxyId(), req.GetAddress(), stream.Context())
+	if err != nil {
+		return err
+	}
+	params.capabilities = req.GetCapabilities()
 
-	peerInfo := PeerIPFromContext(ctx)
-	log.Infof("New proxy connection from %s", peerInfo)
+	conn, proxyRecord, err := s.registerProxyConnection(stream.Context(), params, &proxyConnection{
+		stream: stream,
+	})
+	if err != nil {
+		return err
+	}
 
-	proxyID := req.GetProxyId()
+	if err := s.sendSnapshot(stream.Context(), conn); err != nil {
+		s.cleanupFailedSnapshot(stream.Context(), conn)
+		return fmt.Errorf("send snapshot to proxy %s: %w", params.proxyID, err)
+	}
+
+	errChan := make(chan error, 2)
+	go s.sender(conn, errChan)
+
+	return s.serveProxyConnection(conn, proxyRecord, errChan, false)
+}
+
+// SyncMappings implements the bidirectional SyncMappings RPC.
+// It mirrors GetMappingUpdate but provides application-level back-pressure:
+// management waits for an ack from the proxy before sending the next batch.
+func (s *ProxyServiceServer) SyncMappings(stream proto.ProxyService_SyncMappingsServer) error {
+	init, err := recvSyncInit(stream)
+	if err != nil {
+		return err
+	}
+
+	params, err := s.validateProxyConnect(init.GetProxyId(), init.GetAddress(), stream.Context())
+	if err != nil {
+		return err
+	}
+	params.capabilities = init.GetCapabilities()
+
+	conn, proxyRecord, err := s.registerProxyConnection(stream.Context(), params, &proxyConnection{
+		syncStream: stream,
+	})
+	if err != nil {
+		return err
+	}
+
+	if err := s.sendSnapshotSync(stream.Context(), conn, stream); err != nil {
+		s.cleanupFailedSnapshot(stream.Context(), conn)
+		return fmt.Errorf("send snapshot to proxy %s: %w", params.proxyID, err)
+	}
+
+	errChan := make(chan error, 2)
+	go s.sender(conn, errChan)
+	go s.drainRecv(stream, errChan)
+
+	return s.serveProxyConnection(conn, proxyRecord, errChan, true)
+}
+
+// recvSyncInit receives and validates the first message on a SyncMappings stream.
+func recvSyncInit(stream proto.ProxyService_SyncMappingsServer) (*proto.SyncMappingsInit, error) {
+	firstMsg, err := stream.Recv()
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "receive init: %v", err)
+	}
+	init := firstMsg.GetInit()
+	if init == nil {
+		return nil, status.Errorf(codes.InvalidArgument, "first message must be init")
+	}
+	return init, nil
+}
+
+// validateProxyConnect validates the proxy ID and address, and checks cluster
+// address availability for account-scoped tokens.
+func (s *ProxyServiceServer) validateProxyConnect(proxyID, address string, ctx context.Context) (proxyConnectParams, error) {
 	if proxyID == "" {
-		return status.Errorf(codes.InvalidArgument, "proxy_id is required")
+		return proxyConnectParams{}, status.Errorf(codes.InvalidArgument, "proxy_id is required")
+	}
+	if !isProxyAddressValid(address) {
+		return proxyConnectParams{}, status.Errorf(codes.InvalidArgument, "proxy address is invalid")
 	}
 
-	proxyAddress := req.GetAddress()
-	if !isProxyAddressValid(proxyAddress) {
-		return status.Errorf(codes.InvalidArgument, "proxy address is invalid")
-	}
-
-	var accountID *string
 	token := GetProxyTokenFromContext(ctx)
 	if token != nil && token.AccountID != nil {
-		accountID = token.AccountID
-
-		available, err := s.proxyManager.IsClusterAddressAvailable(ctx, proxyAddress, *accountID)
+		available, err := s.proxyManager.IsClusterAddressAvailable(ctx, address, *token.AccountID)
 		if err != nil {
-			return status.Errorf(codes.Internal, "check cluster address: %v", err)
+			return proxyConnectParams{}, status.Errorf(codes.Internal, "check cluster address: %v", err)
 		}
 		if !available {
-			return status.Errorf(codes.AlreadyExists, "cluster address %s is already in use", proxyAddress)
+			return proxyConnectParams{}, status.Errorf(codes.AlreadyExists, "cluster address %s is already in use", address)
 		}
 	}
 
+	return proxyConnectParams{proxyID: proxyID, address: address}, nil
+}
+
+// registerProxyConnection creates a proxyConnection, registers it with the
+// proxy manager and cluster, and stores it in connectedProxies. The caller
+// provides a partially initialised connSeed with stream-specific fields set;
+// the remaining fields are filled in here.
+func (s *ProxyServiceServer) registerProxyConnection(ctx context.Context, params proxyConnectParams, connSeed *proxyConnection) (*proxyConnection, *proxy.Proxy, error) {
+	peerInfo := PeerIPFromContext(ctx)
+
+	var accountID *string
 	var tokenID string
-	if token != nil {
+	if token := GetProxyTokenFromContext(ctx); token != nil {
+		if token.AccountID != nil {
+			accountID = token.AccountID
+		}
 		tokenID = token.ID
 	}
 
 	sessionID := uuid.NewString()
-
-	if old, loaded := s.connectedProxies.Load(proxyID); loaded {
-		oldConn := old.(*proxyConnection)
-		log.WithFields(log.Fields{
-			"proxy_id":       proxyID,
-			"old_session_id": oldConn.sessionID,
-			"new_session_id": sessionID,
-		}).Info("Superseding existing proxy connection")
-		oldConn.cancel()
-	}
+	s.supersedePriorConnection(params.proxyID, sessionID)
 
 	connCtx, cancel := context.WithCancel(ctx)
-	conn := &proxyConnection{
-		proxyID:      proxyID,
-		sessionID:    sessionID,
-		address:      proxyAddress,
-		accountID:    accountID,
-		tokenID:      tokenID,
-		capabilities: req.GetCapabilities(),
-		stream:       stream,
-		sendChan:     make(chan *proto.GetMappingUpdateResponse, 100),
-		ctx:          connCtx,
-		cancel:       cancel,
-	}
+	connSeed.proxyID = params.proxyID
+	connSeed.sessionID = sessionID
+	connSeed.address = params.address
+	connSeed.accountID = accountID
+	connSeed.tokenID = tokenID
+	connSeed.capabilities = params.capabilities
+	connSeed.sendChan = make(chan *proto.GetMappingUpdateResponse, 100)
+	connSeed.ctx = connCtx
+	connSeed.cancel = cancel
 
 	var caps *proxy.Capabilities
-	if c := req.GetCapabilities(); c != nil {
+	if c := params.capabilities; c != nil {
 		caps = &proxy.Capabilities{
 			SupportsCustomPorts: c.SupportsCustomPorts,
 			RequireSubdomain:    c.RequireSubdomain,
 			SupportsCrowdsec:    c.SupportsCrowdsec,
 		}
 	}
-	proxyRecord, err := s.proxyManager.Connect(ctx, proxyID, sessionID, proxyAddress, peerInfo, accountID, caps)
+
+	proxyRecord, err := s.proxyManager.Connect(ctx, params.proxyID, sessionID, params.address, peerInfo, accountID, caps)
 	if err != nil {
 		cancel()
 		if accountID != nil {
-			return status.Errorf(codes.Internal, "failed to register BYOP proxy: %v", err)
+			return nil, nil, status.Errorf(codes.Internal, "failed to register BYOP proxy: %v", err)
 		}
-		log.WithContext(ctx).Warnf("failed to register proxy %s in database: %v", proxyID, err)
-		return status.Errorf(codes.Internal, "register proxy in database: %v", err)
+		log.WithContext(ctx).Warnf("failed to register proxy %s in database: %v", params.proxyID, err)
+		return nil, nil, status.Errorf(codes.Internal, "register proxy in database: %v", err)
 	}
 
-	s.connectedProxies.Store(proxyID, conn)
-	if err := s.proxyController.RegisterProxyToCluster(ctx, conn.address, proxyID); err != nil {
-		log.WithContext(ctx).Warnf("Failed to register proxy %s in cluster: %v", proxyID, err)
+	s.connectedProxies.Store(params.proxyID, connSeed)
+	if err := s.proxyController.RegisterProxyToCluster(ctx, params.address, params.proxyID); err != nil {
+		log.WithContext(ctx).Warnf("Failed to register proxy %s in cluster: %v", params.proxyID, err)
 	}
 
-	if err := s.sendSnapshot(ctx, conn); err != nil {
-		if s.connectedProxies.CompareAndDelete(proxyID, conn) {
-			if unregErr := s.proxyController.UnregisterProxyFromCluster(context.Background(), conn.address, proxyID); unregErr != nil {
-				log.WithContext(ctx).Debugf("cleanup after snapshot failure for proxy %s: %v", proxyID, unregErr)
-			}
-		}
-		cancel()
-		if disconnErr := s.proxyManager.Disconnect(context.Background(), proxyID, sessionID); disconnErr != nil {
-			log.WithContext(ctx).Debugf("cleanup after snapshot failure for proxy %s: %v", proxyID, disconnErr)
-		}
-		return fmt.Errorf("send snapshot to proxy %s: %w", proxyID, err)
+	return connSeed, proxyRecord, nil
+}
+
+// supersedePriorConnection cancels any existing connection for the given proxy.
+func (s *ProxyServiceServer) supersedePriorConnection(proxyID, newSessionID string) {
+	if old, loaded := s.connectedProxies.Load(proxyID); loaded {
+		oldConn := old.(*proxyConnection)
+		log.WithFields(log.Fields{
+			"proxy_id":       proxyID,
+			"old_session_id": oldConn.sessionID,
+			"new_session_id": newSessionID,
+		}).Info("Superseding existing proxy connection")
+		oldConn.cancel()
 	}
+}
 
-	errChan := make(chan error, 2)
-	go s.sender(conn, errChan)
+// cleanupFailedSnapshot removes the connection from the cluster and store
+// after a snapshot send failure.
+func (s *ProxyServiceServer) cleanupFailedSnapshot(ctx context.Context, conn *proxyConnection) {
+	if s.connectedProxies.CompareAndDelete(conn.proxyID, conn) {
+		if err := s.proxyController.UnregisterProxyFromCluster(context.Background(), conn.address, conn.proxyID); err != nil {
+			log.WithContext(ctx).Debugf("cleanup after snapshot failure for proxy %s: %v", conn.proxyID, err)
+		}
+	}
+	conn.cancel()
+	if err := s.proxyManager.Disconnect(context.Background(), conn.proxyID, conn.sessionID); err != nil {
+		log.WithContext(ctx).Debugf("cleanup after snapshot failure for proxy %s: %v", conn.proxyID, err)
+	}
+}
 
-	log.WithFields(log.Fields{
-		"proxy_id":      proxyID,
-		"session_id":    sessionID,
-		"address":       proxyAddress,
-		"cluster_addr":  proxyAddress,
-		"account_id":    accountID,
-		"total_proxies": len(s.GetConnectedProxies()),
-	}).Info("Proxy registered in cluster")
-	defer func() {
-		if !s.connectedProxies.CompareAndDelete(proxyID, conn) {
-			log.Infof("Proxy %s session %s: skipping cleanup, superseded by new connection", proxyID, sessionID)
-			cancel()
+// drainRecv consumes and discards messages from a bidirectional stream.
+// The proxy sends an ack for every incremental update; we don't need them
+// after the snapshot phase. Recv errors are forwarded to errChan.
+func (s *ProxyServiceServer) drainRecv(stream proto.ProxyService_SyncMappingsServer, errChan chan<- error) {
+	for {
+		if _, err := stream.Recv(); err != nil {
+			errChan <- err
 			return
 		}
+	}
+}
 
-		if err := s.proxyController.UnregisterProxyFromCluster(context.Background(), conn.address, proxyID); err != nil {
-			log.Warnf("Failed to unregister proxy %s from cluster: %v", proxyID, err)
-		}
-		if err := s.proxyManager.Disconnect(context.Background(), proxyID, sessionID); err != nil {
-			log.Warnf("Failed to mark proxy %s as disconnected: %v", proxyID, err)
-		}
+// serveProxyConnection runs the post-snapshot lifecycle: heartbeat, sender,
+// and wait for termination. When bidi is true, normal stream closure (EOF,
+// canceled) is treated as a clean disconnect rather than an error.
+func (s *ProxyServiceServer) serveProxyConnection(conn *proxyConnection, proxyRecord *proxy.Proxy, errChan <-chan error, bidi bool) error {
+	log.WithFields(log.Fields{
+		"proxy_id":      conn.proxyID,
+		"session_id":    conn.sessionID,
+		"address":       conn.address,
+		"cluster_addr":  conn.address,
+		"account_id":    conn.accountID,
+		"total_proxies": len(s.GetConnectedProxies()),
+	}).Info("Proxy registered in cluster")
 
-		cancel()
-		log.Infof("Proxy %s session %s disconnected", proxyID, sessionID)
-	}()
-
-	go s.heartbeat(connCtx, conn, proxyRecord)
+	defer s.disconnectProxy(conn)
+	go s.heartbeat(conn.ctx, conn, proxyRecord)
 
 	select {
 	case err := <-errChan:
-		log.WithContext(ctx).Warnf("Failed to send update: %v", err)
-		return fmt.Errorf("send update to proxy %s: %w", proxyID, err)
-	case <-connCtx.Done():
-		log.WithContext(ctx).Infof("Proxy %s context canceled", proxyID)
-		return connCtx.Err()
+		if bidi && isStreamClosed(err) {
+			log.Infof("Proxy %s stream closed", conn.proxyID)
+			return nil
+		}
+		log.Warnf("Failed to send update: %v", err)
+		return fmt.Errorf("send update to proxy %s: %w", conn.proxyID, err)
+	case <-conn.ctx.Done():
+		log.Infof("Proxy %s context canceled", conn.proxyID)
+		return conn.ctx.Err()
 	}
 }
 
+// disconnectProxy removes the connection from cluster and store, unless it
+// has already been superseded by a newer connection.
+func (s *ProxyServiceServer) disconnectProxy(conn *proxyConnection) {
+	if !s.connectedProxies.CompareAndDelete(conn.proxyID, conn) {
+		log.Infof("Proxy %s session %s: skipping cleanup, superseded by new connection", conn.proxyID, conn.sessionID)
+		conn.cancel()
+		return
+	}
+
+	if err := s.proxyController.UnregisterProxyFromCluster(context.Background(), conn.address, conn.proxyID); err != nil {
+		log.Warnf("Failed to unregister proxy %s from cluster: %v", conn.proxyID, err)
+	}
+	if err := s.proxyManager.Disconnect(context.Background(), conn.proxyID, conn.sessionID); err != nil {
+		log.Warnf("Failed to mark proxy %s as disconnected: %v", conn.proxyID, err)
+	}
+
+	conn.cancel()
+	log.Infof("Proxy %s session %s disconnected", conn.proxyID, conn.sessionID)
+}
+
+// sendSnapshotSync sends the initial snapshot with back-pressure: it sends
+// one batch, then waits for the proxy to ack before sending the next.
+func (s *ProxyServiceServer) sendSnapshotSync(ctx context.Context, conn *proxyConnection, stream proto.ProxyService_SyncMappingsServer) error {
+	if !isProxyAddressValid(conn.address) {
+		return fmt.Errorf("proxy address is invalid")
+	}
+	if s.snapshotBatchSize <= 0 {
+		return fmt.Errorf("invalid snapshot batch size: %d", s.snapshotBatchSize)
+	}
+
+	mappings, err := s.snapshotServiceMappings(ctx, conn)
+	if err != nil {
+		return err
+	}
+
+	for i := 0; i < len(mappings); i += s.snapshotBatchSize {
+		end := i + s.snapshotBatchSize
+		if end > len(mappings) {
+			end = len(mappings)
+		}
+		for _, m := range mappings[i:end] {
+			token, err := s.tokenStore.GenerateToken(m.AccountId, m.Id, s.proxyTokenTTL())
+			if err != nil {
+				return fmt.Errorf("generate auth token for service %s: %w", m.Id, err)
+			}
+			m.AuthToken = token
+		}
+		if err := stream.Send(&proto.SyncMappingsResponse{
+			Mapping:             mappings[i:end],
+			InitialSyncComplete: end == len(mappings),
+		}); err != nil {
+			return fmt.Errorf("send snapshot batch: %w", err)
+		}
+
+		if err := waitForAck(stream); err != nil {
+			return err
+		}
+	}
+
+	if len(mappings) == 0 {
+		if err := stream.Send(&proto.SyncMappingsResponse{
+			InitialSyncComplete: true,
+		}); err != nil {
+			return fmt.Errorf("send snapshot completion: %w", err)
+		}
+
+		if err := waitForAck(stream); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func waitForAck(stream proto.ProxyService_SyncMappingsServer) error {
+	msg, err := stream.Recv()
+	if err != nil {
+		return fmt.Errorf("receive ack: %w", err)
+	}
+	if msg.GetAck() == nil {
+		return fmt.Errorf("expected ack, got %T", msg.GetMsg())
+	}
+	return nil
+}
+
 // heartbeat updates the proxy's last_seen timestamp every minute and
 // disconnects the proxy if its access token has been revoked.
 func (s *ProxyServiceServer) heartbeat(ctx context.Context, conn *proxyConnection, p *proxy.Proxy) {
@@ -381,6 +562,9 @@ func (s *ProxyServiceServer) sendSnapshot(ctx context.Context, conn *proxyConnec
 	if !isProxyAddressValid(conn.address) {
 		return fmt.Errorf("proxy address is invalid")
 	}
+	if s.snapshotBatchSize <= 0 {
+		return fmt.Errorf("invalid snapshot batch size: %d", s.snapshotBatchSize)
+	}
 
 	mappings, err := s.snapshotServiceMappings(ctx, conn)
 	if err != nil {
@@ -460,12 +644,26 @@ func isProxyAddressValid(addr string) bool {
 	return err == nil
 }
 
-// sender handles sending messages to proxy
+// isStreamClosed returns true for errors that indicate normal stream
+// termination: io.EOF, context cancellation, or gRPC Canceled.
+func isStreamClosed(err error) bool {
+	if err == nil {
+		return false
+	}
+	if errors.Is(err, io.EOF) || errors.Is(err, context.Canceled) {
+		return true
+	}
+	return status.Code(err) == codes.Canceled
+}
+
+// sender handles sending messages to proxy.
+// When conn.syncStream is set the message is sent as SyncMappingsResponse;
+// otherwise the legacy GetMappingUpdateResponse stream is used.
 func (s *ProxyServiceServer) sender(conn *proxyConnection, errChan chan<- error) {
 	for {
 		select {
 		case resp := <-conn.sendChan:
-			if err := conn.stream.Send(resp); err != nil {
+			if err := conn.sendResponse(resp); err != nil {
 				errChan <- err
 				return
 			}
@@ -475,6 +673,17 @@ func (s *ProxyServiceServer) sender(conn *proxyConnection, errChan chan<- error)
 	}
 }
 
+// sendResponse sends a mapping update on whichever stream the proxy connected with.
+func (conn *proxyConnection) sendResponse(resp *proto.GetMappingUpdateResponse) error {
+	if conn.syncStream != nil {
+		return conn.syncStream.Send(&proto.SyncMappingsResponse{
+			Mapping:             resp.Mapping,
+			InitialSyncComplete: resp.InitialSyncComplete,
+		})
+	}
+	return conn.stream.Send(resp)
+}
+
 // SendAccessLog processes access log from proxy
 func (s *ProxyServiceServer) SendAccessLog(ctx context.Context, req *proto.SendAccessLogRequest) (*proto.SendAccessLogResponse, error) {
 	accessLog := req.GetLog()
@@ -541,8 +750,8 @@ func (s *ProxyServiceServer) SendServiceUpdate(update *proto.GetMappingUpdateRes
 				return true
 			}
 			connUpdate = &proto.GetMappingUpdateResponse{
-				Mapping:              filtered,
-				InitialSyncComplete:  update.InitialSyncComplete,
+				Mapping:             filtered,
+				InitialSyncComplete: update.InitialSyncComplete,
 			}
 		}
 		resp := s.perProxyMessage(connUpdate, conn.proxyID)

management/internals/shared/grpc/sync_mappings_test.go

@@ -0,0 +1,411 @@
+package grpc
+
+import (
+	"context"
+	"fmt"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/metadata"
+
+	rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
+	"github.com/netbirdio/netbird/shared/management/proto"
+)
+
+// syncRecordingStream is a mock ProxyService_SyncMappingsServer that records
+// sent messages and returns pre-loaded ack responses from Recv.
+type syncRecordingStream struct {
+	grpc.ServerStream
+
+	mu       sync.Mutex
+	sent     []*proto.SyncMappingsResponse
+	recvMsgs []*proto.SyncMappingsRequest
+	recvIdx  int
+}
+
+func (s *syncRecordingStream) Send(m *proto.SyncMappingsResponse) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.sent = append(s.sent, m)
+	return nil
+}
+
+func (s *syncRecordingStream) Recv() (*proto.SyncMappingsRequest, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.recvIdx >= len(s.recvMsgs) {
+		return nil, fmt.Errorf("no more recv messages")
+	}
+	msg := s.recvMsgs[s.recvIdx]
+	s.recvIdx++
+	return msg, nil
+}
+
+func (s *syncRecordingStream) Context() context.Context     { return context.Background() }
+func (s *syncRecordingStream) SetHeader(metadata.MD) error  { return nil }
+func (s *syncRecordingStream) SendHeader(metadata.MD) error { return nil }
+func (s *syncRecordingStream) SetTrailer(metadata.MD)       {}
+func (s *syncRecordingStream) SendMsg(any) error            { return nil }
+func (s *syncRecordingStream) RecvMsg(any) error            { return nil }
+
+func ackMsg() *proto.SyncMappingsRequest {
+	return &proto.SyncMappingsRequest{
+		Msg: &proto.SyncMappingsRequest_Ack{Ack: &proto.SyncMappingsAck{}},
+	}
+}
+
+func TestSendSnapshotSync_BatchesWithAcks(t *testing.T) {
+	const cluster = "cluster.example.com"
+	const batchSize = 3
+	const totalServices = 7 // 3 + 3 + 1 → 3 batches, 3 acks (one per batch, including final)
+
+	ctrl := gomock.NewController(t)
+	mgr := rpservice.NewMockManager(ctrl)
+	mgr.EXPECT().GetGlobalServices(gomock.Any()).Return(makeServices(totalServices, cluster), nil)
+
+	s := newSnapshotTestServer(t, batchSize)
+	s.serviceManager = mgr
+
+	stream := &syncRecordingStream{
+		recvMsgs: []*proto.SyncMappingsRequest{ackMsg(), ackMsg(), ackMsg()},
+	}
+	conn := &proxyConnection{
+		proxyID:    "proxy-a",
+		address:    cluster,
+		syncStream: stream,
+	}
+
+	err := s.sendSnapshotSync(context.Background(), conn, stream)
+	require.NoError(t, err)
+
+	require.Len(t, stream.sent, 3, "should send ceil(7/3) = 3 batches")
+
+	assert.Len(t, stream.sent[0].Mapping, 3)
+	assert.False(t, stream.sent[0].InitialSyncComplete)
+
+	assert.Len(t, stream.sent[1].Mapping, 3)
+	assert.False(t, stream.sent[1].InitialSyncComplete)
+
+	assert.Len(t, stream.sent[2].Mapping, 1)
+	assert.True(t, stream.sent[2].InitialSyncComplete)
+
+	// All 3 acks consumed — including the final batch.
+	assert.Equal(t, 3, stream.recvIdx)
+}
+
+func TestSendSnapshotSync_SingleBatchWaitsForAck(t *testing.T) {
+	const cluster = "cluster.example.com"
+	const batchSize = 100
+	const totalServices = 5
+
+	ctrl := gomock.NewController(t)
+	mgr := rpservice.NewMockManager(ctrl)
+	mgr.EXPECT().GetGlobalServices(gomock.Any()).Return(makeServices(totalServices, cluster), nil)
+
+	s := newSnapshotTestServer(t, batchSize)
+	s.serviceManager = mgr
+
+	stream := &syncRecordingStream{
+		recvMsgs: []*proto.SyncMappingsRequest{ackMsg()},
+	}
+	conn := &proxyConnection{
+		proxyID:    "proxy-a",
+		address:    cluster,
+		syncStream: stream,
+	}
+
+	err := s.sendSnapshotSync(context.Background(), conn, stream)
+	require.NoError(t, err)
+
+	require.Len(t, stream.sent, 1)
+	assert.Len(t, stream.sent[0].Mapping, totalServices)
+	assert.True(t, stream.sent[0].InitialSyncComplete)
+	assert.Equal(t, 1, stream.recvIdx, "final batch ack must be consumed")
+}
+
+func TestSendSnapshotSync_EmptySnapshot(t *testing.T) {
+	const cluster = "cluster.example.com"
+
+	ctrl := gomock.NewController(t)
+	mgr := rpservice.NewMockManager(ctrl)
+	mgr.EXPECT().GetGlobalServices(gomock.Any()).Return(nil, nil)
+
+	s := newSnapshotTestServer(t, 500)
+	s.serviceManager = mgr
+
+	stream := &syncRecordingStream{
+		recvMsgs: []*proto.SyncMappingsRequest{ackMsg()},
+	}
+	conn := &proxyConnection{
+		proxyID:    "proxy-a",
+		address:    cluster,
+		syncStream: stream,
+	}
+
+	err := s.sendSnapshotSync(context.Background(), conn, stream)
+	require.NoError(t, err)
+
+	require.Len(t, stream.sent, 1, "empty snapshot must still send sync-complete")
+	assert.Empty(t, stream.sent[0].Mapping)
+	assert.True(t, stream.sent[0].InitialSyncComplete)
+	assert.Equal(t, 1, stream.recvIdx, "empty snapshot ack must be consumed")
+}
+
+func TestSendSnapshotSync_MissingAckReturnsError(t *testing.T) {
+	const cluster = "cluster.example.com"
+	const batchSize = 2
+	const totalServices = 4 // 2 batches → 1 ack needed, but we provide none
+
+	ctrl := gomock.NewController(t)
+	mgr := rpservice.NewMockManager(ctrl)
+	mgr.EXPECT().GetGlobalServices(gomock.Any()).Return(makeServices(totalServices, cluster), nil)
+
+	s := newSnapshotTestServer(t, batchSize)
+	s.serviceManager = mgr
+
+	// No acks available — Recv will return error.
+	stream := &syncRecordingStream{}
+	conn := &proxyConnection{
+		proxyID:    "proxy-a",
+		address:    cluster,
+		syncStream: stream,
+	}
+
+	err := s.sendSnapshotSync(context.Background(), conn, stream)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "receive ack")
+	// First batch should have been sent before the error.
+	require.Len(t, stream.sent, 1)
+}
+
+func TestSendSnapshotSync_WrongMessageInsteadOfAck(t *testing.T) {
+	const cluster = "cluster.example.com"
+	const batchSize = 2
+	const totalServices = 4
+
+	ctrl := gomock.NewController(t)
+	mgr := rpservice.NewMockManager(ctrl)
+	mgr.EXPECT().GetGlobalServices(gomock.Any()).Return(makeServices(totalServices, cluster), nil)
+
+	s := newSnapshotTestServer(t, batchSize)
+	s.serviceManager = mgr
+
+	// Send an init message instead of an ack.
+	stream := &syncRecordingStream{
+		recvMsgs: []*proto.SyncMappingsRequest{
+			{Msg: &proto.SyncMappingsRequest_Init{Init: &proto.SyncMappingsInit{ProxyId: "bad"}}},
+		},
+	}
+	conn := &proxyConnection{
+		proxyID:    "proxy-a",
+		address:    cluster,
+		syncStream: stream,
+	}
+
+	err := s.sendSnapshotSync(context.Background(), conn, stream)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "expected ack")
+}
+
+func TestSendSnapshotSync_BackPressureOrdering(t *testing.T) {
+	// Verify batches are sent strictly sequentially — batch N+1 is not sent
+	// until the ack for batch N is received, including the final batch.
+	const cluster = "cluster.example.com"
+	const batchSize = 2
+	const totalServices = 6 // 3 batches, 3 acks
+
+	ctrl := gomock.NewController(t)
+	mgr := rpservice.NewMockManager(ctrl)
+	mgr.EXPECT().GetGlobalServices(gomock.Any()).Return(makeServices(totalServices, cluster), nil)
+
+	s := newSnapshotTestServer(t, batchSize)
+	s.serviceManager = mgr
+
+	var mu sync.Mutex
+	var events []string
+
+	// Build a stream that logs send/recv events so we can verify ordering.
+	ackCh := make(chan struct{}, 3)
+	stream := &orderTrackingStream{
+		mu:     &mu,
+		events: &events,
+		ackCh:  ackCh,
+	}
+	conn := &proxyConnection{
+		proxyID:    "proxy-a",
+		address:    cluster,
+		syncStream: stream,
+	}
+
+	// Feed acks asynchronously after a short delay to simulate real proxy.
+	go func() {
+		for range 3 {
+			time.Sleep(10 * time.Millisecond)
+			ackCh <- struct{}{}
+		}
+	}()
+
+	err := s.sendSnapshotSync(context.Background(), conn, stream)
+	require.NoError(t, err)
+
+	mu.Lock()
+	defer mu.Unlock()
+
+	// Expected: send, recv-ack, send, recv-ack, send, recv-ack.
+	require.Len(t, events, 6)
+	assert.Equal(t, "send", events[0])
+	assert.Equal(t, "recv", events[1])
+	assert.Equal(t, "send", events[2])
+	assert.Equal(t, "recv", events[3])
+	assert.Equal(t, "send", events[4])
+	assert.Equal(t, "recv", events[5])
+}
+
+// orderTrackingStream logs "send" and "recv" events and blocks Recv until
+// an ack is signaled via ackCh.
+type orderTrackingStream struct {
+	grpc.ServerStream
+	mu     *sync.Mutex
+	events *[]string
+	ackCh  chan struct{}
+}
+
+func (s *orderTrackingStream) Send(_ *proto.SyncMappingsResponse) error {
+	s.mu.Lock()
+	*s.events = append(*s.events, "send")
+	s.mu.Unlock()
+	return nil
+}
+
+func (s *orderTrackingStream) Recv() (*proto.SyncMappingsRequest, error) {
+	<-s.ackCh
+	s.mu.Lock()
+	*s.events = append(*s.events, "recv")
+	s.mu.Unlock()
+	return ackMsg(), nil
+}
+
+func (s *orderTrackingStream) Context() context.Context     { return context.Background() }
+func (s *orderTrackingStream) SetHeader(metadata.MD) error  { return nil }
+func (s *orderTrackingStream) SendHeader(metadata.MD) error { return nil }
+func (s *orderTrackingStream) SetTrailer(metadata.MD)       {}
+func (s *orderTrackingStream) SendMsg(any) error            { return nil }
+func (s *orderTrackingStream) RecvMsg(any) error            { return nil }
+
+func TestSendSnapshotSync_TokensGeneratedPerBatch(t *testing.T) {
+	const cluster = "cluster.example.com"
+	const batchSize = 2
+	const totalServices = 4
+	const ttl = 100 * time.Millisecond
+	const ackDelay = 200 * time.Millisecond
+
+	ctrl := gomock.NewController(t)
+	mgr := rpservice.NewMockManager(ctrl)
+	mgr.EXPECT().GetGlobalServices(gomock.Any()).Return(makeServices(totalServices, cluster), nil)
+
+	s := newSnapshotTestServer(t, batchSize)
+	s.serviceManager = mgr
+	s.tokenTTL = ttl
+
+	// Build a stream that validates tokens immediately on Send, then
+	// delays the ack to ensure the next batch's tokens are generated fresh.
+	var validateErrs []error
+	ackCh := make(chan struct{}, 2)
+	stream := &tokenValidatingSyncStream{
+		tokenStore:   s.tokenStore,
+		validateErrs: &validateErrs,
+		ackCh:        ackCh,
+	}
+	conn := &proxyConnection{
+		proxyID:    "proxy-a",
+		address:    cluster,
+		syncStream: stream,
+	}
+
+	go func() {
+		// Delay first ack so that if tokens were all generated upfront they'd expire.
+		time.Sleep(ackDelay)
+		ackCh <- struct{}{}
+		// Final batch ack — immediate.
+		ackCh <- struct{}{}
+	}()
+
+	err := s.sendSnapshotSync(context.Background(), conn, stream)
+	require.NoError(t, err)
+	require.Empty(t, validateErrs,
+		"tokens must remain valid: per-batch generation guarantees freshness")
+}
+
+type tokenValidatingSyncStream struct {
+	grpc.ServerStream
+	tokenStore   *OneTimeTokenStore
+	validateErrs *[]error
+	ackCh        chan struct{}
+}
+
+func (s *tokenValidatingSyncStream) Send(m *proto.SyncMappingsResponse) error {
+	for _, mapping := range m.Mapping {
+		if err := s.tokenStore.ValidateAndConsume(mapping.AuthToken, mapping.AccountId, mapping.Id); err != nil {
+			*s.validateErrs = append(*s.validateErrs, fmt.Errorf("svc %s: %w", mapping.Id, err))
+		}
+	}
+	return nil
+}
+
+func (s *tokenValidatingSyncStream) Recv() (*proto.SyncMappingsRequest, error) {
+	<-s.ackCh
+	return ackMsg(), nil
+}
+
+func (s *tokenValidatingSyncStream) Context() context.Context     { return context.Background() }
+func (s *tokenValidatingSyncStream) SetHeader(metadata.MD) error  { return nil }
+func (s *tokenValidatingSyncStream) SendHeader(metadata.MD) error { return nil }
+func (s *tokenValidatingSyncStream) SetTrailer(metadata.MD)       {}
+func (s *tokenValidatingSyncStream) SendMsg(any) error            { return nil }
+func (s *tokenValidatingSyncStream) RecvMsg(any) error            { return nil }
+
+func TestConnectionSendResponse_RoutesToSyncStream(t *testing.T) {
+	stream := &syncRecordingStream{}
+	conn := &proxyConnection{
+		syncStream: stream,
+	}
+
+	resp := &proto.GetMappingUpdateResponse{
+		Mapping: []*proto.ProxyMapping{
+			{Id: "svc-1", AccountId: "acct-1", Domain: "example.com"},
+		},
+		InitialSyncComplete: true,
+	}
+
+	err := conn.sendResponse(resp)
+	require.NoError(t, err)
+
+	require.Len(t, stream.sent, 1)
+	assert.Len(t, stream.sent[0].Mapping, 1)
+	assert.Equal(t, "svc-1", stream.sent[0].Mapping[0].Id)
+	assert.True(t, stream.sent[0].InitialSyncComplete)
+}
+
+func TestConnectionSendResponse_RoutesToLegacyStream(t *testing.T) {
+	stream := &recordingStream{}
+	conn := &proxyConnection{
+		stream: stream,
+	}
+
+	resp := &proto.GetMappingUpdateResponse{
+		Mapping: []*proto.ProxyMapping{
+			{Id: "svc-2", AccountId: "acct-2"},
+		},
+	}
+
+	err := conn.sendResponse(resp)
+	require.NoError(t, err)
+
+	require.Len(t, stream.messages, 1)
+	assert.Equal(t, "svc-2", stream.messages[0].Mapping[0].Id)
+}

management/server/http/testing/integration/networks_handler_integration_test.go

@@ -1319,7 +1319,7 @@ func Test_NetworkRouters_Update(t *testing.T) {
 			},
 		},
 		{
-			name:      "Update non-existing router creates it",
+			name:      "Update non-existing router returns not found",
 			networkId: "testNetworkId",
 			routerId:  "nonExistingRouterId",
 			requestBody: &api.NetworkRouterRequest{
@@ -1328,11 +1328,7 @@ func Test_NetworkRouters_Update(t *testing.T) {
 				Metric:     100,
 				Enabled:    true,
 			},
-			expectedStatus: http.StatusOK,
-			verifyResponse: func(t *testing.T, router *api.NetworkRouter) {
-				t.Helper()
-				assert.Equal(t, "nonExistingRouterId", router.Id)
-			},
+			expectedStatus: http.StatusNotFound,
 		},
 		{
 			name:      "Update router with both peer and peer_groups",

management/server/networks/manager_test.go

@@ -34,8 +34,11 @@ func Test_GetAllNetworksReturnsNetworks(t *testing.T) {
 
 	networks, err := manager.GetAllNetworks(ctx, accountID, userID)
 	require.NoError(t, err)
-	require.Len(t, networks, 1)
-	require.Equal(t, "testNetworkId", networks[0].ID)
+	ids := make([]string, 0, len(networks))
+	for _, n := range networks {
+		ids = append(ids, n.ID)
+	}
+	require.ElementsMatch(t, []string{"testNetworkId", "secondNetworkId"}, ids)
 }
 
 func Test_GetAllNetworksReturnsPermissionDenied(t *testing.T) {

management/server/networks/routers/manager.go

@@ -104,7 +104,7 @@ func (m *managerImpl) CreateRouter(ctx context.Context, userID string, router *t
 
 		router.ID = xid.New().String()
 
-		err = transaction.SaveNetworkRouter(ctx, router)
+		err = transaction.CreateNetworkRouter(ctx, router)
 		if err != nil {
 			return fmt.Errorf("failed to create network router: %w", err)
 		}
@@ -196,24 +196,30 @@ func (m *managerImpl) updateRouterInTransaction(ctx context.Context, transaction
 		return nil, nil, fmt.Errorf("failed to get network: %w", err)
 	}
 
-	if network.ID != router.NetworkID {
+	existing, err := transaction.GetNetworkRouterByID(ctx, store.LockingStrengthUpdate, router.AccountID, router.ID)
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed to get network router: %w", err)
+	}
+
+	if existing.AccountID != router.AccountID {
+		return nil, nil, status.NewNetworkRouterNotFoundError(router.ID)
+	}
+
+	if existing.NetworkID != router.NetworkID {
 		return nil, nil, status.NewRouterNotPartOfNetworkError(router.ID, router.NetworkID)
 	}
 
-	allPeerGroups := router.PeerGroups
+	allPeerGroups := append([]string{}, router.PeerGroups...)
+	allPeerGroups = append(allPeerGroups, existing.PeerGroups...)
 	var directPeers []string
 	if router.Peer != "" {
 		directPeers = append(directPeers, router.Peer)
 	}
-	oldRouter, err := transaction.GetNetworkRouterByID(ctx, store.LockingStrengthNone, router.AccountID, router.ID)
-	if err == nil {
-		allPeerGroups = append(allPeerGroups, oldRouter.PeerGroups...)
-		if oldRouter.Peer != "" {
-			directPeers = append(directPeers, oldRouter.Peer)
-		}
+	if existing.Peer != "" {
+		directPeers = append(directPeers, existing.Peer)
 	}
 
-	if err = transaction.SaveNetworkRouter(ctx, router); err != nil {
+	if err = transaction.UpdateNetworkRouter(ctx, router); err != nil {
 		return nil, nil, fmt.Errorf("failed to update network router: %w", err)
 	}
 

management/server/networks/routers/manager_test.go

@@ -195,6 +195,7 @@ func Test_UpdateRouterSuccessfully(t *testing.T) {
 	if err != nil {
 		require.NoError(t, err)
 	}
+	router.ID = "testRouterId"
 
 	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
 	if err != nil {
@@ -210,6 +211,102 @@ func Test_UpdateRouterSuccessfully(t *testing.T) {
 	require.Equal(t, router.Metric, updatedRouter.Metric)
 }
 
+func Test_UpdateRouterRejectsCrossAccountID(t *testing.T) {
+	ctx := context.Background()
+	userID := "testAdminId"
+
+	// Admin of testAccountId tries to update a router that belongs to otherAccountId
+	// by passing the other account's router ID through the URL.
+	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 1, true)
+	if err != nil {
+		require.NoError(t, err)
+	}
+	router.ID = "otherRouterId"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManager(s)
+	am := mock_server.MockAccountManager{}
+	manager := NewManager(s, permissionsManager, &am)
+
+	updatedRouter, err := manager.UpdateRouter(ctx, userID, router)
+	require.Error(t, err)
+	require.Nil(t, updatedRouter)
+
+	// The other account's router must be untouched.
+	stored, err := s.GetNetworkRouterByID(ctx, store.LockingStrengthNone, "otherAccountId", "otherRouterId")
+	require.NoError(t, err)
+	require.Equal(t, "otherAccountId", stored.AccountID)
+	require.Equal(t, "otherNetworkId", stored.NetworkID)
+	require.Equal(t, "otherPeer", stored.Peer)
+	require.Equal(t, 1, stored.Metric)
+}
+
+func Test_CreateRouterRejectsCrossAccountID(t *testing.T) {
+	ctx := context.Background()
+	userID := "testAdminId"
+
+	// Admin of testAccountId tries to create a router in otherAccountId's network.
+	// The permission check is on router.AccountID (their own), but the network
+	// lookup must fail because (testAccountId, otherNetworkId) does not exist.
+	router, err := types.NewNetworkRouter("testAccountId", "otherNetworkId", "testPeerId", []string{}, false, 1, true)
+	if err != nil {
+		require.NoError(t, err)
+	}
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManager(s)
+	am := mock_server.MockAccountManager{}
+	manager := NewManager(s, permissionsManager, &am)
+
+	createdRouter, err := manager.CreateRouter(ctx, userID, router)
+	require.Error(t, err)
+	require.Nil(t, createdRouter)
+
+	// No router should have been created in either account's scope under otherNetworkId.
+	routersInOther, err := s.GetNetworkRoutersByNetID(ctx, store.LockingStrengthNone, "otherAccountId", "otherNetworkId")
+	require.NoError(t, err)
+	require.Len(t, routersInOther, 1)
+	require.Equal(t, "otherRouterId", routersInOther[0].ID)
+}
+
+func Test_UpdateRouterRejectsNetworkMismatch(t *testing.T) {
+	ctx := context.Background()
+	userID := "testAdminId"
+
+	// The router exists in testNetworkId, but the caller submits secondNetworkId
+	// (a different network in the same account). The update must be refused.
+	router, err := types.NewNetworkRouter("testAccountId", "secondNetworkId", "testPeerId", []string{}, false, 1, true)
+	if err != nil {
+		require.NoError(t, err)
+	}
+	router.ID = "testRouterId"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManager(s)
+	am := mock_server.MockAccountManager{}
+	manager := NewManager(s, permissionsManager, &am)
+
+	updatedRouter, err := manager.UpdateRouter(ctx, userID, router)
+	require.Error(t, err)
+	require.Nil(t, updatedRouter)
+
+	stored, err := s.GetNetworkRouterByID(ctx, store.LockingStrengthNone, "testAccountId", "testRouterId")
+	require.NoError(t, err)
+	require.Equal(t, "testNetworkId", stored.NetworkID)
+}
+
 func Test_UpdateRouterFailsWithPermissionDenied(t *testing.T) {
 	ctx := context.Background()
 	userID := "testUserId"

management/server/store/sql_store.go

@@ -4316,11 +4316,27 @@ func (s *SqlStore) GetNetworkRouterByID(ctx context.Context, lockStrength Lockin
 	return netRouter, nil
 }
 
-func (s *SqlStore) SaveNetworkRouter(ctx context.Context, router *routerTypes.NetworkRouter) error {
-	result := s.db.Save(router)
+func (s *SqlStore) CreateNetworkRouter(ctx context.Context, router *routerTypes.NetworkRouter) error {
+	if err := s.db.Create(router).Error; err != nil {
+		log.WithContext(ctx).Errorf("failed to create network router in store: %v", err)
+		return status.Errorf(status.Internal, "failed to create network router in store")
+	}
+
+	return nil
+}
+
+func (s *SqlStore) UpdateNetworkRouter(ctx context.Context, router *routerTypes.NetworkRouter) error {
+	result := s.db.
+		Select("*").
+		Where(accountAndIDQueryCondition, router.AccountID, router.ID).
+		Updates(router)
 	if result.Error != nil {
-		log.WithContext(ctx).Errorf("failed to save network router to store: %v", result.Error)
-		return status.Errorf(status.Internal, "failed to save network router to store")
+		log.WithContext(ctx).Errorf("failed to update network router in store: %v", result.Error)
+		return status.Errorf(status.Internal, "failed to update network router in store")
+	}
+
+	if result.RowsAffected == 0 {
+		return status.NewNetworkRouterNotFoundError(router.ID)
 	}
 
 	return nil

management/server/store/sql_store_test.go

@@ -2399,7 +2399,7 @@ func TestSqlStore_GetNetworkRouterByID(t *testing.T) {
 	}
 }
 
-func TestSqlStore_SaveNetworkRouter(t *testing.T) {
+func TestSqlStore_CreateNetworkRouter(t *testing.T) {
 	store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir())
 	t.Cleanup(cleanup)
 	require.NoError(t, err)
@@ -2410,7 +2410,7 @@ func TestSqlStore_SaveNetworkRouter(t *testing.T) {
 	netRouter, err := routerTypes.NewNetworkRouter(accountID, networkID, "", []string{"net-router-grp"}, true, 0, true)
 	require.NoError(t, err)
 
-	err = store.SaveNetworkRouter(context.Background(), netRouter)
+	err = store.CreateNetworkRouter(context.Background(), netRouter)
 	require.NoError(t, err)
 
 	savedNetRouter, err := store.GetNetworkRouterByID(context.Background(), LockingStrengthNone, accountID, netRouter.ID)
@@ -2418,6 +2418,39 @@ func TestSqlStore_SaveNetworkRouter(t *testing.T) {
 	require.Equal(t, netRouter, savedNetRouter)
 }
 
+func TestSqlStore_UpdateNetworkRouter(t *testing.T) {
+	store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir())
+	t.Cleanup(cleanup)
+	require.NoError(t, err)
+
+	accountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b"
+	networkID := "ct286bi7qv930dsrrug0"
+	routerID := "ctc20ji7qv9ck2sebc80"
+
+	netRouter := &routerTypes.NetworkRouter{
+		ID:         routerID,
+		AccountID:  accountID,
+		NetworkID:  networkID,
+		Peer:       "",
+		PeerGroups: []string{"net-router-grp"},
+		Masquerade: true,
+		Metric:     42,
+		Enabled:    true,
+	}
+
+	err = store.UpdateNetworkRouter(context.Background(), netRouter)
+	require.NoError(t, err)
+
+	savedNetRouter, err := store.GetNetworkRouterByID(context.Background(), LockingStrengthNone, accountID, routerID)
+	require.NoError(t, err)
+	require.Equal(t, netRouter, savedNetRouter)
+
+	// Updating a router under a different account must not match any row.
+	netRouter.AccountID = "non-existent-account"
+	err = store.UpdateNetworkRouter(context.Background(), netRouter)
+	require.Error(t, err)
+}
+
 func TestSqlStore_DeleteNetworkRouter(t *testing.T) {
 	store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir())
 	t.Cleanup(cleanup)

management/server/store/store.go

@@ -230,7 +230,8 @@ type Store interface {
 	GetNetworkRoutersByNetID(ctx context.Context, lockStrength LockingStrength, accountID, netID string) ([]*routerTypes.NetworkRouter, error)
 	GetNetworkRoutersByAccountID(ctx context.Context, lockStrength LockingStrength, accountID string) ([]*routerTypes.NetworkRouter, error)
 	GetNetworkRouterByID(ctx context.Context, lockStrength LockingStrength, accountID, routerID string) (*routerTypes.NetworkRouter, error)
-	SaveNetworkRouter(ctx context.Context, router *routerTypes.NetworkRouter) error
+	CreateNetworkRouter(ctx context.Context, router *routerTypes.NetworkRouter) error
+	UpdateNetworkRouter(ctx context.Context, router *routerTypes.NetworkRouter) error
 	DeleteNetworkRouter(ctx context.Context, accountID, routerID string) error
 
 	GetNetworkResourcesByNetID(ctx context.Context, lockStrength LockingStrength, accountID, netID string) ([]*resourceTypes.NetworkResource, error)

management/server/store/store_mock.go

@@ -310,6 +310,20 @@ func (mr *MockStoreMockRecorder) CreateGroups(ctx, accountID, groups interface{}
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateGroups", reflect.TypeOf((*MockStore)(nil).CreateGroups), ctx, accountID, groups)
 }
 
+// CreateNetworkRouter mocks base method.
+func (m *MockStore) CreateNetworkRouter(ctx context.Context, router *types0.NetworkRouter) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "CreateNetworkRouter", ctx, router)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// CreateNetworkRouter indicates an expected call of CreateNetworkRouter.
+func (mr *MockStoreMockRecorder) CreateNetworkRouter(ctx, router interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateNetworkRouter", reflect.TypeOf((*MockStore)(nil).CreateNetworkRouter), ctx, router)
+}
+
 // CreatePeerJob mocks base method.
 func (m *MockStore) CreatePeerJob(ctx context.Context, job *types2.Job) error {
 	m.ctrl.T.Helper()
@@ -2642,6 +2656,36 @@ func (mr *MockStoreMockRecorder) MarkPATUsed(ctx, patID interface{}) *gomock.Cal
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkPATUsed", reflect.TypeOf((*MockStore)(nil).MarkPATUsed), ctx, patID)
 }
 
+// MarkPeerConnectedIfNewerSession mocks base method.
+func (m *MockStore) MarkPeerConnectedIfNewerSession(ctx context.Context, accountID, peerID string, newSessionStartedAt int64) (bool, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "MarkPeerConnectedIfNewerSession", ctx, accountID, peerID, newSessionStartedAt)
+	ret0, _ := ret[0].(bool)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// MarkPeerConnectedIfNewerSession indicates an expected call of MarkPeerConnectedIfNewerSession.
+func (mr *MockStoreMockRecorder) MarkPeerConnectedIfNewerSession(ctx, accountID, peerID, newSessionStartedAt interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkPeerConnectedIfNewerSession", reflect.TypeOf((*MockStore)(nil).MarkPeerConnectedIfNewerSession), ctx, accountID, peerID, newSessionStartedAt)
+}
+
+// MarkPeerDisconnectedIfSameSession mocks base method.
+func (m *MockStore) MarkPeerDisconnectedIfSameSession(ctx context.Context, accountID, peerID string, sessionStartedAt int64) (bool, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "MarkPeerDisconnectedIfSameSession", ctx, accountID, peerID, sessionStartedAt)
+	ret0, _ := ret[0].(bool)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// MarkPeerDisconnectedIfSameSession indicates an expected call of MarkPeerDisconnectedIfSameSession.
+func (mr *MockStoreMockRecorder) MarkPeerDisconnectedIfSameSession(ctx, accountID, peerID, sessionStartedAt interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkPeerDisconnectedIfSameSession", reflect.TypeOf((*MockStore)(nil).MarkPeerDisconnectedIfSameSession), ctx, accountID, peerID, sessionStartedAt)
+}
+
 // MarkPendingJobsAsFailed mocks base method.
 func (m *MockStore) MarkPendingJobsAsFailed(ctx context.Context, accountID, peerID, jobID, reason string) error {
 	m.ctrl.T.Helper()
@@ -2852,20 +2896,6 @@ func (mr *MockStoreMockRecorder) SaveNetworkResource(ctx, resource interface{})
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveNetworkResource", reflect.TypeOf((*MockStore)(nil).SaveNetworkResource), ctx, resource)
 }
 
-// SaveNetworkRouter mocks base method.
-func (m *MockStore) SaveNetworkRouter(ctx context.Context, router *types0.NetworkRouter) error {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "SaveNetworkRouter", ctx, router)
-	ret0, _ := ret[0].(error)
-	return ret0
-}
-
-// SaveNetworkRouter indicates an expected call of SaveNetworkRouter.
-func (mr *MockStoreMockRecorder) SaveNetworkRouter(ctx, router interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveNetworkRouter", reflect.TypeOf((*MockStore)(nil).SaveNetworkRouter), ctx, router)
-}
-
 // SavePAT mocks base method.
 func (m *MockStore) SavePAT(ctx context.Context, pat *types2.PersonalAccessToken) error {
 	m.ctrl.T.Helper()
@@ -2922,36 +2952,6 @@ func (mr *MockStoreMockRecorder) SavePeerStatus(ctx, accountID, peerID, status i
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SavePeerStatus", reflect.TypeOf((*MockStore)(nil).SavePeerStatus), ctx, accountID, peerID, status)
 }
 
-// MarkPeerConnectedIfNewerSession mocks base method.
-func (m *MockStore) MarkPeerConnectedIfNewerSession(ctx context.Context, accountID, peerID string, newSessionStartedAt int64) (bool, error) {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "MarkPeerConnectedIfNewerSession", ctx, accountID, peerID, newSessionStartedAt)
-	ret0, _ := ret[0].(bool)
-	ret1, _ := ret[1].(error)
-	return ret0, ret1
-}
-
-// MarkPeerConnectedIfNewerSession indicates an expected call of MarkPeerConnectedIfNewerSession.
-func (mr *MockStoreMockRecorder) MarkPeerConnectedIfNewerSession(ctx, accountID, peerID, newSessionStartedAt interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkPeerConnectedIfNewerSession", reflect.TypeOf((*MockStore)(nil).MarkPeerConnectedIfNewerSession), ctx, accountID, peerID, newSessionStartedAt)
-}
-
-// MarkPeerDisconnectedIfSameSession mocks base method.
-func (m *MockStore) MarkPeerDisconnectedIfSameSession(ctx context.Context, accountID, peerID string, sessionStartedAt int64) (bool, error) {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "MarkPeerDisconnectedIfSameSession", ctx, accountID, peerID, sessionStartedAt)
-	ret0, _ := ret[0].(bool)
-	ret1, _ := ret[1].(error)
-	return ret0, ret1
-}
-
-// MarkPeerDisconnectedIfSameSession indicates an expected call of MarkPeerDisconnectedIfSameSession.
-func (mr *MockStoreMockRecorder) MarkPeerDisconnectedIfSameSession(ctx, accountID, peerID, sessionStartedAt interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkPeerDisconnectedIfSameSession", reflect.TypeOf((*MockStore)(nil).MarkPeerDisconnectedIfSameSession), ctx, accountID, peerID, sessionStartedAt)
-}
-
 // SavePolicy mocks base method.
 func (m *MockStore) SavePolicy(ctx context.Context, policy *types2.Policy) error {
 	m.ctrl.T.Helper()
@@ -3203,6 +3203,20 @@ func (mr *MockStoreMockRecorder) UpdateGroups(ctx, accountID, groups interface{}
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateGroups", reflect.TypeOf((*MockStore)(nil).UpdateGroups), ctx, accountID, groups)
 }
 
+// UpdateNetworkRouter mocks base method.
+func (m *MockStore) UpdateNetworkRouter(ctx context.Context, router *types0.NetworkRouter) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "UpdateNetworkRouter", ctx, router)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// UpdateNetworkRouter indicates an expected call of UpdateNetworkRouter.
+func (mr *MockStoreMockRecorder) UpdateNetworkRouter(ctx, router interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNetworkRouter", reflect.TypeOf((*MockStore)(nil).UpdateNetworkRouter), ctx, router)
+}
+
 // UpdateProxyHeartbeat mocks base method.
 func (m *MockStore) UpdateProxyHeartbeat(ctx context.Context, p *proxy.Proxy) error {
 	m.ctrl.T.Helper()

management/server/testdata/networks.sql

@@ -9,9 +9,13 @@ INSERT INTO peers VALUES('testPeerId','testAccountId','5rvhvriKJZ3S9oxYToVj5TzDM
 
 CREATE TABLE `networks` (`id` text,`account_id` text,`name` text,`description` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_networks` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`));
 INSERT INTO networks VALUES('testNetworkId','testAccountId','some-name','some-description');
+INSERT INTO networks VALUES('secondNetworkId','testAccountId','second-name','second-description');
 
 CREATE TABLE `network_routers` (`id` text,`network_id` text,`account_id` text,`peer` text,`peer_groups` text,`masquerade` numeric,`metric` integer,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_network_routers` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`));
 INSERT INTO network_routers VALUES('testRouterId','testNetworkId','testAccountId','','["csquuo4jcko732k1ag00"]',0,9999);
+INSERT INTO accounts VALUES('otherAccountId','','2024-10-02 16:01:38.000000000+00:00','other.com','private',1,'otherNetworkIdentifier','{"IP":"100.65.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL);
+INSERT INTO networks VALUES('otherNetworkId','otherAccountId','other-net','other-description');
+INSERT INTO network_routers VALUES('otherRouterId','otherNetworkId','otherAccountId','otherPeer',NULL,0,1);
 
 CREATE TABLE `network_resources` (`id` text,`network_id` text,`account_id` text,`name` text,`description` text,`type` text,`address` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_network_resources` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`));
 INSERT INTO network_resources VALUES('testResourceId','testNetworkId','testAccountId','some-name','some-description','host','3.3.3.3/32');