sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-08-27 10:01:28 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix applied zones

Browse Source
This commit is contained in:
pascal
2026-08-21 11:03:58 +02:00
parent 331984a627
commit e83886b77b
10 changed files with 134 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
integration_tests/management/network_map_db/dns_test.go
View File

@@ -16,11 +16,14 @@ func TestGetAppliedZoneCandidatesViaPgxConnection(t *testing.T) {
ctx := context.TODO()
execQuery(t, ctx,
`insert into zones (id, account_id, domain, enable_search_domain, distribution_groups)
VALUES('zone-1','account-1','test-1.com',true,'["group-one-resource-id"]')`)
`insert into zones (id, account_id, domain, enabled, enable_search_domain, distribution_groups)
VALUES('zone-1','account-1','test-1.com',true,true,'["group-one-resource-id"]')`)
execQuery(t, ctx,
`insert into zones (id, account_id, domain, enable_search_domain, distribution_groups)
VALUES('zone-2','account-1','test-2.com',false,'["group-two-resources-id"]')`)
`insert into zones (id, account_id, domain, enabled, enable_search_domain, distribution_groups)
VALUES('zone-2','account-1','test-2.com',true,false,'["group-two-resources-id"]')`)
execQuery(t, ctx,
`insert into zones (id, account_id, domain, enabled, enable_search_domain, distribution_groups)
VALUES('zone-3','account-1','test-3.com',false,true,'["group-one-resource-id"]')`)
execQuery(t, ctx,
`insert into records (id, account_id, zone_id, name, type, ttl, content)
VALUES('record-1','account-1','zone-1','test.test-1.com','A',1800,'1.1.1.1')`)
@@ -33,30 +36,45 @@ func TestGetAppliedZoneCandidatesViaPgxConnection(t *testing.T) {
execQuery(t, ctx,
`insert into records (id, account_id, zone_id, name, type, ttl, content)
VALUES('record-4','account-1','zone-2','test2.test-2.com','CNAME',1800,'test3.test-2.com')`)
execQuery(t, ctx,
`insert into records (id, account_id, zone_id, name, type, ttl, content)
VALUES('record-5','account-1','zone-3','test.test-3.com','A',1800,'1.1.1.3')`)
zoneCandidates, err := conn(t, ctx).GetAppliedZoneCandidates(ctx, "account-1")
assert.NoError(t, err)
// Zone domains and record names are fully qualified, and the zone is served
// non-authoritatively — the account-side builder
// (types.buildAppliedZoneCandidates) states the same shape, and both feed the
// one client-facing map, so the two have to agree.
assert.Contains(t, zoneCandidates, networkmap.AppliedZoneCandidate{
DistributionGroups: []string{"group-one-resource-id"},
Zone: nmdata.CustomZone{
Domain: "test-1.com",
Domain: "test-1.com.",
SearchDomainDisabled: false,
NonAuthoritative: true,
Records: []nmdata.SimpleRecord{
{Name: "test.test-1.com", Type: int(dns.TypeA), Class: "IN", TTL: 1800, RData: "1.1.1.1"},
{Name: "test2.test-1.com", Type: int(dns.TypeA), Class: "IN", TTL: 1800, RData: "1.1.1.2"},
{Name: "test3.test-1.com", Type: int(dns.TypeCNAME), Class: "IN", TTL: 1800, RData: "test4.test-1.com."},
{Name: "test.test-1.com.", Type: int(dns.TypeA), Class: "IN", TTL: 1800, RData: "1.1.1.1"},
{Name: "test2.test-1.com.", Type: int(dns.TypeA), Class: "IN", TTL: 1800, RData: "1.1.1.2"},
{Name: "test3.test-1.com.", Type: int(dns.TypeCNAME), Class: "IN", TTL: 1800, RData: "test4.test-1.com."},
},
},
})
assert.Contains(t, zoneCandidates, networkmap.AppliedZoneCandidate{
DistributionGroups: []string{"group-two-resources-id"},
Zone: nmdata.CustomZone{
Domain: "test-2.com",
Domain: "test-2.com.",
SearchDomainDisabled: true,
NonAuthoritative: true,
Records: []nmdata.SimpleRecord{
{Name: "test2.test-2.com", Type: int(dns.TypeCNAME), Class: "IN", TTL: 1800, RData: "test3.test-2.com."},
{Name: "test2.test-2.com.", Type: int(dns.TypeCNAME), Class: "IN", TTL: 1800, RData: "test3.test-2.com."},
},
},
})
// A zone an admin switched off reaches no peer.
for _, candidate := range zoneCandidates {
assert.NotEqual(t, "test-3.com.", candidate.Zone.Domain, "disabled zone must not be a candidate")
assert.NotEqual(t, "test-3.com", candidate.Zone.Domain, "disabled zone must not be a candidate")
}
}

8
integration_tests/management/network_map_db/network_map_data.sql
View File

