Upgrade go-connlimit to v0.3.0 / return http 429 on too many connections (#8221)

Fixes #7527

I want to highlight this and explain what I think the implications are and make sure we are aware:

* `HTTPConnStateFunc` closes the connection when it is beyond the limit. `Close` does not block.
* `HTTPConnStateFuncWithDefault429Handler(10 * time.Millisecond)` blocks until the following is done (worst case):
  1) `conn.SetDeadline(10*time.Millisecond)` so that
  2) `conn.Write(429error)` is guaranteed to timeout after 10ms, so that the http 429 can be written and 
  3) `conn.Close` can happen

The implication of this change is that accepting any new connection is worst case delayed by 10ms. But only after a client reached the limit already.

agent/agent.go

@@ -1164,7 +1164,7 @@ func (a *Agent) listenHTTP() ([]*HTTPServer, error) {
 			srv.Server.Handler = srv.handler(a.config.EnableDebug)
 
 			// Load the connlimit helper into the server
-			connLimitFn := a.httpConnLimiter.HTTPConnStateFunc()
+			connLimitFn := a.httpConnLimiter.HTTPConnStateFuncWithDefault429Handler(10 * time.Millisecond)
 
 			if proto == "https" {
 				// Enforce TLS handshake timeout

go.mod

@@ -34,7 +34,7 @@ require (
 	github.com/hashicorp/go-bexpr v0.1.2
 	github.com/hashicorp/go-checkpoint v0.0.0-20171009173528-1545e56e46de
 	github.com/hashicorp/go-cleanhttp v0.5.1
-	github.com/hashicorp/go-connlimit v0.2.0
+	github.com/hashicorp/go-connlimit v0.3.0
 	github.com/hashicorp/go-discover v0.0.0-20200501174627-ad1e96bde088
 	github.com/hashicorp/go-hclog v0.12.0
 	github.com/hashicorp/go-memdb v1.1.0

go.sum

@@ -205,8 +205,8 @@ github.com/hashicorp/go-checkpoint v0.0.0-20171009173528-1545e56e46de/go.mod h1:
 github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM=
 github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
-github.com/hashicorp/go-connlimit v0.2.0 h1:OZjcfNxH/hPh/bT2Iw5yOJcLzz+zuIWpsp3I1S4Pjw4=
-github.com/hashicorp/go-connlimit v0.2.0/go.mod h1:OUj9FGL1tPIhl/2RCfzYHrIiWj+VVPGNyVPnUX8AqS0=
+github.com/hashicorp/go-connlimit v0.3.0 h1:oAojHGjFxUTTTA8c5XXnDqWJ2HLuWbDiBPTpWvNzvqM=
+github.com/hashicorp/go-connlimit v0.3.0/go.mod h1:OUj9FGL1tPIhl/2RCfzYHrIiWj+VVPGNyVPnUX8AqS0=
 github.com/hashicorp/go-discover v0.0.0-20200501174627-ad1e96bde088 h1:jBvElOilnIl6mm8S6gva/dfeTJCcMs9TGO6/2C6k52E=
 github.com/hashicorp/go-discover v0.0.0-20200501174627-ad1e96bde088/go.mod h1:vZu6Opqf49xX5lsFAu7iFNewkcVF1sn/wyapZh5ytlg=
 github.com/hashicorp/go-hclog v0.0.0-20180709165350-ff2cf002a8dd/go.mod h1:9bjs9uLqI8l75knNv3lV1kA55veR+WUPSiKIWcQHudI=

vendor/github.com/hashicorp/go-connlimit/README.md

@@ -11,7 +11,7 @@ the resources that can be consumed by a single client.
 
 ### TCP Server
 
-```
+```go
 // During server setup:
 s.limiter = NewLimiter(Config{
   MaxConnsPerClientIP: 10,
@@ -19,7 +19,7 @@ s.limiter = NewLimiter(Config{
 
 ```
 
-```
+```go
 // handleConn is called in its own goroutine for each net.Conn accepted by
 // a net.Listener.
 func (s *Server) handleConn(conn net.Conn) {
@@ -53,7 +53,7 @@ func (s *Server) handleConn(conn net.Conn) {
 
 ### HTTP Server
 
-```
+```go
 lim := NewLimiter(Config{
   MaxConnsPerClientIP: 10,
 })

vendor/github.com/hashicorp/go-connlimit/connlimit.go

@@ -2,16 +2,23 @@ package connlimit
 
 import (
 	"errors"
+	"fmt"
 	"net"
 	"net/http"
 	"sync"
 	"sync/atomic"
+	"time"
 )
 
 var (
 	// ErrPerClientIPLimitReached is returned if accepting a new conn would exceed
 	// the per-client-ip limit set.
 	ErrPerClientIPLimitReached = errors.New("client connection limit reached")
+	tooManyConnsMsg            = "Your IP is issuing too many concurrent connections, please rate limit your calls\n"
+	tooManyRequestsResponse    = []byte(fmt.Sprintf("HTTP/1.1 429 Too Many Requests\r\n"+
+		"Content-Type: text/plain\r\n"+
+		"Content-Length: %d\r\n"+
+		"Connection: close\r\n\r\n%s", len(tooManyConnsMsg), tooManyConnsMsg))
 )
 
 // Limiter implements a simple limiter that tracks the number of connections
@@ -173,7 +180,7 @@ func (l *Limiter) SetConfig(c Config) {
 	l.cfg.Store(c)
 }
 
-// HTTPConnStateFunc returns a func that can be passed as the ConnState field of
+// HTTPConnStateFuncWithErrorHandler returns a func that can be passed as the ConnState field of
 // an http.Server. This intercepts new HTTP connections to the server and
 // applies the limiting to new connections.
 //
@@ -181,13 +188,15 @@ func (l *Limiter) SetConfig(c Config) {
 // in the limiter as if it was closed. Servers that use Hijacking must implement
 // their own calls if they need to continue limiting the number of concurrent
 // hijacked connections.
-func (l *Limiter) HTTPConnStateFunc() func(net.Conn, http.ConnState) {
+// errorHandler MUST close the connection itself
+func (l *Limiter) HTTPConnStateFuncWithErrorHandler(errorHandler func(error, net.Conn)) func(net.Conn, http.ConnState) {
+
 	return func(conn net.Conn, state http.ConnState) {
 		switch state {
 		case http.StateNew:
 			_, err := l.Accept(conn)
 			if err != nil {
-				conn.Close()
+				errorHandler(err, conn)
 			}
 		case http.StateHijacked:
 			l.freeConn(conn)
@@ -199,3 +208,26 @@ func (l *Limiter) HTTPConnStateFunc() func(net.Conn, http.ConnState) {
 		}
 	}
 }
+
+// HTTPConnStateFunc is here for ascending compatibility reasons.
+func (l *Limiter) HTTPConnStateFunc() func(net.Conn, http.ConnState) {
+	return l.HTTPConnStateFuncWithErrorHandler(func(err error, conn net.Conn) {
+		conn.Close()
+	})
+}
+
+// HTTPConnStateFuncWithDefault429Handler return an HTTP 429 if too many connections occur.
+// BEWARE that returning HTTP 429 is done on critical path, you might choose to use
+// HTTPConnStateFuncWithErrorHandler if you want to use a non-blocking strategy.
+func (l *Limiter) HTTPConnStateFuncWithDefault429Handler(writeDeadlineMaxDelay time.Duration) func(net.Conn, http.ConnState) {
+	return l.HTTPConnStateFuncWithErrorHandler(func(err error, conn net.Conn) {
+		if err == ErrPerClientIPLimitReached {
+			// We don't care about slow players
+			if writeDeadlineMaxDelay > 0 {
+				conn.SetDeadline(time.Now().Add(writeDeadlineMaxDelay))
+			}
+			conn.Write(tooManyRequestsResponse)
+		}
+		conn.Close()
+	})
+}

vendor/modules.txt

@@ -197,7 +197,7 @@ github.com/hashicorp/go-bexpr
 github.com/hashicorp/go-checkpoint
 # github.com/hashicorp/go-cleanhttp v0.5.1
 github.com/hashicorp/go-cleanhttp
-# github.com/hashicorp/go-connlimit v0.2.0
+# github.com/hashicorp/go-connlimit v0.3.0
 github.com/hashicorp/go-connlimit
 # github.com/hashicorp/go-discover v0.0.0-20200501174627-ad1e96bde088
 github.com/hashicorp/go-discover