update x/net/http2 to address few bugs (#11144)

additionally also configure http2 healthcheck
values to quickly detect unstable connections
and let them timeout.

also use single transport for proxying requests
This commit is contained in:
Harshavardhana 2020-12-21 21:42:38 -08:00 committed by GitHub
parent c987313431
commit 5c451d1690
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 78 additions and 57 deletions

View File

@ -388,7 +388,7 @@ func getObjectLocation(r *http.Request, domains []string, bucket, object string)
}
proto := handlers.GetSourceScheme(r)
if proto == "" {
proto = getURLScheme(globalIsSSL)
proto = getURLScheme(globalIsTLS)
}
u := &url.URL{
Host: r.Host,

View File

@ -350,7 +350,7 @@ func (sys *BucketTargetSys) getRemoteTargetClient(tcfg *madmin.BucketTarget) (*m
creds := credentials.NewStaticV4(config.AccessKey, config.SecretKey, "")
getRemoteTargetInstanceTransportOnce.Do(func() {
getRemoteTargetInstanceTransport = newGatewayHTTPTransport(1 * time.Hour)
getRemoteTargetInstanceTransport = newGatewayHTTPTransport(10 * time.Minute)
})
core, err := miniogo.NewCore(tcfg.URL().Host, &miniogo.Options{

View File

@ -57,8 +57,8 @@ var encryptRequestTests = []struct {
}
func TestEncryptRequest(t *testing.T) {
defer func(flag bool) { globalIsSSL = flag }(globalIsSSL)
globalIsSSL = true
defer func(flag bool) { globalIsTLS = flag }(globalIsTLS)
globalIsTLS = true
for i, test := range encryptRequestTests {
content := bytes.NewReader(make([]byte, 64))
req := &http.Request{Header: http.Header{}}

View File

@ -18,7 +18,6 @@ package cmd
import (
"context"
"crypto/tls"
"errors"
"fmt"
"net"
@ -38,7 +37,6 @@ import (
"github.com/minio/minio/cmd/config"
xhttp "github.com/minio/minio/cmd/http"
"github.com/minio/minio/cmd/logger"
"github.com/minio/minio/cmd/rest"
"github.com/minio/minio/pkg/env"
"github.com/minio/minio/pkg/mountinfo"
xnet "github.com/minio/minio/pkg/net"
@ -59,7 +57,7 @@ const (
// See proxyRequest() for details.
type ProxyEndpoint struct {
Endpoint
Transport *http.Transport
Transport http.RoundTripper
}
// Endpoint - any type of endpoint.
@ -881,20 +879,9 @@ func GetProxyEndpoints(endpointServerPools EndpointServerPools) []ProxyEndpoint
}
proxyEpSet.Add(host)
var tlsConfig *tls.Config
if globalIsSSL {
tlsConfig = &tls.Config{
ServerName: endpoint.Hostname(),
RootCAs: globalRootCAs,
}
}
// allow transport to be HTTP/1.1 for proxying.
tr := newCustomHTTPProxyTransport(tlsConfig, rest.DefaultTimeout)()
proxyEps = append(proxyEps, ProxyEndpoint{
Endpoint: endpoint,
Transport: tr,
Transport: globalProxyTransport,
})
}
}

View File

@ -179,7 +179,7 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
// Check and load TLS certificates.
var err error
globalPublicCerts, globalTLSCerts, globalIsSSL, err = getTLSConfig()
globalPublicCerts, globalTLSCerts, globalIsTLS, err = getTLSConfig()
logger.FatalIf(err, "Invalid TLS certificate file")
// Check and load Root CAs.
@ -211,7 +211,7 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
if host == "" {
host = sortIPs(localIP4.ToSlice())[0]
}
return fmt.Sprintf("%s://%s", getURLScheme(globalIsSSL), net.JoinHostPort(host, globalMinioPort))
return fmt.Sprintf("%s://%s", getURLScheme(globalIsTLS), net.JoinHostPort(host, globalMinioPort))
}()
// Handle gateway specific env

View File

@ -44,7 +44,7 @@ func printGatewayStartupMessage(apiEndPoints []string, backendType string) {
// SSL is configured reads certification chain, prints
// authority and expiry.
if color.IsTerminal() && !globalCLIContext.Anonymous {
if globalIsSSL {
if globalIsTLS {
printCertificateMsg(globalPublicCerts)
}
}

View File

@ -665,7 +665,7 @@ func (f bucketForwardingHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
}
if globalDomainIPs.Intersection(set.CreateStringSet(getHostsSlice(sr)...)).IsEmpty() {
r.URL.Scheme = "http"
if globalIsSSL {
if globalIsTLS {
r.URL.Scheme = "https"
}
r.URL.Host = getHostFromSrv(sr)
@ -715,7 +715,7 @@ func (f bucketForwardingHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
}
if globalDomainIPs.Intersection(set.CreateStringSet(getHostsSlice(sr)...)).IsEmpty() {
r.URL.Scheme = "http"
if globalIsSSL {
if globalIsTLS {
r.URL.Scheme = "https"
}
r.URL.Host = getHostFromSrv(sr)
@ -798,7 +798,7 @@ type sseTLSHandler struct{ handler http.Handler }
func (h sseTLSHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// Deny SSE-C requests if not made over TLS
if !globalIsSSL && (crypto.SSEC.IsRequested(r.Header) || crypto.SSECopy.IsRequested(r.Header)) {
if !globalIsTLS && (crypto.SSEC.IsRequested(r.Header) || crypto.SSECopy.IsRequested(r.Header)) {
if r.Method == http.MethodHead {
writeErrorResponseHeadersOnly(w, errorCodes.ToAPIErr(ErrInsecureSSECustomerRequest))
} else {

View File

@ -225,13 +225,13 @@ var sseTLSHandlerTests = []struct {
}
func TestSSETLSHandler(t *testing.T) {
defer func(isSSL bool) { globalIsSSL = isSSL }(globalIsSSL) // reset globalIsSSL after test
defer func(isSSL bool) { globalIsTLS = isSSL }(globalIsTLS) // reset globalIsTLS after test
var okHandler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}
for i, test := range sseTLSHandlerTests {
globalIsSSL = test.IsTLS
globalIsTLS = test.IsTLS
w := httptest.NewRecorder()
r := new(http.Request)

View File

@ -171,7 +171,7 @@ var (
globalRootCAs *x509.CertPool
// IsSSL indicates if the server is configured with SSL.
globalIsSSL bool
globalIsTLS bool
globalTLSCerts *certs.Manager
@ -280,6 +280,8 @@ var (
globalInternodeTransport http.RoundTripper
globalProxyTransport http.RoundTripper
globalDNSCache *xhttp.DNSCache
// Add new variable global values here.
)

View File

@ -516,7 +516,7 @@ func proxyRequest(ctx context.Context, w http.ResponseWriter, r *http.Request, e
})
r.URL.Scheme = "http"
if globalIsSSL {
if globalIsTLS {
r.URL.Scheme = "https"
}

View File

@ -165,7 +165,7 @@ func getAPIEndpoints() (apiEndpoints []string) {
}
for _, ip := range ipList {
endpoint := fmt.Sprintf("%s://%s", getURLScheme(globalIsSSL), net.JoinHostPort(ip, globalMinioPort))
endpoint := fmt.Sprintf("%s://%s", getURLScheme(globalIsTLS), net.JoinHostPort(ip, globalMinioPort))
apiEndpoints = append(apiEndpoints, endpoint)
}

View File

@ -752,7 +752,7 @@ var getRemoteInstanceClient = func(r *http.Request, host string) (*miniogo.Core,
// and hence expected to have same credentials.
core, err := miniogo.NewCore(host, &miniogo.Options{
Creds: credentials.NewStaticV4(cred.AccessKey, cred.SecretKey, ""),
Secure: globalIsSSL,
Secure: globalIsTLS,
Transport: getRemoteInstanceTransport,
})
if err != nil {

View File

@ -213,8 +213,8 @@ func testAPIHeadObjectHandlerWithEncryption(obj ObjectLayer, instanceType, bucke
credentials auth.Credentials, t *testing.T) {
// Set SSL to on to do encryption tests
globalIsSSL = true
defer func() { globalIsSSL = false }()
globalIsTLS = true
defer func() { globalIsTLS = false }()
var (
oneMiB int64 = 1024 * 1024
@ -659,8 +659,8 @@ func testAPIGetObjectWithMPHandler(obj ObjectLayer, instanceType, bucketName str
credentials auth.Credentials, t *testing.T) {
// Set SSL to on to do encryption tests
globalIsSSL = true
defer func() { globalIsSSL = false }()
globalIsTLS = true
defer func() { globalIsTLS = false }()
var (
oneMiB int64 = 1024 * 1024
@ -857,8 +857,8 @@ func testAPIGetObjectWithPartNumberHandler(obj ObjectLayer, instanceType, bucket
credentials auth.Credentials, t *testing.T) {
// Set SSL to on to do encryption tests
globalIsSSL = true
defer func() { globalIsSSL = false }()
globalIsTLS = true
defer func() { globalIsTLS = false }()
var (
oneMiB int64 = 1024 * 1024

View File

@ -859,7 +859,7 @@ func newPeerRestClients(endpoints EndpointServerPools) (remote, all []*peerRESTC
// Returns a peer rest client.
func newPeerRESTClient(peer *xnet.Host) *peerRESTClient {
scheme := "http"
if globalIsSSL {
if globalIsTLS {
scheme = "https"
}

View File

@ -173,7 +173,7 @@ func IsServerResolvable(endpoint Endpoint) error {
}
var tlsConfig *tls.Config
if globalIsSSL {
if globalIsTLS {
tlsConfig = &tls.Config{
ServerName: endpoint.Hostname(),
RootCAs: globalRootCAs,

View File

@ -119,7 +119,7 @@ func serverHandleCmdArgs(ctx *cli.Context) {
var setupType SetupType
// Check and load TLS certificates.
globalPublicCerts, globalTLSCerts, globalIsSSL, err = getTLSConfig()
globalPublicCerts, globalTLSCerts, globalIsTLS, err = getTLSConfig()
logger.FatalIf(err, "Unable to load the TLS configuration")
// Check and load Root CAs.
@ -140,6 +140,10 @@ func serverHandleCmdArgs(ctx *cli.Context) {
globalEndpoints, setupType, err = createServerEndpoints(globalCLIContext.Addr, serverCmdArgs(ctx)...)
logger.FatalIf(err, "Invalid command line arguments")
// allow transport to be HTTP/1.1 for proxying.
globalProxyTransport = newCustomHTTPProxyTransport(&tls.Config{
RootCAs: globalRootCAs,
}, rest.DefaultTimeout)()
globalProxyEndpoints = GetProxyEndpoints(globalEndpoints)
globalInternodeTransport = newInternodeHTTPTransport(&tls.Config{
RootCAs: globalRootCAs,
@ -384,15 +388,15 @@ func serverMain(ctx *cli.Context) {
if host == "" {
host = sortIPs(localIP4.ToSlice())[0]
}
return fmt.Sprintf("%s://%s", getURLScheme(globalIsSSL), net.JoinHostPort(host, globalMinioPort))
return fmt.Sprintf("%s://%s", getURLScheme(globalIsTLS), net.JoinHostPort(host, globalMinioPort))
}()
// Is distributed setup, error out if no certificates are found for HTTPS endpoints.
if globalIsDistErasure {
if globalEndpoints.HTTPS() && !globalIsSSL {
if globalEndpoints.HTTPS() && !globalIsTLS {
logger.Fatal(config.ErrNoCertsAndHTTPSEndpoints(nil), "Unable to start the server")
}
if !globalEndpoints.HTTPS() && globalIsSSL {
if !globalEndpoints.HTTPS() && globalIsTLS {
logger.Fatal(config.ErrCertsAndHTTPEndpoints(nil), "Unable to start the server")
}
}

View File

@ -85,7 +85,7 @@ func printStartupMessage(apiEndpoints []string, err error) {
// SSL is configured reads certification chain, prints
// authority and expiry.
if color.IsTerminal() && !globalCLIContext.Anonymous {
if globalIsSSL {
if globalIsTLS {
printCertificateMsg(globalPublicCerts)
}
}

View File

@ -463,7 +463,7 @@ func ToS3ETag(etag string) string {
return etag
}
func newInternodeHTTPTransport(tlsConfig *tls.Config, dialTimeout time.Duration) func() *http.Transport {
func newInternodeHTTPTransport(tlsConfig *tls.Config, dialTimeout time.Duration) func() http.RoundTripper {
// For more details about various values used here refer
// https://golang.org/pkg/net/http/#Transport documentation
tr := &http.Transport{
@ -481,11 +481,23 @@ func newInternodeHTTPTransport(tlsConfig *tls.Config, dialTimeout time.Duration)
DisableCompression: true,
}
if tlsConfig != nil {
http2.ConfigureTransport(tr)
if globalIsTLS {
trhttp2, _ := http2.ConfigureTransports(tr)
if trhttp2 != nil {
// ReadIdleTimeout is the timeout after which a health check using ping
// frame will be carried out if no frame is received on the
// connection. 5 minutes is sufficient time for any idle connection.
trhttp2.ReadIdleTimeout = 5 * time.Minute
// PingTimeout is the timeout after which the connection will be closed
// if a response to Ping is not received.
trhttp2.PingTimeout = dialTimeout
// DisableCompression, if true, prevents the Transport from
// requesting compression with an "Accept-Encoding: gzip"
trhttp2.DisableCompression = true
}
}
return func() *http.Transport {
return func() http.RoundTripper {
return tr
}
}
@ -532,8 +544,23 @@ func newCustomHTTPTransport(tlsConfig *tls.Config, dialTimeout time.Duration) fu
DisableCompression: true,
}
if tlsConfig != nil {
http2.ConfigureTransport(tr)
if globalIsTLS {
trhttp2, _ := http2.ConfigureTransports(tr)
if trhttp2 != nil {
// ReadIdleTimeout is the timeout after which a health check using ping
// frame will be carried out if no frame is received on the
// connection. 5 minutes is above maximum sane scrape interval,
// we should not have this small overhead on the scrape connections.
// For other cases, this is used to validate that the connection can
// still be used.
trhttp2.ReadIdleTimeout = 5 * time.Minute
// PingTimeout is the timeout after which the connection will be closed
// if a response to Ping is not received.
trhttp2.PingTimeout = dialTimeout
// DisableCompression, if true, prevents the Transport from
// requesting compression with an "Accept-Encoding: gzip"
trhttp2.DisableCompression = true
}
}
return func() *http.Transport {
@ -543,8 +570,6 @@ func newCustomHTTPTransport(tlsConfig *tls.Config, dialTimeout time.Duration) fu
// NewGatewayHTTPTransport returns a new http configuration
// used while communicating with the cloud backends.
// This sets the value for MaxIdleConnsPerHost from 2 (go default)
// to 256.
func NewGatewayHTTPTransport() *http.Transport {
return newGatewayHTTPTransport(1 * time.Minute)
}

View File

@ -2208,7 +2208,7 @@ func (web *webAPIHandlers) LoginSTS(r *http.Request, args *LoginSTSArgs, reply *
if sourceScheme := handlers.GetSourceScheme(r); sourceScheme != "" {
scheme = sourceScheme
}
if globalIsSSL {
if globalIsTLS {
scheme = "https"
}
@ -2227,8 +2227,6 @@ func (web *webAPIHandlers) LoginSTS(r *http.Request, args *LoginSTSArgs, reply *
clnt := &http.Client{
Transport: NewGatewayHTTPTransport(),
}
defer clnt.CloseIdleConnections()
resp, err := clnt.Do(req)
if err != nil {
return toJSONError(ctx, err)

4
go.mod
View File

@ -81,8 +81,8 @@ require (
github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c
go.etcd.io/etcd v0.0.0-20201125193152-8a03d2e9614b
golang.org/x/crypto v0.0.0-20201124201722-c8d3bf9c5392
golang.org/x/net v0.0.0-20200904194848-62affa334b73
golang.org/x/sys v0.0.0-20201101102859-da207088b7d1
golang.org/x/net v0.0.0-20201216054612-986b41b23924
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68
golang.org/x/tools v0.0.0-20200929223013-bf155c11ec6f // indirect
google.golang.org/api v0.5.0
gopkg.in/jcmturner/gokrb5.v7 v7.5.0

5
go.sum
View File

@ -713,6 +713,8 @@ golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81R
golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
golang.org/x/net v0.0.0-20200904194848-62affa334b73 h1:MXfv8rhZWmFeqX3GNZRsd6vOLoaCHjYEX3qkRo3YBUA=
golang.org/x/net v0.0.0-20200904194848-62affa334b73/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
golang.org/x/net v0.0.0-20201216054612-986b41b23924 h1:QsnDpLLOKwHBBDa8nDws4DYNc/ryVW2vCpxCs09d4PY=
golang.org/x/net v0.0.0-20201216054612-986b41b23924/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421 h1:Wo7BWFiOk0QRFMLYMqJGFMd9CgUAcGx7V+qEg/h5IBI=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@ -759,7 +761,10 @@ golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20201015000850-e3ed0017c211/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201101102859-da207088b7d1 h1:a/mKvvZr9Jcc8oKfcmgzyp7OwF73JPWsQLvH1z2Kxck=
golang.org/x/sys v0.0.0-20201101102859-da207088b7d1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68 h1:nxC68pudNYkKU6jWhgrqdreuFiOQWj1Fs7T3VrH4Pjw=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.1-0.20181227161524-e6919f6577db/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=