Skip to content

Commit

Permalink
Disable HTTP/2 (#249)
Browse files Browse the repository at this point in the history
* Add an option to disable HTTP2

Signed-off-by: Julien Pivotto <[email protected]>
  • Loading branch information
roidelapluie authored Aug 10, 2020
1 parent 217fd62 commit 273427a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 15 deletions.
22 changes: 15 additions & 7 deletions config/http_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ func newClient(rt http.RoundTripper) *http.Client {

// NewClientFromConfig returns a new HTTP client configured for the
// given config.HTTPClientConfig. The name is used as go-conntrack metric label.
func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives bool) (*http.Client, error) {
rt, err := NewRoundTripperFromConfig(cfg, name, disableKeepAlives)
func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, enableHTTP2 bool) (*http.Client, error) {
rt, err := NewRoundTripperFromConfig(cfg, name, disableKeepAlives, enableHTTP2)
if err != nil {
return nil, err
}
Expand All @@ -133,7 +133,7 @@ func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives bo

// NewRoundTripperFromConfig returns a new HTTP RoundTripper configured for the
// given config.HTTPClientConfig. The name is used as go-conntrack metric label.
func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives bool) (http.RoundTripper, error) {
func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, enableHTTP2 bool) (http.RoundTripper, error) {
newRT := func(tlsConfig *tls.Config) (http.RoundTripper, error) {
// The only timeout we care about is the configured scrape timeout.
// It is applied on request. So we leave out any timings here.
Expand All @@ -154,10 +154,18 @@ func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAli
conntrack.DialWithName(name),
),
}
// TODO: use ForceAttemptHTTP2 when we move to Go 1.13+.
err := http2.ConfigureTransport(rt.(*http.Transport))
if err != nil {
return nil, err
if enableHTTP2 {
// HTTP/2 support is golang has many problematic cornercases where
// dead connections would be kept and used in connection pools.
// https://github.com/golang/go/issues/32388
// https://github.com/golang/go/issues/39337
// https://github.com/golang/go/issues/39750
// TODO: Re-Enable HTTP/2 once upstream issue is fixed.
// TODO: use ForceAttemptHTTP2 when we move to Go 1.13+.
err := http2.ConfigureTransport(rt.(*http.Transport))
if err != nil {
return nil, err
}
}

// If a bearer token is provided, create a round tripper that will set the
Expand Down
16 changes: 8 additions & 8 deletions config/http_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func TestNewClientFromConfig(t *testing.T) {
}
defer testServer.Close()

client, err := NewClientFromConfig(validConfig.clientConfig, "test", false)
client, err := NewClientFromConfig(validConfig.clientConfig, "test", false, true)
if err != nil {
t.Errorf("Can't create a client from this config: %+v", validConfig.clientConfig)
continue
Expand Down Expand Up @@ -255,7 +255,7 @@ func TestNewClientFromInvalidConfig(t *testing.T) {
}

for _, invalidConfig := range newClientInvalidConfig {
client, err := NewClientFromConfig(invalidConfig.clientConfig, "test", false)
client, err := NewClientFromConfig(invalidConfig.clientConfig, "test", false, true)
if client != nil {
t.Errorf("A client instance was returned instead of nil using this config: %+v", invalidConfig.clientConfig)
}
Expand Down Expand Up @@ -294,7 +294,7 @@ func TestMissingBearerAuthFile(t *testing.T) {
}
defer testServer.Close()

client, err := NewClientFromConfig(cfg, "test", false)
client, err := NewClientFromConfig(cfg, "test", false, true)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -483,7 +483,7 @@ func TestBasicAuthNoPassword(t *testing.T) {
if err != nil {
t.Fatalf("Error loading HTTP client config: %v", err)
}
client, err := NewClientFromConfig(*cfg, "test", false)
client, err := NewClientFromConfig(*cfg, "test", false, true)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand All @@ -509,7 +509,7 @@ func TestBasicAuthNoUsername(t *testing.T) {
if err != nil {
t.Fatalf("Error loading HTTP client config: %v", err)
}
client, err := NewClientFromConfig(*cfg, "test", false)
client, err := NewClientFromConfig(*cfg, "test", false, true)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand All @@ -535,7 +535,7 @@ func TestBasicAuthPasswordFile(t *testing.T) {
if err != nil {
t.Fatalf("Error loading HTTP client config: %v", err)
}
client, err := NewClientFromConfig(*cfg, "test", false)
client, err := NewClientFromConfig(*cfg, "test", false, true)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand Down Expand Up @@ -686,7 +686,7 @@ func TestTLSRoundTripper(t *testing.T) {
writeCertificate(bs, tc.cert, cert)
writeCertificate(bs, tc.key, key)
if c == nil {
c, err = NewClientFromConfig(cfg, "test", false)
c, err = NewClientFromConfig(cfg, "test", false, true)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand Down Expand Up @@ -758,7 +758,7 @@ func TestTLSRoundTripperRaces(t *testing.T) {
writeCertificate(bs, TLSCAChainPath, ca)
writeCertificate(bs, ClientCertificatePath, cert)
writeCertificate(bs, ClientKeyNoPassPath, key)
c, err = NewClientFromConfig(cfg, "test", false)
c, err = NewClientFromConfig(cfg, "test", false, true)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand Down

0 comments on commit 273427a

Please sign in to comment.