diff --git a/client/internal/engine.go b/client/internal/engine.go index e2ff901ed..21c6a4fe8 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -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) } diff --git a/client/internal/engine_bundle_test.go b/client/internal/engine_bundle_test.go index 135453613..37a989d3f 100644 --- a/client/internal/engine_bundle_test.go +++ b/client/internal/engine_bundle_test.go @@ -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()) } diff --git a/management/internals/shared/grpc/conversion.go b/management/internals/shared/grpc/conversion.go index 7067fa787..9fb4fccb5 100644 --- a/management/internals/shared/grpc/conversion.go +++ b/management/internals/shared/grpc/conversion.go @@ -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 }