mirror of
https://github.com/goauthentik/authentik
synced 2026-04-25 17:15:26 +02:00
internal/outpost: serialize websocket writes to prevent panic (#21728)
The outpost API controller shares a single *websocket.Conn across multiple goroutines: the event-handler loop, the 10s health ticker (SendEventHello), the shutdown path (WriteMessage close), initEvent writing the hello frame on (re)connect, and RAC session handlers that also invoke SendEventHello. gorilla/websocket explicitly documents that concurrent WriteMessage/WriteJSON calls are unsafe and will panic with "concurrent write to websocket connection", which takes the outpost (and embedded-outpost authentik-server) pod down. Fix by adding a sync.Mutex on APIController guarding every write path on eventConn (initEvent hello, Shutdown close message, SendEventHello). Reads (ReadJSON in startEventHandler) are left unsynchronized as gorilla permits a single concurrent reader alongside a writer. Minimal, localized change: no API changes, no behavior changes, writes are already infrequent so lock contention is negligible. Refs #11090 Co-authored-by: curiosity <curiosity@somni.dev>
This commit is contained in:
@@ -11,6 +11,7 @@ import (
|
|||||||
"os"
|
"os"
|
||||||
"os/signal"
|
"os/signal"
|
||||||
"runtime"
|
"runtime"
|
||||||
|
"sync"
|
||||||
"syscall"
|
"syscall"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -45,6 +46,7 @@ type APIController struct {
|
|||||||
reloadOffset time.Duration
|
reloadOffset time.Duration
|
||||||
|
|
||||||
eventConn *websocket.Conn
|
eventConn *websocket.Conn
|
||||||
|
eventConnMu sync.Mutex
|
||||||
lastWsReconnect time.Time
|
lastWsReconnect time.Time
|
||||||
wsIsReconnecting bool
|
wsIsReconnecting bool
|
||||||
eventHandlers []EventHandler
|
eventHandlers []EventHandler
|
||||||
|
|||||||
@@ -77,7 +77,12 @@ func (ac *APIController) initEvent(outpostUUID string, attempt int) error {
|
|||||||
Instruction: EventKindHello,
|
Instruction: EventKindHello,
|
||||||
Args: ac.getEventPingArgs(),
|
Args: ac.getEventPingArgs(),
|
||||||
}
|
}
|
||||||
|
// Serialize this write against concurrent SendEventHello callers (health
|
||||||
|
// ticker, RAC handlers) sharing the same *websocket.Conn. Gorilla's Conn
|
||||||
|
// does not permit concurrent writes.
|
||||||
|
ac.eventConnMu.Lock()
|
||||||
err = ws.WriteJSON(msg)
|
err = ws.WriteJSON(msg)
|
||||||
|
ac.eventConnMu.Unlock()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ac.logger.WithField("logger", "authentik.outpost.events").WithError(err).Warning("Failed to hello to authentik")
|
ac.logger.WithField("logger", "authentik.outpost.events").WithError(err).Warning("Failed to hello to authentik")
|
||||||
return err
|
return err
|
||||||
@@ -91,7 +96,9 @@ func (ac *APIController) initEvent(outpostUUID string, attempt int) error {
|
|||||||
func (ac *APIController) Shutdown() {
|
func (ac *APIController) Shutdown() {
|
||||||
// Cleanly close the connection by sending a close message and then
|
// Cleanly close the connection by sending a close message and then
|
||||||
// waiting (with timeout) for the server to close the connection.
|
// waiting (with timeout) for the server to close the connection.
|
||||||
|
ac.eventConnMu.Lock()
|
||||||
err := ac.eventConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""))
|
err := ac.eventConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""))
|
||||||
|
ac.eventConnMu.Unlock()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ac.logger.WithError(err).Warning("failed to write close message")
|
ac.logger.WithError(err).Warning("failed to write close message")
|
||||||
return
|
return
|
||||||
@@ -252,6 +259,10 @@ func (a *APIController) SendEventHello(args map[string]any) error {
|
|||||||
Instruction: EventKindHello,
|
Instruction: EventKindHello,
|
||||||
Args: allArgs,
|
Args: allArgs,
|
||||||
}
|
}
|
||||||
|
// Gorilla *websocket.Conn does not permit concurrent writes. This method
|
||||||
|
// is invoked from the health ticker and from RAC session handlers.
|
||||||
|
a.eventConnMu.Lock()
|
||||||
err := a.eventConn.WriteJSON(aliveMsg)
|
err := a.eventConn.WriteJSON(aliveMsg)
|
||||||
|
a.eventConnMu.Unlock()
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user