Merge remote-tracking branch 'origin/revert/component-types' into revert/component-types

Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io>

management/server/account_request_buffer.go

@@ -2,117 +2,38 @@ package server
 
 import (
 	"context"
-	"os"
-	"sync"
 	"time"
 
 	log "github.com/sirupsen/logrus"
 
+	"github.com/netbirdio/netbird/management/internals/shared/requestbuffer"
 	"github.com/netbirdio/netbird/management/server/store"
 	"github.com/netbirdio/netbird/management/server/types"
 )
 
-// AccountRequest holds the result channel to return the requested account.
-type AccountRequest struct {
-	AccountID  string
-	ResultChan chan *AccountResult
-}
-
-// AccountResult holds the account data or an error.
-type AccountResult struct {
-	Account *types.Account
-	Err     error
-}
+const defaultAccountBufferInterval = 100 * time.Millisecond
 
 type AccountRequestBuffer struct {
-	store               store.Store
-	getAccountRequests  map[string][]*AccountRequest
-	mu                  sync.Mutex
-	getAccountRequestCh chan *AccountRequest
-	bufferInterval      time.Duration
+	buffer *requestbuffer.Buffer[*types.Account]
 }
 
 func NewAccountRequestBuffer(ctx context.Context, store store.Store) *AccountRequestBuffer {
-	bufferIntervalStr := os.Getenv("NB_GET_ACCOUNT_BUFFER_INTERVAL")
-	bufferInterval, err := time.ParseDuration(bufferIntervalStr)
-	if err != nil {
-		if bufferIntervalStr != "" {
-			log.WithContext(ctx).Warnf("failed to parse account request buffer interval: %s", err)
-		}
-		bufferInterval = 100 * time.Millisecond
+	interval := requestbuffer.Interval(ctx, "NB_GET_ACCOUNT_BUFFER_INTERVAL", defaultAccountBufferInterval)
+	log.WithContext(ctx).Infof("set account request buffer interval to %s", interval)
+
+	return &AccountRequestBuffer{
+		buffer: requestbuffer.New(ctx, "account request buffer", interval, store.GetAccount),
 	}
-
-	log.WithContext(ctx).Infof("set account request buffer interval to %s", bufferInterval)
-
-	ac := AccountRequestBuffer{
-		store:               store,
-		getAccountRequests:  make(map[string][]*AccountRequest),
-		getAccountRequestCh: make(chan *AccountRequest),
-		bufferInterval:      bufferInterval,
-	}
-
-	go ac.processGetAccountRequests(ctx)
-
-	return &ac
 }
+
 func (ac *AccountRequestBuffer) GetAccountWithBackpressure(ctx context.Context, accountID string) (*types.Account, error) {
-	req := &AccountRequest{
-		AccountID:  accountID,
-		ResultChan: make(chan *AccountResult, 1),
+	account, err := ac.buffer.Get(ctx, accountID)
+	if err != nil || account == nil {
+		return account, err
 	}
 
-	log.WithContext(ctx).Tracef("requesting account %s with backpressure", accountID)
-	startTime := time.Now()
-	ac.getAccountRequestCh <- req
-
-	result := <-req.ResultChan
-	log.WithContext(ctx).Tracef("got account with backpressure after %s", time.Since(startTime))
-	return result.Account, result.Err
-}
-
-func (ac *AccountRequestBuffer) processGetAccountBatch(ctx context.Context, accountID string) {
-	ac.mu.Lock()
-	requests := ac.getAccountRequests[accountID]
-	delete(ac.getAccountRequests, accountID)
-	ac.mu.Unlock()
-
-	if len(requests) == 0 {
-		return
-	}
-
-	startTime := time.Now()
-	account, err := ac.store.GetAccount(ctx, accountID)
-	log.WithContext(ctx).Tracef("getting account %s in batch took %s", accountID, time.Since(startTime))
-	result := &AccountResult{Account: account, Err: err}
-
-	for _, req := range requests {
-		if account != nil {
-			// Shallow copy the account so each goroutine gets its own struct value.
-			// This prevents data races when callers mutate fields like Policies.
-			accountCopy := *account
-			req.ResultChan <- &AccountResult{Account: &accountCopy, Err: err}
-		} else {
-			req.ResultChan <- result
-		}
-		close(req.ResultChan)
-	}
-}
-
-func (ac *AccountRequestBuffer) processGetAccountRequests(ctx context.Context) {
-	for {
-		select {
-		case req := <-ac.getAccountRequestCh:
-			ac.mu.Lock()
-			ac.getAccountRequests[req.AccountID] = append(ac.getAccountRequests[req.AccountID], req)
-			if len(ac.getAccountRequests[req.AccountID]) == 1 {
-				go func(ctx context.Context, accountID string) {
-					time.Sleep(ac.bufferInterval)
-					ac.processGetAccountBatch(ctx, accountID)
-				}(ctx, req.AccountID)
-			}
-			ac.mu.Unlock()
-		case <-ctx.Done():
-			return
-		}
-	}
+	// Shallow copy the account so each caller gets its own struct value.
+	// This prevents data races when callers mutate fields like Policies.
+	accountCopy := *account
+	return &accountCopy, nil
 }

management/server/types/legacynmap/equivalence_test.go

@@ -50,7 +50,7 @@ import (
 	"gorm.io/gorm"
 	gormlogger "gorm.io/gorm/logger"
 
-	nbdns "github.com/netbirdio/netbird/dns"
+	"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller"
 	"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller/cache"
 	networkmapdb "github.com/netbirdio/netbird/management/internals/network_map_db"
 	networkmap_pgsql "github.com/netbirdio/netbird/management/internals/network_map_db/pgsql"
@@ -60,7 +60,7 @@ import (
 	"github.com/netbirdio/netbird/management/server/store"
 	"github.com/netbirdio/netbird/management/server/types"
 	"github.com/netbirdio/netbird/management/server/types/legacynmap"
-	"github.com/netbirdio/netbird/shared/management/networkmap/nmdata"
+	"github.com/netbirdio/netbird/shared/management/networkmap"
 	"github.com/netbirdio/netbird/shared/management/proto"
 )
 
@@ -108,7 +108,7 @@ func TestNetworkMapProtoEquivalence(t *testing.T) {
 			continue
 		}
 
-		checkAccount(ctx, t, nmStore, account, maxPeers, stats)
+		checkAccount(ctx, t, testStore, nmStore, account, maxPeers, stats)
 
 		account = nil
 		debug.FreeOSMemory()
@@ -126,7 +126,7 @@ func TestNetworkMapProtoEquivalence(t *testing.T) {
 
 // checkAccount compares both paths for every peer of one account. Nothing is
 // retained across peers, so memory stays flat within an account.
-func checkAccount(ctx context.Context, t *testing.T, nmStore *networkmapdb.NetworkMapDBStoreImpl, account *types.Account, maxPeers int, stats *equivStats) {
+func checkAccount(ctx context.Context, t *testing.T, accountStore store.Store, nmStore *networkmapdb.NetworkMapDBStoreImpl, account *types.Account, maxPeers int, stats *equivStats) {
 	t.Helper()
 
 	if len(account.Peers) == 0 {
@@ -150,11 +150,14 @@ func checkAccount(ctx context.Context, t *testing.T, nmStore *networkmapdb.Netwo
 	// Production fills ValidatedPeers via the integrated-validator wrapper; here
 	// every peer counts as validated, matching the legacy side's map.
 	nmData.ValidatedPeers = validated
-	// The legacy side receives no account zones (main sourced them from the
-	// external zones manager), so the DB-sourced applied-zone candidates must be
-	// dropped to keep the comparison surface identical. PrivateServiceCandidates
-	// stay: all paths derive them from account/DB data.
-	nmData.AppliedZoneCandidates = nil
+
+	// Custom DNS zones are built twice from the same rows — the account side
+	// from the zones manager, the store side in SQL — so both are fed in and
+	// compared rather than dropped. The same goes for the peers zone below:
+	// each side computes it with its own helper, which is where an AAAA gate
+	// that disagrees between the two would show up.
+	accountZones, err := accountStore.GetAccountZones(ctx, store.LockingStrengthNone, account.Id)
+	require.NoError(t, err, "account %s: load account zones", account.Id)
 
 	resourcePolicies := account.GetResourcePoliciesMap()
 	routers := account.GetResourceRoutersMap()
@@ -176,6 +179,9 @@ func checkAccount(ctx context.Context, t *testing.T, nmStore *networkmapdb.Netwo
 		settings = &types.Settings{}
 	}
 
+	accountPeersZone := account.GetPeersCustomZone(ctx, equivDNSName)
+	storePeersZone := networkmap.PeersCustomZone(ctx, account.Id, equivDNSName, nmData.Peers, controller.IPv6AllowedPeersFromData(nmData))
+
 	for _, peerID := range peerIDs {
 		peer := account.Peers[peerID]
 		if peer == nil {
@@ -188,7 +194,7 @@ func checkAccount(ctx context.Context, t *testing.T, nmStore *networkmapdb.Netwo
 
 		// STORE PATH — nmdata store through the production computation, mirroring
 		// the controller's networkMapFromData.
-		components := nmData.GetPeerNetworkMapComponents(peerID, nmdata.CustomZone{})
+		components := nmData.GetPeerNetworkMapComponents(peerID, storePeersZone)
 		storeNM := &types.NetworkMap{Network: components.Network}
 		if !components.IsEmpty() {
 			storeNM = types.CalculateNetworkMapFromComponents(ctx, components)
@@ -202,7 +208,7 @@ func checkAccount(ctx context.Context, t *testing.T, nmStore *networkmapdb.Netwo
 
 		// ACCOUNT PATH — Account → toNetworkMapData twins → components.
 		acctNM := account.GetPeerNetworkMapFromComponents(
-			ctx, peerID, nbdns.CustomZone{}, nil, validated, resourcePolicies, routers, nil, groupUsers,
+			ctx, peerID, accountPeersZone, accountZones, validated, resourcePolicies, routers, nil, groupUsers,
 		)
 		acctProto := mgmtgrpc.ToSyncResponse(
 			ctx, nil, nil, nil, types.TwinPeer(peer), nil, nil, acctNM, equivDNSName, nil,
@@ -211,7 +217,7 @@ func checkAccount(ctx context.Context, t *testing.T, nmStore *networkmapdb.Netwo
 
 		// LEGACY PATH — main's frozen copy.
 		legacyNM := legacynmap.GetPeerNetworkMapFromComponents(
-			&legacyAccount, ctx, peerID, nbdns.CustomZone{}, nil, validated, legacyResourcePolicies, routers, nil, groupUsers,
+			&legacyAccount, ctx, peerID, accountPeersZone, accountZones, validated, legacyResourcePolicies, routers, nil, groupUsers,
 		)
 		if legacyNM == nil {
 			t.Fatalf("after %d peers: account=%s peer=%s legacy NetworkMap nil, new non-nil", stats.peersChecked, account.Id, peerID)

management/server/types/legacynmap/proto_legacy.go

@@ -10,7 +10,7 @@ import (
 	"github.com/netbirdio/netbird/client/ssh/auth"
 	nbconfig "github.com/netbirdio/netbird/management/internals/server/config"
 	nbpeer "github.com/netbirdio/netbird/management/server/peer"
-	types "github.com/netbirdio/netbird/management/server/types"
+	"github.com/netbirdio/netbird/management/server/types"
 	nbroute "github.com/netbirdio/netbird/route"
 	"github.com/netbirdio/netbird/shared/management/networkmap"
 	"github.com/netbirdio/netbird/shared/management/proto"
@@ -89,7 +89,7 @@ func buildJWTConfig(config *nbconfig.HttpServerConfig, deviceFlowConfig *nbconfi
 
 	return &proto.JWTConfig{
 		Issuer:       issuer,
-		Audience:     audience,
+		Audience:     audience, //nolint:staticcheck
 		Audiences:    audiences,
 		KeysLocation: keysLocation,
 	}