Fleet-Server abruptly closes connection when limit is reached instead of returning TooManyRequests (429)

[belimawr]

When limits.max_connections is set, Fleet-Server will abruptly close connections, which makes the clients (usually Elastic-Agent) to get a read: connection reset by peer. This is not helpful and does not enable the client to correctly adapt its behaviour.

The problem comes from the limitListener (L 69):

fleet-server/internal/pkg/limit/listener.go

Lines 55 to 80 in 8ff01e3

	func (l *limitListener) Accept() (net.Conn, error) {
	// Accept the connection irregardless
	c, err := l.Listener.Accept()
	if err != nil {
	return nil, err
	}
	// If we cannot acquire the semaphore, close the connection
	if acquired := l.acquire(); !acquired {
	zlog := log.Warn()
	var err error
	if c != nil {
	err = c.Close()
	zlog.Str(logger.ECSServerAddress, c.LocalAddr().String())
	zlog.Str(logger.ECSClientAddress, c.RemoteAddr().String())
	zlog.Err(err)
	}
	zlog.Int("max", cap(l.sem)).Msg("Connection closed due to max limit")
	return c, nil
	}
	return &limitListenerConn{Conn: c, release: l.release}, nil
	}

If there is a proxy in front of Fleet-Server this can cause a very hard situation to debug from the client's (e.g: Elastic-Agent) perspective: Fleet Server gets a connection that is over the limit, closes the connection, which results in an EOF for the proxy, which translates it into a 502, which the elastic-agent enrol command swallows and does not display.

[cmacknz]

Our cloud proxy will specifically translate an EOF from the fleet-server backend into a 502, which leads us to searching the Fleet Server code for places where 502 could be returned that don't exist.

[michel-laterman]

If we wanted to change fleet-server's behaviour when the connection limit is reached to return a proper HTTP response, we would need to move our connection tracking; however we currently have the assumption that we track connections before the TLS handshake.

fleet-server/internal/pkg/api/server.go

Lines 79 to 84 in 8ff01e3

	// Conn Limiter must be before the TLS handshake in the stack;
	// The server should not eat the cost of the handshake if there
	// is no capacity to service the connection.
	// Also, it appears the HTTP2 implementation depends on the tls.Listener
	// being at the top of the stack.
	ln = wrapConnLimitter(ctx, ln, s.cfg)

Changing how we do this will have performance implications

[blakerouse]

It was specifically implemented this way to not require TLS handshake of max connections.