mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-14 01:19:09 +02:00
fix(proxy): background ctx for already-started AddPeer notification
The earlier ctx fix covered the async runClientStartup path but missed the synchronous branch: when a service is added to an already-started client, AddPeer called NotifyStatus with the caller's request-scoped ctx. A cancelled request/stream could drop the connected notification to management. Use context.Background() here too, matching notifyClientReady. Extends TestNetBird_AddPeer_ExistingStartedClient_NotifiesStatus to pass a pre-cancelled caller ctx and assert the notification still ran on a non-cancelled context.
This commit is contained in:
@@ -213,7 +213,11 @@ func (n *NetBird) AddPeer(ctx context.Context, accountID types.AccountID, key Se
|
|||||||
}).Debug("registered service with existing client")
|
}).Debug("registered service with existing client")
|
||||||
|
|
||||||
if started && n.statusNotifier != nil {
|
if started && n.statusNotifier != nil {
|
||||||
if err := n.statusNotifier.NotifyStatus(ctx, accountID, serviceID, true); err != nil {
|
// Use a background context, not the caller's: the management
|
||||||
|
// connection notification must land even if the request /
|
||||||
|
// stream that triggered this registration is cancelled.
|
||||||
|
// Mirrors the async runClientStartup path.
|
||||||
|
if err := n.statusNotifier.NotifyStatus(context.Background(), accountID, serviceID, true); err != nil {
|
||||||
n.logger.WithFields(log.Fields{
|
n.logger.WithFields(log.Fields{
|
||||||
"account_id": accountID,
|
"account_id": accountID,
|
||||||
"service_key": key,
|
"service_key": key,
|
||||||
|
|||||||
@@ -299,8 +299,12 @@ func TestNetBird_AddPeer_ExistingStartedClient_NotifiesStatus(t *testing.T) {
|
|||||||
nb.clients[accountID].started = true
|
nb.clients[accountID].started = true
|
||||||
nb.clientsMux.Unlock()
|
nb.clientsMux.Unlock()
|
||||||
|
|
||||||
// Add second service — should notify immediately since client is already started.
|
// Add second service with an already-cancelled caller context —
|
||||||
err = nb.AddPeer(context.Background(), accountID, "domain2.test", "key-1", types.ServiceID("svc-2"))
|
// should notify immediately (client is started) AND the notification
|
||||||
|
// must not inherit the cancelled ctx.
|
||||||
|
cancelledCtx, cancel := context.WithCancel(context.Background())
|
||||||
|
cancel()
|
||||||
|
err = nb.AddPeer(cancelledCtx, accountID, "domain2.test", "key-1", types.ServiceID("svc-2"))
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
calls := notifier.calls()
|
calls := notifier.calls()
|
||||||
@@ -308,6 +312,9 @@ func TestNetBird_AddPeer_ExistingStartedClient_NotifiesStatus(t *testing.T) {
|
|||||||
assert.Equal(t, accountID, calls[0].accountID)
|
assert.Equal(t, accountID, calls[0].accountID)
|
||||||
assert.Equal(t, types.ServiceID("svc-2"), calls[0].serviceID)
|
assert.Equal(t, types.ServiceID("svc-2"), calls[0].serviceID)
|
||||||
assert.True(t, calls[0].connected)
|
assert.True(t, calls[0].connected)
|
||||||
|
require.NotNil(t, calls[0].ctx, "NotifyStatus must receive a context")
|
||||||
|
require.NoError(t, calls[0].ctx.Err(),
|
||||||
|
"already-started NotifyStatus must use a background ctx, not the cancelled caller ctx")
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestNetBird_IdentityForIP_UnknownAccountReturnsFalse confirms that the
|
// TestNetBird_IdentityForIP_UnknownAccountReturnsFalse confirms that the
|
||||||
|
|||||||
Reference in New Issue
Block a user