add configurable 'shutdown-timeout' for HTTP server (#13771)

fixes #12317

cmd/gateway-main.go

@@ -20,7 +20,8 @@ package cmd
 import (
 	"context"
 	"fmt"
-	"net"
+	"io/ioutil"
+	"log"
 	"net/url"
 	"os"
 	"os/signal"
@@ -269,10 +270,13 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 		addrs = append(addrs, globalMinioAddr)
 	}
 
-	httpServer := xhttp.NewServer(addrs, setCriticalErrorHandler(corsHandler(router)), getCert)
-	httpServer.BaseContext = func(listener net.Listener) context.Context {
-		return GlobalContext
-	}
+	httpServer := xhttp.NewServer(addrs).
+		UseHandler(setCriticalErrorHandler(corsHandler(router))).
+		UseTLSConfig(newTLSConfig(getCert)).
+		UseShutdownTimeout(ctx.Duration("shutdown-timeout")).
+		UseBaseContext(GlobalContext).
+		UseCustomLogger(log.New(ioutil.Discard, "", 0)) // Turn-off random logging by Go stdlib
+
 	go func() {
 		globalHTTPServerErrorCh <- httpServer.Start(GlobalContext)
 	}()

cmd/server-main.go

@@ -23,9 +23,9 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"log"
 	"math/rand"
-	"net"
 	"os"
 	"os/signal"
 	"strings"
@@ -64,6 +64,12 @@ var ServerFlags = []cli.Flag{
 		Name:  "console-address",
 		Usage: "bind to a specific ADDRESS:PORT for embedded Console UI, ADDRESS can be an IP or hostname",
 	},
+	cli.DurationFlag{
+		Name:   "shutdown-timeout",
+		Value:  xhttp.DefaultShutdownTimeout,
+		Usage:  "shutdown timeout to gracefully shutdown server",
+		Hidden: true,
+	},
 }
 
 var serverCmd = cli.Command{
@@ -412,12 +418,6 @@ func initConfigSubsystem(ctx context.Context, newObject ObjectLayer) ([]BucketIn
 	return buckets, nil
 }
 
-type nullWriter struct{}
-
-func (lw nullWriter) Write(b []byte) (int, error) {
-	return len(b), nil
-}
-
 // serverMain handler called for 'minio server' command.
 func serverMain(ctx *cli.Context) {
 	signal.Notify(globalOSSignalCh, os.Interrupt, syscall.SIGTERM, syscall.SIGQUIT)
@@ -492,12 +492,13 @@ func serverMain(ctx *cli.Context) {
 		addrs = append(addrs, globalMinioAddr)
 	}
 
-	httpServer := xhttp.NewServer(addrs, setCriticalErrorHandler(corsHandler(handler)), getCert)
-	httpServer.BaseContext = func(listener net.Listener) context.Context {
-		return GlobalContext
-	}
-	// Turn-off random logging by Go internally
-	httpServer.ErrorLog = log.New(&nullWriter{}, "", 0)
+	httpServer := xhttp.NewServer(addrs).
+		UseHandler(setCriticalErrorHandler(corsHandler(handler))).
+		UseTLSConfig(newTLSConfig(getCert)).
+		UseShutdownTimeout(ctx.Duration("shutdown-timeout")).
+		UseBaseContext(GlobalContext).
+		UseCustomLogger(log.New(ioutil.Discard, "", 0)) // Turn-off random logging by Go stdlib
+
 	go func() {
 		globalHTTPServerErrorCh <- httpServer.Start(GlobalContext)
 	}()

cmd/utils.go

@@ -46,12 +46,17 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/minio/madmin-go"
 	miniogopolicy "github.com/minio/minio-go/v7/pkg/policy"
+	"github.com/minio/minio/internal/config"
+	"github.com/minio/minio/internal/config/api"
+	xtls "github.com/minio/minio/internal/config/identity/tls"
+	"github.com/minio/minio/internal/fips"
 	"github.com/minio/minio/internal/handlers"
 	xhttp "github.com/minio/minio/internal/http"
 	"github.com/minio/minio/internal/logger"
 	"github.com/minio/minio/internal/logger/message/audit"
 	"github.com/minio/minio/internal/rest"
 	"github.com/minio/pkg/certs"
+	"github.com/minio/pkg/env"
 )
 
 const (
@@ -1097,3 +1102,34 @@ func speedTest(ctx context.Context, opts speedTestOpts) chan madmin.SpeedTestRes
 	}()
 	return ch
 }
+
+func newTLSConfig(getCert certs.GetCertificateFunc) *tls.Config {
+	if getCert == nil {
+		return nil
+	}
+
+	tlsConfig := &tls.Config{
+		PreferServerCipherSuites: true,
+		MinVersion:               tls.VersionTLS12,
+		NextProtos:               []string{"http/1.1", "h2"},
+		GetCertificate:           getCert,
+	}
+
+	tlsClientIdentity := env.Get(xtls.EnvIdentityTLSEnabled, "") == config.EnableOn
+	if tlsClientIdentity {
+		tlsConfig.ClientAuth = tls.RequestClientCert
+	}
+
+	secureCiphers := env.Get(api.EnvAPISecureCiphers, config.EnableOn) == config.EnableOn
+	if secureCiphers || fips.Enabled {
+		// Hardened ciphers
+		tlsConfig.CipherSuites = fips.CipherSuitesTLS()
+		tlsConfig.CurvePreferences = fips.EllipticCurvesTLS()
+	} else {
+		// Default ciphers while excluding those with security issues
+		for _, cipher := range tls.CipherSuites() {
+			tlsConfig.CipherSuites = append(tlsConfig.CipherSuites, cipher.ID)
+		}
+	}
+	return tlsConfig
+}

