Merge pull request kubernetes#82090 from liggitt/webhook-http2

Use http/1.1 for apiserver->webhook clients

Author: k8s-ci-robot

pkg/kubelet/certificate/transport.go

@@ -153,6 +153,7 @@ func addCertRotation(stopCh <-chan struct{}, period time.Duration, clientConfig
 	clientConfig.CAData = nil
 	clientConfig.CAFile = ""
 	clientConfig.Insecure = false
+	clientConfig.NextProtos = nil
 
 	return nil
 }

pkg/kubelet/client/kubelet_client.go

@@ -109,12 +109,13 @@ func MakeTransport(config *KubeletClientConfig) (http.RoundTripper, error) {
 func (c *KubeletClientConfig) transportConfig() *transport.Config {
 	cfg := &transport.Config{
 		TLS: transport.TLSConfig{
-			CAFile:   c.CAFile,
-			CAData:   c.CAData,
-			CertFile: c.CertFile,
-			CertData: c.CertData,
-			KeyFile:  c.KeyFile,
-			KeyData:  c.KeyData,
+			CAFile:     c.CAFile,
+			CAData:     c.CAData,
+			CertFile:   c.CertFile,
+			CertData:   c.CertData,
+			KeyFile:    c.KeyFile,
+			KeyData:    c.KeyData,
+			NextProtos: c.NextProtos,
 		},
 		BearerToken: c.BearerToken,
 	}

staging/src/k8s.io/apimachinery/pkg/util/net/http.go

@@ -101,6 +101,9 @@ func SetOldTransportDefaults(t *http.Transport) *http.Transport {
 	if t.TLSHandshakeTimeout == 0 {
 		t.TLSHandshakeTimeout = defaultTransport.TLSHandshakeTimeout
 	}
+	if t.IdleConnTimeout == 0 {
+		t.IdleConnTimeout = defaultTransport.IdleConnTimeout
+	}
 	return t
 }
 
@@ -111,14 +114,29 @@ func SetTransportDefaults(t *http.Transport) *http.Transport {
 	// Allow clients to disable http2 if needed.
 	if s := os.Getenv("DISABLE_HTTP2"); len(s) > 0 {
 		klog.Infof("HTTP2 has been explicitly disabled")
-	} else {
+	} else if allowsHTTP2(t) {
 		if err := http2.ConfigureTransport(t); err != nil {
 			klog.Warningf("Transport failed http2 configuration: %v", err)
 		}
 	}
 	return t
 }
 
+func allowsHTTP2(t *http.Transport) bool {
+	if t.TLSClientConfig == nil || len(t.TLSClientConfig.NextProtos) == 0 {
+		// the transport expressed no NextProto preference, allow
+		return true
+	}
+	for _, p := range t.TLSClientConfig.NextProtos {
+		if p == http2.NextProtoTLS {
+			// the transport explicitly allowed http/2
+			return true
+		}
+	}
+	// the transport explicitly set NextProtos and excluded http/2
+	return false
+}
+
 type RoundTripperWrapper interface {
 	http.RoundTripper
 	WrappedRoundTripper() http.RoundTripper

staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go

@@ -439,3 +439,56 @@ func TestConnectWithRedirects(t *testing.T) {
 		})
 	}
 }
+
+func TestAllowsHTTP2(t *testing.T) {
+	testcases := []struct {
+		Name         string
+		Transport    *http.Transport
+		ExpectAllows bool
+	}{
+		{
+			Name:         "empty",
+			Transport:    &http.Transport{},
+			ExpectAllows: true,
+		},
+		{
+			Name:         "empty tlsconfig",
+			Transport:    &http.Transport{TLSClientConfig: &tls.Config{}},
+			ExpectAllows: true,
+		},
+		{
+			Name:         "zero-length NextProtos",
+			Transport:    &http.Transport{TLSClientConfig: &tls.Config{NextProtos: []string{}}},
+			ExpectAllows: true,
+		},
+		{
+			Name:         "includes h2 in NextProtos after",
+			Transport:    &http.Transport{TLSClientConfig: &tls.Config{NextProtos: []string{"http/1.1", "h2"}}},
+			ExpectAllows: true,
+		},
+		{
+			Name:         "includes h2 in NextProtos before",
+			Transport:    &http.Transport{TLSClientConfig: &tls.Config{NextProtos: []string{"h2", "http/1.1"}}},
+			ExpectAllows: true,
+		},
+		{
+			Name:         "includes h2 in NextProtos between",
+			Transport:    &http.Transport{TLSClientConfig: &tls.Config{NextProtos: []string{"http/1.1", "h2", "h3"}}},
+			ExpectAllows: true,
+		},
+		{
+			Name:         "excludes h2 in NextProtos",
+			Transport:    &http.Transport{TLSClientConfig: &tls.Config{NextProtos: []string{"http/1.1"}}},
+			ExpectAllows: false,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.Name, func(t *testing.T) {
+			allows := allowsHTTP2(tc.Transport)
+			if allows != tc.ExpectAllows {
+				t.Errorf("expected %v, got %v", tc.ExpectAllows, allows)
+			}
+		})
+	}
+}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go

