Close HTTP response body on failed GET attempts (#105591)

* Close response body when it's not needed

* Code cleanups

staging/src/k8s.io/cli-runtime/pkg/resource/builder.go

@@ -1155,10 +1155,9 @@ func (b *Builder) Do() *Result {
 		helpers = append(helpers, RetrieveLazy)
 	}
 	if b.continueOnError {
-		r.visitor = NewDecoratedVisitor(ContinueOnErrorVisitor{r.visitor}, helpers...)
+		r.visitor = ContinueOnErrorVisitor{Visitor: r.visitor}
-	} else {
-		r.visitor = NewDecoratedVisitor(r.visitor, helpers...)
 	}
+	r.visitor = NewDecoratedVisitor(r.visitor, helpers...)
 	return r
 }
 

staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go

@@ -204,9 +204,9 @@ type EagerVisitorList []Visitor
 // Visit implements Visitor, and gathers errors that occur during processing until
 // all sub visitors have been visited.
 func (l EagerVisitorList) Visit(fn VisitorFunc) error {
-	errs := []error(nil)
+	var errs []error
 	for i := range l {
-		if err := l[i].Visit(func(info *Info, err error) error {
+		err := l[i].Visit(func(info *Info, err error) error {
 			if err != nil {
 				errs = append(errs, err)
 				return nil
@@ -215,7 +215,8 @@ func (l EagerVisitorList) Visit(fn VisitorFunc) error {
 				errs = append(errs, err)
 			}
 			return nil
-		}); err != nil {
+		})
+		if err != nil {
 			errs = append(errs, err)
 		}
 	}
@@ -253,13 +254,15 @@ func (v *URLVisitor) Visit(fn VisitorFunc) error {
 // readHttpWithRetries tries to http.Get the v.URL retries times before giving up.
 func readHttpWithRetries(get httpget, duration time.Duration, u string, attempts int) (io.ReadCloser, error) {
 	var err error
-	var body io.ReadCloser
 	if attempts <= 0 {
 		return nil, fmt.Errorf("http attempts must be greater than 0, was %d", attempts)
 	}
 	for i := 0; i < attempts; i++ {
-		var statusCode int
+		var (
-		var status string
+			statusCode int
+			status     string
+			body       io.ReadCloser
+		)
 		if i > 0 {
 			time.Sleep(duration)
 		}
@@ -272,10 +275,12 @@ func readHttpWithRetries(get httpget, duration time.Duration, u string, attempts
 			continue
 		}
 
-		// Error - Set the error condition from the StatusCode
+		if statusCode == http.StatusOK {
-		if statusCode != http.StatusOK {
+			return body, nil
-			err = fmt.Errorf("unable to read URL %q, server reported %s, status code=%d", u, status, statusCode)
 		}
+		body.Close()
+		// Error - Set the error condition from the StatusCode
+		err = fmt.Errorf("unable to read URL %q, server reported %s, status code=%d", u, status, statusCode)
 
 		if statusCode >= 500 && statusCode < 600 {
 			// Retry 500's
@@ -285,7 +290,7 @@ func readHttpWithRetries(get httpget, duration time.Duration, u string, attempts
 			break
 		}
 	}
-	return body, err
+	return nil, err
 }
 
 // httpget Defines function to retrieve a url and return the results.  Exists for unit test stubbing.
@@ -346,7 +351,7 @@ type ContinueOnErrorVisitor struct {
 // returned by the visitor directly may still result in some items
 // not being visited.
 func (v ContinueOnErrorVisitor) Visit(fn VisitorFunc) error {
-	errs := []error{}
+	var errs []error
 	err := v.Visitor.Visit(func(info *Info, err error) error {
 		if err != nil {
 			errs = append(errs, err)
@@ -420,7 +425,7 @@ func (v FlattenListVisitor) Visit(fn VisitorFunc) error {
 		if info.Mapping != nil && !info.Mapping.GroupVersionKind.Empty() {
 			preferredGVKs = append(preferredGVKs, info.Mapping.GroupVersionKind)
 		}
-		errs := []error{}
+		var errs []error
 		for i := range items {
 			item, err := v.mapper.infoForObject(items[i], v.typer, preferredGVKs)
 			if err != nil {
@@ -439,7 +444,6 @@ func (v FlattenListVisitor) Visit(fn VisitorFunc) error {
 			}
 		}
 		return utilerrors.NewAggregate(errs)
-
 	})
 }
 

staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go

@@ -75,7 +75,7 @@ func TestVisitorHttpGet(t *testing.T) {
 			httpRetries: func(url string) (int, string, io.ReadCloser, error) {
 				assert.Equal(t, "hello", url)
 				i++
-				return 501, "Status", nil, nil
+				return 501, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil
 			},
 			args: httpArgs{
 				duration: 0,
@@ -89,7 +89,7 @@ func TestVisitorHttpGet(t *testing.T) {
 			httpRetries: func(url string) (int, string, io.ReadCloser, error) {
 				assert.Equal(t, "hello", url)
 				i++
-				return 300, "Status", nil, nil
+				return 300, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil
 
 			},
 			args: httpArgs{
@@ -104,7 +104,7 @@ func TestVisitorHttpGet(t *testing.T) {
 			httpRetries: func(url string) (int, string, io.ReadCloser, error) {
 				assert.Equal(t, "hello", url)
 				i++
-				return 501, "Status", nil, nil
+				return 501, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil
 
 			},
 			args: httpArgs{
@@ -135,7 +135,7 @@ func TestVisitorHttpGet(t *testing.T) {
 				if i > 1 {
 					return 200, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil
 				}
-				return 501, "Status", nil, nil
+				return 501, "Status", ioutil.NopCloser(new(bytes.Buffer)), nil
 
 			},
 			args: httpArgs{