Skip to content

Commit 321ad7d

Browse files
daniloviddanilovid
authored
fix(helm): pass arrays via per-element --set-string args (#20)
1 parent 9d6ee98 commit 321ad7d

2 files changed

Lines changed: 76 additions & 89 deletions

File tree

‎cluster-manager/pkg/helm/values.go‎

Lines changed: 62 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package helm
22

33
import (
4-
"encoding/json"
54
"errors"
65
"fmt"
76
"reflect"
@@ -20,8 +19,7 @@ const (
2019
)
2120

2221
var (
23-
errInputNotStruct = errors.New("input must be a struct")
24-
errMarshal = errors.New("failed to marshal field")
22+
errUnsupported = errors.New("unsupported field type")
2523
)
2624

2725
// Values represents helm values returned as a complete install command or yaml.
@@ -234,21 +232,23 @@ type TLSGlobal struct {
234232
} `json:"tls"`
235233
}
236234

237-
// buildHelmArgs recursively converts a struct's fields to Helm command-line arguments.
238-
// It traverses the struct and generates the appropriate --set or --set-string arguments
239-
// based on field types.
235+
// buildHelmArgs recursively converts a value to Helm command-line arguments.
236+
// It traverses the value and generates the appropriate --set or --set-string arguments
237+
// based on the value's kind.
240238
//
241-
// Each field is processed according to its kind:
239+
// Each value is processed according to its kind:
242240
// - Strings use --set-string
243241
// - Bool, numeric types use --set
244-
// - Arrays/Slices are JSON marshaled and use --set
245-
// - Structs are processed recursively
242+
// - Arrays/Slices are expanded into per-element args using the `name[i]` form
243+
// - Structs are processed recursively over their fields
246244
//
247-
// Fields with `json:"-"` are skipped.
248-
// Fields with `json:",omitempty"` or `json:",omitzero"` are skipped if they contain zero values.
245+
// Struct fields with `json:"-"` are skipped.
246+
// Struct fields with `json:",omitempty"` or `json:",omitzero"` are skipped if they
247+
// contain zero values; this is communicated through the `hasOmit` argument when
248+
// recursing.
249249
//
250-
// Returns an error if JSON marshaling fails for array/slice fields.
251-
func buildHelmArgs(v any, prefix string) ([]string, error) {
250+
// Returns errUnsupported if the value's kind is not handled.
251+
func buildHelmArgs(v any, prefix string, hasOmit bool) ([]string, error) {
252252
var res []string
253253

254254
val := reflect.ValueOf(v)
@@ -257,66 +257,72 @@ func buildHelmArgs(v any, prefix string) ([]string, error) {
257257
val = val.Elem()
258258
}
259259

260-
if val.Kind() != reflect.Struct {
261-
return nil, errInputNotStruct
260+
// skip field on `omitempty` and `omitzero`
261+
// theoretically `field.IsZero()` can possibly panic
262+
// but currently it should never happen
263+
if hasOmit && val.IsZero() {
264+
return nil, nil
262265
}
263266

264-
t := val.Type()
267+
switch val.Kind() {
268+
case reflect.String:
269+
res = append(res, fmt.Sprintf("--set-string '%s=%s'", prefix, val.String()))
265270

266-
for i := range val.NumField() {
267-
field := val.Field(i)
268-
fieldType := t.Field(i)
271+
case reflect.Bool:
272+
res = append(res, fmt.Sprintf("--set '%s=%t'", prefix, val.Bool()))
269273

270-
tag := fieldType.Tag.Get("json")
271-
if tag == "-" {
272-
continue
273-
}
274+
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
275+
res = append(res, fmt.Sprintf("--set '%s=%d'", prefix, val.Int()))
274276

275-
parts := strings.Split(tag, ",")
276-
fieldName := parts[0]
277-
if fieldName == "" {
278-
fieldName = fieldType.Name
279-
}
280-
hasOmit := (slices.Index(parts, "omitempty") != -1) || (slices.Index(parts, "omitzero") != -1)
277+
case reflect.Float32, reflect.Float64:
278+
res = append(res, fmt.Sprintf("--set '%s=%f'", prefix, val.Float()))
281279

282-
// skip field on `omitempty` and `omitzero`
283-
// theoretically `field.IsZero()` can possibly panic
284-
// but currently it should never happen
285-
if hasOmit && field.IsZero() {
286-
continue
280+
case reflect.Array, reflect.Slice:
281+
if hasOmit && val.Len() == 0 {
282+
return nil, nil
287283
}
288-
289-
currentPrefix := fieldName
290-
if prefix != "" {
291-
currentPrefix = prefix + "." + fieldName
284+
for i := range val.Len() {
285+
nestedRes, err := buildHelmArgs(val.Index(i).Interface(), fmt.Sprintf("%s[%d]", prefix, i), false)
286+
if err != nil {
287+
return nil, err
288+
}
289+
res = append(res, nestedRes...)
292290
}
291+
case reflect.Struct:
292+
t := val.Type()
293+
294+
for i := range val.NumField() {
295+
field := val.Field(i)
296+
fieldType := t.Field(i)
293297

294-
switch field.Kind() {
295-
case reflect.String:
296-
res = append(res, fmt.Sprintf("--set-string '%s=%s'", currentPrefix, field.String()))
297-
case reflect.Bool:
298-
res = append(res, fmt.Sprintf("--set '%s=%t'", currentPrefix, field.Bool()))
299-
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
300-
res = append(res, fmt.Sprintf("--set '%s=%d'", currentPrefix, field.Int()))
301-
case reflect.Float32, reflect.Float64:
302-
res = append(res, fmt.Sprintf("--set '%s=%f'", currentPrefix, field.Float()))
303-
case reflect.Array, reflect.Slice:
304-
if hasOmit && field.Len() == 0 {
298+
tag := fieldType.Tag.Get("json")
299+
if tag == "-" {
305300
continue
306301
}
307-
b, err := json.Marshal(field.Interface())
308-
if err != nil {
309-
return nil, fmt.Errorf("%w: %w", errMarshal, err)
302+
303+
parts := strings.Split(tag, ",")
304+
fieldName := parts[0]
305+
if fieldName == "" {
306+
fieldName = fieldType.Name
310307
}
311-
res = append(res, fmt.Sprintf("--set '%s=%s'", currentPrefix, string(b)))
312-
case reflect.Struct:
313-
nestedRes, err := buildHelmArgs(field.Interface(), currentPrefix)
308+
309+
hasOmit := (slices.Index(parts, "omitempty") != -1) || (slices.Index(parts, "omitzero") != -1)
310+
311+
currentPrefix := fieldName
312+
if prefix != "" {
313+
currentPrefix = prefix + "." + fieldName
314+
}
315+
316+
nestedRes, err := buildHelmArgs(field.Interface(), currentPrefix, hasOmit)
314317
if err != nil {
315318
return nil, err
316319
}
317320
res = append(res, nestedRes...)
318321
}
322+
default:
323+
return nil, errUnsupported
319324
}
325+
320326
return res, nil
321327
}
322328

@@ -330,5 +336,5 @@ func (v *Values) ToYAML() (string, error) {
330336
}
331337

332338
func (v *Values) ToHelmArgs() ([]string, error) {
333-
return buildHelmArgs(v, "")
339+
return buildHelmArgs(v, "", false)
334340
}

‎cluster-manager/pkg/helm/values_test.go‎

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,6 @@ import (
1212
func TestBuildHelmArgs(t *testing.T) {
1313
t.Parallel()
1414

15-
// BadType contains cyclic reference to make `json.Marshal` return error
16-
type BadType struct {
17-
CyclicRef *BadType
18-
}
19-
badArg := BadType{}
20-
badArg.CyclicRef = &badArg
21-
2215
tests := []struct {
2316
name string
2417
input any
@@ -68,11 +61,11 @@ func TestBuildHelmArgs(t *testing.T) {
6861
Nested struct {
6962
Field1 string `json:"field1"`
7063
Field2 int `json:"field2"`
71-
} `json:"nested,omitempty"`
64+
} `json:"nested"`
7265
NestedEmpty struct {
7366
Field1 string `json:"field1"`
7467
Field2 int `json:"field2"`
75-
} `json:"nestedEmpty,omitempty"`
68+
} `json:"nestedEmpty,omitzero"`
7669
NestedZero struct {
7770
Field1 string `json:"field1"`
7871
Field2 int `json:"field2"`
@@ -166,7 +159,9 @@ func TestBuildHelmArgs(t *testing.T) {
166159
},
167160
prefix: "",
168161
expected: []string{
169-
"--set 'names=[\"alice\",\"bob\",\"charlie\"]'",
162+
"--set-string 'names[0]=alice'",
163+
"--set-string 'names[1]=bob'",
164+
"--set-string 'names[2]=charlie'",
170165
},
171166
expectedErr: nil,
172167
},
@@ -188,37 +183,20 @@ func TestBuildHelmArgs(t *testing.T) {
188183
},
189184
prefix: "",
190185
expected: []string{
191-
"--set 'env=[{\"name\":\"ENV1\",\"value\":\"value1\"},{\"name\":\"ENV2\",\"value\":\"value2\"}]'",
186+
"--set-string 'env[0].name=ENV1'",
187+
"--set-string 'env[0].value=value1'",
188+
"--set-string 'env[1].name=ENV2'",
189+
"--set-string 'env[1].value=value2'",
192190
},
193191
expectedErr: nil,
194192
},
195-
{
196-
name: "incorrect type",
197-
input: []string{"one", "two", "three"},
198-
prefix: "",
199-
expected: nil,
200-
expectedErr: errInputNotStruct,
201-
},
202-
{
203-
name: "unmarshalable slice content",
204-
input: struct {
205-
BadSlice []BadType `json:"bad_slice"`
206-
}{
207-
BadSlice: []BadType{
208-
badArg,
209-
},
210-
},
211-
prefix: "",
212-
expected: nil,
213-
expectedErr: errMarshal,
214-
},
215193
}
216194

217195
for _, tt := range tests {
218196
t.Run(tt.name, func(t *testing.T) {
219197
t.Parallel()
220198

221-
args, err := buildHelmArgs(tt.input, tt.prefix)
199+
args, err := buildHelmArgs(tt.input, tt.prefix, false)
222200

223201
if !errors.Is(err, tt.expectedErr) {
224202
t.Fatalf("expected error '%v', got '%v'", tt.expectedErr, err)
@@ -357,7 +335,10 @@ func TestValuesToHelmArgs(t *testing.T) {
357335
"--set-string 'reverse-proxy.ingress.hostname=cs.example.com'",
358336
"--set-string 'reverse-proxy.ingress.secretName=cs-tls'",
359337
"--set-string 'reverse-proxy.service.type=ClusterIP'",
360-
"--set 'notifier.overwriteEnv=[{\"name\":\"HTTP_PROXY\",\"value\":\"http://proxy.local\"},{\"name\":\"HTTPS_PROXY\",\"value\":\"https://proxy.local\"}]'",
338+
"--set-string 'notifier.overwriteEnv[0].name=HTTP_PROXY'",
339+
"--set-string 'notifier.overwriteEnv[0].value=http://proxy.local'",
340+
"--set-string 'notifier.overwriteEnv[1].name=HTTPS_PROXY'",
341+
"--set-string 'notifier.overwriteEnv[1].value=https://proxy.local'",
361342
"--set-string 'cs-manager.registrationToken=registration-token'",
362343
"--set-string 'auth-center.administrator.username=user'",
363344
"--set-string 'auth-center.administrator.password=pass'",

0 commit comments

Comments
 (0)