refactor(cli): decouple EnvSettings from pkg/kube to avoid import cycles#31970
refactor(cli): decouple EnvSettings from pkg/kube to avoid import cycles#31970isumitsolanki wants to merge 3 commits intohelm:mainfrom
Conversation
Move the retrying round tripper used by EnvSettings into pkg/cli so pkg/cli no longer imports pkg/kube. This preserves retry behavior while breaking the import edge that triggers cycles for Helm library consumers (such as the kustomize integration described in helm#31965). Signed-off-by: Sumit Solanki <sumit.solanki@ibm.com>
pull-request-size
bot
added
the
size/M
There was a problem hiding this comment.
Pull request overview
Refactors pkg/cli.EnvSettings to remove its dependency on pkg/kube, preventing import cycles for Helm-as-a-library consumers while preserving the existing retry behavior for transient Kubernetes API server errors.
Changes:
- Add a
pkg/cli-local retryinghttp.RoundTripperimplementation. - Update
EnvSettingsREST config wrapping to use the newpkg/cliround-tripper and drop thepkg/kubeimport.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/cli/environment.go | Removes pkg/kube dependency by switching the REST config wrapper to a pkg/cli-local round-tripper. |
| pkg/cli/roundtripper.go | Introduces the retrying round-tripper implementation previously sourced from pkg/kube. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move the retrying HTTP round-tripper used by EnvSettings into pkg/cli so pkg/cli no longer imports pkg/kube, avoiding import cycles for Helm library consumers while preserving retry behavior for transient API server errors. Add pkg/cli/roundtripper_test.go with parity coverage for the moved logic. Signed-off-by: Sumit Solanki <sumit.solanki@ibm.com> Made-with: Cursor
pull-request-size
bot
added
size/L
There was a problem hiding this comment.
I think rather than duplicating the logic from pkg/kube/roundtripper.go into pkg/cli/roundtripper.go, it would be preferred to push pkg/kube. RetryingRoundTripper into a "lower level" package
ie.
create pkg/kubeenv/roundtripper.go
pkg/cli/environment.go would use kubeenv.RetryingRoundTripper
pkg/kube/roundtripper.go would do type RetryingRoundTripper = kubeenv.RetryingRoundTripper (to preserve api compatibility)
(I don't think kubeenv is the best name, was the best I could come up with for this review comment)
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>
gjenkins8
left a comment
gjenkins8
left a comment
There was a problem hiding this comment.
I can't think of a better name than kubeenv. If someone else can, please suggest.
gjenkins8
added
the
Has One Approval
closes #31965
What this PR does / why we need it:
This PR decouples pkg/cli from pkg/kube in the EnvSettings code path to prevent import cycles for Helm library consumers.
Today, pkg/cli/environment.go imports pkg/kube only to use RetryingRoundTripper. That import pulls in a larger dependency chain and can create cycles for consumers (for example, the kustomize integration described in #31965) when importing Helm packages that depend on pkg/cli.
To fix this, the retrying round-tripper logic used by EnvSettings is moved into pkg/cli, and the pkg/cli -> pkg/kube import edge is removed. Behavior is preserved; this is a dependency-graph refactor, not a functional change.
Special notes for your reviewer:
Scope is intentionally minimal and focused on import decoupling.
Retry behavior for transient Kubernetes API server errors remains the same.
No public API signatures were changed.
Files changed:
pkg/cli/environment.go
pkg/cli/roundtripper.go
If applicable:
docs neededlabel should be applied if so)