[8.x](backport #4402) Use chi/middleware.Throttle instead of custom conn limiter

changelog/fragments/1738613859-Return-HTTP-429-when-conn-limit-is-reached.yaml

@@ -0,0 +1,33 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# Change summary; a 80ish characters long description of the change.
+summary: Return HTTP 429 when conn limit is reached
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+description: |
+  fleet-server will return a 429 instead of silently close connections when the conn limit is reached.
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: fleet-server
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+#pr: https://github.com/owner/repo/1234
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+issue: https://github.com/elastic/fleet-server/issues/4200

fleet-server.reference.yml

@@ -149,7 +149,7 @@ fleet:
 #       policy_throttle: 5ms # 1ms min is forced
 #       # max_header_byte_size is the request header size limit
 #       max_header_byte_size: 8192 # 8Kib
-#       # max_connections is the maximum number of connnections per API endpoint
+#       # max_connections is the maximum number of in-flight HTTP requests per API listener
 #       max_connections: 0
 #
 #       # action_limit is a limiter for the action dispatcher, it is added to control how fast the checkin endpoint writes responses when an action effecting multiple agents is detected.

internal/pkg/api/router.go

@@ -9,14 +9,15 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/elastic/fleet-server/v7/internal/pkg/config"
-	"github.com/elastic/fleet-server/v7/internal/pkg/limit"
-	"github.com/elastic/fleet-server/v7/internal/pkg/logger"
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
 	"github.com/rs/zerolog"
 	"go.elastic.co/apm/module/apmchiv5/v2"
 	"go.elastic.co/apm/v2"
+
+	"github.com/elastic/fleet-server/v7/internal/pkg/config"
+	"github.com/elastic/fleet-server/v7/internal/pkg/limit"
+	"github.com/elastic/fleet-server/v7/internal/pkg/logger"
 )
 
 func newRouter(cfg *config.ServerLimits, si ServerInterface, tracer *apm.Tracer) http.Handler {
@@ -26,6 +27,9 @@ func newRouter(cfg *config.ServerLimits, si ServerInterface, tracer *apm.Tracer)
 	}
 	r.Use(logger.Middleware) // Attach middlewares to router directly so the occur before any request parsing/validation
 	r.Use(middleware.Recoverer)
+	if cfg.MaxConnections > 0 {
+		r.Use(middleware.Throttle(cfg.MaxConnections))
+	}
 	r.Use(Limiter(cfg).middleware)
 	return HandlerWithOptions(si, ChiServerOptions{
 		BaseRouter:       r,

internal/pkg/api/server.go

@@ -15,7 +15,6 @@ import (
 
 	"github.com/elastic/elastic-agent-libs/transport/tlscommon"
 	"github.com/elastic/fleet-server/v7/internal/pkg/config"
-	"github.com/elastic/fleet-server/v7/internal/pkg/limit"
 	"github.com/elastic/fleet-server/v7/internal/pkg/logger"
 
 	"github.com/rs/zerolog"
@@ -29,7 +28,7 @@ type server struct {
 
 // NewServer creates a new HTTP api for the passed addr.
 //
-// The server has a listener specific conn limit and endpoint specific rate-limits.
+// The server has an http request limit and endpoint specific rate-limits.
 // The underlying API structs (such as *CheckinT) may be shared between servers.
 func NewServer(addr string, cfg *config.Server, opts ...APIOpt) *server {
 	a := &apiServer{}
@@ -76,13 +75,6 @@ func (s *server) Run(ctx context.Context) error {
 		}
 	}()
 
-	// 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)
-
 	if s.cfg.TLS != nil && s.cfg.TLS.IsEnabled() {
 		commonTLSCfg, err := tlscommon.LoadTLSServerConfig(s.cfg.TLS)
 		if err != nil {
@@ -151,22 +143,6 @@ func getDiagConnFunc(ctx context.Context) func(c net.Conn, s http.ConnState) {
 	}
 }
 
-func wrapConnLimitter(ctx context.Context, ln net.Listener, cfg *config.Server) net.Listener {
-	hardLimit := cfg.Limits.MaxConnections
-
-	if hardLimit != 0 {
-		zerolog.Ctx(ctx).Info().
-			Int("hardConnLimit", hardLimit).
-			Msg("server hard connection limiter installed")
-
-		ln = limit.Listener(ln, hardLimit, zerolog.Ctx(ctx))
-	} else {
-		zerolog.Ctx(ctx).Info().Msg("server hard connection limiter disabled")
-	}
-
-	return ln
-}
-
 type stubLogger struct {
 	log zerolog.Logger
 }