From 9d5733173cfd688d97734f5d80b1779fc47bdab8 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Tue, 17 Nov 2020 11:31:39 -0500 Subject: [PATCH] Close http bodies and add linter for it Not closing the response body presumably claused flakyness in the registry replacer when it tries to read a response body[0], see[1] for why this can happen. As this seems to be a common mistake in our codebase, this change activates a linter for it. [0] https://issues.redhat.com/browse/DPTP-1692 [1] https://github.com/golang/go/issues/36700 --- .golangci.yml | 1 + pkg/backporter/httpcache.go | 3 +++ pkg/github/github.go | 1 + pkg/job-runtime-analyzer/job-runtime-analyzer.go | 1 + pkg/load/load.go | 2 ++ pkg/release/candidate/client.go | 1 + pkg/release/official/client.go | 1 + pkg/release/prerelease/client.go | 1 + 8 files changed, 11 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index c764aa85017..5a0cd0d7b9a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,6 +14,7 @@ linters: - errcheck - ineffassign - misspell + - bodyclose disable-all: true issues: diff --git a/pkg/backporter/httpcache.go b/pkg/backporter/httpcache.go index 2e65a6684fd..a1674cbdb72 100644 --- a/pkg/backporter/httpcache.go +++ b/pkg/backporter/httpcache.go @@ -67,6 +67,9 @@ func (t *cachingTransport) RoundTrip(req *http.Request) (*http.Response, error) g := errgroup.Group{} g.Go(func() error { var err error + // Disable the bodyclose linter, in this particular case the caller is responsible + // for closing the body. + // nolint:bodyclose resp, err = t.transport.RoundTrip(req) if err != nil { return err diff --git a/pkg/github/github.go b/pkg/github/github.go index c5a3c745608..b5682cde6ca 100644 --- a/pkg/github/github.go +++ b/pkg/github/github.go @@ -52,6 +52,7 @@ func FileGetterFactory(org, repo, branch string, opts ...Opt) FileGetter { if err != nil { return nil, fmt.Errorf("failed to GET %s: %w", url, err) } + defer resp.Body.Close() if resp.StatusCode == http.StatusNotFound { return nil, nil } diff --git a/pkg/job-runtime-analyzer/job-runtime-analyzer.go b/pkg/job-runtime-analyzer/job-runtime-analyzer.go index 30882d8271e..fe44c6bd7d8 100644 --- a/pkg/job-runtime-analyzer/job-runtime-analyzer.go +++ b/pkg/job-runtime-analyzer/job-runtime-analyzer.go @@ -168,6 +168,7 @@ func fetchFromURL(urlString string) ([]byte, error) { if err != nil { return nil, fmt.Errorf("failed to GET %s: %w", parsedURL.String(), err) } + defer result.Body.Close() body, err := ioutil.ReadAll(result.Body) if err != nil { return nil, fmt.Errorf("failed to read response body for request to %s: %w", parsedURL.String(), err) diff --git a/pkg/load/load.go b/pkg/load/load.go index c9bce6749b6..06e18942a08 100644 --- a/pkg/load/load.go +++ b/pkg/load/load.go @@ -192,6 +192,7 @@ func configFromResolver(info *ResolverInfo) (*api.ReleaseBuildConfiguration, err if err != nil { return nil, fmt.Errorf("failed to make request to configresolver: %w", err) } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("response from configresolver == %d (%s)", resp.StatusCode, http.StatusText(resp.StatusCode)) } @@ -221,6 +222,7 @@ func literalConfigFromResolver(raw []byte, address string) (*api.ReleaseBuildCon if err != nil { return nil, fmt.Errorf("failed to request resolved config: %w", err) } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("response from configresolver == %d (%s)", resp.StatusCode, http.StatusText(resp.StatusCode)) } diff --git a/pkg/release/candidate/client.go b/pkg/release/candidate/client.go index e5bc39476be..6c5c7c2ed79 100644 --- a/pkg/release/candidate/client.go +++ b/pkg/release/candidate/client.go @@ -76,6 +76,7 @@ func resolvePullSpec(client release.HTTPClient, endpoint string, relative int) ( if resp == nil { return "", errors.New("failed to request latest release: got a nil response") } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("failed to request latest release: server responded with %d: %s", resp.StatusCode, resp.Body) } diff --git a/pkg/release/official/client.go b/pkg/release/official/client.go index 60cd6e14a00..72f6307c5cb 100644 --- a/pkg/release/official/client.go +++ b/pkg/release/official/client.go @@ -45,6 +45,7 @@ func resolvePullSpec(client release.HTTPClient, endpoint string, release api.Rel if resp == nil { return "", "", errors.New("failed to request latest release: got a nil response") } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return "", "", fmt.Errorf("failed to request latest release: server responded with %d: %s", resp.StatusCode, resp.Body) } diff --git a/pkg/release/prerelease/client.go b/pkg/release/prerelease/client.go index 7551ec2fce3..d230682af14 100644 --- a/pkg/release/prerelease/client.go +++ b/pkg/release/prerelease/client.go @@ -45,6 +45,7 @@ func resolvePullSpec(client release.HTTPClient, endpoint string, bounds api.Vers if resp == nil { return "", errors.New("failed to request latest release: got a nil response") } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("failed to request latest release: server responded with %d: %s", resp.StatusCode, resp.Body) }