refactor(cli): share RetryingRoundTripper via pkg/kubeenv

Move implementation to pkg/kubeenv per review; kube.RetryingRoundTripper
remains a type alias for API compatibility. pkg/cli uses kubeenv only.

Signed-off-by: Sumit Solanki <sumit.solanki@ibm.com>

pkg/cli/environment.go

@@ -35,6 +35,7 @@ import (
 
 	"helm.sh/helm/v4/internal/version"
 	"helm.sh/helm/v4/pkg/helmpath"
+	"helm.sh/helm/v4/pkg/kubeenv"
 )
 
 // defaultMaxHistory sets the maximum number of releases to 0: unlimited
@@ -133,7 +134,7 @@ func New() *EnvSettings {
 			config.Burst = env.BurstLimit
 			config.QPS = env.QPS
 			config.Wrap(func(rt http.RoundTripper) http.RoundTripper {
-				return &retryingRoundTripper{wrapped: rt}
+				return &kubeenv.RetryingRoundTripper{Wrapped: rt}
 			})
 			config.UserAgent = version.GetUserAgent()
 			return config

pkg/cli/roundtripper_test.go

@@ -1,161 +0,0 @@
-/*
-Copyright The Helm Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package cli
-
-import (
-	"encoding/json"
-	"errors"
-	"io"
-	"net/http"
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-type fakeRoundTripper struct {
-	resp  *http.Response
-	err   error
-	calls int
-}
-
-func (f *fakeRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) {
-	f.calls++
-	return f.resp, f.err
-}
-
-func newRespWithBody(statusCode int, contentType, body string) *http.Response {
-	return &http.Response{
-		StatusCode: statusCode,
-		Header:     http.Header{"Content-Type": []string{contentType}},
-		Body:       io.NopCloser(strings.NewReader(body)),
-	}
-}
-
-func TestRetryingRoundTripper_RoundTrip(t *testing.T) {
-	marshalErr := func(code int, msg string) string {
-		b, _ := json.Marshal(kubernetesError{
-			Code:    code,
-			Message: msg,
-		})
-		return string(b)
-	}
-
-	tests := []struct {
-		name          string
-		resp          *http.Response
-		err           error
-		expectedCalls int
-		expectedErr   string
-		expectedCode  int
-	}{
-		{
-			name:          "no retry, status < 500 returns response",
-			resp:          newRespWithBody(200, "application/json", `{"message":"ok","code":200}`),
-			err:           nil,
-			expectedCalls: 1,
-			expectedCode:  200,
-		},
-		{
-			name:          "error from wrapped RoundTripper propagates",
-			resp:          nil,
-			err:           errors.New("wrapped error"),
-			expectedCalls: 1,
-			expectedErr:   "wrapped error",
-		},
-		{
-			name:          "no retry, content-type not application/json",
-			resp:          newRespWithBody(500, "text/plain", "server error"),
-			err:           nil,
-			expectedCalls: 1,
-			expectedCode:  500,
-		},
-		{
-			name: "error reading body returns error",
-			resp: &http.Response{
-				StatusCode: http.StatusInternalServerError,
-				Header:     http.Header{"Content-Type": []string{"application/json"}},
-				Body:       &errReader{},
-			},
-			err:           nil,
-			expectedCalls: 1,
-			expectedErr:   "read error",
-		},
-		{
-			name:          "error decoding JSON returns error",
-			resp:          newRespWithBody(500, "application/json", `invalid-json`),
-			err:           nil,
-			expectedCalls: 1,
-			expectedErr:   "invalid character",
-		},
-		{
-			name:          "retry on etcdserver leader changed message",
-			resp:          newRespWithBody(500, "application/json", marshalErr(500, "some error etcdserver: leader changed")),
-			err:           nil,
-			expectedCalls: 2,
-			expectedCode:  500,
-		},
-		{
-			name:          "retry on raft proposal dropped message",
-			resp:          newRespWithBody(500, "application/json", marshalErr(500, "rpc error: code = Unknown desc = raft proposal dropped")),
-			err:           nil,
-			expectedCalls: 2,
-			expectedCode:  500,
-		},
-		{
-			name:          "no retry on other error message",
-			resp:          newRespWithBody(500, "application/json", marshalErr(500, "other server error")),
-			err:           nil,
-			expectedCalls: 1,
-			expectedCode:  500,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			fakeRT := &fakeRoundTripper{
-				resp: tt.resp,
-				err:  tt.err,
-			}
-			rt := retryingRoundTripper{
-				wrapped: fakeRT,
-			}
-			req, _ := http.NewRequest(http.MethodGet, "http://example.com", nil)
-			resp, err := rt.RoundTrip(req)
-
-			if tt.expectedErr != "" {
-				assert.Error(t, err)
-				assert.Contains(t, err.Error(), tt.expectedErr)
-				return
-			}
-			assert.NoError(t, err)
-
-			assert.Equal(t, tt.expectedCode, resp.StatusCode)
-			assert.Equal(t, tt.expectedCalls, fakeRT.calls)
-		})
-	}
-}
-
-type errReader struct{}
-
-func (e *errReader) Read(_ []byte) (int, error) {
-	return 0, errors.New("read error")
-}
-
-func (e *errReader) Close() error {
-	return nil
-}

pkg/kube/roundtripper.go

@@ -16,65 +16,9 @@ limitations under the License.
 
 package kube
 
-import (
+import "helm.sh/helm/v4/pkg/kubeenv"
-	"bytes"
-	"encoding/json"
-	"io"
-	"net/http"
-	"strings"
-)
 
-type RetryingRoundTripper struct {
+// RetryingRoundTripper retries transient Kubernetes API server errors on a
-	Wrapped http.RoundTripper
+// wrapped [http.RoundTripper]. The implementation lives in [kubeenv] so
-}
+// consumers can depend on that package without importing all of kube.
-
+type RetryingRoundTripper = kubeenv.RetryingRoundTripper
-func (rt *RetryingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
-	return rt.roundTrip(req, 1, nil)
-}
-
-func (rt *RetryingRoundTripper) roundTrip(req *http.Request, retry int, prevResp *http.Response) (*http.Response, error) {
-	if retry < 0 {
-		return prevResp, nil
-	}
-	resp, rtErr := rt.Wrapped.RoundTrip(req)
-	if rtErr != nil {
-		return resp, rtErr
-	}
-	if resp.StatusCode < 500 {
-		return resp, rtErr
-	}
-	if resp.Header.Get("content-type") != "application/json" {
-		return resp, rtErr
-	}
-	b, err := io.ReadAll(resp.Body)
-	resp.Body.Close()
-	if err != nil {
-		return resp, err
-	}
-
-	var ke kubernetesError
-	r := bytes.NewReader(b)
-	err = json.NewDecoder(r).Decode(&ke)
-	r.Seek(0, io.SeekStart)
-	resp.Body = io.NopCloser(r)
-	if err != nil {
-		return resp, err
-	}
-	if ke.Code < 500 {
-		return resp, nil
-	}
-	// Matches messages like "etcdserver: leader changed"
-	if strings.HasSuffix(ke.Message, "etcdserver: leader changed") {
-		return rt.roundTrip(req, retry-1, resp)
-	}
-	// Matches messages like "rpc error: code = Unknown desc = raft proposal dropped"
-	if strings.HasSuffix(ke.Message, "raft proposal dropped") {
-		return rt.roundTrip(req, retry-1, resp)
-	}
-	return resp, nil
-}
-
-type kubernetesError struct {
-	Message string `json:"message"`
-	Code    int    `json:"code"`
-}

pkg/kubeenv/roundtripper.go

@@ -14,7 +14,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cli
+// Package kubeenv holds small, cycle-free Kubernetes client helpers shared by
+// higher-level packages (for example pkg/cli and pkg/kube).
+package kubeenv
 
 import (
 	"bytes"
@@ -24,21 +26,21 @@ import (
 	"strings"
 )
 
-// retryingRoundTripper retries selected transient Kubernetes API server failures.
+// RetryingRoundTripper retries transient Kubernetes API server errors on a
-// Keeping this in pkg/cli avoids importing pkg/kube from EnvSettings.
+// wrapped [http.RoundTripper].
-type retryingRoundTripper struct {
+type RetryingRoundTripper struct {
-	wrapped http.RoundTripper
+	Wrapped http.RoundTripper
 }
 
-func (rt *retryingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+func (rt *RetryingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	return rt.roundTrip(req, 1, nil)
 }
 
-func (rt *retryingRoundTripper) roundTrip(req *http.Request, retry int, prevResp *http.Response) (*http.Response, error) {
+func (rt *RetryingRoundTripper) roundTrip(req *http.Request, retry int, prevResp *http.Response) (*http.Response, error) {
 	if retry < 0 {
 		return prevResp, nil
 	}
-	resp, rtErr := rt.wrapped.RoundTrip(req)
+	resp, rtErr := rt.Wrapped.RoundTrip(req)
 	if rtErr != nil {
 		return resp, rtErr
 	}

pkg/kubeenv/roundtripper_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package kube
+package kubeenv
 
 import (
 	"encoding/json"