Skip to content

bug: Respect provided kubeconfig in helm collector - #1833

Merged
emosbaugh merged 1 commit into
replicatedhq:mainfrom
Danil-Grigorev:respect-helm-kubeconfig-helm
Oct 2, 2025
Merged

bug: Respect provided kubeconfig in helm collector#1833
emosbaugh merged 1 commit into
replicatedhq:mainfrom
Danil-Grigorev:respect-helm-kubeconfig-helm

Conversation

@Danil-Grigorev

@Danil-Grigorev Danil-Grigorev commented Aug 31, 2025

Copy link
Copy Markdown
Contributor

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 server https://localhost:8080 while performing operations on a host without KUBECONFIG or ~/.kube/config. In a scenario, when the support-bundle was used as a library, this resulted in a failed collection for helm.

To fix this, a new configGetter type was introduced that implements genericclioptions.RESTClientGetter. Helm collector now passes c.ClientConfig through.

Checklist

  • New and existing tests pass locally with introduced changes.
  • Tests for the changes have been added (for bug fixes / features).
  • The commit message(s) are informative and highlight any breaking changes.
  • Any documentation required has been added/updated. For changes to https://troubleshoot.sh/ create a PR here.

Does this PR introduce a breaking change?

  • Yes
  • No

@Danil-Grigorev
Danil-Grigorev requested a review from a team as a code owner August 31, 2025 21:54
@Danil-Grigorev Danil-Grigorev changed the title Respect provided kubeconfig in helm collector bug: Respect provided kubeconfig in helm collector Sep 1, 2025
@emosbaugh emosbaugh added type::bug Something isn't working bug::normal labels Sep 3, 2025
Comment thread pkg/collect/helm.go
Comment on lines +57 to +59
func (c configGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) {
return nil, nil
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/collect/helm.go
Comment thread pkg/collect/helm.go
@AmberAlston

Copy link
Copy Markdown
Member

@Danil-Grigorev Making sure you saw the PR feedback above from @emosbaugh

cursor[bot]

This comment was marked as outdated.

@AmberAlston
AmberAlston requested a review from emosbaugh October 1, 2025 21:30

@emosbaugh emosbaugh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Danil-Grigorev force-pushed the respect-helm-kubeconfig-helm branch from 30df521 to 4606444 Compare October 2, 2025 06:22
@emosbaugh
emosbaugh merged commit fcf46d4 into replicatedhq:main Oct 2, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug::normal type::bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants