Skip to content

Commit

Permalink
Close http bodies and add linter for it
Browse files Browse the repository at this point in the history
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] golang/go#36700
  • Loading branch information
alvaroaleman committed Nov 17, 2020
1 parent 15d4876 commit 9d57331
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 0 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ linters:
- errcheck
- ineffassign
- misspell
- bodyclose
disable-all: true

issues:
Expand Down
3 changes: 3 additions & 0 deletions pkg/backporter/httpcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions pkg/job-runtime-analyzer/job-runtime-analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions pkg/load/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down
1 change: 1 addition & 0 deletions pkg/release/candidate/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/release/official/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/release/prerelease/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 9d57331

Please sign in to comment.