fix(sse): cancel active streams on user logout#6073
Conversation
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
Bot
added
the
size/L
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR ensures server-side SSE streams are actively terminated when a user logs out, preventing continued event delivery after JWT revocation and aligning SSE behavior with the existing exec-session cancellation lifecycle.
Changes:
- Added a per-user SSE session registry in
sse.goand exposedCancelUserSSEStreams(userID)to cancel all active streams for a user. - Registered/unregistered SSE stream cancel funcs in
streamClustersso logout can promptly tear down in-flight streams. - Updated
AuthHandler.Logoutto cancel SSE streams after revoking the token, and added unit tests covering the registry lifecycle and cancellation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/api/handlers/sse.go | Adds per-user SSE stream cancellation registry and registers stream cancel funcs from streamClusters. |
| pkg/api/handlers/sse_test.go | Adds unit tests validating cancellation, isolation across users, and unregister behavior. |
| pkg/api/handlers/auth.go | Cancels active SSE streams for the logging-out user via CancelUserSSEStreams. |
| // Create a cancellable context with the overall deadline so that all | ||
| // spawned goroutines are cancelled when the client disconnects or the | ||
| // deadline expires. Previously context.Background() was used, which | ||
| // caused goroutine leaks on client disconnect (see #3291). | ||
| streamCtx, streamCancel := context.WithTimeout(context.Background(), sseOverallDeadline) | ||
| defer streamCancel() |
clubanderson
force-pushed
the
fix/6029-sse-cancellation
branch
from
ccc5dda to
6c93085
Compare
Fixes #6029. SSE streaming endpoints (streamClusters in sse.go) ran inside SetBodyStreamWriter callbacks that blocked for up to sseOverallDeadline (~30s), so when a user logged out any in-flight SSE streams continued to emit cluster_data events until the deadline fired or the client disconnected. Mirrors the exec session cancellation pattern added in #6031 for /ws/exec sessions (#6024): - Add a per-user SSE session registry in sse.go with registerSSESession / unregisterSSESession / CancelUserSSEStreams, guarded by a plain sync.Mutex. - In streamClusters, capture the authenticated user ID before the deferred SetBodyStreamWriter callback runs (the fiber.Ctx may be reused by then) and register the stream context's cancel func in the registry, with a deferred unregister for normal stream end. - In auth.Logout, after CancelUserExecSessions, call CancelUserSSEStreams(claims.UserID) so logout tears down any live SSE streams the user had open. - Add sse_test.go with the four lifecycle tests mirroring exec_test.go: CancelsRegisteredContexts, OtherUserUnaffected, UnregisterSSESession_RemovesEntry, and NoSessions (no panic on empty map). streamDemoSSE is left unchanged — demo streams are instant, they do not block, so they need no cancellation path. Signed-off-by: Andrew Anderson <[email protected]>
clubanderson
force-pushed
the
fix/6029-sse-cancellation
branch
from
6c93085 to
93d7919
Compare
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
❌ Post-Merge Verification: failedCommit: |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
Fixes #6029. Recreated from #6033 after base branch auto-deletion closed the original.
Problem
SSE streaming endpoints (
streamClustersinpkg/api/handlers/sse.go) run insideSetBodyStreamWritercallbacks that block until either the client disconnects orsseOverallDeadline(~30s) fires. When a user logged out, any in-flight SSE streams kept emittingcluster_dataevents until that deadline — the token was revoked but the already-established stream had no active cancellation path.Fix
Mirrors the exec-session cancellation pattern added in #6031 for
/ws/execsessions (#6024).sse.go:sseSessions map[uuid.UUID]map[int64]context.CancelFuncguarded by a plainsync.MutexregisterSSESession/unregisterSSESession/CancelUserSSEStreamsstreamClusterscaptures the authenticated user ID before entering the deferredSetBodyStreamWritercallback, registers the stream context'scancelfunc, and unregisters on normal stream end.auth.LogoutcallsCancelUserSSEStreams(claims.UserID)immediately after the existingCancelUserExecSessionscall.streamDemoSSEintentionally unchanged — demo streams are instant, no cancellation path needed.Tests
New
pkg/api/handlers/sse_test.go:TestCancelUserSSEStreams_CancelsRegisteredContextsTestCancelUserSSEStreams_OtherUserUnaffectedTestUnregisterSSESession_RemovesEntryTestCancelUserSSEStreams_NoSessionsVerification
go build ./...,go vet ./...,go test ./pkg/api/handlers/...,cd web && npm run build— all clean.