Skip to content

Commit 3add14b

Browse files
manusamanusa
authored
fix(go): update NewCfg to return error for better error handling (#312)
1 parent c80c442 commit 3add14b

11 files changed

Lines changed: 118 additions & 27 deletions

File tree

‎native/internal/helm/helm.go‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ type CertOptions struct {
5656
Keyring string
5757
}
5858

59-
func NewCfg(options *CfgOptions) *action.Configuration {
59+
func NewCfg(options *CfgOptions) (*action.Configuration, error) {
6060
settings := cli.New()
6161
settings.KubeConfig = options.KubeConfig
6262
actionConfig := new(action.Configuration)
@@ -73,23 +73,24 @@ func NewCfg(options *CfgOptions) *action.Configuration {
7373
effectiveNamespace = ""
7474
}
7575
restClientGetter := settings.RESTClientGetter()
76-
restClientGetter.(*genericclioptions.ConfigFlags).WrapConfigFn = func(original *rest.Config) *rest.Config {
77-
if options.KubeConfigContents != "" {
78-
// TODO: we could actually merge both kubeconfigs
79-
config, err := clientcmd.RESTConfigFromKubeConfig([]byte(options.KubeConfigContents))
80-
if err != nil {
81-
panic(err)
82-
}
83-
return config
76+
// Validate KubeConfigContents upfront if provided
77+
if options.KubeConfigContents != "" {
78+
// TODO: we could actually merge both kubeconfigs
79+
parsedConfig, err := clientcmd.RESTConfigFromKubeConfig([]byte(options.KubeConfigContents))
80+
if err != nil {
81+
return nil, fmt.Errorf("failed to parse kubeconfig contents: %w", err)
82+
}
83+
// Use the validated config in the wrapper
84+
restClientGetter.(*genericclioptions.ConfigFlags).WrapConfigFn = func(original *rest.Config) *rest.Config {
85+
return parsedConfig
8486
}
85-
return original
8687
}
8788
err := actionConfig.Init(restClientGetter, effectiveNamespace, os.Getenv("HELM_DRIVER"), log)
8889
if err != nil {
89-
panic(err)
90+
return nil, fmt.Errorf("failed to initialize action configuration: %w", err)
9091
}
9192
actionConfig.RegistryClient = options.RegistryClient
92-
return actionConfig
93+
return actionConfig, nil
9394
}
9495

9596
func StatusReport(release *release.Release, showDescription bool, debug bool) string {

‎native/internal/helm/helm_test.go‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,12 @@ users:
5858
`
5959

6060
func TestNewCfg_KubeConfigContents(t *testing.T) {
61-
cfg := NewCfg(&CfgOptions{
61+
cfg, err := NewCfg(&CfgOptions{
6262
KubeConfigContents: kubeConfigContentsForTests,
6363
})
64+
if err != nil {
65+
t.Fatalf("Expected NewCfg to succeed, got %s", err)
66+
}
6467
restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
6568
if err != nil {
6669
t.Fatalf("Expected converting to succeed, got %s", err)
@@ -75,3 +78,35 @@ func TestNewCfg_KubeConfigContents(t *testing.T) {
7578
t.Fatalf("Expected test-password, got %s", restConfig.Password)
7679
}
7780
}
81+
82+
func TestNewCfg_InvalidKubeConfigContents(t *testing.T) {
83+
t.Run("should return error for invalid kubeconfig contents", func(t *testing.T) {
84+
cfg, err := NewCfg(&CfgOptions{
85+
KubeConfigContents: "invalid yaml content {[}",
86+
})
87+
if err == nil {
88+
t.Fatal("Expected NewCfg to return error for invalid kubeconfig, got nil")
89+
}
90+
if cfg != nil {
91+
t.Errorf("Expected cfg to be nil when error occurs, got %v", cfg)
92+
}
93+
})
94+
95+
t.Run("should return error for malformed kubeconfig", func(t *testing.T) {
96+
malformedConfig := `
97+
apiVersion: v1
98+
kind: Config
99+
clusters:
100+
- this is not valid yaml structure
101+
`
102+
cfg, err := NewCfg(&CfgOptions{
103+
KubeConfigContents: malformedConfig,
104+
})
105+
if err == nil {
106+
t.Fatal("Expected NewCfg to return error for malformed kubeconfig, got nil")
107+
}
108+
if cfg != nil {
109+
t.Errorf("Expected cfg to be nil when error occurs, got %v", cfg)
110+
}
111+
})
112+
}

‎native/internal/helm/install.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,11 @@ func install(options *InstallOptions) (*release.Release, *installOutputs, error)
106106
if options.Debug {
107107
cfgOptions.KubeOut = outputs.kubeOut
108108
}
109-
client := action.NewInstall(NewCfg(cfgOptions))
109+
cfg, err := NewCfg(cfgOptions)
110+
if err != nil {
111+
return nil, outputs, err
112+
}
113+
client := action.NewInstall(cfg)
110114
client.GenerateName = options.GenerateName
111115
client.NameTemplate = options.NameTemplate
112116
client.Version = options.Version

‎native/internal/helm/list.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,15 @@ type ListOptions struct {
4040
}
4141

4242
func List(options *ListOptions) (string, error) {
43-
cfg := NewCfg(&CfgOptions{
43+
cfg, err := NewCfg(&CfgOptions{
4444
KubeConfig: options.KubeConfig,
4545
KubeConfigContents: options.KubeConfigContents,
4646
Namespace: options.Namespace,
4747
AllNamespaces: options.AllNamespaces,
4848
})
49+
if err != nil {
50+
return "", err
51+
}
4952
client := action.NewList(cfg)
5053
client.All = options.All
5154
client.AllNamespaces = options.AllNamespaces

‎native/internal/helm/plugins_test.go‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,13 @@ users:
128128

129129
func TestAuthPlugins(t *testing.T) {
130130
t.Run("should support azure via exec plugin (kubelogin)", func(t *testing.T) {
131-
cfg := NewCfg(&CfgOptions{
131+
cfg, err := NewCfg(&CfgOptions{
132132
KubeConfigContents: azureExecKubeConfig,
133133
})
134+
if err != nil {
135+
t.Errorf("Expected NewCfg to succeed, got %s", err)
136+
return
137+
}
134138
restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
135139
if err != nil {
136140
t.Errorf("Expected azure exec kubeconfig to parse successfully, got %s", err)
@@ -145,9 +149,13 @@ func TestAuthPlugins(t *testing.T) {
145149
}
146150
})
147151
t.Run("should support gcp via exec plugin (gke-gcloud-auth-plugin)", func(t *testing.T) {
148-
cfg := NewCfg(&CfgOptions{
152+
cfg, err := NewCfg(&CfgOptions{
149153
KubeConfigContents: gcpExecKubeConfig,
150154
})
155+
if err != nil {
156+
t.Errorf("Expected NewCfg to succeed, got %s", err)
157+
return
158+
}
151159
restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
152160
if err != nil {
153161
t.Errorf("Expected gcp exec kubeconfig to parse successfully, got %s", err)
@@ -162,9 +170,13 @@ func TestAuthPlugins(t *testing.T) {
162170
}
163171
})
164172
t.Run("should register oidc auth provider plugin", func(t *testing.T) {
165-
cfg := NewCfg(&CfgOptions{
173+
cfg, err := NewCfg(&CfgOptions{
166174
KubeConfigContents: oidcKubeConfig,
167175
})
176+
if err != nil {
177+
t.Errorf("Expected NewCfg to succeed, got %s", err)
178+
return
179+
}
168180
restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
169181
if err != nil {
170182
t.Errorf("Expected oidc kubeconfig to parse successfully, got %s", err)
@@ -180,9 +192,13 @@ func TestAuthPlugins(t *testing.T) {
180192
}
181193
})
182194
t.Run("should support exec auth provider", func(t *testing.T) {
183-
cfg := NewCfg(&CfgOptions{
195+
cfg, err := NewCfg(&CfgOptions{
184196
KubeConfigContents: execKubeConfig,
185197
})
198+
if err != nil {
199+
t.Errorf("Expected NewCfg to succeed, got %s", err)
200+
return
201+
}
186202
restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
187203
if err != nil {
188204
t.Errorf("Expected exec kubeconfig to parse successfully, got %s", err)
@@ -219,9 +235,13 @@ users:
219235
config:
220236
some-key: some-value
221237
`
222-
cfg := NewCfg(&CfgOptions{
238+
cfg, err := NewCfg(&CfgOptions{
223239
KubeConfigContents: unknownAuthConfig,
224240
})
241+
if err != nil {
242+
t.Errorf("Expected NewCfg to succeed, got %s", err)
243+
return
244+
}
225245
restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
226246
if err != nil {
227247
t.Errorf("Expected parsing to succeed, got %s", err)

‎native/internal/helm/push.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,13 @@ func Push(options *PushOptions) (string, error) {
4040
return "", err
4141
}
4242

43+
cfg, err := NewCfg(&CfgOptions{RegistryClient: registryClient})
44+
if err != nil {
45+
return "", err
46+
}
47+
4348
pushOptions := []action.PushOpt{
44-
action.WithPushConfig(NewCfg(&CfgOptions{RegistryClient: registryClient})),
49+
action.WithPushConfig(cfg),
4550
action.WithTLSClientConfig(options.CertFile, options.KeyFile, options.CaFile),
4651
action.WithInsecureSkipTLSVerify(options.InsecureSkipTLSverify),
4752
action.WithPlainHTTP(options.PlainHttp),

‎native/internal/helm/registry.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,11 @@ func RegistryLogin(options *RegistryOptions) (string, error) {
5050
// Manually set the logrus (which is used) output to out
5151
logrus.SetOutput(debugBuffer)
5252
}
53-
err = action.NewRegistryLogin(NewCfg(&CfgOptions{RegistryClient: registryClient})).Run(
53+
cfg, err := NewCfg(&CfgOptions{RegistryClient: registryClient})
54+
if err != nil {
55+
return "", err
56+
}
57+
err = action.NewRegistryLogin(cfg).Run(
5458
debugBuffer /* ignored */, options.Hostname, options.Username, options.Password,
5559
action.WithCertFile(options.CertFile),
5660
action.WithKeyFile(options.KeyFile),
@@ -78,7 +82,11 @@ func RegistryLogout(options *RegistryOptions) (string, error) {
7882
// Manually set the logrus (which is used) output to out
7983
logrus.SetOutput(debugBuffer)
8084
}
81-
err = action.NewRegistryLogout(NewCfg(&CfgOptions{RegistryClient: registryClient})).Run(
85+
cfg, err := NewCfg(&CfgOptions{RegistryClient: registryClient})
86+
if err != nil {
87+
return "", err
88+
}
89+
err = action.NewRegistryLogout(cfg).Run(
8290
debugBuffer /* ignored */, options.Hostname)
8391
return appendToOutOrErr(debugBuffer, getRegistryClientOut().String(), err)
8492
}

‎native/internal/helm/show.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,11 @@ func Show(options *ShowOptions) (string, error) {
6464
if err != nil {
6565
return "", err
6666
}
67-
client := action.NewShowWithConfig(format, NewCfg(&CfgOptions{}))
67+
cfg, err := NewCfg(&CfgOptions{})
68+
if err != nil {
69+
return "", err
70+
}
71+
client := action.NewShowWithConfig(format, cfg)
6872
client.SetRegistryClient(registryClient)
6973
client.Version = options.Version
7074
cp, err := client.ChartPathOptions.LocateChart(options.Path, cli.New())

‎native/internal/helm/test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ func Test(options *TestOptions) (string, error) {
3636
KubeConfigContents: options.KubeConfigContents,
3737
Namespace: options.Namespace,
3838
}
39-
client := action.NewReleaseTesting(NewCfg(cfgOptions))
39+
cfg, err := NewCfg(cfgOptions)
40+
if err != nil {
41+
return "", err
42+
}
43+
client := action.NewReleaseTesting(cfg)
4044
client.Namespace = options.Namespace
4145
client.Timeout = options.Timeout
4246
release, err := client.Run(options.ReleaseName)

‎native/internal/helm/uninstall.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ func Uninstall(options *UninstallOptions) (string, error) {
4545
if options.Debug {
4646
cfgOptions.KubeOut = kubeOut
4747
}
48-
client := action.NewUninstall(NewCfg(cfgOptions))
48+
cfg, err := NewCfg(cfgOptions)
49+
if err != nil {
50+
return "", err
51+
}
52+
client := action.NewUninstall(cfg)
4953
client.DryRun = options.DryRun
5054
client.DisableHooks = options.NoHooks
5155
client.IgnoreNotFound = options.IgnoreNotFound

0 commit comments

Comments
 (0)