@@ -96,9 +96,11 @@ func BenchmarkAdmit(b *testing.B) {
 			}
 
 			b.ResetTimer()
-			for i := 0; i < b.N; i++ {
-				wh.Admit(context.TODO(), attr, objectInterfaces)
-			}
+			b.RunParallel(func(pb *testing.PB) {
+				for pb.Next() {
+					wh.Admit(context.TODO(), attr, objectInterfaces)
+				}
+			})
 		})
 	}
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go

@@ -85,9 +85,11 @@ func BenchmarkValidate(b *testing.B) {
 			attr := webhooktesting.NewAttribute(ns, nil, tt.IsDryRun)
 
 			b.ResetTimer()
-			for i := 0; i < b.N; i++ {
-				wh.Validate(context.TODO(), attr, objectInterfaces)
-			}
+			b.RunParallel(func(pb *testing.PB) {
+				for pb.Next() {
+					wh.Validate(context.TODO(), attr, objectInterfaces)
+				}
+			})
 		})
 	}
 }

staging/src/k8s.io/apiserver/pkg/util/webhook/client.go

@@ -136,6 +136,11 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
 		}
 		cfg.TLSClientConfig.CAData = append(cfg.TLSClientConfig.CAData, cc.CABundle...)
 
+		// Use http/1.1 instead of http/2.
+		// This is a workaround for http/2-enabled clients not load-balancing concurrent requests to multiple backends.
+		// See http://issue.k8s.io/75791 for details.
+		cfg.NextProtos = []string{"http/1.1"}
+
 		cfg.ContentConfig.NegotiatedSerializer = cm.negotiatedSerializer
 		cfg.ContentConfig.ContentType = runtime.ContentTypeJSON
 		client, err := rest.UnversionedRESTClientFor(cfg)

staging/src/k8s.io/client-go/rest/config.go

@@ -211,6 +211,12 @@ type TLSClientConfig struct {
 	// CAData holds PEM-encoded bytes (typically read from a root certificates bundle).
 	// CAData takes precedence over CAFile
 	CAData []byte
+
+	// NextProtos is a list of supported application level protocols, in order of preference.
+	// Used to populate tls.Config.NextProtos.
+	// To indicate to the server http/1.1 is preferred over http/2, set to ["http/1.1", "h2"] (though the server is free to ignore that preference).
+	// To use only http/1.1, set to ["http/1.1"].
+	NextProtos []string
 }
 
 var _ fmt.Stringer = TLSClientConfig{}
