mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-13 00:49:07 +02:00
[management,client] Carry the debug upload destination on PeerConfig
A destination set from the dashboard never reached a connected peer. The peer fan-out that follows a settings change builds its sync with a nil server config, and toNetbirdConfig returns nil for that, so no NetbirdConfig went out at all and the peer kept whatever it had. The new URL only arrived when the peer reconnected, and until then its bundles kept going to NetBird's upload service — which is the disclosure this PR exists to close. PeerConfig is the message the fan-out already fills from the account settings, alongside the other knobs an operator flips at runtime: lazy connection, MTU, routing-peer DNS resolution, auto-update. The destination belongs with them. Sending a partial NetbirdConfig from the fan-out instead would be worse: clients treat a non-nil one as authoritative for relay and would drop their relay URLs. Start now reads the destination off the PeerConfig it was given in the previous commit, for the same reason it used to read it off NetbirdConfig: without it, a bundle collected between login and the first sync sees no destination, and the first seconds after a start are exactly when someone collects a bundle about a start that went wrong. DebugConfig is removed rather than deprecated. It was added earlier in this branch and has never been in a release, so no deployed client reads it.
This commit is contained in:
+18
-18
@@ -622,12 +622,13 @@ func (e *Engine) Start(netbirdConfig *mgmProto.NetbirdConfig, peerConfig *mgmPro
|
||||
log.Warnf("failed to populate DNS cache: %v", err)
|
||||
}
|
||||
|
||||
// The login response carries the same NetbirdConfig a sync does, but Start
|
||||
// does not run it through updateNetbirdConfig. Without this, a bundle
|
||||
// requested between login and the first sync sees no published destination
|
||||
// and falls back to the service NetBird runs, even where the deployment
|
||||
// configured its own.
|
||||
e.handleDebugUploadUpdate(netbirdConfig.GetDebug())
|
||||
// The login response carries the same PeerConfig a sync does, but Start does
|
||||
// not run it through updateNetworkMap. Without this, a bundle requested
|
||||
// between login and the first sync sees no published destination and falls
|
||||
// back to the service NetBird runs, even where the deployment configured its
|
||||
// own — and the first seconds after a start are exactly when someone collects
|
||||
// a bundle about a start that went wrong.
|
||||
e.handleDebugUploadUpdate(peerConfig)
|
||||
|
||||
e.routeManager = routemanager.NewManager(routemanager.ManagerConfig{
|
||||
Context: e.ctx,
|
||||
@@ -1171,8 +1172,6 @@ func (e *Engine) updateNetbirdConfig(wCfg *mgmProto.NetbirdConfig) error {
|
||||
|
||||
e.handleMetricsUpdate(wCfg.GetMetrics())
|
||||
|
||||
e.handleDebugUploadUpdate(wCfg.GetDebug())
|
||||
|
||||
if err := e.PopulateNetbirdConfig(wCfg, nil); err != nil {
|
||||
log.Warnf("Failed to update DNS server config: %v", err)
|
||||
}
|
||||
@@ -1251,20 +1250,19 @@ func (e *Engine) handleMetricsUpdate(config *mgmProto.MetricsConfig) {
|
||||
}
|
||||
|
||||
// handleDebugUploadUpdate records the debug-bundle destination the management
|
||||
// server published.
|
||||
// server published for this peer.
|
||||
//
|
||||
// A nil DebugConfig carries no information and is left alone: the partial
|
||||
// updates that refresh TURN and relay credentials ship a NetbirdConfig holding
|
||||
// only those fields, and treating their absent Debug as "no destination" would
|
||||
// silently drop the operator's choice on every credential refresh. An operator
|
||||
// clearing the destination is an empty UploadUrl on a full config, which does
|
||||
// reach the store below.
|
||||
func (e *Engine) handleDebugUploadUpdate(config *mgmProto.DebugConfig) {
|
||||
if config == nil {
|
||||
// A nil PeerConfig carries no information and is left alone: a partial update
|
||||
// ships a SyncResponse without one, and reading that absence as "no
|
||||
// destination" would silently drop the operator's choice. An operator clearing
|
||||
// the destination sends a PeerConfig with an empty URL, which does reach the
|
||||
// store below.
|
||||
func (e *Engine) handleDebugUploadUpdate(peerConfig *mgmProto.PeerConfig) {
|
||||
if peerConfig == nil {
|
||||
return
|
||||
}
|
||||
|
||||
url := config.GetUploadUrl()
|
||||
url := peerConfig.GetDebugBundleUploadUrl()
|
||||
e.debugUploadURL.Store(&url)
|
||||
}
|
||||
|
||||
@@ -1635,6 +1633,8 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap) error {
|
||||
if err := e.connMgr.UpdatedRemoteFeatureFlag(e.ctx, peerConfig.GetLazyConnectionEnabled()); err != nil {
|
||||
log.Errorf("failed to update lazy connection feature flag: %v", err)
|
||||
}
|
||||
|
||||
e.handleDebugUploadUpdate(peerConfig)
|
||||
}
|
||||
|
||||
if e.firewall != nil {
|
||||
|
||||
@@ -47,18 +47,17 @@ func TestEngineDebugUploadURL(t *testing.T) {
|
||||
e.handleDebugUploadUpdate(nil)
|
||||
assert.Empty(t, e.DebugUploadURL(), "a management server predating the field publishes none")
|
||||
|
||||
e.handleDebugUploadUpdate(&mgmProto.DebugConfig{UploadUrl: "https://upload.example.com/upload-url"})
|
||||
e.handleDebugUploadUpdate(&mgmProto.PeerConfig{DebugBundleUploadUrl: "https://upload.example.com/upload-url"})
|
||||
assert.Equal(t, "https://upload.example.com/upload-url", e.DebugUploadURL())
|
||||
|
||||
// The partial updates that refresh TURN and relay credentials carry a
|
||||
// NetbirdConfig with no Debug at all. Treating that as "no destination"
|
||||
// would drop the operator's choice on every credential refresh.
|
||||
// A partial update ships a SyncResponse with no PeerConfig at all. Treating
|
||||
// that as "no destination" would drop the operator's choice.
|
||||
e.handleDebugUploadUpdate(nil)
|
||||
assert.Equal(t, "https://upload.example.com/upload-url", e.DebugUploadURL(),
|
||||
"a partial config update must not clear the published destination")
|
||||
"a partial update must not clear the published destination")
|
||||
|
||||
// An operator that removes the destination sends an empty UploadUrl on a
|
||||
// full config, and that does reach the peer.
|
||||
e.handleDebugUploadUpdate(&mgmProto.DebugConfig{})
|
||||
// An operator that removes the destination sends a PeerConfig with an empty
|
||||
// URL, and that does reach the peer.
|
||||
e.handleDebugUploadUpdate(&mgmProto.PeerConfig{})
|
||||
assert.Empty(t, e.DebugUploadURL())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user