Skip to content

refactor(helm): resolve config in constructor following Kiali pattern#1006

Merged
nader-ziada merged 1 commit into
containers:mainfrom
marcnuri-forks:review/helm-add-backend-storage-driver
Apr 2, 2026
Merged

refactor(helm): resolve config in constructor following Kiali pattern#1006
nader-ziada merged 1 commit into
containers:mainfrom
marcnuri-forks:review/helm-add-backend-storage-driver

Conversation

@manusa

@manusa manusa commented Apr 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Move Helm toolset config resolution from individual tool handlers into NewHelm constructor
  • Remove duplicated GetToolsetConfig/type-assertion boilerplate from helmInstall, helmList, and helmUninstall
  • Drop *Config parameter from Install, List, Uninstall, and newAction methods — config is now read from the Helm struct
  • Aligns with the Kiali toolset pattern where config is resolved once at instantiation time

Follows up on #998.

@manusa
manusa marked this pull request as ready for review April 2, 2026 11:05
@manusa manusa added this to the 0.1.0 milestone Apr 2, 2026
@manusa
manusa marked this pull request as draft April 2, 2026 11:06
@manusa
manusa force-pushed the review/helm-add-backend-storage-driver branch from ac02aeb to 2db61ce Compare April 2, 2026 11:09
Move Helm config resolution from individual tool handlers into
NewHelm, eliminating duplicated boilerplate across helmInstall,
helmList, and helmUninstall. This aligns with the Kiali toolset
pattern where config is resolved once at instantiation time.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Marc Nuri <marc@marcnuri.com>
@manusa
manusa force-pushed the review/helm-add-backend-storage-driver branch from 2db61ce to a690f36 Compare April 2, 2026 11:19
@manusa
manusa marked this pull request as ready for review April 2, 2026 11:23
@difrost

difrost commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

@manusa +1 for this ... I was planning to go this path but didn't wanted to break the interface. This opens up for some more controls over helm via config file that I would like to propose in next few days.

@manusa
manusa requested review from Cali0707 and nader-ziada April 2, 2026 15:33

@nader-ziada nader-ziada left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@nader-ziada
nader-ziada merged commit 0fb47b5 into containers:main Apr 2, 2026
9 checks passed
@manusa
manusa deleted the review/helm-add-backend-storage-driver branch April 3, 2026 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants