From 0586d5d40d0f3804a94a0a074b539fa81e547118 Mon Sep 17 00:00:00 2001
From: Tom Meadows <tom@tmlabs.co.uk>
Date: Wed, 8 Oct 2025 15:15:42 +0100
Subject: [PATCH] k8s-operator/sessionrecording: gives the connection to the
 recorder from the hijacker a dedicated context (#17403)

The hijacker on k8s-proxy's reverse proxy is used to stream recordings
to tsrecorder as they pass through the proxy to the kubernetes api
server. The connection to the recorder was using the client's
(e.g., kubectl) context, rather than a dedicated one. This was causing
the recording stream to get cut off in scenarios where the client
cancelled the context before streaming could be completed.

By using a dedicated context, we can continue streaming even if the
client cancels the context (for example if the client request
completes).

Fixes #17404

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
---
 k8s-operator/sessionrecording/hijacker.go      | 13 +++++++++++--
 k8s-operator/sessionrecording/hijacker_test.go |  2 +-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/k8s-operator/sessionrecording/hijacker.go b/k8s-operator/sessionrecording/hijacker.go
index ebd77641b..2d6c94710 100644
--- a/k8s-operator/sessionrecording/hijacker.go
+++ b/k8s-operator/sessionrecording/hijacker.go
@@ -122,7 +122,7 @@ func (h *Hijacker) Hijack() (net.Conn, *bufio.ReadWriter, error) {
 		return nil, nil, fmt.Errorf("error hijacking connection: %w", err)
 	}
 
-	conn, err := h.setUpRecording(h.req.Context(), reqConn)
+	conn, err := h.setUpRecording(reqConn)
 	if err != nil {
 		return nil, nil, fmt.Errorf("error setting up session recording: %w", err)
 	}
@@ -133,7 +133,7 @@ func (h *Hijacker) Hijack() (net.Conn, *bufio.ReadWriter, error) {
 // spdyHijacker.addrs. Returns conn from provided opts, wrapped in recording
 // logic. If connecting to the recorder fails or an error is received during the
 // session and spdyHijacker.failOpen is false, connection will be closed.
-func (h *Hijacker) setUpRecording(ctx context.Context, conn net.Conn) (net.Conn, error) {
+func (h *Hijacker) setUpRecording(conn net.Conn) (_ net.Conn, retErr error) {
 	const (
 		// https://docs.asciinema.org/manual/asciicast/v2/
 		asciicastv2  = 2
@@ -147,6 +147,14 @@ func (h *Hijacker) setUpRecording(ctx context.Context, conn net.Conn) (net.Conn,
 		errChan <-chan error
 	)
 	h.log.Infof("kubectl %s session will be recorded, recorders: %v, fail open policy: %t", h.sessionType, h.addrs, h.failOpen)
+	// NOTE: (ChaosInTheCRD) we want to use a dedicated context here, rather than the context from the request,
+	// otherwise the context can be cancelled by the client (kubectl) while we are still streaming to tsrecorder.
+	ctx, cancel := context.WithCancel(context.Background())
+	defer func() {
+		if retErr != nil {
+			cancel()
+		}
+	}()
 	qp := h.req.URL.Query()
 	container := strings.Join(qp[containerKey], "")
 	var recorderAddr net.Addr
@@ -213,6 +221,7 @@ func (h *Hijacker) setUpRecording(ctx context.Context, conn net.Conn) (net.Conn,
 	}
 
 	go func() {
+		defer cancel()
 		var err error
 		select {
 		case <-ctx.Done():
diff --git a/k8s-operator/sessionrecording/hijacker_test.go b/k8s-operator/sessionrecording/hijacker_test.go
index cac6f55c7..fb45820a7 100644
--- a/k8s-operator/sessionrecording/hijacker_test.go
+++ b/k8s-operator/sessionrecording/hijacker_test.go
@@ -95,7 +95,7 @@ func Test_Hijacker(t *testing.T) {
 				proto:    tt.proto,
 			}
 			ctx := context.Background()
-			_, err := h.setUpRecording(ctx, tc)
+			_, err := h.setUpRecording(tc)
 			if (err != nil) != tt.wantsSetupErr {
 				t.Errorf("spdyHijacker.setupRecording() error = %v, wantErr %v", err, tt.wantsSetupErr)
 				return