Merge pull request #2324 from yarpc/pulkitbhatia/release

Preparing release v1.75.2

CHANGELOG.md

@@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 =======
+## [1.75.2] - 2024-11-19
+### Added
+- HTTP2 server support for the HTTP inbound.
+  - The HTTP inbound now supports HTTP2 connections as well by default.
+  - Inbound config option to disable HTTP2 server support (this will only allow HTTP1.1 connections).
+
 ## [1.75.1] - 2024-11-19
 ### Changed
 - Updated grpc-go to v1.59.0
@@ -1529,6 +1535,7 @@ This release requires regeneration of ThriftRW code.
 ## 0.1.0 - 2016-08-31
 
 - Initial release.
+[1.75.2]: https://github.com/yarpc/yarpc-go/compare/v1.75.1...1.75.2
 [1.75.1]: https://github.com/yarpc/yarpc-go/compare/v1.75.0...1.75.1
 [1.75.0]: https://github.com/yarpc/yarpc-go/compare/v1.73.2...1.75.0
 [1.73.2]: https://github.com/yarpc/yarpc-go/compare/v1.73.1...1.73.2

internal/integrationtest/util_test.go

@@ -30,7 +30,7 @@ import (
 	"go.uber.org/yarpc/transport/http"
 )
 
-var spec = integrationtest.TransportSpec{
+var http1spec = integrationtest.TransportSpec{
 	Identify: hostport.Identify,
 	NewServerTransport: func(t *testing.T, addr string) peer.Transport {
 		return http.NewTransport()
@@ -42,13 +42,39 @@ var spec = integrationtest.TransportSpec{
 		return x.(*http.Transport).NewOutbound(pc)
 	},
 	NewInbound: func(x peer.Transport, addr string) transport.Inbound {
-		return x.(*http.Transport).NewInbound(addr)
+		// disable http2
+		return x.(*http.Transport).NewInbound(addr, http.DisableHTTP2(true))
 	},
 	Addr: func(x peer.Transport, ib transport.Inbound) string {
 		return ib.(*http.Inbound).Addr().String()
 	},
 }
 
 func TestIntegrationWithHTTP(t *testing.T) {
-	spec.Test(t)
+	http1spec.Test(t)
+}
+
+var http2spec = integrationtest.TransportSpec{
+	Identify: hostport.Identify,
+	NewServerTransport: func(t *testing.T, addr string) peer.Transport {
+		return http.NewTransport()
+	},
+	NewClientTransport: func(t *testing.T) peer.Transport {
+		// TODO: will update this once we add client support
+		return http.NewTransport()
+	},
+	NewUnaryOutbound: func(x peer.Transport, pc peer.Chooser) transport.UnaryOutbound {
+		return x.(*http.Transport).NewOutbound(pc)
+	},
+	NewInbound: func(x peer.Transport, addr string) transport.Inbound {
+		// we don't disable http2
+		return x.(*http.Transport).NewInbound(addr)
+	},
+	Addr: func(x peer.Transport, ib transport.Inbound) string {
+		return ib.(*http.Inbound).Addr().String()
+	},
+}
+
+func TestIntegrationWithHTTP2(t *testing.T) {
+	http2spec.Test(t)
 }

transport/http/config.go

@@ -174,6 +174,8 @@ type InboundConfig struct {
 	ShutdownTimeout *time.Duration `config:"shutdownTimeout"`
 	// TLS configuration of the inbound.
 	TLSConfig TLSConfig `config:"tls"`
+	// DisableHTTP2 configure to reject http2 requests.
+	DisableHTTP2 bool `config:"disableHTTP2"`
 }
 
 // TLSConfig specifies the TLS configuration of the HTTP inbound.
@@ -202,6 +204,8 @@ func (ts *transportSpec) buildInbound(ic *InboundConfig, t transport.Transport,
 		inboundOptions = append(inboundOptions, ShutdownTimeout(*ic.ShutdownTimeout))
 	}
 
+	inboundOptions = append(inboundOptions, DisableHTTP2(ic.DisableHTTP2))
+
 	return t.(*Transport).NewInbound(ic.Address, inboundOptions...), nil
 }
 

transport/http/config_test.go

@@ -81,6 +81,7 @@ func TestTransportSpec(t *testing.T) {
 		GrabHeaders     map[string]struct{}
 		ShutdownTimeout time.Duration
 		TLSMode         yarpctls.Mode
+		DisableHTTP2    bool
 	}
 
 	type inboundTest struct {
@@ -239,6 +240,24 @@ func TestTransportSpec(t *testing.T) {
 			cfg:        attrs{"address": ":8080", "shutdownTimeout": "-1s"},
 			wantErrors: []string{`shutdownTimeout must not be negative, got: "-1s"`},
 		},
+		{
+			desc:        "disableHTTP2 - true",
+			cfg:         attrs{"address": ":8080", "disableHTTP2": true},
+			opts:        []Option{},
+			wantInbound: &wantInbound{Address: ":8080", ShutdownTimeout: defaultShutdownTimeout, DisableHTTP2: true},
+		},
+		{
+			desc:        "disableHTTP2 - false",
+			cfg:         attrs{"address": ":8080", "disableHTTP2": false},
+			opts:        []Option{},
+			wantInbound: &wantInbound{Address: ":8080", ShutdownTimeout: defaultShutdownTimeout, DisableHTTP2: false},
+		},
+		{
+			desc:        "disableHTTP2 - default (false)",
+			cfg:         attrs{"address": ":8080"},
+			opts:        []Option{},
+			wantInbound: &wantInbound{Address: ":8080", ShutdownTimeout: defaultShutdownTimeout, DisableHTTP2: false},
+		},
 	}
 
 	outboundTests := []outboundTest{
@@ -563,6 +582,7 @@ func TestTransportSpec(t *testing.T) {
 				assert.Equal(t, want.ShutdownTimeout, ib.shutdownTimeout, "shutdownTimeout should match")
 				assert.Equal(t, "foo", ib.transport.serviceName, "service name must match")
 				assert.Equal(t, want.TLSMode, ib.tlsMode, "tlsMode should match")
+				assert.Equal(t, want.DisableHTTP2, ib.disableHTTP2, "disableHTTP2 should match")
 			}
 		}
 

transport/http/inbound.go

@@ -24,6 +24,7 @@ import (
 	"context"
 	"crypto/tls"
 	"errors"
+	"fmt"
 	"net"
 	"net/http"
 	"strings"
@@ -38,15 +39,19 @@ import (
 	"go.uber.org/yarpc/transport/internal/tls/muxlistener"
 	"go.uber.org/yarpc/yarpcerrors"
 	"go.uber.org/zap"
+	"golang.org/x/net/http2"
+	"golang.org/x/net/http2/h2c"
 )
 
-// We want a value that's around 5 seconds, but slightly higher than how
-// long a successful HTTP shutdown can take.
-// There's a specific path in the HTTP shutdown path that can take 5 seconds:
-// https://golang.org/src/net/http/server.go?s=83923:83977#L2710
-// This avoids timeouts in shutdown caused by new idle connections, without
-// making the timeout too large.
-const defaultShutdownTimeout = 6 * time.Second
+const (
+	// We want a value that's around 5 seconds, but slightly higher than how
+	// long a successful HTTP shutdown can take.
+	// There's a specific path in the HTTP shutdown path that can take 5 seconds:
+	// https://golang.org/src/net/http/server.go?s=83923:83977#L2710
+	// This avoids timeouts in shutdown caused by new idle connections, without
+	// making the timeout too large.
+	defaultShutdownTimeout = 6 * time.Second
+)
 
 // InboundOption customizes the behavior of an HTTP Inbound constructed with
 // NewInbound.
@@ -124,6 +129,13 @@ func InboundTLSMode(mode yarpctls.Mode) InboundOption {
 	}
 }
 
+// DisableHTTP2 returns an InboundOption that disables HTTP/2 support.
+func DisableHTTP2(flag bool) InboundOption {
+	return func(i *Inbound) {
+		i.disableHTTP2 = flag
+	}
+}
+
 // NewInbound builds a new HTTP inbound that listens on the given address and
 // sharing this transport.
 func (t *Transport) NewInbound(addr string, opts ...InboundOption) *Inbound {
@@ -136,6 +148,7 @@ func (t *Transport) NewInbound(addr string, opts ...InboundOption) *Inbound {
 		transport:         t,
 		grabHeaders:       make(map[string]struct{}),
 		bothResponseError: true,
+		disableHTTP2:      false,
 	}
 	for _, opt := range opts {
 		opt(i)
@@ -165,6 +178,8 @@ type Inbound struct {
 
 	tlsConfig *tls.Config
 	tlsMode   yarpctls.Mode
+
+	disableHTTP2 bool
 }
 
 // Tracer configures a tracer on this inbound.
@@ -221,10 +236,19 @@ func (i *Inbound) start() error {
 		httpHandler = i.mux
 	}
 
-	i.server = intnet.NewHTTPServer(&http.Server{
+	server := &http.Server{
 		Addr:    i.addr,
 		Handler: httpHandler,
-	})
+	}
+	if !i.disableHTTP2 {
+		h2s := &http2.Server{}
+		server.Handler = h2c.NewHandler(server.Handler, h2s)
+		err := http2.ConfigureServer(server, h2s)
+		if err != nil {
+			return fmt.Errorf("failed to configure HTTP/2 server: %w", err)
+		}
+	}
+	i.server = intnet.NewHTTPServer(server)
 
 	addr := i.addr
 	if addr == "" {

transport/http/inbound_test.go

@@ -23,6 +23,7 @@ package http
 import (
 	"bytes"
 	"context"
+	"crypto/tls"
 	"fmt"
 	"io"
 	"net"
@@ -31,6 +32,8 @@ import (
 	"syscall"
 	"testing"
 
+	"golang.org/x/net/http2"
+
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -378,6 +381,83 @@ func TestRequestAfterStop(t *testing.T) {
 	assert.Error(t, err, "requests should fail once inbound is stopped")
 }
 
+func TestInboundWithHTTPVersion(t *testing.T) {
+	t.Run("HTTP1", func(t *testing.T) {
+		// Create a new transport that supports both HTTP/1.1 and HTTP/2
+		testTransport := NewTransport()
+		inbound := testTransport.NewInbound("127.0.0.1:8888")
+
+		dispatcher := yarpc.NewDispatcher(yarpc.Config{
+			Name:     "myservice",
+			Inbounds: yarpc.Inbounds{inbound},
+		})
+		require.NoError(t, dispatcher.Start(), "failed to start dispatcher")
+		t.Cleanup(func() { _ = dispatcher.Stop() })
+
+		// Make a request to the /health endpoint.
+		res, err := http.Get("http://127.0.0.1:8888/health")
+		require.Nil(t, err, "got error making request: %v", err)
+		t.Cleanup(func() { _ = res.Body.Close() })
+	})
+
+	t.Run("HTTP2", func(t *testing.T) {
+		// Create a new transport that supports both HTTP/1.1 and HTTP/2
+		testTransport := NewTransport()
+		inbound := testTransport.NewInbound("127.0.0.1:8888")
+
+		dispatcher := yarpc.NewDispatcher(yarpc.Config{
+			Name:     "myservice",
+			Inbounds: yarpc.Inbounds{inbound},
+		})
+		require.NoError(t, dispatcher.Start(), "failed to start dispatcher")
+		t.Cleanup(func() { _ = dispatcher.Stop() })
+
+		client := http.Client{
+			Transport: &http2.Transport{
+				AllowHTTP: true,
+				DialTLSContext: func(ctx context.Context, network, addr string, _ *tls.Config) (net.Conn, error) {
+					var d net.Dialer
+					return d.DialContext(ctx, network, addr)
+				},
+			},
+		}
+		t.Cleanup(client.CloseIdleConnections)
+		// Make a request to the /health endpoint.
+		res, err := client.Get("http://127.0.0.1:8888/health")
+		require.Nil(t, err, "got error making request: %v", err)
+		t.Cleanup(func() { _ = res.Body.Close() })
+	})
+
+	t.Run("HTTP2 should fail when disabledHTTP2 flag is set", func(t *testing.T) {
+		// Create a new transport that only supports HTTP/1.1
+		testTransport := NewTransport()
+		// Disable HTTP/2
+		inbound := testTransport.NewInbound("127.0.0.1:8888", DisableHTTP2(true))
+
+		dispatcher := yarpc.NewDispatcher(yarpc.Config{
+			Name:     "myservice",
+			Inbounds: yarpc.Inbounds{inbound},
+		})
+		require.NoError(t, dispatcher.Start(), "failed to start dispatcher")
+		t.Cleanup(func() { _ = dispatcher.Stop() })
+
+		client := http.Client{
+			Transport: &http2.Transport{
+				AllowHTTP: true,
+				DialTLSContext: func(ctx context.Context, network, addr string, _ *tls.Config) (net.Conn, error) {
+					var d net.Dialer
+					return d.DialContext(ctx, network, addr)
+				},
+			},
+		}
+		t.Cleanup(client.CloseIdleConnections)
+		// Make a request to the /health endpoint.
+		res, err := client.Get("http://127.0.0.1:8888/health")
+		require.Error(t, err, "expected error making request")
+		require.Nil(t, res, "expected response to be nil")
+	})
+}
+
 func httpGet(t *testing.T, url string) (*http.Response, string, error) {
 	resp, err := http.Get(url)
 	if err != nil {

version.go

@@ -21,4 +21,4 @@
 package yarpc // import "go.uber.org/yarpc"
 
 // Version is the current version of YARPC.
-const Version = "1.75.1"
+const Version = "1.75.2"