[management,client] Keep the published upload destination across partial config updates

pushNewTURNTokens and pushNewRelayTokens send a SyncResponse whose NetbirdConfig
carries only Turns and Relay. handleDebugUploadUpdate read the absent Debug as
an empty destination and stored it, so every TURN credential refresh — every few
minutes — silently dropped the operator's choice and the next bundle went to the
service NetBird runs. That is the exact failure this branch exists to prevent.

A nil DebugConfig now carries no information and is left alone. To keep an
operator's clear reaching the peer, toNetbirdConfig always emits Debug on the
full config it builds, empty URL included, so the peer can tell "cleared" from
"not mentioned".

Reported by cubic on #7514.
This commit is contained in:
riccardom
2026-09-14 09:16:10 +02:00
parent a8ba9d0149
commit 803b0d6db0
3 changed files with 25 additions and 8 deletions
+12 -3
View File
@@ -1239,10 +1239,19 @@ func (e *Engine) handleMetricsUpdate(config *mgmProto.MetricsConfig) {
}
// handleDebugUploadUpdate records the debug-bundle destination the management
// server published. A nil DebugConfig clears it: a management server that stops
// publishing a destination must take it away from the peer, not leave the peer
// uploading to a host the operator has since removed.
// server published.
//
// 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 {
return
}
url := config.GetUploadUrl()
e.debugUploadURL.Store(&url)
}
+9 -2
View File
@@ -50,8 +50,15 @@ func TestEngineDebugUploadURL(t *testing.T) {
e.handleDebugUploadUpdate(&mgmProto.DebugConfig{UploadUrl: "https://upload.example.com/upload-url"})
assert.Equal(t, "https://upload.example.com/upload-url", e.DebugUploadURL())
// An operator that removes the destination must take it away from the peer,
// not leave it uploading to a host that no longer exists.
// 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.
e.handleDebugUploadUpdate(nil)
assert.Equal(t, "https://upload.example.com/upload-url", e.DebugUploadURL(),
"a partial config 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{})
assert.Empty(t, e.DebugUploadURL())
}
@@ -123,9 +123,10 @@ func toNetbirdConfig(config *nbconfig.Config, turnCredentials *Token, relayToken
if settings != nil && settings.DebugBundleUploadURL != "" {
debugUploadURL = settings.DebugBundleUploadURL
}
if debugUploadURL != "" {
nbConfig.Debug = &proto.DebugConfig{UploadUrl: debugUploadURL}
}
// Always sent, empty included: this is a full config, so the peer can tell an
// operator clearing the destination from the partial updates that carry only
// TURN or relay credentials and say nothing about it.
nbConfig.Debug = &proto.DebugConfig{UploadUrl: debugUploadURL}
return nbConfig
}