@@ -43,8 +43,12 @@ insert into peers (id,account_id,"key", ssh_key, dns_label, extra_dns_labels, us
'[1,2]','{"RosenpassEnabled":false,"RosenpassPermissive":false,"ServerSSHAllowed":true,"DisableClientRoutes":false,"DisableServerRoutes":false,"DisableDNS":false,"DisableFirewall":false,"BlockLANAccess":false,"BlockInbound":false,"DisableIPv6":false,"LazyConnectionEnabled":false}',1,
'DE','Berlin','"46.201.150.187"');
insert into zones (id, account_id, domain, enable_search_domain, distribution_groups)
VALUES('zone-331','account-33','test-331.com',true,'["33-group-one-resource-id"]');
insert into zones (id, account_id, domain, enabled, enable_search_domain, distribution_groups)
VALUES('zone-331','account-33','test-331.com',true,true,'["33-group-one-resource-id"]');
insert into zones (id, account_id, domain, enabled, enable_search_domain, distribution_groups)
VALUES('zone-332','account-33','disabled-331.com',false,true,'["33-group-one-resource-id"]');
insert into records (id, account_id, zone_id, name, type, ttl, content)
VALUES('record-333','account-33','zone-332','test.disabled-331.com','A',1800,'1.1.1.9');
insert into records (id, account_id, zone_id, name, type, ttl, content)
VALUES('record-331','account-33','zone-331','test.test-331.com','A',1800,'1.1.1.1');
insert into records (id, account_id, zone_id, name, type, ttl, content)

8
integration_tests/management/network_map_db/network_map_data_golden.json
View File

@@ -493,17 +493,17 @@
"33-group-one-resource-id"
],
"Zone": {
"Domain": "test-331.com",
"Domain": "test-331.com.",
"Records": [
{
"Name": "test.test-331.com",
"Name": "test.test-331.com.",
"Type": 1,
"Class": "IN",
"TTL": 1800,
"RData": "1.1.1.1"
},
{
"Name": "test2.test-331.com",
"Name": "test2.test-331.com.",
"Type": 1,
"Class": "IN",
"TTL": 1800,
@@ -511,7 +511,7 @@
}
],
"SearchDomainDisabled": false,
"NonAuthoritative": false
"NonAuthoritative": true
}
}
],

12
integration_tests/management/network_map_db/network_map_data_test.go
View File

