From 69300f50122d1ab1ee79fef1ff220eaf54594c6a Mon Sep 17 00:00:00 2001 From: Thibault Deutsch Date: Sat, 8 Apr 2023 12:02:56 +0100 Subject: [PATCH] fix: memory leak in the webhook TLS healthcheck - The resp.Body was never closed, thus causing one connection to be leaked for each executions. - Creating a new transport based on the default transport, to inherit some of the default timeouts. Most importantly, this ensure that there is a default TLSHandshakeTimeout (10s) and dial timeout (30s). - Disable http keep alive, to avoid reuse of the same http connection. Otherwise, we may fail the healthcheck when the certs is rotated (new cert on disk wouldn't match the cert attached to the reused connection opened earlier). Fix #2654 Signed-off-by: Thibault Deutsch --- pkg/webhook/health_check.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/pkg/webhook/health_check.go b/pkg/webhook/health_check.go index b0dd29f2c71..e0960f681dd 100644 --- a/pkg/webhook/health_check.go +++ b/pkg/webhook/health_check.go @@ -4,23 +4,27 @@ import ( "bytes" "crypto/tls" "fmt" + "io" "net/http" "path/filepath" logf "sigs.k8s.io/controller-runtime/pkg/log" ) -// disabling gosec linting here as the http client used in this checking is intended to skip CA verification -// -//nolint:gosec -var tr = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, -} -var insecureClient = &http.Client{Transport: tr} - var tlsCheckerLog = logf.Log.WithName("webhook-tls-checker") func NewTLSChecker(certDir string, port int) func(*http.Request) error { + //nolint:forcetypeassert + tr := http.DefaultTransport.(*http.Transport).Clone() + // disabling gosec linting here as the http client used in this checking is intended to skip CA verification + // + //nolint:gosec + tr.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + // disable keep alives to ensure that http connection aren't reused, otherwise the check may + // fail if the cert was rotated in between + tr.DisableKeepAlives = true + insecureClient := &http.Client{Transport: tr} + returnFunc := func(_ *http.Request) error { resp, err := insecureClient.Get(fmt.Sprintf("https://127.0.0.1:%d", port)) if err != nil { @@ -28,6 +32,10 @@ func NewTLSChecker(certDir string, port int) func(*http.Request) error { tlsCheckerLog.Error(newErr, "error in connecting to webhook server with https") return newErr } + defer resp.Body.Close() + // explicitly discard the body to avoid any memory leak + _, _ = io.Copy(io.Discard, resp.Body) + if len(resp.TLS.PeerCertificates) == 0 { newErr := fmt.Errorf("webhook does not serve TLS certificate") tlsCheckerLog.Error(newErr, "error in connecting to webhook server with https")