From 099c7e9444a5d56918b8221672fc8d6a09a5d389 Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Thu, 11 Apr 2024 15:48:04 +0200 Subject: [PATCH] Revert LingeringTimeout and change default value for ReadTimeout Co-authored-by: Romain --- docs/content/migration/v2.md | 35 +++++- .../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 | 84 ++++---------- pkg/api/handler_entrypoint_test.go | 22 +--- pkg/api/testdata/entrypoints.json | 16 +-- pkg/config/static/static_config.go | 103 ++---------------- 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, 125 insertions(+), 389 deletions(-) diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index 0a80cf43d..669ebf705 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -578,7 +578,7 @@ the maximum user-defined router priority value is: - `(MaxInt32 - 1000)` for 32-bit platforms, - `(MaxInt64 - 1000)` for 64-bit platforms. -### .Transport.RespondingTimeouts. +### EntryPoint.Transport.RespondingTimeouts. Starting with `v2.11.1` the following timeout options are deprecated: @@ -592,7 +592,7 @@ They have been replaced by: - `.transport.respondingTimeouts.http.writeTimeout` - `.transport.respondingTimeouts.http.idleTimeout` -### .Transport.RespondingTimeouts.TCP.LingeringTimeout +### EntryPoint.Transport.RespondingTimeouts.TCP.LingeringTimeout Starting with `v2.11.1` a new `lingeringTimeout` entryPoints option has been introduced, with a default value of 2s. @@ -609,3 +609,34 @@ Increasing the `lingeringTimeout` value could be the solution notably if you are - 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` + +## v2.11.2 + +### LingeringTimeout + +Starting with `v2.11.2` the `.transport.respondingTimeouts.tcp.lingeringTimeout` introduced in `v2.11.1` has been removed. + +### RespondingTimeouts.TCP and RespondingTimeouts.HTTP + +Starting with `v2.11.2` the `respondingTimeouts.tcp` and `respondingTimeouts.http` sections introduced in `v2.11.1` have been removed. +To configure responding timeouts + +### EntryPoint.Transport.RespondingTimeouts.ReadTimeout + +Starting with `v2.11.2` the entryPoints `readTimeout` option default value changed to 5 seconds. + +For HTTP, this option defines the maximum duration for reading the entire request, including the body. +For TCP, this option defines the maximum duration for the first bytes to be read on the connection. + +The default value was previously set to zero, which means no timeout. + +This change has been done to avoid Traefik instances with the default configuration to be hanging forever while waiting for bytes to be read on the connection. + +We suggest to adapt this value accordingly to your situation, as the new default value is purposely narrowed, +it can make the connection be closed too early. + +Increasing the `readTimeout` 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 e3ae10e2a..81aed712a 100644 --- a/docs/content/reference/static-configuration/cli-ref.md +++ b/docs/content/reference/static-configuration/cli-ref.md @@ -183,26 +183,14 @@ 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.http.idletimeout`: +`--entrypoints..transport.respondingtimeouts.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.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.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```) +ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set. (Default: ```5```) `--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```) +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 9ae961694..605aecfed 100644 --- a/docs/content/reference/static-configuration/env-ref.md +++ b/docs/content/reference/static-configuration/env-ref.md @@ -183,26 +183,14 @@ 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_HTTP_IDLETIMEOUT`: +`TRAEFIK_ENTRYPOINTS__TRANSPORT_RESPONDINGTIMEOUTS_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_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_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```) +ReadTimeout is the maximum duration for reading the entire request, including the body. If zero, no timeout is set. (Default: ```5```) `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```) +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 bb402467c..42c4e6fdf 100644 --- a/docs/content/reference/static-configuration/file.toml +++ b/docs/content/reference/static-configuration/file.toml @@ -26,12 +26,6 @@ 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 d5265aa1d..abb8e05d8 100644 --- a/docs/content/reference/static-configuration/file.yaml +++ b/docs/content/reference/static-configuration/file.yaml @@ -24,12 +24,6 @@ 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 c6a79ba8e..685db070c 100644 --- a/docs/content/routing/entrypoints.md +++ b/docs/content/routing/entrypoints.md @@ -397,19 +397,19 @@ You can configure Traefik to trust the forwarded headers information (`X-Forward #### `respondingTimeouts` -##### `http` +`respondingTimeouts` are timeouts for incoming requests to the Traefik instance. +Setting them has no effect for UDP entryPoints. -`respondingTimeouts.http` are timeouts for incoming requests to the Traefik instance. +??? info "`transport.respondingTimeouts.readTimeout`" -??? info "`transport.respondingTimeouts.http.readTimeout`" - - _Optional, Default=0s_ + _Optional, Default=5s_ `readTimeout` is the maximum duration for reading the entire request, including the body. If zero, no timeout exists. 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. + For requests with large payloads, this timeout value might be increased. ```yaml tab="File (YAML)" ## Static configuration @@ -418,8 +418,7 @@ You can configure Traefik to trust the forwarded headers information (`X-Forward address: ":8888" transport: respondingTimeouts: - http: - readTimeout: 42 + readTimeout: 42 ``` ```toml tab="File (TOML)" @@ -427,17 +426,18 @@ You can configure Traefik to trust the forwarded headers information (`X-Forward [entryPoints] [entryPoints.name] address = ":8888" - [entryPoints.name.transport.respondingTimeouts.http] - readTimeout = 42 + [entryPoints.name.transport] + [entryPoints.name.transport.respondingTimeouts] + readTimeout = 42 ``` ```bash tab="CLI" ## Static configuration --entryPoints.name.address=:8888 - --entryPoints.name.transport.respondingTimeouts.http.readTimeout=42 + --entryPoints.name.transport.respondingTimeouts.readTimeout=42 ``` -??? info "`transport.respondingTimeouts.http.writeTimeout`" +??? info "`transport.respondingTimeouts.writeTimeout`" _Optional, Default=0s_ @@ -455,8 +455,7 @@ You can configure Traefik to trust the forwarded headers information (`X-Forward address: ":8888" transport: respondingTimeouts: - http: - writeTimeout: 42 + writeTimeout: 42 ``` ```toml tab="File (TOML)" @@ -464,17 +463,18 @@ You can configure Traefik to trust the forwarded headers information (`X-Forward [entryPoints] [entryPoints.name] address = ":8888" - [entryPoints.name.transport.respondingTimeouts.http] - writeTimeout = 42 + [entryPoints.name.transport] + [entryPoints.name.transport.respondingTimeouts] + writeTimeout = 42 ``` ```bash tab="CLI" ## Static configuration --entryPoints.name.address=:8888 - --entryPoints.name.transport.respondingTimeouts.http.writeTimeout=42 + --entryPoints.name.transport.respondingTimeouts.writeTimeout=42 ``` -??? info "`transport.respondingTimeouts.http.idleTimeout`" +??? info "`transport.respondingTimeouts.idleTimeout`" _Optional, Default=180s_ @@ -491,8 +491,7 @@ You can configure Traefik to trust the forwarded headers information (`X-Forward address: ":8888" transport: respondingTimeouts: - http: - idleTimeout: 42 + idleTimeout: 42 ``` ```toml tab="File (TOML)" @@ -500,54 +499,15 @@ You can configure Traefik to trust the forwarded headers information (`X-Forward [entryPoints] [entryPoints.name] address = ":8888" - [entryPoints.name.transport.respondingTimeouts.http] - idleTimeout = 42 + [entryPoints.name.transport] + [entryPoints.name.transport.respondingTimeouts] + idleTimeout = 42 ``` ```bash tab="CLI" ## Static configuration --entryPoints.name.address=:8888 - --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 + --entryPoints.name.transport.respondingTimeouts.idleTimeout=42 ``` #### `lifeCycle` diff --git a/pkg/api/handler_entrypoint_test.go b/pkg/api/handler_entrypoint_test.go index 79b92a65f..d5d6a5d1f 100644 --- a/pkg/api/handler_entrypoint_test.go +++ b/pkg/api/handler_entrypoint_test.go @@ -13,7 +13,6 @@ 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" ) @@ -56,11 +55,9 @@ func TestHandler_EntryPoints(t *testing.T) { GraceTimeOut: 2, }, RespondingTimeouts: &static.RespondingTimeouts{ - HTTP: &static.HTTPRespondingTimeouts{ - ReadTimeout: paerserDurationPtr(3), - WriteTimeout: paerserDurationPtr(4), - IdleTimeout: paerserDurationPtr(5), - }, + ReadTimeout: 3, + WriteTimeout: 4, + IdleTimeout: 5, }, }, ProxyProtocol: &static.ProxyProtocol{ @@ -80,11 +77,9 @@ func TestHandler_EntryPoints(t *testing.T) { GraceTimeOut: 20, }, RespondingTimeouts: &static.RespondingTimeouts{ - HTTP: &static.HTTPRespondingTimeouts{ - ReadTimeout: paerserDurationPtr(3), - WriteTimeout: paerserDurationPtr(4), - IdleTimeout: paerserDurationPtr(5), - }, + ReadTimeout: 30, + WriteTimeout: 40, + IdleTimeout: 50, }, }, ProxyProtocol: &static.ProxyProtocol{ @@ -268,8 +263,3 @@ 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 93f56a401..d93d07bfc 100644 --- a/pkg/api/testdata/entrypoints.json +++ b/pkg/api/testdata/entrypoints.json @@ -23,11 +23,9 @@ "requestAcceptGraceTimeout": "1ns" }, "respondingTimeouts": { - "http": { - "idleTimeout": "5ns", - "readTimeout": "3ns", - "writeTimeout": "4ns" - } + "idleTimeout": "5ns", + "readTimeout": "3ns", + "writeTimeout": "4ns" } } }, @@ -55,11 +53,9 @@ "requestAcceptGraceTimeout": "10ns" }, "respondingTimeouts": { - "http": { - "idleTimeout": "5ns", - "readTimeout": "3ns", - "writeTimeout": "4ns" - } + "idleTimeout": "50ns", + "readTimeout": "30ns", + "writeTimeout": "40ns" } } } diff --git a/pkg/config/static/static_config.go b/pkg/config/static/static_config.go index 2a47763a0..6e43e3105 100644 --- a/pkg/config/static/static_config.go +++ b/pkg/config/static/static_config.go @@ -50,15 +50,15 @@ const ( // DefaultIdleTimeout before closing an idle connection. DefaultIdleTimeout = 180 * time.Second + // DefaultReadTimeout defines the default maximum duration for reading the entire request, including the body. + DefaultReadTimeout = 5 * time.Second + // DefaultAcmeCAServer is the default ACME API endpoint. DefaultAcmeCAServer = "https://acme-v02.api.letsencrypt.org/directory" // 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. @@ -121,44 +121,17 @@ func (a *API) SetDefaults() { a.Dashboard = true } -// RespondingTimeouts contains timeout configurations. +// RespondingTimeouts contains timeout configurations for incoming requests to the Traefik instance. type RespondingTimeouts struct { - // 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"` + 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"` } // SetDefaults sets the default values. func (a *RespondingTimeouts) SetDefaults() { - noTimeout := ptypes.Duration(0) - defaultIdleTimeout := ptypes.Duration(DefaultIdleTimeout) - a.HTTP = &HTTPRespondingTimeouts{ - ReadTimeout: &noTimeout, - WriteTimeout: &noTimeout, - IdleTimeout: &defaultIdleTimeout, - } - - a.TCP = &TCPRespondingTimeouts{ - LingeringTimeout: ptypes.Duration(defaultLingeringTimeout), - } + a.ReadTimeout = ptypes.Duration(DefaultReadTimeout) + a.IdleTimeout = ptypes.Duration(DefaultIdleTimeout) } // ForwardingTimeouts contains timeout configurations for forwarding requests to the backend servers. @@ -242,39 +215,6 @@ 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) || @@ -380,31 +320,6 @@ 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 4cc080683..368f2d956 100644 --- a/pkg/redactor/redactor_config_test.go +++ b/pkg/redactor/redactor_config_test.go @@ -520,8 +520,6 @@ func TestDo_staticConfiguration(t *testing.T) { }, } - paerserDuration := ptypes.Duration(111 * time.Second) - config.EntryPoints = static.EntryPoints{ "foobar": { Address: "foo Address", @@ -531,14 +529,9 @@ func TestDo_staticConfiguration(t *testing.T) { GraceTimeOut: ptypes.Duration(111 * time.Second), }, RespondingTimeouts: &static.RespondingTimeouts{ - HTTP: &static.HTTPRespondingTimeouts{ - ReadTimeout: &paerserDuration, - WriteTimeout: &paerserDuration, - IdleTimeout: &paerserDuration, - }, - TCP: &static.TCPRespondingTimeouts{ - LingeringTimeout: ptypes.Duration(111 * time.Second), - }, + ReadTimeout: ptypes.Duration(111 * time.Second), + WriteTimeout: ptypes.Duration(111 * time.Second), + IdleTimeout: 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 dd25c1fa3..09e0d796d 100644 --- a/pkg/redactor/testdata/anonymized-static-config.json +++ b/pkg/redactor/testdata/anonymized-static-config.json @@ -26,14 +26,9 @@ "graceTimeOut": "1m51s" }, "respondingTimeouts": { - "http": { - "readTimeout": "1m51s", - "writeTimeout": "1m51s", - "idleTimeout": "1m51s" - }, - "tcp": { - "lingeringTimeout": "1m51s" - } + "readTimeout": "1m51s", + "writeTimeout": "1m51s", + "idleTimeout": "1m51s" } }, "proxyProtocol": { diff --git a/pkg/server/router/tcp/router.go b/pkg/server/router/tcp/router.go index 551bb43e7..0d8524398 100644 --- a/pkg/server/router/tcp/router.go +++ b/pkg/server/router/tcp/router.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "slices" + "time" "github.com/go-acme/lego/v4/challenge/tlsalpn01" "github.com/traefik/traefik/v2/pkg/log" @@ -116,6 +117,17 @@ 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 b5aaa1ac0..1fb371d08 100644 --- a/pkg/server/server_entrypoint_tcp.go +++ b/pkg/server/server_entrypoint_tcp.go @@ -244,15 +244,24 @@ 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)) }) } @@ -382,55 +391,6 @@ 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 { @@ -459,7 +419,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.HTTP.ReadTimeout + timeout := entryPoint.Transport.RespondingTimeouts.ReadTimeout // proxyproto use 200ms if ReadHeaderTimeout is set to 0 and not no timeout if timeout == 0 { timeout = -1 @@ -628,9 +588,9 @@ func createHTTPServer(ctx context.Context, ln net.Listener, configuration *stati serverHTTP := &http.Server{ Handler: handler, ErrorLog: httpServerLogger, - ReadTimeout: time.Duration(*configuration.Transport.RespondingTimeouts.HTTP.ReadTimeout), - WriteTimeout: time.Duration(*configuration.Transport.RespondingTimeouts.HTTP.WriteTimeout), - IdleTimeout: time.Duration(*configuration.Transport.RespondingTimeouts.HTTP.IdleTimeout), + ReadTimeout: time.Duration(configuration.Transport.RespondingTimeouts.ReadTimeout), + WriteTimeout: time.Duration(configuration.Transport.RespondingTimeouts.WriteTimeout), + IdleTimeout: time.Duration(configuration.Transport.RespondingTimeouts.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 6b177099f..f0b12c8dd 100644 --- a/pkg/server/server_entrypoint_tcp_test.go +++ b/pkg/server/server_entrypoint_tcp_test.go @@ -69,10 +69,8 @@ func testShutdown(t *testing.T, router *tcprouter.Router) { epConfig.LifeCycle.RequestAcceptGraceTimeout = 0 epConfig.LifeCycle.GraceTimeOut = 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 + epConfig.RespondingTimeouts.ReadTimeout = ptypes.Duration(5 * time.Second) + epConfig.RespondingTimeouts.WriteTimeout = ptypes.Duration(5 * time.Second) entryPoint, err := NewTCPEntryPoint(context.Background(), &static.EntryPoint{ // We explicitly use an IPV4 address because on Alpine, with an IPV6 address @@ -159,8 +157,7 @@ func startEntrypoint(entryPoint *TCPEntryPoint, router *tcprouter.Router) (net.C func TestReadTimeoutWithoutFirstByte(t *testing.T) { epConfig := &static.EntryPointsTransport{} epConfig.SetDefaults() - readTimeout := ptypes.Duration(2 * time.Second) - epConfig.RespondingTimeouts.HTTP.ReadTimeout = &readTimeout + epConfig.RespondingTimeouts.ReadTimeout = ptypes.Duration(2 * time.Second) entryPoint, err := NewTCPEntryPoint(context.Background(), &static.EntryPoint{ Address: ":0", @@ -197,84 +194,7 @@ func TestReadTimeoutWithoutFirstByte(t *testing.T) { func TestReadTimeoutWithFirstByte(t *testing.T) { epConfig := &static.EntryPointsTransport{} epConfig.SetDefaults() - 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) + epConfig.RespondingTimeouts.ReadTimeout = ptypes.Duration(2 * time.Second) entryPoint, err := NewTCPEntryPoint(context.Background(), &static.EntryPoint{ Address: ":0",