mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-06 15:01:28 +02:00
[management] Serve networks with peer-based routers from the SQLite network map (#7418)
The SQLite network-map query expanded a router's groups with from network_routers, json_each(peer_groups). That comma is an inner join, so a router row survives only when json_each returns at least one row. A router targeting an individual peer carries no groups — the write path stores NULL for a nil slice and '[]' for an empty one — and json_each yields nothing for either, so the join erased the router before it could be keyed by its peer. Postgres reads the same rows through a correlated subquery and was never affected. The fix expands the groups with a left join, so the router survives with a NULL group_peers.peer_id and the existing scan loop keys it by router.Peer. Group routers still fan out one row per member.
This commit is contained in:
120
management/internals/network_map_db/network_router_store_test.go
Normal file
120
management/internals/network_map_db/network_router_store_test.go
Normal file
@@ -0,0 +1,120 @@
|
||||
package networkmapdb_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/netip"
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
networkmapdb "github.com/netbirdio/netbird/management/internals/network_map_db"
|
||||
networkmap_pgsql "github.com/netbirdio/netbird/management/internals/network_map_db/pgsql"
|
||||
networkmap_sqlite "github.com/netbirdio/netbird/management/internals/network_map_db/sqlite"
|
||||
routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types"
|
||||
nbpeer "github.com/netbirdio/netbird/management/server/peer"
|
||||
"github.com/netbirdio/netbird/management/server/store"
|
||||
"github.com/netbirdio/netbird/management/server/testutil"
|
||||
"github.com/netbirdio/netbird/management/server/types"
|
||||
)
|
||||
|
||||
// newEngineStores opens both stores on the selected engine's database.
|
||||
func newEngineStores(t *testing.T) (store.Store, networkmapdb.NetworkMapDBStore) {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
|
||||
switch engine := types.Engine(os.Getenv("NETBIRD_STORE_ENGINE")); engine {
|
||||
case types.PostgresStoreEngine:
|
||||
cleanup, dsn, err := testutil.CreatePostgresTestContainer()
|
||||
require.NoError(t, err, "start postgres test container")
|
||||
t.Cleanup(cleanup)
|
||||
|
||||
accountStore, err := store.NewPostgresqlStore(ctx, dsn, nil, false)
|
||||
require.NoError(t, err, "connect account store")
|
||||
t.Cleanup(func() { _ = accountStore.Close(ctx) })
|
||||
|
||||
nmStore, err := networkmap_pgsql.NewPostgresqlStore(ctx, dsn)
|
||||
require.NoError(t, err, "connect networkmap store")
|
||||
t.Cleanup(func() { nmStore.Pool.Close() })
|
||||
return accountStore, nmStore
|
||||
case types.SqliteStoreEngine, "":
|
||||
dataDir := t.TempDir()
|
||||
accountStore, err := store.NewSqliteStore(ctx, dataDir, nil, false)
|
||||
require.NoError(t, err, "open account store")
|
||||
t.Cleanup(func() { _ = accountStore.Close(ctx) })
|
||||
|
||||
nmStore, err := networkmap_sqlite.NewSqliteStore("store.db", dataDir)
|
||||
require.NoError(t, err, "open networkmap store")
|
||||
t.Cleanup(func() { _ = nmStore.Db.Close() })
|
||||
return accountStore, nmStore
|
||||
default:
|
||||
t.Skipf("networkmap store does not support engine %q", engine)
|
||||
return nil, nil
|
||||
}
|
||||
}
|
||||
|
||||
// Peer-based routers must survive the network-map read on every engine.
|
||||
func TestGetNetworkRouters_ServesPeerBasedRouters(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
accountStore, nmStore := newEngineStores(t)
|
||||
|
||||
const (
|
||||
accountID = "acc-nmap-routers"
|
||||
groupID = "grp-router-members"
|
||||
memberID = "peer-member"
|
||||
)
|
||||
|
||||
// Postgres enforces the groups-to-accounts FK that SQLite ignores.
|
||||
require.NoError(t, accountStore.SaveAccount(ctx, &types.Account{
|
||||
Id: accountID,
|
||||
Peers: map[string]*nbpeer.Peer{
|
||||
memberID: {
|
||||
ID: memberID,
|
||||
AccountID: accountID,
|
||||
Key: memberID + "-key",
|
||||
IP: netip.MustParseAddr("100.64.0.10"),
|
||||
Status: &nbpeer.PeerStatus{},
|
||||
},
|
||||
},
|
||||
Groups: map[string]*types.Group{
|
||||
groupID: {
|
||||
ID: groupID,
|
||||
AccountID: accountID,
|
||||
Name: "router members",
|
||||
Issued: types.GroupIssuedAPI,
|
||||
Peers: []string{memberID},
|
||||
},
|
||||
},
|
||||
}), "seed the account the routers belong to")
|
||||
|
||||
routers := []*routerTypes.NetworkRouter{
|
||||
{ID: "router-peer-nil", AccountID: accountID, NetworkID: "net-peer-nil", PublicID: "pub-peer-nil", Peer: "peer-direct-nil", Enabled: true, Metric: 9999},
|
||||
{ID: "router-peer-empty", AccountID: accountID, NetworkID: "net-peer-empty", PublicID: "pub-peer-empty", Peer: "peer-direct-empty", PeerGroups: []string{}, Enabled: true, Metric: 9999},
|
||||
{ID: "router-group", AccountID: accountID, NetworkID: "net-group", PublicID: "pub-group", PeerGroups: []string{groupID}, Enabled: true, Metric: 9999},
|
||||
}
|
||||
for _, router := range routers {
|
||||
require.NoError(t, accountStore.CreateNetworkRouter(ctx, router))
|
||||
}
|
||||
|
||||
tx, err := nmStore.BeginTx(ctx)
|
||||
require.NoError(t, err, "begin networkmap read transaction")
|
||||
t.Cleanup(func() { _ = tx.RollbackTx(ctx) })
|
||||
|
||||
got, err := tx.GetNetworkRouters(ctx, accountID)
|
||||
require.NoError(t, err, "read network routers")
|
||||
|
||||
assert.Contains(t, got, "net-peer-nil",
|
||||
"a router referencing an individual peer (peer_groups stored as NULL) must reach the network map")
|
||||
assert.Contains(t, got["net-peer-nil"], "peer-direct-nil",
|
||||
"the individual-peer router must be keyed by its peer")
|
||||
|
||||
assert.Contains(t, got, "net-peer-empty",
|
||||
"a router referencing an individual peer (peer_groups stored as '[]') must reach the network map")
|
||||
assert.Contains(t, got["net-peer-empty"], "peer-direct-empty",
|
||||
"the individual-peer router must be keyed by its peer")
|
||||
|
||||
assert.Contains(t, got, "net-group", "a group router must reach the network map")
|
||||
assert.Contains(t, got["net-group"], memberID,
|
||||
"the group router must fan out to the group's member peers")
|
||||
}
|
||||
@@ -11,9 +11,11 @@ import (
|
||||
)
|
||||
|
||||
const (
|
||||
// Outer join: a groupless router must survive.
|
||||
GetNetworkRouterQuery = `
|
||||
select public_id, peer, network_id, masquerade, metric, enabled, peer_groups, group_peers.peer_id
|
||||
from network_routers, json_each(peer_groups)
|
||||
from network_routers
|
||||
left join json_each(network_routers.peer_groups) on true
|
||||
left join group_peers on group_peers.account_id=? and group_peers.group_id=json_each.value
|
||||
where network_routers.account_id=?
|
||||
`
|
||||
|
||||
Reference in New Issue
Block a user