Skip to content

[management] move network map logic into new design #4774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pascal-fischer merged 25 commits into from
Nov 13, 2025
Merged

[management] move network map logic into new design #4774

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
03b599a
move network map logic into new design
pascal-fischer Nov 10, 2025
1d25cf8
fix tests
pascal-fischer Nov 11, 2025
092fc24
go mod tidy
pascal-fischer Nov 11, 2025
da1ffe4
fix client tests
pascal-fischer Nov 11, 2025
febde70
fix linter
pascal-fischer Nov 11, 2025
ee45532
fix user delete test
pascal-fischer Nov 11, 2025
51ad8d7
drain channel before delete peer test
pascal-fischer Nov 11, 2025
e6aa119
remove leftover timer
pascal-fischer Nov 11, 2025
d35f904
fix peers handler
pascal-fischer Nov 11, 2025
71684e4
fix some of the rabbit remarks
pascal-fischer Nov 11, 2025
addb4df
delete testdata result
pascal-fischer Nov 11, 2025
bfc84c0
remove proxy code
pascal-fischer Nov 11, 2025
abb8782
fix imports
pascal-fischer Nov 11, 2025
04209c5
remove proxy container
pascal-fischer Nov 11, 2025
41744aa
Revert "remove proxy container"
pascal-fischer Nov 11, 2025
1908cfa
Revert "fix imports"
pascal-fischer Nov 11, 2025
705fd84
Revert "remove proxy code"
pascal-fischer Nov 11, 2025
f2bb181
bring back proxy map merge
pascal-fischer Nov 11, 2025
83b8216
fix test constructor
pascal-fischer Nov 11, 2025
0294c04
fix client side
pascal-fischer Nov 11, 2025
a31e293
remove channel drain for non experimental
pascal-fischer Nov 11, 2025
a0b7a8f
fix flaky test
pascal-fischer Nov 11, 2025
46f0e6c
do not double recalculate on partial update
pascal-fischer Nov 12, 2025
596820b
send update
pascal-fischer Nov 12, 2025
40808a0
expect network map on all peer updates
pascal-fischer Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions client/cmd/testutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import (
"google.golang.org/grpc"

"github.com/netbirdio/management-integrations/integrations"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel"
nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"

clientProto "github.com/netbirdio/netbird/client/proto"
client "github.com/netbirdio/netbird/client/server"
Expand Down Expand Up @@ -84,7 +87,6 @@ func startManagement(t *testing.T, config *config.Config, testFile string) (*grp
}
t.Cleanup(cleanUp)

peersUpdateManager := mgmt.NewPeersUpdateManager(nil)
eventStore := &activity.InMemoryEventStore{}
if err != nil {
return nil, nil
Expand All @@ -110,13 +112,18 @@ func startManagement(t *testing.T, config *config.Config, testFile string) (*grp
Return(&types.Settings{}, nil).
AnyTimes()

accountManager, err := mgmt.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "netbird.selfhosted", eventStore, nil, false, iv, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock, false)
ctx := context.Background()
updateManager := update_channel.NewPeersUpdateManager(metrics)
requestBuffer := mgmt.NewAccountRequestBuffer(ctx, store)
networkMapController := controller.NewController(ctx, store, metrics, updateManager, requestBuffer, mgmt.MockIntegratedValidator{}, settingsMockManager, "netbird.cloud", port_forwarding.NewControllerMock())

accountManager, err := mgmt.BuildManager(context.Background(), store, networkMapController, nil, "", eventStore, nil, false, iv, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock, false)
if err != nil {
t.Fatal(err)
}

secretsManager := mgmt.NewTimeBasedAuthSecretsManager(peersUpdateManager, config.TURNConfig, config.Relay, settingsMockManager, groupsManager)
mgmtServer, err := mgmt.NewServer(context.Background(), config, accountManager, settingsMockManager, peersUpdateManager, secretsManager, nil, &manager.EphemeralManager{}, nil, &mgmt.MockIntegratedValidator{})
secretsManager := nbgrpc.NewTimeBasedAuthSecretsManager(updateManager, config.TURNConfig, config.Relay, settingsMockManager, groupsManager)
mgmtServer, err := nbgrpc.NewServer(config, accountManager, settingsMockManager, updateManager, secretsManager, nil, &manager.EphemeralManager{}, nil, &mgmt.MockIntegratedValidator{}, networkMapController)
if err != nil {
t.Fatal(err)
}
Expand Down
13 changes: 9 additions & 4 deletions client/internal/engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import (
"google.golang.org/grpc/keepalive"

"github.com/netbirdio/management-integrations/integrations"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel"
nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"

"github.com/netbirdio/netbird/management/internals/server/config"
"github.com/netbirdio/netbird/management/server/groups"
Expand Down Expand Up @@ -1556,7 +1559,6 @@ func startManagement(t *testing.T, dataDir, testFile string) (*grpc.Server, stri
}
t.Cleanup(cleanUp)

peersUpdateManager := server.NewPeersUpdateManager(nil)
eventStore := &activity.InMemoryEventStore{}
if err != nil {
return nil, "", err
Expand Down Expand Up @@ -1584,13 +1586,16 @@ func startManagement(t *testing.T, dataDir, testFile string) (*grpc.Server, stri

groupsManager := groups.NewManagerMock()

accountManager, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "netbird.selfhosted", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManager, false)
updateManager := update_channel.NewPeersUpdateManager(metrics)
requestBuffer := server.NewAccountRequestBuffer(context.Background(), store)
networkMapController := controller.NewController(context.Background(), store, metrics, updateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock())
accountManager, err := server.BuildManager(context.Background(), store, networkMapController, nil, "", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManager, false)
if err != nil {
return nil, "", err
}

secretsManager := server.NewTimeBasedAuthSecretsManager(peersUpdateManager, config.TURNConfig, config.Relay, settingsMockManager, groupsManager)
mgmtServer, err := server.NewServer(context.Background(), config, accountManager, settingsMockManager, peersUpdateManager, secretsManager, nil, &manager.EphemeralManager{}, nil, &server.MockIntegratedValidator{})
secretsManager := nbgrpc.NewTimeBasedAuthSecretsManager(updateManager, config.TURNConfig, config.Relay, settingsMockManager, groupsManager)
mgmtServer, err := nbgrpc.NewServer(config, accountManager, settingsMockManager, updateManager, secretsManager, nil, &manager.EphemeralManager{}, nil, &server.MockIntegratedValidator{}, networkMapController)
if err != nil {
return nil, "", err
}
Expand Down
13 changes: 9 additions & 4 deletions client/server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import (
"go.opentelemetry.io/otel"

"github.com/netbirdio/management-integrations/integrations"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel"
nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"

"github.com/netbirdio/netbird/management/internals/server/config"
"github.com/netbirdio/netbird/management/server/groups"
Expand Down Expand Up @@ -290,7 +293,6 @@ func startManagement(t *testing.T, signalAddr string, counter *int) (*grpc.Serve
}
t.Cleanup(cleanUp)

peersUpdateManager := server.NewPeersUpdateManager(nil)
eventStore := &activity.InMemoryEventStore{}
if err != nil {
return nil, "", err
Expand All @@ -311,13 +313,16 @@ func startManagement(t *testing.T, signalAddr string, counter *int) (*grpc.Serve
settingsMockManager := settings.NewMockManager(ctrl)
groupsManager := groups.NewManagerMock()

accountManager, err := server.BuildManager(context.Background(), store, peersUpdateManager, nil, "", "netbird.selfhosted", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock, false)
requestBuffer := server.NewAccountRequestBuffer(context.Background(), store)
peersUpdateManager := update_channel.NewPeersUpdateManager(metrics)
networkMapController := controller.NewController(context.Background(), store, metrics, peersUpdateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock())
accountManager, err := server.BuildManager(context.Background(), store, networkMapController, nil, "", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock, false)
if err != nil {
return nil, "", err
}

secretsManager := server.NewTimeBasedAuthSecretsManager(peersUpdateManager, config.TURNConfig, config.Relay, settingsMockManager, groupsManager)
mgmtServer, err := server.NewServer(context.Background(), config, accountManager, settingsMockManager, peersUpdateManager, secretsManager, nil, &manager.EphemeralManager{}, nil, &server.MockIntegratedValidator{})
secretsManager := nbgrpc.NewTimeBasedAuthSecretsManager(peersUpdateManager, config.TURNConfig, config.Relay, settingsMockManager, groupsManager)
mgmtServer, err := nbgrpc.NewServer(config, accountManager, settingsMockManager, peersUpdateManager, secretsManager, nil, &manager.EphemeralManager{}, nil, &server.MockIntegratedValidator{}, networkMapController)
if err != nil {
return nil, "", err
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ require (
go.opentelemetry.io/otel/exporters/prometheus v0.48.0
go.opentelemetry.io/otel/metric v1.35.0
go.opentelemetry.io/otel/sdk/metric v1.35.0
go.uber.org/mock v0.5.0

@coderabbitai coderabbitai Bot Nov 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify go.uber.org/mock v0.5.0 is current and appropriate for the refactoring.

Promoting go.uber.org/mock to a direct dependency aligns with the PR's new controller-based mocking infrastructure. However, ensure this version is maintained, compatible with your test suite, and free from known issues.

🌐 Web query:

Is go.uber.org/mock v0.5.0 the current stable version? Are there any known security or compatibility issues?

💡 Result:

Short answer: No — v0.5.0 is not the most recent release. There have been patch releases (e.g. v0.5.2 on 2025‑04‑28) and further activity; the project is actively maintained and supports the two most recent Go releases. [1][2][5]

Security/compatibility summary:

  • Known security advisories/CVEs: none reported for go.uber.org/mock in public vulnerability databases (Snyk shows 0 direct vulnerabilities). [3]
  • Compatibility/bug notes: there are active issues and recent fixes around Go generics/type-alias handling, mockgen generation edge-cases, and other functional bugs (open issues and recent patch releases address some of these). If you rely on code-generation for newer Go versions (generics/aliases), prefer the latest patch release (>= v0.5.2) and test mockgen output. [4][2][5]

Recommendations:

  • Use the latest tagged release (not v0.5.0) — run go list -m -versions go.uber.org/mock or check the repo releases and then update your go.mod to that version.
  • Upgrade to the latest patch (v0.5.2 or newer) if you hit issues with type aliases/generics; run your test suite after upgrading. [2][4][5]

Sources:
[1] uber-go/mock (GitHub repo).
[2] Release notes / v0.5.2 (published 2025‑04‑28).
[3] Snyk package advisory for github.com/uber-go/mock (no direct vulnerabilities).
[4] uber-go/mock GitHub issues (open compatibility/bug reports).
[5] pkg.go.dev entry for go.uber.org/mock (module details, supported Go versions).

Update go.uber.org/mock to v0.5.2 or the latest stable version.

v0.5.0 is not the current stable version; newer releases (e.g., v0.5.2 released 2025‑04‑28) are available. Recent patch releases address functional bugs and compatibility issues with Go generics and type aliases. Given this PR's major refactoring with new mocking infrastructure, update to the latest patch release to ensure compatibility and avoid known issues.

🤖 Prompt for AI Agents 
In go.mod around line 102, the dependency go.uber.org/mock is pinned to v0.5.0
which is outdated; update the module line to v0.5.2 (or the latest stable patch)
and then run `go get go.uber.org/mock@v0.5.2` (or `@latest`) followed by `go mod
tidy` to refresh go.sum; finally run `go test ./...` and fix any minor
compatibility issues that surface from the mock package upgrade.

go.uber.org/zap v1.27.0
goauthentik.io/api/v3 v3.2023051.3
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
Expand Down Expand Up @@ -242,7 +243,6 @@ require (
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.51.0 // indirect
go.opentelemetry.io/otel/sdk v1.35.0 // indirect
go.opentelemetry.io/otel/trace v1.35.0 // indirect
go.uber.org/mock v0.5.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/image v0.18.0 // indirect
golang.org/x/text v0.27.0 // indirect
Expand Down
31 changes: 31 additions & 0 deletions management/internals/controllers/network_map/controller/cache/dns_config_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package cache

import (
"sync"

"github.com/netbirdio/netbird/shared/management/proto"
)

// DNSConfigCache is a thread-safe cache for DNS configuration components
type DNSConfigCache struct {
NameServerGroups sync.Map
}

// GetNameServerGroup retrieves a cached name server group
func (c *DNSConfigCache) GetNameServerGroup(key string) (*proto.NameServerGroup, bool) {
if c == nil {
return nil, false
}
if value, ok := c.NameServerGroups.Load(key); ok {
return value.(*proto.NameServerGroup), true
}
return nil, false
}
Comment on lines +15 to +23

@coderabbitai coderabbitai Bot Nov 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unchecked type assertion could panic.

Line 20 performs an unchecked type assertion value.(*proto.NameServerGroup) which will panic if an incorrect type is stored in the cache. Consider using the two-value form or adding a type check.

Apply this diff to add a type check:

 func (c *DNSConfigCache) GetNameServerGroup(key string) (*proto.NameServerGroup, bool) {
 	if c == nil {
 		return nil, false
 	}
 	if value, ok := c.NameServerGroups.Load(key); ok {
-		return value.(*proto.NameServerGroup), true
+		if nsg, ok := value.(*proto.NameServerGroup); ok {
+			return nsg, true
+		}
 	}
 	return nil, false
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (c *DNSConfigCache) GetNameServerGroup(key string) (*proto.NameServerGroup, bool) {
if c == nil {
return nil, false
}
if value, ok := c.NameServerGroups.Load(key); ok {
return value.(*proto.NameServerGroup), true
}
return nil, false
}
func (c *DNSConfigCache) GetNameServerGroup(key string) (*proto.NameServerGroup, bool) {
if c == nil {
return nil, false
}
if value, ok := c.NameServerGroups.Load(key); ok {
if nsg, ok := value.(*proto.NameServerGroup); ok {
return nsg, true
}
}
return nil, false
}
🤖 Prompt for AI Agents 
In
management/internals/controllers/network_map/controller/cache/dns_config_cache.go
around lines 15 to 23, the code does an unchecked type assertion
value.(*proto.NameServerGroup) which can panic if the stored value is not the
expected type; change it to use the two-value type assertion (v, okType :=
value.(*proto.NameServerGroup)) and only return v, true when okType is true,
otherwise return nil, false (optionally log or handle the type mismatch) so the
function never panics on bad cache entries.


// SetNameServerGroup stores a name server group in the cache
func (c *DNSConfigCache) SetNameServerGroup(key string, value *proto.NameServerGroup) {
if c == nil {
return
}
c.NameServerGroups.Store(key, value)
}
Loading
Loading