@@ -236,6 +242,7 @@ func (c TLSClientConfig) String() string {
 		CertData:   c.CertData,
 		KeyData:    c.KeyData,
 		CAData:     c.CAData,
+		NextProtos: c.NextProtos,
 	}
 	// Explicitly mark non-empty credential fields as redacted.
 	if len(cc.CertData) != 0 {
@@ -503,6 +510,7 @@ func AnonymousClientConfig(config *Config) *Config {
 			ServerName: config.ServerName,
 			CAFile:     config.TLSClientConfig.CAFile,
 			CAData:     config.TLSClientConfig.CAData,
+			NextProtos: config.TLSClientConfig.NextProtos,
 		},
 		RateLimiter:        config.RateLimiter,
 		UserAgent:          config.UserAgent,
@@ -541,6 +549,7 @@ func CopyConfig(config *Config) *Config {
 			CertData:   config.TLSClientConfig.CertData,
 			KeyData:    config.TLSClientConfig.KeyData,
 			CAData:     config.TLSClientConfig.CAData,
+			NextProtos: config.TLSClientConfig.NextProtos,
 		},
 		UserAgent:          config.UserAgent,
 		DisableCompression: config.DisableCompression,

staging/src/k8s.io/client-go/rest/config_test.go

@@ -493,10 +493,11 @@ func TestConfigSprint(t *testing.T) {
 			Env:     []clientcmdapi.ExecEnvVar{{Name: "secret", Value: "s3cr3t"}},
 		},
 		TLSClientConfig: TLSClientConfig{
-			CertFile: "a.crt",
-			KeyFile:  "a.key",
-			CertData: []byte("fake cert"),
-			KeyData:  []byte("fake key"),
+			CertFile:   "a.crt",
+			KeyFile:    "a.key",
+			CertData:   []byte("fake cert"),
+			KeyData:    []byte("fake key"),
+			NextProtos: []string{"h2", "http/1.1"},
 		},
 		UserAgent:     "gobot",
 		Transport:     &fakeRoundTripper{},
@@ -508,7 +509,7 @@ func TestConfigSprint(t *testing.T) {
 		Dial:          fakeDialFunc,
 	}
 	want := fmt.Sprintf(
-		`&rest.Config{Host:"localhost:8080", APIPath:"v1", ContentConfig:rest.ContentConfig{AcceptContentTypes:"application/json", ContentType:"application/json", GroupVersion:(*schema.GroupVersion)(nil), NegotiatedSerializer:runtime.NegotiatedSerializer(nil)}, Username:"gopher", Password:"--- REDACTED ---", BearerToken:"--- REDACTED ---", BearerTokenFile:"", Impersonate:rest.ImpersonationConfig{UserName:"gopher2", Groups:[]string(nil), Extra:map[string][]string(nil)}, AuthProvider:api.AuthProviderConfig{Name: "gopher", Config: map[string]string{--- REDACTED ---}}, AuthConfigPersister:rest.AuthProviderConfigPersister(--- REDACTED ---), ExecProvider:api.AuthProviderConfig{Command: "sudo", Args: []string{"--- REDACTED ---"}, Env: []ExecEnvVar{--- REDACTED ---}, APIVersion: ""}, TLSClientConfig:rest.sanitizedTLSClientConfig{Insecure:false, ServerName:"", CertFile:"a.crt", KeyFile:"a.key", CAFile:"", CertData:[]uint8{0x2d, 0x2d, 0x2d, 0x20, 0x54, 0x52, 0x55, 0x4e, 0x43, 0x41, 0x54, 0x45, 0x44, 0x20, 0x2d, 0x2d, 0x2d}, KeyData:[]uint8{0x2d, 0x2d, 0x2d, 0x20, 0x52, 0x45, 0x44, 0x41, 0x43, 0x54, 0x45, 0x44, 0x20, 0x2d, 0x2d, 0x2d}, CAData:[]uint8(nil)}, UserAgent:"gobot", DisableCompression:false, Transport:(*rest.fakeRoundTripper)(%p), WrapTransport:(transport.WrapperFunc)(%p), QPS:1, Burst:2, RateLimiter:(*rest.fakeLimiter)(%p), Timeout:3000000000, Dial:(func(context.Context, string, string) (net.Conn, error))(%p)}`,
+		`&rest.Config{Host:"localhost:8080", APIPath:"v1", ContentConfig:rest.ContentConfig{AcceptContentTypes:"application/json", ContentType:"application/json", GroupVersion:(*schema.GroupVersion)(nil), NegotiatedSerializer:runtime.NegotiatedSerializer(nil)}, Username:"gopher", Password:"--- REDACTED ---", BearerToken:"--- REDACTED ---", BearerTokenFile:"", Impersonate:rest.ImpersonationConfig{UserName:"gopher2", Groups:[]string(nil), Extra:map[string][]string(nil)}, AuthProvider:api.AuthProviderConfig{Name: "gopher", Config: map[string]string{--- REDACTED ---}}, AuthConfigPersister:rest.AuthProviderConfigPersister(--- REDACTED ---), ExecProvider:api.AuthProviderConfig{Command: "sudo", Args: []string{"--- REDACTED ---"}, Env: []ExecEnvVar{--- REDACTED ---}, APIVersion: ""}, TLSClientConfig:rest.sanitizedTLSClientConfig{Insecure:false, ServerName:"", CertFile:"a.crt", KeyFile:"a.key", CAFile:"", CertData:[]uint8{0x2d, 0x2d, 0x2d, 0x20, 0x54, 0x52, 0x55, 0x4e, 0x43, 0x41, 0x54, 0x45, 0x44, 0x20, 0x2d, 0x2d, 0x2d}, KeyData:[]uint8{0x2d, 0x2d, 0x2d, 0x20, 0x52, 0x45, 0x44, 0x41, 0x43, 0x54, 0x45, 0x44, 0x20, 0x2d, 0x2d, 0x2d}, CAData:[]uint8(nil), NextProtos:[]string{"h2", "http/1.1"}}, UserAgent:"gobot", DisableCompression:false, Transport:(*rest.fakeRoundTripper)(%p), WrapTransport:(transport.WrapperFunc)(%p), QPS:1, Burst:2, RateLimiter:(*rest.fakeLimiter)(%p), Timeout:3000000000, Dial:(func(context.Context, string, string) (net.Conn, error))(%p)}`,
 		c.Transport, fakeWrapperFunc, c.RateLimiter, fakeDialFunc,
 	)
 

staging/src/k8s.io/client-go/rest/transport.go

@@ -74,6 +74,7 @@ func (c *Config) TransportConfig() (*transport.Config, error) {
 			CertData:   c.CertData,
 			KeyFile:    c.KeyFile,
 			KeyData:    c.KeyData,
+			NextProtos: c.NextProtos,
 		},
 		Username:        c.Username,
 		Password:        c.Password,