diff --git a/pkg/util/webhook/client.go b/pkg/util/webhook/client.go index ec3585b45..63ea4e266 100644 --- a/pkg/util/webhook/client.go +++ b/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,20 @@ 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) { + 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 @@ -139,11 +154,6 @@ 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 https://issue.k8s.io/75791 for details. - cfg.NextProtos = []string{"http/1.1"} - cfg.ContentConfig.NegotiatedSerializer = cm.negotiatedSerializer cfg.ContentConfig.ContentType = runtime.ContentTypeJSON @@ -153,12 +163,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 { @@ -173,6 +178,12 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) { return nil, err } cfg := rest.CopyConfig(restConfig) + + // 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"} + serverName := cc.Service.Name + "." + cc.Service.Namespace + ".svc" host := net.JoinHostPort(serverName, strconv.Itoa(int(port))) @@ -225,6 +236,22 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) { cfg := rest.CopyConfig(restConfig) cfg.Host = u.Scheme + "://" + u.Host cfg.APIPath = u.Path + if !isLocalHost(u) { + cfg.NextProtos = []string{"http/1.1"} + } 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 +} diff --git a/pkg/util/webhook/client_test.go b/pkg/util/webhook/client_test.go new file mode 100644 index 000000000..dd00f238d --- /dev/null +++ b/pkg/util/webhook/client_test.go @@ -0,0 +1,91 @@ +/* +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" + + "golang.org/x/net/http2" + "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 && !allowHTTP2(cfg.NextProtos) { + t.Errorf("expected allow http/2, got: %v", cfg.NextProtos) + } + }) + } +} + +func allowHTTP2(nextProtos []string) bool { + if len(nextProtos) == 0 { + // the transport expressed no NextProto preference, allow + return true + } + for _, p := range nextProtos { + if p == http2.NextProtoTLS { + // the transport explicitly allowed http/2 + return true + } + } + // the transport explicitly set NextProtos and excluded http/2 + return false +}