1
0
Fork 0

Revert LingeringTimeout and change default value for ReadTimeout

Co-authored-by: Romain <rtribotte@users.noreply.github.com>
This commit is contained in:
Kevin Pollet 2024-04-11 15:48:04 +02:00 committed by GitHub
parent d53f5f01a0
commit 099c7e9444
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 125 additions and 389 deletions

View file

@ -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
}

View file

@ -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"
}
}
}

View file

@ -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
}

View file

@ -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{

View file

@ -26,14 +26,9 @@
"graceTimeOut": "1m51s"
},
"respondingTimeouts": {
"http": {
"readTimeout": "1m51s",
"writeTimeout": "1m51s",
"idleTimeout": "1m51s"
},
"tcp": {
"lingeringTimeout": "1m51s"
}
"readTimeout": "1m51s",
"writeTimeout": "1m51s",
"idleTimeout": "1m51s"
}
},
"proxyProtocol": {

View file

@ -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)

View file

@ -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 {

View file

@ -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",