Skip to content

Commit 648f541

Browse files
committed
webhook: fix memory leak in the 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 open-policy-agent#2654
1 parent effa347 commit 648f541

File tree

1 file changed

+16
-8
lines changed

1 file changed

+16
-8
lines changed

pkg/webhook/health_check.go

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,38 @@ import (
44
"bytes"
55
"crypto/tls"
66
"fmt"
7+
"io"
78
"net/http"
89
"path/filepath"
910

1011
logf "sigs.k8s.io/controller-runtime/pkg/log"
1112
)
1213

13-
// disabling gosec linting here as the http client used in this checking is intended to skip CA verification
14-
//
15-
//nolint:gosec
16-
var tr = &http.Transport{
17-
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
18-
}
19-
var insecureClient = &http.Client{Transport: tr}
20-
2114
var tlsCheckerLog = logf.Log.WithName("webhook-tls-checker")
2215

2316
func NewTLSChecker(certDir string, port int) func(*http.Request) error {
17+
//nolint:forcetypeassert
18+
tr := http.DefaultTransport.(*http.Transport).Clone()
19+
// disabling gosec linting here as the http client used in this checking is intended to skip CA verification
20+
//
21+
//nolint:gosec
22+
tr.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
23+
// disable keep alives to ensure that http connection aren't reused, otherwise the check may
24+
// fail if the cert was rotated in between
25+
tr.DisableKeepAlives = true
26+
insecureClient := &http.Client{Transport: tr}
27+
2428
returnFunc := func(_ *http.Request) error {
2529
resp, err := insecureClient.Get(fmt.Sprintf("https://127.0.0.1:%d", port))
2630
if err != nil {
2731
newErr := fmt.Errorf("unable to connect to server: %w", err)
2832
tlsCheckerLog.Error(newErr, "error in connecting to webhook server with https")
2933
return newErr
3034
}
35+
defer resp.Body.Close()
36+
// explicitly discard the body to avoid any memory leak
37+
_, _ = io.Copy(io.Discard, resp.Body)
38+
3139
if len(resp.TLS.PeerCertificates) == 0 {
3240
newErr := fmt.Errorf("webhook does not serve TLS certificate")
3341
tlsCheckerLog.Error(newErr, "error in connecting to webhook server with https")

0 commit comments

Comments
 (0)