go-helm-client icon indicating copy to clipboard operation
go-helm-client copied to clipboard

Support configuration of chartPathOptions in install/upgrade client interfaces

Open vl306 opened this issue 2 years ago • 2 comments

The existing helm install/upgrade client interface functions doesn't seem to support configuration of the chartPathOptions. We have a use case where we want to set "client.ChartPathOptions.InsecureSkipTLSverify = true".

One option is to modify the functions to take the additional argument.

If this the right/preferred approach?.

Existing Client interface:

 InstallOrUpgradeChart(ctx context.Context, spec *ChartSpec, opts *GenericHelmOptions) (*release.Release, error)
 InstallChart(ctx context.Context, spec *ChartSpec, opts *GenericHelmOptions) (*release.Release, error)
 UpgradeChart(ctx context.Context, spec *ChartSpec, opts *GenericHelmOptions) (*release.Release, error)

Proposed Changes:

  InstallOrUpgradeChart(ctx context.Context, spec *ChartSpec, opts *GenericHelmOptions, chartPathOptions *action.ChartPathOptions) (*release.Release, error)
  InstallChart(ctx context.Context, spec *ChartSpec, opts *GenericHelmOptions, chartPathOptions *action.ChartPathOptions) (*release.Release, error)
  UpgradeChart(ctx context.Context, spec *ChartSpec, opts *GenericHelmOptions, chartPathOptions *action.ChartPathOptions) (*release.Release, error)

Alternatively, to avoid changing the interface, the ChartPathOptions can be added to GenericHelmOptions:

@@ -88,8 +88,9 @@ type HelmClient struct {
 }

 type GenericHelmOptions struct {
-       PostRenderer postrender.PostRenderer
-       RollBack     RollBack
+       PostRenderer     postrender.PostRenderer
+       RollBack         RollBack
+       ChartPathOptions *action.ChartPathOptions
 }

vl306 avatar Aug 12 '23 01:08 vl306

Agreed, and yes please.

My case is not about skipping TLS verification, albeit related. Instead of disabling verification I wish to provide the necessary supporting certificates through the ChartPathOptions.CaFile field.

andreas-kupries avatar Oct 12 '23 13:10 andreas-kupries

The following pull request contains the suggested changes. Just waiting for someone with permissions to merge the pull request:

https://github.com/mittwald/go-helm-client/pull/182

vl306 avatar Oct 12 '23 13:10 vl306