From baaa44e8e8c3e20beb875788c536d38bc1a5c34d Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Fri, 15 Sep 2023 14:39:32 +0100 Subject: [PATCH] Partially revert #30248 This PR removes the `ReadTimeout` and `WriteTimeout` settings from `kube/proxy.Server`. The revert is required because both settings were terminating watch streams earlier and causing several when parsing the long lived data stream. Signed-off-by: Tiago Silva --- lib/kube/proxy/responsewriters/watcher.go | 2 +- lib/kube/proxy/server.go | 13 ++++++++----- lib/kube/proxy/utils_testing.go | 3 ++- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/kube/proxy/responsewriters/watcher.go b/lib/kube/proxy/responsewriters/watcher.go index c2db48f0d87ed..b879361ec221e 100644 --- a/lib/kube/proxy/responsewriters/watcher.go +++ b/lib/kube/proxy/responsewriters/watcher.go @@ -207,7 +207,7 @@ func (w *WatcherResponseWriter) watchDecoder(contentType string, writer io.Write // wait for events received from upstream until the connection is terminated. for { eventType, obj, err := w.decodeStreamingMessage(streamingDecoder, objectDecoder) - if errors.Is(err, io.EOF) { + if errors.Is(err, io.EOF) || errors.Is(err, io.ErrClosedPipe) { return nil } else if err != nil { return trace.Wrap(err) diff --git a/lib/kube/proxy/server.go b/lib/kube/proxy/server.go index 62fb94c57bdc6..a54c696016091 100644 --- a/lib/kube/proxy/server.go +++ b/lib/kube/proxy/server.go @@ -239,11 +239,14 @@ func NewTLSServer(cfg TLSServerConfig) (*TLSServer, error) { Server: &http.Server{ Handler: httplib.MakeTracingHandler(limiter, teleport.ComponentKube), ReadHeaderTimeout: apidefaults.DefaultIOTimeout * 2, - ReadTimeout: apidefaults.DefaultIOTimeout, - WriteTimeout: apidefaults.DefaultIOTimeout, - IdleTimeout: apidefaults.DefaultIdleTimeout, - TLSConfig: cfg.TLS, - ConnState: ingress.HTTPConnStateReporter(ingress.Kube, cfg.IngressReporter), + // Setting ReadTimeout and WriteTimeout will cause the server to + // terminate long running requests. This will cause issues with + // long running watch streams. The server will close the connection + // and the client will receive incomplete data and will fail to + // parse it. + IdleTimeout: apidefaults.DefaultIdleTimeout, + TLSConfig: cfg.TLS, + ConnState: ingress.HTTPConnStateReporter(ingress.Kube, cfg.IngressReporter), ConnContext: func(ctx context.Context, c net.Conn) context.Context { return utils.ClientAddrContext(ctx, c.RemoteAddr(), c.LocalAddr()) }, diff --git a/lib/kube/proxy/utils_testing.go b/lib/kube/proxy/utils_testing.go index a61f08d495db5..b03b4eef97276 100644 --- a/lib/kube/proxy/utils_testing.go +++ b/lib/kube/proxy/utils_testing.go @@ -336,7 +336,8 @@ func SetupTestContext(ctx context.Context, t *testing.T, cfg TestConfig) *TestCo Log: log, }) require.NoError(t, err) - + require.Equal(t, testCtx.KubeServer.Server.ReadTimeout, time.Duration(0), "kube server write timeout must be 0") + require.Equal(t, testCtx.KubeServer.Server.WriteTimeout, time.Duration(0), "kube server write timeout must be 0") // Waits for len(clusters) heartbeats to start waitForHeartbeats := len(cfg.Clusters)