From cef842245c032c063a463f2baaf7c92ecc5a8070 Mon Sep 17 00:00:00 2001 From: Romain Date: Mon, 8 Apr 2024 17:16:04 +0200 Subject: [PATCH] Introduce Lingering Timeout Co-authored-by: Baptiste Mayelle Co-authored-by: Kevin Pollet --- docs/content/migration/v2.md | 32 ++++++ .../reference/static-configuration/cli-ref.md | 18 +++- .../reference/static-configuration/env-ref.md | 18 +++- .../reference/static-configuration/file.toml | 6 ++ .../reference/static-configuration/file.yaml | 6 ++ docs/content/routing/entrypoints.md | 81 +++++++++++---- pkg/api/handler_entrypoint_test.go | 22 +++-- pkg/api/testdata/entrypoints.json | 16 +-- pkg/config/static/static_config.go | 99 ++++++++++++++++++- pkg/redactor/redactor_config_test.go | 13 ++- .../testdata/anonymized-static-config.json | 11 ++- pkg/server/router/tcp/router.go | 12 --- pkg/server/server_entrypoint_tcp.go | 82 +++++++++++---- pkg/server/server_entrypoint_tcp_test.go | 88 ++++++++++++++++- 14 files changed, 418 insertions(+), 86 deletions(-) diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index 0f3114fc3..0a80cf43d 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -577,3 +577,35 @@ the maximum user-defined router priority value is: - `(MaxInt32 - 1000)` for 32-bit platforms, - `(MaxInt64 - 1000)` for 64-bit platforms. + +### .Transport.RespondingTimeouts. + +Starting with `v2.11.1` the following timeout options are deprecated: + +- `.transport.respondingTimeouts.readTimeout` +- `.transport.respondingTimeouts.writeTimeout` +- `.transport.respondingTimeouts.idleTimeout` + +They have been replaced by: + +- `.transport.respondingTimeouts.http.readTimeout` +- `.transport.respondingTimeouts.http.writeTimeout` +- `.transport.respondingTimeouts.http.idleTimeout` + +### .Transport.RespondingTimeouts.TCP.LingeringTimeout + +Starting with `v2.11.1` a new `lingeringTimeout` entryPoints option has been introduced, with a default value of 2s. + +The lingering timeout defines the maximum duration between each TCP read operation on the connection. +As a layer 4 timeout, it applies during HTTP handling but respects the configured HTTP server `readTimeout`. + +This change avoids Traefik instances with the default configuration hanging while waiting for bytes to be read on the connection. + +We suggest to adapt this value accordingly to your situation. +The new default value is purposely narrowed and can close the connection too early. + +Increasing the `lingeringTimeout` value could be the solution notably if you are dealing with the following errors: + +- TCP: `Error while handling TCP connection: readfrom tcp X.X.X.X:X->X.X.X.X:X: read tcp X.X.X.X:X->X.X.X.X:X: i/o timeout` +- HTTP: `'499 Client Closed Request' caused by: context canceled` +- HTTP: `ReverseProxy read error during body copy: read tcp X.X.X.X:X->X.X.X.X:X: use of closed network connection` diff --git a/docs/content/reference/static-configuration/cli-ref.md b/docs/content/reference/static-configuration/cli-ref.md index 60a8c6513..e3ae10e2a 100644 --- a/docs/content/reference/static-configuration/cli-ref.md +++ b/docs/content/reference/static-configuration/cli-ref.md @@ -183,15 +183,27 @@ Duration to give active requests a chance to finish before Traefik stops. (Defau `--entrypoints..transport.lifecycle.requestacceptgracetimeout`: Duration to keep accepting requests before Traefik initiates the graceful shutdown procedure. (Default: ```0```) -`--entrypoints..transport.respondingtimeouts.idletimeout`: +`--entrypoints..transport.respondingtimeouts.http.idletimeout`: IdleTimeout is the maximum amount duration an idle (keep-alive) connection will remain idle before closing itself. If zero, no timeout is set. (Default: ```180```) -`--entrypoints..transport.respondingtimeouts.readtimeout`: +`--entrypoints..transport.respondingtimeouts.http.readtimeout`: ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set. (Default: ```0```) -`--entrypoints..transport.respondingtimeouts.writetimeout`: +`--entrypoints..transport.respondingtimeouts.http.writetimeout`: WriteTimeout is the maximum duration before timing out writes of the response. If zero, no timeout is set. (Default: ```0```) +`--entrypoints..transport.respondingtimeouts.idletimeout`: +(Deprecated) IdleTimeout is the maximum amount duration an idle (keep-alive) connection will remain idle before closing itself. If zero, no timeout is set. (Default: ```0```) + +`--entrypoints..transport.respondingtimeouts.readtimeout`: +(Deprecated) ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set. (Default: ```0```) + +`--entrypoints..transport.respondingtimeouts.tcp.lingeringtimeout`: +LingeringTimeout is the maximum duration between each TCP read operation on the connection. If zero, no timeout is set. (Default: ```2```) + +`--entrypoints..transport.respondingtimeouts.writetimeout`: +(Deprecated) WriteTimeout is the maximum duration before timing out writes of the response. If zero, no timeout is set. (Default: ```0```) + `--entrypoints..udp.timeout`: Timeout defines how long to wait on an idle session before releasing the related resources. (Default: ```3```) diff --git a/docs/content/reference/static-configuration/env-ref.md b/docs/content/reference/static-configuration/env-ref.md index 849601f63..9ae961694 100644 --- a/docs/content/reference/static-configuration/env-ref.md +++ b/docs/content/reference/static-configuration/env-ref.md @@ -183,15 +183,27 @@ Duration to give active requests a chance to finish before Traefik stops. (Defau `TRAEFIK_ENTRYPOINTS__TRANSPORT_LIFECYCLE_REQUESTACCEPTGRACETIMEOUT`: Duration to keep accepting requests before Traefik initiates the graceful shutdown procedure. (Default: ```0```) -`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_IDLETIMEOUT`: +`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_HTTP_IDLETIMEOUT`: IdleTimeout is the maximum amount duration an idle (keep-alive) connection will remain idle before closing itself. If zero, no timeout is set. (Default: ```180```) -`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_READTIMEOUT`: +`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_HTTP_READTIMEOUT`: ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set. (Default: ```0```) -`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_WRITETIMEOUT`: +`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_HTTP_WRITETIMEOUT`: WriteTimeout is the maximum duration before timing out writes of the response. If zero, no timeout is set. (Default: ```0```) +`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_IDLETIMEOUT`: +(Deprecated) IdleTimeout is the maximum amount duration an idle (keep-alive) connection will remain idle before closing itself. If zero, no timeout is set. (Default: ```0```) + +`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_READTIMEOUT`: +(Deprecated) ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set. (Default: ```0```) + +`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_TCP_LINGERINGTIMEOUT`: +LingeringTimeout is the maximum duration between each TCP read operation on the connection. If zero, no timeout is set. (Default: ```2```) + +`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_WRITETIMEOUT`: +(Deprecated) WriteTimeout is the maximum duration before timing out writes of the response. If zero, no timeout is set. (Default: ```0```) + `TRAEFIK_ENTRYPOINTS__UDP_TIMEOUT`: Timeout defines how long to wait on an idle session before releasing the related resources. (Default: ```3```) diff --git a/docs/content/reference/static-configuration/file.toml b/docs/content/reference/static-configuration/file.toml index 42c4e6fdf..bb402467c 100644 --- a/docs/content/reference/static-configuration/file.toml +++ b/docs/content/reference/static-configuration/file.toml @@ -26,6 +26,12 @@ readTimeout = "42s" writeTimeout = "42s" idleTimeout = "42s" + [entryPoints.EntryPoint0.transport.respondingTimeouts.http] + readTimeout = "42s" + writeTimeout = "42s" + idleTimeout = "42s" + [entryPoints.EntryPoint0.transport.respondingTimeouts.tcp] + lingeringTimeout = "42s" [entryPoints.EntryPoint0.proxyProtocol] insecure = true trustedIPs = ["foobar", "foobar"] diff --git a/docs/content/reference/static-configuration/file.yaml b/docs/content/reference/static-configuration/file.yaml index abb8e05d8..d5265aa1d 100644 --- a/docs/content/reference/static-configuration/file.yaml +++ b/docs/content/reference/static-configuration/file.yaml @@ -24,6 +24,12 @@ entryPoints: readTimeout: 42s writeTimeout: 42s idleTimeout: 42s + http: + readTimeout: 42s + writeTimeout: 42s + idleTimeout: 42s + tcp: + lingeringTimeout: 42s keepAliveMaxTime: 42s keepAliveMaxRequests: 42 proxyProtocol: diff --git a/docs/content/routing/entrypoints.md b/docs/content/routing/entrypoints.md index 70ce11cf5..c6a79ba8e 100644 --- a/docs/content/routing/entrypoints.md +++ b/docs/content/routing/entrypoints.md @@ -397,10 +397,11 @@ You can configure Traefik to trust the forwarded headers information (`X-Forward #### `respondingTimeouts` -`respondingTimeouts` are timeouts for incoming requests to the Traefik instance. -Setting them has no effect for UDP entryPoints. +##### `http` -??? info "`transport.respondingTimeouts.readTimeout`" +`respondingTimeouts.http` are timeouts for incoming requests to the Traefik instance. + +??? info "`transport.respondingTimeouts.http.readTimeout`" _Optional, Default=0s_ @@ -417,7 +418,8 @@ Setting them has no effect for UDP entryPoints. address: ":8888" transport: respondingTimeouts: - readTimeout: 42 + http: + readTimeout: 42 ``` ```toml tab="File (TOML)" @@ -425,18 +427,17 @@ Setting them has no effect for UDP entryPoints. [entryPoints] [entryPoints.name] address = ":8888" - [entryPoints.name.transport] - [entryPoints.name.transport.respondingTimeouts] - readTimeout = 42 + [entryPoints.name.transport.respondingTimeouts.http] + readTimeout = 42 ``` ```bash tab="CLI" ## Static configuration --entryPoints.name.address=:8888 - --entryPoints.name.transport.respondingTimeouts.readTimeout=42 + --entryPoints.name.transport.respondingTimeouts.http.readTimeout=42 ``` -??? info "`transport.respondingTimeouts.writeTimeout`" +??? info "`transport.respondingTimeouts.http.writeTimeout`" _Optional, Default=0s_ @@ -454,7 +455,8 @@ Setting them has no effect for UDP entryPoints. address: ":8888" transport: respondingTimeouts: - writeTimeout: 42 + http: + writeTimeout: 42 ``` ```toml tab="File (TOML)" @@ -462,18 +464,17 @@ Setting them has no effect for UDP entryPoints. [entryPoints] [entryPoints.name] address = ":8888" - [entryPoints.name.transport] - [entryPoints.name.transport.respondingTimeouts] - writeTimeout = 42 + [entryPoints.name.transport.respondingTimeouts.http] + writeTimeout = 42 ``` ```bash tab="CLI" ## Static configuration --entryPoints.name.address=:8888 - --entryPoints.name.transport.respondingTimeouts.writeTimeout=42 + --entryPoints.name.transport.respondingTimeouts.http.writeTimeout=42 ``` -??? info "`transport.respondingTimeouts.idleTimeout`" +??? info "`transport.respondingTimeouts.http.idleTimeout`" _Optional, Default=180s_ @@ -490,7 +491,8 @@ Setting them has no effect for UDP entryPoints. address: ":8888" transport: respondingTimeouts: - idleTimeout: 42 + http: + idleTimeout: 42 ``` ```toml tab="File (TOML)" @@ -498,15 +500,54 @@ Setting them has no effect for UDP entryPoints. [entryPoints] [entryPoints.name] address = ":8888" - [entryPoints.name.transport] - [entryPoints.name.transport.respondingTimeouts] - idleTimeout = 42 + [entryPoints.name.transport.respondingTimeouts.http] + idleTimeout = 42 ``` ```bash tab="CLI" ## Static configuration --entryPoints.name.address=:8888 - --entryPoints.name.transport.respondingTimeouts.idleTimeout=42 + --entryPoints.name.transport.respondingTimeouts.http.idleTimeout=42 + +##### `tcp` + +`respondingTimeouts.tcp` are timeouts for client connections to the Traefik instance. + +??? info "`transport.respondingTimeouts.tcp.lingeringTimeout`" + + _Optional, Default=2s_ + + `lingeringTimeout` is the maximum duration between each TCP read operation on the connection. + As a layer 4 timeout, it also applies during HTTP handling, but respect the configured HTTP server `readTimeout`. + + If zero, the lingering is disabled. + Can be provided in a format supported by [time.ParseDuration](https://golang.org/pkg/time/#ParseDuration) or as raw values (digits). + If no units are provided, the value is parsed assuming seconds. + + ```yaml tab="File (YAML)" + ## Static configuration + entryPoints: + name: + address: ":8888" + transport: + respondingTimeouts: + tcp: + lingeringTimeout: 42 + ``` + + ```toml tab="File (TOML)" + ## Static configuration + [entryPoints] + [entryPoints.name] + address = ":8888" + [entryPoints.name.transport.respondingTimeouts.tcp] + lingeringTimeout = 42 + ``` + + ```bash tab="CLI" + ## Static configuration + --entryPoints.name.address=:8888 + --entryPoints.name.transport.respondingTimeouts.tcp.lingeringTimeout=42 ``` #### `lifeCycle` diff --git a/pkg/api/handler_entrypoint_test.go b/pkg/api/handler_entrypoint_test.go index d5d6a5d1f..79b92a65f 100644 --- a/pkg/api/handler_entrypoint_test.go +++ b/pkg/api/handler_entrypoint_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + ptypes "github.com/traefik/paerser/types" "github.com/traefik/traefik/v2/pkg/config/runtime" "github.com/traefik/traefik/v2/pkg/config/static" ) @@ -55,9 +56,11 @@ func TestHandler_EntryPoints(t *testing.T) { GraceTimeOut: 2, }, RespondingTimeouts: &static.RespondingTimeouts{ - ReadTimeout: 3, - WriteTimeout: 4, - IdleTimeout: 5, + HTTP: &static.HTTPRespondingTimeouts{ + ReadTimeout: paerserDurationPtr(3), + WriteTimeout: paerserDurationPtr(4), + IdleTimeout: paerserDurationPtr(5), + }, }, }, ProxyProtocol: &static.ProxyProtocol{ @@ -77,9 +80,11 @@ func TestHandler_EntryPoints(t *testing.T) { GraceTimeOut: 20, }, RespondingTimeouts: &static.RespondingTimeouts{ - ReadTimeout: 30, - WriteTimeout: 40, - IdleTimeout: 50, + HTTP: &static.HTTPRespondingTimeouts{ + ReadTimeout: paerserDurationPtr(3), + WriteTimeout: paerserDurationPtr(4), + IdleTimeout: paerserDurationPtr(5), + }, }, }, ProxyProtocol: &static.ProxyProtocol{ @@ -263,3 +268,8 @@ func generateEntryPoints(nb int) map[string]*static.EntryPoint { return eps } + +func paerserDurationPtr(duration int) *ptypes.Duration { + d := ptypes.Duration(duration) + return &d +} diff --git a/pkg/api/testdata/entrypoints.json b/pkg/api/testdata/entrypoints.json index d93d07bfc..93f56a401 100644 --- a/pkg/api/testdata/entrypoints.json +++ b/pkg/api/testdata/entrypoints.json @@ -23,9 +23,11 @@ "requestAcceptGraceTimeout": "1ns" }, "respondingTimeouts": { - "idleTimeout": "5ns", - "readTimeout": "3ns", - "writeTimeout": "4ns" + "http": { + "idleTimeout": "5ns", + "readTimeout": "3ns", + "writeTimeout": "4ns" + } } } }, @@ -53,9 +55,11 @@ "requestAcceptGraceTimeout": "10ns" }, "respondingTimeouts": { - "idleTimeout": "50ns", - "readTimeout": "30ns", - "writeTimeout": "40ns" + "http": { + "idleTimeout": "5ns", + "readTimeout": "3ns", + "writeTimeout": "4ns" + } } } } diff --git a/pkg/config/static/static_config.go b/pkg/config/static/static_config.go index dcf9b3dba..2a47763a0 100644 --- a/pkg/config/static/static_config.go +++ b/pkg/config/static/static_config.go @@ -56,6 +56,9 @@ const ( // DefaultUDPTimeout defines how long to wait by default on an idle session, // before releasing all resources related to that session. DefaultUDPTimeout = 3 * time.Second + + // defaultLingeringTimeout defines the default maximum duration between each read operation on the connection. + defaultLingeringTimeout = 2 * time.Second ) // Configuration is the static configuration. @@ -118,16 +121,44 @@ func (a *API) SetDefaults() { a.Dashboard = true } -// RespondingTimeouts contains timeout configurations for incoming requests to the Traefik instance. +// RespondingTimeouts contains timeout configurations. type RespondingTimeouts struct { - ReadTimeout ptypes.Duration `description:"ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set." json:"readTimeout,omitempty" toml:"readTimeout,omitempty" yaml:"readTimeout,omitempty" export:"true"` - WriteTimeout ptypes.Duration `description:"WriteTimeout is the maximum duration before timing out writes of the response. If zero, no timeout is set." json:"writeTimeout,omitempty" toml:"writeTimeout,omitempty" yaml:"writeTimeout,omitempty" export:"true"` - IdleTimeout ptypes.Duration `description:"IdleTimeout is the maximum amount duration an idle (keep-alive) connection will remain idle before closing itself. If zero, no timeout is set." json:"idleTimeout,omitempty" toml:"idleTimeout,omitempty" yaml:"idleTimeout,omitempty" export:"true"` + // Deprecated: please use `respondingTimeouts.http.readTimeout` instead. + ReadTimeout *ptypes.Duration `description:"(Deprecated) ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set." json:"readTimeout,omitempty" toml:"readTimeout,omitempty" yaml:"readTimeout,omitempty" export:"true"` + // Deprecated: please use `respondingTimeouts.http.writeTimeout` instead. + WriteTimeout *ptypes.Duration `description:"(Deprecated) WriteTimeout is the maximum duration before timing out writes of the response. If zero, no timeout is set." json:"writeTimeout,omitempty" toml:"writeTimeout,omitempty" yaml:"writeTimeout,omitempty" export:"true"` + // Deprecated: please use `respondingTimeouts.http.idleTimeout` instead. + IdleTimeout *ptypes.Duration `description:"(Deprecated) IdleTimeout is the maximum amount duration an idle (keep-alive) connection will remain idle before closing itself. If zero, no timeout is set." json:"idleTimeout,omitempty" toml:"idleTimeout,omitempty" yaml:"idleTimeout,omitempty" export:"true"` + + HTTP *HTTPRespondingTimeouts `description:"Defines the HTTP responding timeouts." json:"http,omitempty" toml:"http,omitempty" yaml:"http,omitempty" export:"true"` + TCP *TCPRespondingTimeouts `description:"Defines the TCP responding timeouts." json:"tcp,omitempty" toml:"tcp,omitempty" yaml:"tcp,omitempty" export:"true"` +} + +// HTTPRespondingTimeouts contains HTTP timeout configurations for incoming requests to the Traefik instance. +type HTTPRespondingTimeouts struct { + ReadTimeout *ptypes.Duration `description:"ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set." json:"readTimeout,omitempty" toml:"readTimeout,omitempty" yaml:"readTimeout,omitempty" export:"true"` + WriteTimeout *ptypes.Duration `description:"WriteTimeout is the maximum duration before timing out writes of the response. If zero, no timeout is set." json:"writeTimeout,omitempty" toml:"writeTimeout,omitempty" yaml:"writeTimeout,omitempty" export:"true"` + IdleTimeout *ptypes.Duration `description:"IdleTimeout is the maximum amount duration an idle (keep-alive) connection will remain idle before closing itself. If zero, no timeout is set." json:"idleTimeout,omitempty" toml:"idleTimeout,omitempty" yaml:"idleTimeout,omitempty" export:"true"` +} + +// TCPRespondingTimeouts contains TCP timeout configurations for client connections to the Traefik instance. +type TCPRespondingTimeouts struct { + LingeringTimeout ptypes.Duration `description:"LingeringTimeout is the maximum duration between each TCP read operation on the connection. If zero, no timeout is set." json:"lingeringTimeout,omitempty" toml:"lingeringTimeout,omitempty" yaml:"lingeringTimeout,omitempty" export:"true"` } // SetDefaults sets the default values. func (a *RespondingTimeouts) SetDefaults() { - a.IdleTimeout = ptypes.Duration(DefaultIdleTimeout) + noTimeout := ptypes.Duration(0) + defaultIdleTimeout := ptypes.Duration(DefaultIdleTimeout) + a.HTTP = &HTTPRespondingTimeouts{ + ReadTimeout: &noTimeout, + WriteTimeout: &noTimeout, + IdleTimeout: &defaultIdleTimeout, + } + + a.TCP = &TCPRespondingTimeouts{ + LingeringTimeout: ptypes.Duration(defaultLingeringTimeout), + } } // ForwardingTimeouts contains timeout configurations for forwarding requests to the backend servers. @@ -211,6 +242,39 @@ func (c *Configuration) SetEffectiveConfiguration() { c.EntryPoints["http"] = ep } + for _, entrypoint := range c.EntryPoints { + if entrypoint.Transport == nil || + entrypoint.Transport.RespondingTimeouts == nil { + continue + } + + respondingTimeouts := entrypoint.Transport.RespondingTimeouts + + if respondingTimeouts.ReadTimeout != nil && + respondingTimeouts.HTTP != nil && + respondingTimeouts.HTTP.ReadTimeout == nil { + log.WithoutContext().Warnf("Option `respondingTimeouts.readTimeout` is deprecated, please use `respondingTimeouts.http.readTimeout` instead.") + respondingTimeouts.HTTP.ReadTimeout = respondingTimeouts.ReadTimeout + respondingTimeouts.ReadTimeout = nil + } + + if respondingTimeouts.WriteTimeout != nil && + respondingTimeouts.HTTP != nil && + respondingTimeouts.HTTP.WriteTimeout == nil { + log.WithoutContext().Warnf("Option `respondingTimeouts.writeTimeout` is deprecated, please use `respondingTimeouts.http.writeTimeout` instead.") + respondingTimeouts.HTTP.WriteTimeout = respondingTimeouts.WriteTimeout + respondingTimeouts.WriteTimeout = nil + } + + if respondingTimeouts.IdleTimeout != nil && + respondingTimeouts.HTTP != nil && + respondingTimeouts.HTTP.IdleTimeout == nil { + log.WithoutContext().Warnf("Option `respondingTimeouts.idleTimeout` is deprecated, please use `respondingTimeouts.http.idleTimeout` instead.") + respondingTimeouts.HTTP.IdleTimeout = respondingTimeouts.IdleTimeout + respondingTimeouts.IdleTimeout = nil + } + } + // Creates the internal traefik entry point if needed if (c.API != nil && c.API.Insecure) || (c.Ping != nil && !c.Ping.ManualRouting && c.Ping.EntryPoint == DefaultInternalEntryPointName) || @@ -316,6 +380,31 @@ func (c *Configuration) ValidateConfiguration() error { return errors.New("Nomad provider cannot have both namespace and namespaces options configured") } + for epName, entrypoint := range c.EntryPoints { + if entrypoint.Transport == nil || + entrypoint.Transport.RespondingTimeouts == nil || + entrypoint.Transport.RespondingTimeouts.HTTP == nil { + continue + } + + respondingTimeouts := entrypoint.Transport.RespondingTimeouts + + if respondingTimeouts.ReadTimeout != nil && + respondingTimeouts.HTTP.ReadTimeout != nil { + return fmt.Errorf("entrypoint %q has `readTimeout` option is defined multiple times (`respondingTimeouts.readTimeout` is deprecated)", epName) + } + + if respondingTimeouts.WriteTimeout != nil && + respondingTimeouts.HTTP.WriteTimeout != nil { + return fmt.Errorf("entrypoint %q has `writeTimeout` option is defined multiple times (`respondingTimeouts.writeTimeout` is deprecated)", epName) + } + + if respondingTimeouts.IdleTimeout != nil && + respondingTimeouts.HTTP.IdleTimeout != nil { + return fmt.Errorf("entrypoint %q has `idleTimeout` option is defined multiple times (`respondingTimeouts.idleTimeout` is deprecated)", epName) + } + } + return nil } diff --git a/pkg/redactor/redactor_config_test.go b/pkg/redactor/redactor_config_test.go index 368f2d956..4cc080683 100644 --- a/pkg/redactor/redactor_config_test.go +++ b/pkg/redactor/redactor_config_test.go @@ -520,6 +520,8 @@ func TestDo_staticConfiguration(t *testing.T) { }, } + paerserDuration := ptypes.Duration(111 * time.Second) + config.EntryPoints = static.EntryPoints{ "foobar": { Address: "foo Address", @@ -529,9 +531,14 @@ func TestDo_staticConfiguration(t *testing.T) { GraceTimeOut: ptypes.Duration(111 * time.Second), }, RespondingTimeouts: &static.RespondingTimeouts{ - ReadTimeout: ptypes.Duration(111 * time.Second), - WriteTimeout: ptypes.Duration(111 * time.Second), - IdleTimeout: ptypes.Duration(111 * time.Second), + HTTP: &static.HTTPRespondingTimeouts{ + ReadTimeout: &paerserDuration, + WriteTimeout: &paerserDuration, + IdleTimeout: &paerserDuration, + }, + TCP: &static.TCPRespondingTimeouts{ + LingeringTimeout: ptypes.Duration(111 * time.Second), + }, }, }, ProxyProtocol: &static.ProxyProtocol{ diff --git a/pkg/redactor/testdata/anonymized-static-config.json b/pkg/redactor/testdata/anonymized-static-config.json index 09e0d796d..dd25c1fa3 100644 --- a/pkg/redactor/testdata/anonymized-static-config.json +++ b/pkg/redactor/testdata/anonymized-static-config.json @@ -26,9 +26,14 @@ "graceTimeOut": "1m51s" }, "respondingTimeouts": { - "readTimeout": "1m51s", - "writeTimeout": "1m51s", - "idleTimeout": "1m51s" + "http": { + "readTimeout": "1m51s", + "writeTimeout": "1m51s", + "idleTimeout": "1m51s" + }, + "tcp": { + "lingeringTimeout": "1m51s" + } } }, "proxyProtocol": { diff --git a/pkg/server/router/tcp/router.go b/pkg/server/router/tcp/router.go index 0d8524398..551bb43e7 100644 --- a/pkg/server/router/tcp/router.go +++ b/pkg/server/router/tcp/router.go @@ -9,7 +9,6 @@ import ( "net" "net/http" "slices" - "time" "github.com/go-acme/lego/v4/challenge/tlsalpn01" "github.com/traefik/traefik/v2/pkg/log" @@ -117,17 +116,6 @@ func (r *Router) ServeTCP(conn tcp.WriteCloser) { return } - // Remove read/write deadline and delegate this to underlying tcp server (for now only handled by HTTP Server) - err = conn.SetReadDeadline(time.Time{}) - if err != nil { - log.WithoutContext().Errorf("Error while setting read deadline: %v", err) - } - - err = conn.SetWriteDeadline(time.Time{}) - if err != nil { - log.WithoutContext().Errorf("Error while setting write deadline: %v", err) - } - connData, err := tcpmuxer.NewConnData(hello.serverName, conn, hello.protos) if err != nil { log.WithoutContext().Errorf("Error while reading TCP connection data: %v", err) diff --git a/pkg/server/server_entrypoint_tcp.go b/pkg/server/server_entrypoint_tcp.go index 1fb371d08..b5aaa1ac0 100644 --- a/pkg/server/server_entrypoint_tcp.go +++ b/pkg/server/server_entrypoint_tcp.go @@ -244,24 +244,15 @@ func (e *TCPEntryPoint) Start(ctx context.Context) { panic(err) } + if e.transportConfiguration != nil && + e.transportConfiguration.RespondingTimeouts != nil && + e.transportConfiguration.RespondingTimeouts.TCP != nil && + e.transportConfiguration.RespondingTimeouts.TCP.LingeringTimeout > 0 { + lingeringTimeout := time.Duration(e.transportConfiguration.RespondingTimeouts.TCP.LingeringTimeout) + writeCloser = newLingeringConnection(writeCloser, lingeringTimeout) + } + safe.Go(func() { - // Enforce read/write deadlines at the connection level, - // because when we're peeking the first byte to determine whether we are doing TLS, - // the deadlines at the server level are not taken into account. - if e.transportConfiguration.RespondingTimeouts.ReadTimeout > 0 { - err := writeCloser.SetReadDeadline(time.Now().Add(time.Duration(e.transportConfiguration.RespondingTimeouts.ReadTimeout))) - if err != nil { - logger.Errorf("Error while setting read deadline: %v", err) - } - } - - if e.transportConfiguration.RespondingTimeouts.WriteTimeout > 0 { - err = writeCloser.SetWriteDeadline(time.Now().Add(time.Duration(e.transportConfiguration.RespondingTimeouts.WriteTimeout))) - if err != nil { - logger.Errorf("Error while setting write deadline: %v", err) - } - } - e.switcher.ServeTCP(newTrackedConnection(writeCloser, e.tracker)) }) } @@ -391,6 +382,55 @@ func writeCloser(conn net.Conn) (tcp.WriteCloser, error) { } } +// lingeringConn represents a writeCloser with lingeringTimeout handling. +type lingeringConn struct { + tcp.WriteCloser + + lingeringTimeout time.Duration + + rdlMu sync.RWMutex + // readDeadline is the current readDeadline set by an upper caller. + // In case of HTTP, the HTTP go server manipulates deadlines on the connection. + readDeadline time.Time +} + +// newLingeringConnection returns the given writeCloser augmented with lingeringTimeout handling. +func newLingeringConnection(conn tcp.WriteCloser, timeout time.Duration) tcp.WriteCloser { + return &lingeringConn{ + WriteCloser: conn, + lingeringTimeout: timeout, + } +} + +// Read reads data from the connection and postpones the connection readDeadline according to the lingeringTimeout config. +// It also ensures that the upper level set readDeadline is enforced. +func (l *lingeringConn) Read(b []byte) (int, error) { + if l.lingeringTimeout > 0 { + deadline := time.Now().Add(l.lingeringTimeout) + + l.rdlMu.RLock() + if !l.readDeadline.IsZero() && deadline.After(l.readDeadline) { + deadline = l.readDeadline + } + l.rdlMu.RUnlock() + + if err := l.WriteCloser.SetReadDeadline(deadline); err != nil { + return 0, err + } + } + + return l.WriteCloser.Read(b) +} + +// SetReadDeadline sets and save the read deadline. +func (l *lingeringConn) SetReadDeadline(t time.Time) error { + l.rdlMu.Lock() + l.readDeadline = t + l.rdlMu.Unlock() + + return l.WriteCloser.SetReadDeadline(t) +} + // tcpKeepAliveListener sets TCP keep-alive timeouts on accepted // connections. type tcpKeepAliveListener struct { @@ -419,7 +459,7 @@ func (ln tcpKeepAliveListener) Accept() (net.Conn, error) { } func buildProxyProtocolListener(ctx context.Context, entryPoint *static.EntryPoint, listener net.Listener) (net.Listener, error) { - timeout := entryPoint.Transport.RespondingTimeouts.ReadTimeout + timeout := *entryPoint.Transport.RespondingTimeouts.HTTP.ReadTimeout // proxyproto use 200ms if ReadHeaderTimeout is set to 0 and not no timeout if timeout == 0 { timeout = -1 @@ -588,9 +628,9 @@ func createHTTPServer(ctx context.Context, ln net.Listener, configuration *stati serverHTTP := &http.Server{ Handler: handler, ErrorLog: httpServerLogger, - ReadTimeout: time.Duration(configuration.Transport.RespondingTimeouts.ReadTimeout), - WriteTimeout: time.Duration(configuration.Transport.RespondingTimeouts.WriteTimeout), - IdleTimeout: time.Duration(configuration.Transport.RespondingTimeouts.IdleTimeout), + ReadTimeout: time.Duration(*configuration.Transport.RespondingTimeouts.HTTP.ReadTimeout), + WriteTimeout: time.Duration(*configuration.Transport.RespondingTimeouts.HTTP.WriteTimeout), + IdleTimeout: time.Duration(*configuration.Transport.RespondingTimeouts.HTTP.IdleTimeout), } if debugConnection || (configuration.Transport != nil && (configuration.Transport.KeepAliveMaxTime > 0 || configuration.Transport.KeepAliveMaxRequests > 0)) { serverHTTP.ConnContext = func(ctx context.Context, c net.Conn) context.Context { diff --git a/pkg/server/server_entrypoint_tcp_test.go b/pkg/server/server_entrypoint_tcp_test.go index f0b12c8dd..6b177099f 100644 --- a/pkg/server/server_entrypoint_tcp_test.go +++ b/pkg/server/server_entrypoint_tcp_test.go @@ -69,8 +69,10 @@ func testShutdown(t *testing.T, router *tcprouter.Router) { epConfig.LifeCycle.RequestAcceptGraceTimeout = 0 epConfig.LifeCycle.GraceTimeOut = ptypes.Duration(5 * time.Second) - epConfig.RespondingTimeouts.ReadTimeout = ptypes.Duration(5 * time.Second) - epConfig.RespondingTimeouts.WriteTimeout = ptypes.Duration(5 * time.Second) + readTimeout := ptypes.Duration(5 * time.Second) + epConfig.RespondingTimeouts.HTTP.ReadTimeout = &readTimeout + writeTimeout := ptypes.Duration(5 * time.Second) + epConfig.RespondingTimeouts.HTTP.WriteTimeout = &writeTimeout entryPoint, err := NewTCPEntryPoint(context.Background(), &static.EntryPoint{ // We explicitly use an IPV4 address because on Alpine, with an IPV6 address @@ -157,7 +159,8 @@ func startEntrypoint(entryPoint *TCPEntryPoint, router *tcprouter.Router) (net.C func TestReadTimeoutWithoutFirstByte(t *testing.T) { epConfig := &static.EntryPointsTransport{} epConfig.SetDefaults() - epConfig.RespondingTimeouts.ReadTimeout = ptypes.Duration(2 * time.Second) + readTimeout := ptypes.Duration(2 * time.Second) + epConfig.RespondingTimeouts.HTTP.ReadTimeout = &readTimeout entryPoint, err := NewTCPEntryPoint(context.Background(), &static.EntryPoint{ Address: ":0", @@ -194,7 +197,84 @@ func TestReadTimeoutWithoutFirstByte(t *testing.T) { func TestReadTimeoutWithFirstByte(t *testing.T) { epConfig := &static.EntryPointsTransport{} epConfig.SetDefaults() - epConfig.RespondingTimeouts.ReadTimeout = ptypes.Duration(2 * time.Second) + readTimeout := ptypes.Duration(2 * time.Second) + epConfig.RespondingTimeouts.HTTP.ReadTimeout = &readTimeout + + entryPoint, err := NewTCPEntryPoint(context.Background(), &static.EntryPoint{ + Address: ":0", + Transport: epConfig, + ForwardedHeaders: &static.ForwardedHeaders{}, + HTTP2: &static.HTTP2Config{}, + }, nil) + require.NoError(t, err) + + router := &tcprouter.Router{} + router.SetHTTPHandler(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(http.StatusOK) + })) + + conn, err := startEntrypoint(entryPoint, router) + require.NoError(t, err) + + _, err = conn.Write([]byte("GET /some HTTP/1.1\r\n")) + require.NoError(t, err) + + errChan := make(chan error) + + go func() { + b := make([]byte, 2048) + _, err := conn.Read(b) + errChan <- err + }() + + select { + case err := <-errChan: + require.Equal(t, io.EOF, err) + case <-time.Tick(5 * time.Second): + t.Error("Timeout while read") + } +} + +func TestLingeringTimeoutWithoutFirstByte(t *testing.T) { + epConfig := &static.EntryPointsTransport{} + epConfig.SetDefaults() + + entryPoint, err := NewTCPEntryPoint(context.Background(), &static.EntryPoint{ + Address: ":0", + Transport: epConfig, + ForwardedHeaders: &static.ForwardedHeaders{}, + HTTP2: &static.HTTP2Config{}, + }, nil) + require.NoError(t, err) + + router := &tcprouter.Router{} + router.SetHTTPHandler(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(http.StatusOK) + })) + + conn, err := startEntrypoint(entryPoint, router) + require.NoError(t, err) + + errChan := make(chan error) + + go func() { + b := make([]byte, 2048) + _, err := conn.Read(b) + errChan <- err + }() + + select { + case err := <-errChan: + require.Equal(t, io.EOF, err) + case <-time.Tick(5 * time.Second): + t.Error("Timeout while read") + } +} + +func TestLingeringTimeoutWithFirstByte(t *testing.T) { + epConfig := &static.EntryPointsTransport{} + epConfig.SetDefaults() + epConfig.RespondingTimeouts.TCP.LingeringTimeout = ptypes.Duration(time.Second) entryPoint, err := NewTCPEntryPoint(context.Background(), &static.EntryPoint{ Address: ":0",