Use http/2 for localhost webhook

Signed-off-by: Eric Lin <exlin@google.com>

go.mod

@@ -32,6 +32,7 @@ require (
 	github.com/docker/go-units v0.5.0
 	github.com/emicklei/go-restful/v3 v3.11.0
 	github.com/evanphx/json-patch v4.12.0+incompatible
+	github.com/foxcpp/go-mockdns v1.0.0
 	github.com/fsnotify/fsnotify v1.7.0
 	github.com/go-logr/logr v1.3.0
 	github.com/godbus/dbus/v5 v5.1.0
@@ -194,6 +195,7 @@ require (
 	github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
+	github.com/miekg/dns v1.1.25 // indirect
 	github.com/mistifyio/go-zfs v2.1.2-0.20190413222219-f784269be439+incompatible // indirect
 	github.com/mitchellh/go-wordwrap v1.0.1 // indirect
 	github.com/moby/spdystream v0.2.0 // indirect

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

@@ -24,6 +24,7 @@ import (
 	"net"
 	"net/url"
 	"strconv"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -32,6 +33,7 @@ import (
 	"k8s.io/apiserver/pkg/util/x509metrics"
 	"k8s.io/client-go/rest"
 	"k8s.io/utils/lru"
+	netutils "k8s.io/utils/net"
 )
 
 const (
@@ -128,7 +130,21 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
 		return client.(*rest.RESTClient), nil
 	}
 
-	complete := func(cfg *rest.Config) (*rest.RESTClient, error) {
+	cfg, err := cm.hookClientConfig(cc)
+	if err != nil {
+		return nil, err
+	}
+
+	client, err := rest.UnversionedRESTClientFor(cfg)
+	if err == nil {
+		cm.cache.Add(string(cacheKey), client)
+	}
+	return client, err
+}
+
+func (cm *ClientManager) hookClientConfig(cc ClientConfig) (*rest.Config, error) {
+	forceHTTP1 := true
+	complete := func(cfg *rest.Config) (*rest.Config, error) {
 		// Avoid client-side rate limiting talking to the webhook backend.
 		// Rate limiting should happen when deciding how many requests to serve.
 		cfg.QPS = -1
@@ -142,7 +158,9 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
 		// 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 https://issue.k8s.io/75791 for details.
-		cfg.NextProtos = []string{"http/1.1"}
+		if forceHTTP1 {
+			cfg.NextProtos = []string{"http/1.1"}
+		}
 
 		cfg.ContentConfig.NegotiatedSerializer = cm.negotiatedSerializer
 		cfg.ContentConfig.ContentType = runtime.ContentTypeJSON
@@ -153,12 +171,7 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
 			x509MissingSANCounter,
 			x509InsecureSHA1Counter,
 		))
-
-		client, err := rest.UnversionedRESTClientFor(cfg)
-		if err == nil {
-			cm.cache.Add(string(cacheKey), client)
-		}
-		return client, err
+		return cfg, nil
 	}
 
 	if cc.Service != nil {
@@ -211,6 +224,10 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
 		return nil, &ErrCallingWebhook{WebhookName: cc.Name, Reason: fmt.Errorf("Unparsable URL: %v", err)}
 	}
 
+	if isLocalHost(u) {
+		forceHTTP1 = false
+	}
+
 	hostPort := u.Host
 	if len(u.Port()) == 0 {
 		// Default to port 443 if no port is specified
@@ -228,3 +245,16 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
 
 	return complete(cfg)
 }
+
+func isLocalHost(u *url.URL) bool {
+	host := u.Hostname()
+	if strings.EqualFold(host, "localhost") {
+		return true
+	}
+
+	netIP := netutils.ParseIPSloppy(host)
+	if netIP != nil {
+		return netIP.IsLoopback()
+	}
+	return false
+}

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

@@ -0,0 +1,75 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package webhook
+
+import (
+	"testing"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+func TestWebhookClientConfig(t *testing.T) {
+	cm, _ := NewClientManager([]schema.GroupVersion{})
+	authInfoResolver, err := NewDefaultAuthenticationInfoResolver("")
+	if err != nil {
+		t.Fatal(err)
+	}
+	cm.SetAuthenticationInfoResolver(authInfoResolver)
+	cm.SetServiceResolver(NewDefaultServiceResolver())
+
+	tests := []struct {
+		name             string
+		url              string
+		expectAllowHTTP2 bool
+	}{
+		{
+			name:             "force http1",
+			url:              "https://webhook.example.com",
+			expectAllowHTTP2: false,
+		},
+		{
+			name:             "allow http2 for localhost",
+			url:              "https://localhost",
+			expectAllowHTTP2: true,
+		},
+		{
+			name:             "allow http2 for 127.0.0.1",
+			url:              "https://127.0.0.1",
+			expectAllowHTTP2: true,
+		},
+		{
+			name:             "allow http2 for [::1]:0",
+			url:              "https://[::1]",
+			expectAllowHTTP2: true,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+
+			cc := ClientConfig{
+				URL: tc.url,
+			}
+			cfg, err := cm.hookClientConfig(cc)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tc.expectAllowHTTP2 && len(cfg.NextProtos) != 0 {
+				t.Errorf("expected allow http/2, got: %v", cfg.NextProtos)
+			}
+		})
+	}
+}

test/integration/apiserver/admissionwebhook/load_balance_test.go

@@ -40,6 +40,8 @@ import (
 	"k8s.io/client-go/rest"
 	kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
 	"k8s.io/kubernetes/test/integration/framework"
+
+	"github.com/foxcpp/go-mockdns"
 )
 
 const (
@@ -48,6 +50,19 @@ const (
 
 // TestWebhookLoadBalance ensures that the admission webhook opens multiple connections to backends to satisfy concurrent requests
 func TestWebhookLoadBalance(t *testing.T) {
+	srv, err := mockdns.NewServer(map[string]mockdns.Zone{
+		"example.com.": {
+			A:    []string{"127.0.0.1"},
+			AAAA: []string{"::1"},
+		},
+	}, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer srv.Close()
+
+	srv.PatchNet(net.DefaultResolver)
+	defer mockdns.UnpatchNet(net.DefaultResolver)
 
 	roots := x509.NewCertPool()
 	if !roots.AppendCertsFromPEM(localhostCert) {
@@ -80,7 +95,8 @@ func TestWebhookLoadBalance(t *testing.T) {
 	}()
 	defer httpServer.Close()
 
-	webhookURL := "https://" + localListener.Addr().String()
+	port := localListener.Addr().(*net.TCPAddr).Port
+	webhookURL := fmt.Sprintf("https://example.com:%d", port)
 
 	s := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{
 		"--disable-admission-plugins=ServiceAccount",