@@ -12,13 +12,14 @@ import (
"strings"
"testing"
"github.com/golang/mock/gomock"
legacygomock "github.com/golang/mock/gomock"
networkmapdb "github.com/netbirdio/netbird/management/internals/network_map_db"
"github.com/netbirdio/netbird/management/server/integrations/integrated_validator"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/types"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
)
//go:embed network_map_data.sql
@@ -31,13 +32,14 @@ const EnvUpdateGoldenData = "NMAP_UPDATE_GOLDEN_DATA"
func TestGetNetworkMapData(t *testing.T) {
ctx := context.TODO()
ctrl := gomock.NewController(t)
extraSettingsManager := settings.NewMockManager(ctrl)
// The two mocks are generated by different mock frameworks, so each needs a
// controller of its own kind.
extraSettingsManager := settings.NewMockManager(gomock.NewController(t))
extraSettingsManager.EXPECT().GetExtraSettings(gomock.Any(), gomock.Any()).Return(&types.ExtraSettings{}, nil)
peerValidators := integrated_validator.NewMockIntegratedValidator(ctrl)
peerValidators.EXPECT().GetValidatedPeers(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
peerValidators := integrated_validator.NewMockIntegratedValidator(legacygomock.NewController(t))
peerValidators.EXPECT().GetValidatedPeers(legacygomock.Any(), legacygomock.Any(), legacygomock.Any(), legacygomock.Any(), legacygomock.Any()).Return(
map[string]struct{}{
"peer-id-1": {},
"peer-id-2": {},

21
management/internals/controllers/network_map/controller/controller.go
View File

@@ -500,15 +500,18 @@ func (c *Controller) getDNSDomainFromData(settings *nmdata.AccountSettingsInfo)
func IPv6AllowedPeersFromData(nmData *networkmap.NetworkMapData) map[string]struct{} {
result := make(map[string]struct{})
if nmData.AccountSettings != nil {
for _, groupID := range nmData.AccountSettings.IPv6EnabledGroups {
group := nmData.Groups[groupID]
if group == nil {
continue
}
for _, peerID := range group.Peers {
result[peerID] = struct{}{}
}
// An account with no IPv6-enabled group runs no overlay at all, so the
// embedded-proxy carve-out below has nothing to reach and stays shut.
if nmData.AccountSettings == nil || len(nmData.AccountSettings.IPv6EnabledGroups) == 0 {
return result
}
for _, groupID := range nmData.AccountSettings.IPv6EnabledGroups {
group := nmData.Groups[groupID]
if group == nil {
continue
}
for _, peerID := range group.Peers {
result[peerID] = struct{}{}
}
}
for id, p := range nmData.Peers {

47
management/internals/controllers/network_map/controller/ipv6_allowed_test.go Normal file
View File

@@ -0,0 +1,47 @@
package controller
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/netbirdio/netbird/shared/management/networkmap"
"github.com/netbirdio/netbird/shared/management/networkmap/nmdata"
)
// The account-side builder (types.Account.peerIPv6AllowedSet) is the reference:
// an account with no IPv6-enabled group runs no IPv6 overlay at all, embedded
// proxy peers included — see TestPeerIPv6AllowedEmbeddedProxy. Both builders
// gate the same AAAA records, so the store-backed one has to agree.
func TestIPv6AllowedPeersFromData(t *testing.T) {
data := func(enabledGroups []string) *networkmap.NetworkMapData {
return &networkmap.NetworkMapData{
AccountSettings: &nmdata.AccountSettingsInfo{IPv6EnabledGroups: enabledGroups},
Peers: map[string]*nmdata.Peer{
"peer1": {ID: "peer1"},
"lonely": {ID: "lonely"},
"proxy": {ID: "proxy", ProxyMeta: nmdata.ProxyMeta{Embedded: true, Cluster: "netbird.test"}},
},
Groups: map[string]*nmdata.Group{
"group-devs": {ID: "group-devs", Peers: []string{"peer1"}},
},
}
}
t.Run("embedded proxy allowed when any v6 group exists, without group membership", func(t *testing.T) {
allowed := IPv6AllowedPeersFromData(data([]string{"group-devs"}))
assert.Contains(t, allowed, "proxy", "embedded proxy participates in v6 overlay")
assert.Contains(t, allowed, "peer1", "regular peer in enabled group still allowed")
})
t.Run("embedded proxy denied when no v6 group enabled", func(t *testing.T) {
allowed := IPv6AllowedPeersFromData(data(nil))
assert.NotContains(t, allowed, "proxy", "v6 disabled account-wide denies embedded proxies too")
assert.Empty(t, allowed, "no peer participates in the v6 overlay")
})
t.Run("non-embedded peer outside any enabled group is not pulled in", func(t *testing.T) {
allowed := IPv6AllowedPeersFromData(data([]string{"group-devs"}))
assert.NotContains(t, allowed, "lonely", "embedded-proxy bypass must not leak to regular peers")
})
}

4
management/internals/network_map_db/pgsql/dns.go
View File

@@ -13,8 +13,8 @@ const (
select zones.id as id, domain, not enable_search_domain as search_domain_disabled, distribution_groups,
r.name as record_name, r.type as record_type, 'IN' record_class, r.ttl as record_ttl, r.content as record_rdata
from zones
left join records as r on r.zone_id = zones.id
where zones.account_id=$1
left join records as r on r.zone_id = zones.id
where zones.account_id=$1 and zones.enabled
`
)

8
management/internals/network_map_db/shared_types.go
View File

@@ -248,6 +248,12 @@ func ZonesToAppliedZoneCandidates(zones []Zone) ([]networkmap.AppliedZoneCandida
}
if z.Id != currentZoneId {
// The account-side builder (types.buildAppliedZoneCandidates) states
// the shape of an applied zone: names fully qualified, served
// non-authoritatively. Both builders feed the same client-facing map,
// so this one has to produce the same value.
zone.Domain = dns.Fqdn(zone.Domain)
zone.NonAuthoritative = true
zone.Records = []nmdata.SimpleRecord{}
toret = append(toret, AppliedZoneCandidateFromZone(zone, distributionGroups))
currentZoneId = z.Id
@@ -263,7 +269,7 @@ func ZonesToAppliedZoneCandidates(zones []Zone) ([]networkmap.AppliedZoneCandida
lastZone := &toret[len(toret)-1]
lastZone.Zone.Records = append(lastZone.Zone.Records, nmdata.SimpleRecord{
Name: z.RecordName.String,
Name: dns.Fqdn(z.RecordName.String),
Class: z.RecordClass.String,
TTL: int(z.RecordTTL.Int64),
RData: rdata,

4
management/internals/network_map_db/sqlite/dns.go
View File

@@ -12,8 +12,8 @@ const (
select zones.id as id, domain, not enable_search_domain as search_domain_disabled, distribution_groups,
r.name as record_name, r.type as record_type, 'IN' record_class, r.ttl as record_ttl, r.content as record_rdata
from zones
left join records as r on r.zone_id = zones.id
where zones.account_id=?
left join records as r on r.zone_id = zones.id
where zones.account_id=? and zones.enabled
`
)

32
management/server/types/legacynmap/equivalence_test.go
View File

@@ -42,15 +42,15 @@ import (
"strings"
"testing"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"google.golang.org/protobuf/encoding/prototext"
goproto "google.golang.org/protobuf/proto"
"gorm.io/driver/postgres"
"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)