Skip to content

Commit

Permalink
client-go/transport: fix memory leak when using rest.Config Dial func…
Browse files Browse the repository at this point in the history
…tion with client.New and kubernetes.NewForConfig
  • Loading branch information
lizardruss committed May 7, 2024
1 parent b0ed45d commit 5b7ded5
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 9 deletions.
2 changes: 1 addition & 1 deletion rest/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (c *Config) TransportConfig() (*transport.Config, error) {
}

if c.Dial != nil {
conf.DialHolder = &transport.DialHolder{Dial: c.Dial}
conf.DialHolder = &transport.DialHolder{Dial: c.Dial, DisableCache: true}
}

if c.ExecProvider != nil && c.AuthProvider != nil {
Expand Down
4 changes: 4 additions & 0 deletions transport/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ func tlsConfigKey(c *Config) (tlsCacheKey, bool, error) {
return tlsCacheKey{}, false, nil
}

if c.DialHolder != nil && c.DialHolder.DisableCache {
return tlsCacheKey{}, false, nil
}

k := tlsCacheKey{
insecure: c.TLS.Insecure,
caData: string(c.TLS.CAData),
Expand Down
17 changes: 9 additions & 8 deletions transport/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,14 @@ func TestTLSConfigKey(t *testing.T) {
dialer := net.Dialer{}
getCert := &GetCertHolder{GetCert: func() (*tls.Certificate, error) { return nil, nil }}
uniqueConfigurations := map[string]*Config{
"proxy": {Proxy: func(request *http.Request) (*url.URL, error) { return nil, nil }},
"no tls": {},
"dialer": {DialHolder: &DialHolder{Dial: dialer.DialContext}},
"dialer2": {DialHolder: &DialHolder{Dial: func(ctx context.Context, network, address string) (net.Conn, error) { return nil, nil }}},
"insecure": {TLS: TLSConfig{Insecure: true}},
"cadata 1": {TLS: TLSConfig{CAData: []byte{1}}},
"cadata 2": {TLS: TLSConfig{CAData: []byte{2}}},
"proxy": {Proxy: func(request *http.Request) (*url.URL, error) { return nil, nil }},
"no tls": {},
"dialer": {DialHolder: &DialHolder{Dial: dialer.DialContext}},
"dialer with cache disabled": {DialHolder: &DialHolder{DisableCache: true, Dial: dialer.DialContext}},
"dialer2": {DialHolder: &DialHolder{Dial: func(ctx context.Context, network, address string) (net.Conn, error) { return nil, nil }}},
"insecure": {TLS: TLSConfig{Insecure: true}},
"cadata 1": {TLS: TLSConfig{CAData: []byte{1}}},
"cadata 2": {TLS: TLSConfig{CAData: []byte{2}}},
"cert 1, key 1": {
TLS: TLSConfig{
CertData: []byte{1},
Expand Down Expand Up @@ -157,7 +158,7 @@ func TestTLSConfigKey(t *testing.T) {
continue
}

shouldCacheA := valueA.Proxy == nil
shouldCacheA := valueA.Proxy == nil && (valueA.DialHolder == nil || !valueA.DialHolder.DisableCache)
if shouldCacheA != canCacheA {
t.Errorf("Unexpected canCache=false for " + nameA)
}
Expand Down
3 changes: 3 additions & 0 deletions transport/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ type Config struct {
// DialHolder is used to make the wrapped function comparable so that it can be used as a map key.
type DialHolder struct {
Dial func(ctx context.Context, network, address string) (net.Conn, error)

// DisableCache disables TLS config caching for this DialHolder.
DisableCache bool
}

// ImpersonationConfig has all the available impersonation options
Expand Down

0 comments on commit 5b7ded5

Please sign in to comment.