From 871061dfb15382905241ec13559bd04abed28b5b Mon Sep 17 00:00:00 2001 From: Ilya Danilov Date: Thu, 14 May 2026 13:12:58 +0300 Subject: [PATCH] fix(helm): pass arrays via per-element --set-string args --- cluster-manager/pkg/helm/values.go | 118 +++++++++++++----------- cluster-manager/pkg/helm/values_test.go | 47 +++------- 2 files changed, 76 insertions(+), 89 deletions(-) diff --git a/cluster-manager/pkg/helm/values.go b/cluster-manager/pkg/helm/values.go index d4ef1221..27e50039 100644 --- a/cluster-manager/pkg/helm/values.go +++ b/cluster-manager/pkg/helm/values.go @@ -1,7 +1,6 @@ package helm import ( - "encoding/json" "errors" "fmt" "reflect" @@ -20,8 +19,7 @@ const ( ) var ( - errInputNotStruct = errors.New("input must be a struct") - errMarshal = errors.New("failed to marshal field") + errUnsupported = errors.New("unsupported field type") ) // Values represents helm values returned as a complete install command or yaml. @@ -234,21 +232,23 @@ type TLSGlobal struct { } `json:"tls"` } -// buildHelmArgs recursively converts a struct's fields to Helm command-line arguments. -// It traverses the struct and generates the appropriate --set or --set-string arguments -// based on field types. +// buildHelmArgs recursively converts a value to Helm command-line arguments. +// It traverses the value and generates the appropriate --set or --set-string arguments +// based on the value's kind. // -// Each field is processed according to its kind: +// Each value is processed according to its kind: // - Strings use --set-string // - Bool, numeric types use --set -// - Arrays/Slices are JSON marshaled and use --set -// - Structs are processed recursively +// - Arrays/Slices are expanded into per-element args using the `name[i]` form +// - Structs are processed recursively over their fields // -// Fields with `json:"-"` are skipped. -// Fields with `json:",omitempty"` or `json:",omitzero"` are skipped if they contain zero values. +// Struct fields with `json:"-"` are skipped. +// Struct fields with `json:",omitempty"` or `json:",omitzero"` are skipped if they +// contain zero values; this is communicated through the `hasOmit` argument when +// recursing. // -// Returns an error if JSON marshaling fails for array/slice fields. -func buildHelmArgs(v any, prefix string) ([]string, error) { +// Returns errUnsupported if the value's kind is not handled. +func buildHelmArgs(v any, prefix string, hasOmit bool) ([]string, error) { var res []string val := reflect.ValueOf(v) @@ -257,66 +257,72 @@ func buildHelmArgs(v any, prefix string) ([]string, error) { val = val.Elem() } - if val.Kind() != reflect.Struct { - return nil, errInputNotStruct + // skip field on `omitempty` and `omitzero` + // theoretically `field.IsZero()` can possibly panic + // but currently it should never happen + if hasOmit && val.IsZero() { + return nil, nil } - t := val.Type() + switch val.Kind() { + case reflect.String: + res = append(res, fmt.Sprintf("--set-string '%s=%s'", prefix, val.String())) - for i := range val.NumField() { - field := val.Field(i) - fieldType := t.Field(i) + case reflect.Bool: + res = append(res, fmt.Sprintf("--set '%s=%t'", prefix, val.Bool())) - tag := fieldType.Tag.Get("json") - if tag == "-" { - continue - } + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + res = append(res, fmt.Sprintf("--set '%s=%d'", prefix, val.Int())) - parts := strings.Split(tag, ",") - fieldName := parts[0] - if fieldName == "" { - fieldName = fieldType.Name - } - hasOmit := (slices.Index(parts, "omitempty") != -1) || (slices.Index(parts, "omitzero") != -1) + case reflect.Float32, reflect.Float64: + res = append(res, fmt.Sprintf("--set '%s=%f'", prefix, val.Float())) - // skip field on `omitempty` and `omitzero` - // theoretically `field.IsZero()` can possibly panic - // but currently it should never happen - if hasOmit && field.IsZero() { - continue + case reflect.Array, reflect.Slice: + if hasOmit && val.Len() == 0 { + return nil, nil } - - currentPrefix := fieldName - if prefix != "" { - currentPrefix = prefix + "." + fieldName + for i := range val.Len() { + nestedRes, err := buildHelmArgs(val.Index(i).Interface(), fmt.Sprintf("%s[%d]", prefix, i), false) + if err != nil { + return nil, err + } + res = append(res, nestedRes...) } + case reflect.Struct: + t := val.Type() + + for i := range val.NumField() { + field := val.Field(i) + fieldType := t.Field(i) - switch field.Kind() { - case reflect.String: - res = append(res, fmt.Sprintf("--set-string '%s=%s'", currentPrefix, field.String())) - case reflect.Bool: - res = append(res, fmt.Sprintf("--set '%s=%t'", currentPrefix, field.Bool())) - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - res = append(res, fmt.Sprintf("--set '%s=%d'", currentPrefix, field.Int())) - case reflect.Float32, reflect.Float64: - res = append(res, fmt.Sprintf("--set '%s=%f'", currentPrefix, field.Float())) - case reflect.Array, reflect.Slice: - if hasOmit && field.Len() == 0 { + tag := fieldType.Tag.Get("json") + if tag == "-" { continue } - b, err := json.Marshal(field.Interface()) - if err != nil { - return nil, fmt.Errorf("%w: %w", errMarshal, err) + + parts := strings.Split(tag, ",") + fieldName := parts[0] + if fieldName == "" { + fieldName = fieldType.Name } - res = append(res, fmt.Sprintf("--set '%s=%s'", currentPrefix, string(b))) - case reflect.Struct: - nestedRes, err := buildHelmArgs(field.Interface(), currentPrefix) + + hasOmit := (slices.Index(parts, "omitempty") != -1) || (slices.Index(parts, "omitzero") != -1) + + currentPrefix := fieldName + if prefix != "" { + currentPrefix = prefix + "." + fieldName + } + + nestedRes, err := buildHelmArgs(field.Interface(), currentPrefix, hasOmit) if err != nil { return nil, err } res = append(res, nestedRes...) } + default: + return nil, errUnsupported } + return res, nil } @@ -330,5 +336,5 @@ func (v *Values) ToYAML() (string, error) { } func (v *Values) ToHelmArgs() ([]string, error) { - return buildHelmArgs(v, "") + return buildHelmArgs(v, "", false) } diff --git a/cluster-manager/pkg/helm/values_test.go b/cluster-manager/pkg/helm/values_test.go index cd0a5e75..0738a291 100644 --- a/cluster-manager/pkg/helm/values_test.go +++ b/cluster-manager/pkg/helm/values_test.go @@ -12,13 +12,6 @@ import ( func TestBuildHelmArgs(t *testing.T) { t.Parallel() - // BadType contains cyclic reference to make `json.Marshal` return error - type BadType struct { - CyclicRef *BadType - } - badArg := BadType{} - badArg.CyclicRef = &badArg - tests := []struct { name string input any @@ -68,11 +61,11 @@ func TestBuildHelmArgs(t *testing.T) { Nested struct { Field1 string `json:"field1"` Field2 int `json:"field2"` - } `json:"nested,omitempty"` + } `json:"nested"` NestedEmpty struct { Field1 string `json:"field1"` Field2 int `json:"field2"` - } `json:"nestedEmpty,omitempty"` + } `json:"nestedEmpty,omitzero"` NestedZero struct { Field1 string `json:"field1"` Field2 int `json:"field2"` @@ -166,7 +159,9 @@ func TestBuildHelmArgs(t *testing.T) { }, prefix: "", expected: []string{ - "--set 'names=[\"alice\",\"bob\",\"charlie\"]'", + "--set-string 'names[0]=alice'", + "--set-string 'names[1]=bob'", + "--set-string 'names[2]=charlie'", }, expectedErr: nil, }, @@ -188,37 +183,20 @@ func TestBuildHelmArgs(t *testing.T) { }, prefix: "", expected: []string{ - "--set 'env=[{\"name\":\"ENV1\",\"value\":\"value1\"},{\"name\":\"ENV2\",\"value\":\"value2\"}]'", + "--set-string 'env[0].name=ENV1'", + "--set-string 'env[0].value=value1'", + "--set-string 'env[1].name=ENV2'", + "--set-string 'env[1].value=value2'", }, expectedErr: nil, }, - { - name: "incorrect type", - input: []string{"one", "two", "three"}, - prefix: "", - expected: nil, - expectedErr: errInputNotStruct, - }, - { - name: "unmarshalable slice content", - input: struct { - BadSlice []BadType `json:"bad_slice"` - }{ - BadSlice: []BadType{ - badArg, - }, - }, - prefix: "", - expected: nil, - expectedErr: errMarshal, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - args, err := buildHelmArgs(tt.input, tt.prefix) + args, err := buildHelmArgs(tt.input, tt.prefix, false) if !errors.Is(err, tt.expectedErr) { t.Fatalf("expected error '%v', got '%v'", tt.expectedErr, err) @@ -357,7 +335,10 @@ func TestValuesToHelmArgs(t *testing.T) { "--set-string 'reverse-proxy.ingress.hostname=cs.example.com'", "--set-string 'reverse-proxy.ingress.secretName=cs-tls'", "--set-string 'reverse-proxy.service.type=ClusterIP'", - "--set 'notifier.overwriteEnv=[{\"name\":\"HTTP_PROXY\",\"value\":\"http://proxy.local\"},{\"name\":\"HTTPS_PROXY\",\"value\":\"https://proxy.local\"}]'", + "--set-string 'notifier.overwriteEnv[0].name=HTTP_PROXY'", + "--set-string 'notifier.overwriteEnv[0].value=http://proxy.local'", + "--set-string 'notifier.overwriteEnv[1].name=HTTPS_PROXY'", + "--set-string 'notifier.overwriteEnv[1].value=https://proxy.local'", "--set-string 'cs-manager.registrationToken=registration-token'", "--set-string 'auth-center.administrator.username=user'", "--set-string 'auth-center.administrator.password=pass'",