Skip to content

Commit 62bdabb

Browse files
Deeven-SeruYuan325
andauthored
fix: error on invalid v1 config docs (#2993)
## Summary - return hard errors for invalid v1 docs in ConvertConfig (no silent drop) - include doc index + key path + v2 hint in error - add tests for invalid v1 docs ## Testing - go test ./cmd/internal -run TestConvertConfig fixes #2926 --------- Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>
1 parent 10880ed commit 62bdabb

2 files changed

Lines changed: 38 additions & 17 deletions

File tree

cmd/internal/config.go

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,17 +128,19 @@ func ConvertConfig(raw []byte) ([]byte, error) {
128128
encoder := yaml.NewEncoder(&buf)
129129

130130
v1keys := []string{"sources", "authServices", "embeddingModels", "tools", "toolsets", "prompts"}
131+
docIndex := 0
131132
for {
132133
if err := decoder.Decode(&input); err != nil {
133134
if err == io.EOF {
134135
break
135136
}
136137
return nil, err
137138
}
139+
docIndex++
138140
for _, item := range input {
139141
key, ok := item.Key.(string)
140142
if !ok {
141-
return nil, fmt.Errorf("unexpected non-string key in input: %v", item.Key)
143+
return nil, fmt.Errorf("doc %d: unexpected non-string key in input: %v", docIndex, item.Key)
142144
}
143145
// check if the key is config file v1's key
144146
if slices.Contains(v1keys, key) {
@@ -163,7 +165,7 @@ func ConvertConfig(raw []byte) ([]byte, error) {
163165
}
164166
transformed, err := transformDocs(key, slice)
165167
if err != nil {
166-
return nil, err
168+
return nil, fmt.Errorf("doc %d: invalid config format at key %q: %w", docIndex, key, err)
167169
}
168170
// encode per-doc
169171
for _, doc := range transformed {
@@ -172,15 +174,14 @@ func ConvertConfig(raw []byte) ([]byte, error) {
172174
}
173175
}
174176
} else {
175-
// invalid input will be ignored
176-
// we don't want to throw error here since the config could
177-
// be valid but with a different order such as:
178-
// ---
179-
// tools:
180-
// - tool_a
181-
// kind: toolset
182-
// ---
183-
continue
177+
if hasKindField(input) {
178+
// this doc is already v2, encode to buf
179+
if err := encoder.Encode(input); err != nil {
180+
return nil, err
181+
}
182+
break
183+
}
184+
return nil, fmt.Errorf("doc %d: invalid config format at key %q: expected map", docIndex, key)
184185
}
185186
} else {
186187
// this doc is already v2, encode to buf
@@ -194,6 +195,15 @@ func ConvertConfig(raw []byte) ([]byte, error) {
194195
return buf.Bytes(), nil
195196
}
196197

198+
func hasKindField(input yaml.MapSlice) bool {
199+
for _, item := range input {
200+
if key, ok := item.Key.(string); ok && key == "kind" {
201+
return true
202+
}
203+
}
204+
return false
205+
}
206+
197207
// transformDocs transforms the configuration file from v1 format to v2
198208
// yaml.MapSlice will preserve the order in a map
199209
func transformDocs(kind string, input yaml.MapSlice) ([]yaml.MapSlice, error) {

cmd/internal/config_test.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -531,19 +531,30 @@ tools:
531531
`,
532532
},
533533
{
534-
desc: "invalid source",
535-
in: `sources: invalid`,
536-
want: "",
534+
desc: "invalid source",
535+
in: `sources: invalid`,
536+
isErr: true,
537+
errStr: `doc 1: invalid config format at key "sources": expected map`,
537538
},
538539
{
539-
desc: "invalid toolset",
540-
in: `toolsets: invalid`,
541-
want: "",
540+
desc: "invalid toolset",
541+
in: `toolsets: invalid`,
542+
isErr: true,
543+
errStr: `doc 1: invalid config format at key "toolsets": expected map`,
542544
},
543545
}
544546
for _, tc := range tcs {
545547
t.Run(tc.desc, func(t *testing.T) {
546548
output, err := ConvertConfig([]byte(tc.in))
549+
if tc.isErr {
550+
if err == nil {
551+
t.Fatalf("expected error")
552+
}
553+
if tc.errStr != "" && err.Error() != tc.errStr {
554+
t.Fatalf("incorrect error string: got %s, want %s", err, tc.errStr)
555+
}
556+
return
557+
}
547558
if err != nil {
548559
t.Fatalf("unexpected error: %s", err)
549560
}

0 commit comments

Comments
 (0)