From ee889d83cf7a95b81adb7c37499e3fcedd702e69 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 7 May 2026 12:26:23 -0700 Subject: [PATCH 1/2] feat(overrides): support []string slice fields in ApplyOverrides ApplyOverrides previously rejected []string fields with "unsupported field type", blocking SeiConfig.TxIndex.Indexer and any future []string field from being set via the controller's spec.config.overrides or the sidecar's ConfigApplyTask. setReflectValue now accepts reflect.Slice when the element kind is String. Values are comma-separated; whitespace is trimmed and empty entries are dropped. Empty input yields a non-nil zero-length slice so override-set fields always render into TOML. Non-string slice element kinds remain rejected with a clear error. Closes #13. Co-Authored-By: Claude Opus 4.7 (1M context) --- config_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++ resolve.go | 29 ++++++++++++++++++++++++++ resolve_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 resolve_test.go diff --git a/config_test.go b/config_test.go index 7a29538..8eda244 100644 --- a/config_test.go +++ b/config_test.go @@ -467,6 +467,56 @@ func TestApplyOverrides_Empty(t *testing.T) { } } +func TestApplyOverrides_StringSlice(t *testing.T) { + cases := []struct { + name string + in string + want []string + }{ + {"single value", "kv", []string{"kv"}}, + {"multi value", "kv,psql", []string{"kv", "psql"}}, + {"trims whitespace", " kv , psql ", []string{"kv", "psql"}}, + {"drops empty entries", "kv,,psql,", []string{"kv", "psql"}}, + {"empty string yields empty slice", "", []string{}}, + {"only commas yields empty slice", ",,,", []string{}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := Default() + if err := ApplyOverrides(cfg, map[string]string{ + "tx_index.indexer": tc.in, + }); err != nil { + t.Fatalf("ApplyOverrides: %v", err) + } + got := cfg.TxIndex.Indexer + if len(got) != len(tc.want) { + t.Fatalf("indexer: got %v (len %d), want %v (len %d)", + got, len(got), tc.want, len(tc.want)) + } + for i := range got { + if got[i] != tc.want[i] { + t.Errorf("indexer[%d]: got %q, want %q", i, got[i], tc.want[i]) + } + } + if got == nil { + t.Error("indexer slice must be non-nil to render into TOML") + } + }) + } +} + +func TestApplyOverrides_StringSliceOverwritesDefault(t *testing.T) { + cfg := Default() + if err := ApplyOverrides(cfg, map[string]string{ + "tx_index.indexer": "kv", + }); err != nil { + t.Fatalf("ApplyOverrides: %v", err) + } + if len(cfg.TxIndex.Indexer) != 1 || cfg.TxIndex.Indexer[0] != "kv" { + t.Errorf("indexer: got %v, want [kv]", cfg.TxIndex.Indexer) + } +} + func TestResolveEnv(t *testing.T) { cfg := Default() t.Setenv("SEI_CHAIN_MIN_GAS_PRICES", "0.5usei") diff --git a/resolve.go b/resolve.go index 2b5003b..6ce620b 100644 --- a/resolve.go +++ b/resolve.go @@ -167,6 +167,16 @@ func setReflectValue(v reflect.Value, s string) error { return fmt.Errorf("value %g overflows %s", n, v.Type()) } v.SetFloat(n) + case reflect.Slice: + if v.Type().Elem().Kind() != reflect.String { + return fmt.Errorf("unsupported slice element type: %s", v.Type().Elem()) + } + out := parseStringSlice(s) + sliceVal := reflect.MakeSlice(v.Type(), len(out), len(out)) + for i, p := range out { + sliceVal.Index(i).SetString(p) + } + v.Set(sliceVal) default: return fmt.Errorf("unsupported field type: %s", v.Type()) } @@ -190,3 +200,22 @@ func parseFloat64(s string) (float64, error) { _, err := fmt.Sscanf(s, "%f", &n) return n, err } + +// parseStringSlice splits a comma-separated string into a slice, trimming +// whitespace and dropping empty entries. An empty input yields a non-nil +// zero-length slice so override-set fields always render into TOML. +func parseStringSlice(s string) []string { + if s == "" { + return []string{} + } + parts := strings.Split(s, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p == "" { + continue + } + out = append(out, p) + } + return out +} diff --git a/resolve_test.go b/resolve_test.go new file mode 100644 index 0000000..29aebdc --- /dev/null +++ b/resolve_test.go @@ -0,0 +1,55 @@ +package seiconfig + +import ( + "reflect" + "testing" +) + +func TestSetReflectValue_StringSlice(t *testing.T) { + var s []string + v := reflect.ValueOf(&s).Elem() + + if err := setReflectValue(v, "a, b ,c"); err != nil { + t.Fatalf("setReflectValue: %v", err) + } + want := []string{"a", "b", "c"} + if !reflect.DeepEqual(s, want) { + t.Errorf("got %v, want %v", s, want) + } +} + +func TestSetReflectValue_RejectsNonStringSlice(t *testing.T) { + var s []int + v := reflect.ValueOf(&s).Elem() + + err := setReflectValue(v, "1,2,3") + if err == nil { + t.Fatal("expected error for []int slice") + } + if got := err.Error(); got != "unsupported slice element type: int" { + t.Errorf("got %q, want %q", got, "unsupported slice element type: int") + } +} + +func TestParseStringSlice(t *testing.T) { + cases := []struct { + in string + want []string + }{ + {"", []string{}}, + {"a", []string{"a"}}, + {"a,b,c", []string{"a", "b", "c"}}, + {" a , b , c ", []string{"a", "b", "c"}}, + {",a,,b,", []string{"a", "b"}}, + {",,,", []string{}}, + } + for _, tc := range cases { + got := parseStringSlice(tc.in) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("parseStringSlice(%q): got %v, want %v", tc.in, got, tc.want) + } + if got == nil { + t.Errorf("parseStringSlice(%q) returned nil; want non-nil empty slice", tc.in) + } + } +} From ad1bcf719247c857ca3ef9ce6cf6c15e5c4bb1b0 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 7 May 2026 12:36:46 -0700 Subject: [PATCH 2/2] refactor(overrides): strict empty-entry rejection + clearer error Address coral platform-engineer review of #14: - Reject empty entries in parseStringSlice (",,", ",a", "a,", " "). Operator typos in security-relevant allow/deny lists (evm.deny_list, evm.enabled_legacy_sei_apis) fail loudly at config-apply time instead of silently dropping. - "unsupported slice element type: []string" -> "unsupported slice element kind: ". The previous message was misleading for [][]string fields (e.g. metrics.global_labels). - Add round-trip test covering ApplyOverrides -> WriteConfigToDir -> ReadConfigFromDir for tx_index.indexer empty + non-empty, locking the BurntSushi/toml encoder behavior the empty-non-nil contract depends on. Co-Authored-By: Claude Opus 4.7 (1M context) --- config_test.go | 59 ++++++++++++++++++++++++++++++++++++++++-- resolve.go | 28 +++++++++++--------- resolve_test.go | 68 ++++++++++++++++++++++++++++++++++++++----------- 3 files changed, 126 insertions(+), 29 deletions(-) diff --git a/config_test.go b/config_test.go index 8eda244..b499879 100644 --- a/config_test.go +++ b/config_test.go @@ -476,9 +476,7 @@ func TestApplyOverrides_StringSlice(t *testing.T) { {"single value", "kv", []string{"kv"}}, {"multi value", "kv,psql", []string{"kv", "psql"}}, {"trims whitespace", " kv , psql ", []string{"kv", "psql"}}, - {"drops empty entries", "kv,,psql,", []string{"kv", "psql"}}, {"empty string yields empty slice", "", []string{}}, - {"only commas yields empty slice", ",,,", []string{}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -505,6 +503,21 @@ func TestApplyOverrides_StringSlice(t *testing.T) { } } +func TestApplyOverrides_StringSliceRejectsEmptyEntries(t *testing.T) { + cases := []string{"kv,,psql", ",kv", "kv,", ",,,", "kv, ,psql"} + for _, in := range cases { + t.Run(in, func(t *testing.T) { + cfg := Default() + err := ApplyOverrides(cfg, map[string]string{ + "tx_index.indexer": in, + }) + if err == nil { + t.Fatalf("expected error for input %q, got nil", in) + } + }) + } +} + func TestApplyOverrides_StringSliceOverwritesDefault(t *testing.T) { cfg := Default() if err := ApplyOverrides(cfg, map[string]string{ @@ -517,6 +530,48 @@ func TestApplyOverrides_StringSliceOverwritesDefault(t *testing.T) { } } +func TestApplyOverrides_StringSliceRoundTripTOML(t *testing.T) { + dir := t.TempDir() + + cases := []struct { + name string + in string + want []string + }{ + {"non-empty list survives round-trip", "kv,psql", []string{"kv", "psql"}}, + {"empty list survives round-trip as []", "", []string{}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := DefaultForMode(ModeFull) + if err := ApplyOverrides(cfg, map[string]string{ + "tx_index.indexer": tc.in, + }); err != nil { + t.Fatalf("ApplyOverrides: %v", err) + } + subdir := t.TempDir() + if err := WriteConfigToDir(cfg, subdir); err != nil { + t.Fatalf("WriteConfigToDir: %v", err) + } + loaded, err := ReadConfigFromDir(subdir) + if err != nil { + t.Fatalf("ReadConfigFromDir: %v", err) + } + got := loaded.TxIndex.Indexer + if len(got) != len(tc.want) { + t.Fatalf("after round-trip: got %v (len %d), want %v (len %d)", + got, len(got), tc.want, len(tc.want)) + } + for i := range got { + if got[i] != tc.want[i] { + t.Errorf("indexer[%d]: got %q, want %q", i, got[i], tc.want[i]) + } + } + }) + } + _ = dir +} + func TestResolveEnv(t *testing.T) { cfg := Default() t.Setenv("SEI_CHAIN_MIN_GAS_PRICES", "0.5usei") diff --git a/resolve.go b/resolve.go index 6ce620b..855ecdd 100644 --- a/resolve.go +++ b/resolve.go @@ -169,9 +169,12 @@ func setReflectValue(v reflect.Value, s string) error { v.SetFloat(n) case reflect.Slice: if v.Type().Elem().Kind() != reflect.String { - return fmt.Errorf("unsupported slice element type: %s", v.Type().Elem()) + return fmt.Errorf("unsupported slice element kind: %s", v.Type().Elem().Kind()) + } + out, err := parseStringSlice(s) + if err != nil { + return err } - out := parseStringSlice(s) sliceVal := reflect.MakeSlice(v.Type(), len(out), len(out)) for i, p := range out { sliceVal.Index(i).SetString(p) @@ -201,21 +204,22 @@ func parseFloat64(s string) (float64, error) { return n, err } -// parseStringSlice splits a comma-separated string into a slice, trimming -// whitespace and dropping empty entries. An empty input yields a non-nil -// zero-length slice so override-set fields always render into TOML. -func parseStringSlice(s string) []string { +// parseStringSlice splits a comma-separated string, trims whitespace, and +// rejects empty entries so operator typos fail loudly. Empty input yields a +// non-nil zero-length slice: BurntSushi/toml encodes nil as omitted, +// []string{} as "field = []". +func parseStringSlice(s string) ([]string, error) { if s == "" { - return []string{} + return []string{}, nil } parts := strings.Split(s, ",") out := make([]string, 0, len(parts)) for _, p := range parts { - p = strings.TrimSpace(p) - if p == "" { - continue + trimmed := strings.TrimSpace(p) + if trimmed == "" { + return nil, fmt.Errorf("empty entry in string slice value %q", s) } - out = append(out, p) + out = append(out, trimmed) } - return out + return out, nil } diff --git a/resolve_test.go b/resolve_test.go index 29aebdc..d7cbddb 100644 --- a/resolve_test.go +++ b/resolve_test.go @@ -26,30 +26,68 @@ func TestSetReflectValue_RejectsNonStringSlice(t *testing.T) { if err == nil { t.Fatal("expected error for []int slice") } - if got := err.Error(); got != "unsupported slice element type: int" { - t.Errorf("got %q, want %q", got, "unsupported slice element type: int") + if got := err.Error(); got != "unsupported slice element kind: int" { + t.Errorf("got %q, want %q", got, "unsupported slice element kind: int") + } +} + +func TestSetReflectValue_RejectsSliceOfSlice(t *testing.T) { + var s [][]string + v := reflect.ValueOf(&s).Elem() + + err := setReflectValue(v, "anything") + if err == nil { + t.Fatal("expected error for [][]string") + } + if got := err.Error(); got != "unsupported slice element kind: slice" { + t.Errorf("got %q, want %q", got, "unsupported slice element kind: slice") } } func TestParseStringSlice(t *testing.T) { cases := []struct { + name string in string want []string }{ - {"", []string{}}, - {"a", []string{"a"}}, - {"a,b,c", []string{"a", "b", "c"}}, - {" a , b , c ", []string{"a", "b", "c"}}, - {",a,,b,", []string{"a", "b"}}, - {",,,", []string{}}, + {"empty yields non-nil empty", "", []string{}}, + {"single value", "a", []string{"a"}}, + {"multi value", "a,b,c", []string{"a", "b", "c"}}, + {"trims whitespace", " a , b , c ", []string{"a", "b", "c"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := parseStringSlice(tc.in) + if err != nil { + t.Fatalf("parseStringSlice(%q): %v", tc.in, err) + } + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("parseStringSlice(%q): got %v, want %v", tc.in, got, tc.want) + } + if got == nil { + t.Errorf("parseStringSlice(%q) returned nil; want non-nil empty slice", tc.in) + } + }) + } +} + +func TestParseStringSlice_RejectsEmptyEntries(t *testing.T) { + cases := []struct { + name string + in string + }{ + {"leading comma", ",a"}, + {"trailing comma", "a,"}, + {"consecutive commas", "a,,b"}, + {"only whitespace entry", "a, ,b"}, + {"only commas", ",,,"}, } for _, tc := range cases { - got := parseStringSlice(tc.in) - if !reflect.DeepEqual(got, tc.want) { - t.Errorf("parseStringSlice(%q): got %v, want %v", tc.in, got, tc.want) - } - if got == nil { - t.Errorf("parseStringSlice(%q) returned nil; want non-nil empty slice", tc.in) - } + t.Run(tc.name, func(t *testing.T) { + _, err := parseStringSlice(tc.in) + if err == nil { + t.Fatalf("parseStringSlice(%q): expected error, got nil", tc.in) + } + }) } }