bug: Respect provided kubeconfig in helm collector - #1833
Merged
emosbaugh merged 1 commit intoOct 2, 2025
Conversation
emosbaugh
reviewed
Sep 3, 2025
Comment on lines
+57
to
+59
| func (c configGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { | ||
| return nil, nil | ||
| } |
Contributor
There was a problem hiding this comment.
I am unclear if this function is used by the helm client. were you able to confirm either way? it may be possible to implement it like so.
Suggested change
| func (c configGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { | |
| return nil, nil | |
| } | |
| func (c configGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { | |
| discoveryClient, err := discovery.NewDiscoveryClientForConfig(c.restConfig) | |
| if err != nil { | |
| return nil, err | |
| } | |
| cached := memory.NewMemCacheClient(discoveryClient) | |
| return cached, nil | |
| } |
Contributor
Author
There was a problem hiding this comment.
I’ll check tomorrow, but from what I observed, only the implemented method was used. Helm operations there simply listed known resources, so it didn’t require discovery. Committed anyway, thanks for pointing that out.
Member
|
@Danil-Grigorev Making sure you saw the PR feedback above from @emosbaugh |
emosbaugh
suggested changes
Oct 1, 2025
emosbaugh
left a comment
Contributor
There was a problem hiding this comment.
Looks like this is now failing to compile
Signed-off-by: Danil-Grigorev <daniil.grigorev.dev@gmail.com> Co-authored-by: Ethan Mosbaugh <emosbaugh@gmail.com>
Danil-Grigorev
force-pushed
the
respect-helm-kubeconfig-helm
branch
from
October 2, 2025 06:22
30df521 to
4606444
Compare
emosbaugh
approved these changes
Oct 2, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description, Motivation and Context
This change updates the Helm collector to properly use provided client configuration.
Previously, the collector used
actionConfig.Init(nil, ...), which relied on implicit defaults and did not respect the configured cluster context. This resulted in using serverhttps://localhost:8080while performing operations on a host withoutKUBECONFIGor~/.kube/config. In a scenario, when thesupport-bundlewas used as a library, this resulted in a failed collection forhelm.To fix this, a new
configGettertype was introduced that implementsgenericclioptions.RESTClientGetter. Helm collector now passesc.ClientConfigthrough.Checklist
Does this PR introduce a breaking change?