internal/http/server.go

@@ -22,6 +22,8 @@ import (
 	"crypto/tls"
 	"errors"
 	"io/ioutil"
+	"log"
+	"net"
 	"net/http"
 	"runtime/pprof"
 	"sync"
@@ -29,19 +31,12 @@ import (
 	"time"
 
 	humanize "github.com/dustin/go-humanize"
-
-	"github.com/minio/minio/internal/config"
-	"github.com/minio/minio/internal/config/api"
-	xtls "github.com/minio/minio/internal/config/identity/tls"
-	"github.com/minio/minio/internal/fips"
-	"github.com/minio/pkg/certs"
-	"github.com/minio/pkg/env"
 )
 
 const (
 	serverShutdownPoll = 500 * time.Millisecond
 
-	// DefaultShutdownTimeout - default shutdown timeout used for graceful http server shutdown.
+	// DefaultShutdownTimeout - default shutdown timeout to gracefully shutdown server.
 	DefaultShutdownTimeout = 5 * time.Second
 
 	// DefaultMaxHeaderBytes - default maximum HTTP header size in bytes.
@@ -160,42 +155,44 @@ func (srv *Server) Shutdown() error {
 	}
 }
 
+// UseShutdownTimeout configure server shutdown timeout
+func (srv *Server) UseShutdownTimeout(d time.Duration) *Server {
+	srv.ShutdownTimeout = d
+	return srv
+}
+
+// UseHandler configure final handler for this HTTP *Server
+func (srv *Server) UseHandler(h http.Handler) *Server {
+	srv.Handler = h
+	return srv
+}
+
+// UseTLSConfig pass configured TLSConfig for this HTTP *Server
+func (srv *Server) UseTLSConfig(cfg *tls.Config) *Server {
+	srv.TLSConfig = cfg
+	return srv
+}
+
+// UseBaseContext use custom base context for this HTTP *Server
+func (srv *Server) UseBaseContext(ctx context.Context) *Server {
+	srv.BaseContext = func(listener net.Listener) context.Context {
+		return ctx
+	}
+	return srv
+}
+
+// UseCustomLogger use customized logger for this HTTP *Server
+func (srv *Server) UseCustomLogger(l *log.Logger) *Server {
+	srv.ErrorLog = l
+	return srv
+}
+
 // NewServer - creates new HTTP server using given arguments.
-func NewServer(addrs []string, handler http.Handler, getCert certs.GetCertificateFunc) *Server {
-	secureCiphers := env.Get(api.EnvAPISecureCiphers, config.EnableOn) == config.EnableOn
-	var tlsConfig *tls.Config
-	if getCert != nil {
-		tlsConfig = &tls.Config{
-			PreferServerCipherSuites: true,
-			MinVersion:               tls.VersionTLS12,
-			NextProtos:               []string{"http/1.1", "h2"},
-			GetCertificate:           getCert,
-		}
-
-		tlsClientIdentity := env.Get(xtls.EnvIdentityTLSEnabled, "") == config.EnableOn
-		if tlsClientIdentity {
-			tlsConfig.ClientAuth = tls.RequestClientCert
-		}
-
-		if secureCiphers || fips.Enabled {
-			// Hardened ciphers
-			tlsConfig.CipherSuites = fips.CipherSuitesTLS()
-			tlsConfig.CurvePreferences = fips.EllipticCurvesTLS()
-		} else {
-			// Default ciphers while excluding those with security issues
-			for _, cipher := range tls.CipherSuites() {
-				tlsConfig.CipherSuites = append(tlsConfig.CipherSuites, cipher.ID)
-			}
-		}
-	}
-
+func NewServer(addrs []string) *Server {
 	httpServer := &Server{
-		Addrs:           addrs,
-		ShutdownTimeout: DefaultShutdownTimeout,
+		Addrs: addrs,
 	}
-	httpServer.Handler = handler
-	httpServer.TLSConfig = tlsConfig
+	// This is not configurable for now.
 	httpServer.MaxHeaderBytes = DefaultMaxHeaderBytes
-
 	return httpServer
 }

internal/http/server_test.go

@@ -18,6 +18,7 @@
 package http
 
 import (
+	"crypto/tls"
 	"fmt"
 	"net/http"
 	"reflect"
@@ -46,7 +47,15 @@ func TestNewServer(t *testing.T) {
 	}
 
 	for i, testCase := range testCases {
-		server := NewServer(testCase.addrs, testCase.handler, testCase.certFn)
+		server := NewServer(testCase.addrs).
+			UseHandler(testCase.handler).
+			UseShutdownTimeout(DefaultShutdownTimeout)
+		if testCase.certFn != nil {
+			server = server.UseTLSConfig(&tls.Config{
+				PreferServerCipherSuites: true,
+				GetCertificate:           testCase.certFn,
+			})
+		}
 		if server == nil {
 			t.Fatalf("Case %v: server: expected: <non-nil>, got: <nil>", (i + 1))
 		}