Skip to content

Commit 6aa9b06

Browse files
smirasmira
committed
fix: skip empty documents on config decoding
Fixes #12649 The cryptic error was coming from our code, as it never worked if the decoded node is not mapping node. Also annotate errors with line numbers (or document kinds) to make understanding the problem better, specifically for multi-doc and long configs. Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
1 parent 4944924 commit 6aa9b06

File tree

3 files changed

+48
-13
lines changed

3 files changed

+48
-13
lines changed

‎internal/app/machined/pkg/controllers/config/acquire_test.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,10 +349,15 @@ func (suite *AcquireSuite) TestFromDiskFailure() {
349349
ev := suite.platformEvent.getEvents()[0]
350350
suite.Assert().Equal(platform.EventTypeFailure, ev.Type)
351351
suite.Assert().Equal("Error loading and validating Talos machine config.", ev.Message)
352-
suite.Assert().Equal("failed to load \"config.yaml\" from STATE: unknown keys found during decoding:\naaaversion: v1alpha1 # Indicates the schema used to decode the contents.\n", ev.Error.Error())
352+
suite.Assert().Equal(
353+
"failed to load \"config.yaml\" from STATE: error decoding document /v1alpha1/ (line 1): unknown keys found during decoding:\n"+
354+
"aaaversion: v1alpha1 # Indicates the schema used to decode the contents.\n",
355+
ev.Error.Error(),
356+
)
353357

354358
suite.Assert().Equal(&machineapi.ConfigLoadErrorEvent{
355-
Error: "failed to load \"config.yaml\" from STATE: unknown keys found during decoding:\naaaversion: v1alpha1 # Indicates the schema used to decode the contents.\n",
359+
Error: "failed to load \"config.yaml\" from STATE: error decoding document /v1alpha1/ (line 1): unknown keys found during decoding:\n" +
360+
"aaaversion: v1alpha1 # Indicates the schema used to decode the contents.\n",
356361
}, suite.eventPublisher.getEvents()[0])
357362
}
358363

‎pkg/machinery/config/configloader/internal/decoder/decoder.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,13 @@ func parse(r io.Reader, allowPatchDelete bool) (decoded []config.Document, err e
8585
}
8686

8787
if manifests.Kind != yaml.DocumentNode {
88-
return nil, errors.New("expected a document")
88+
return nil, fmt.Errorf("expected a document at line %d", manifests.Line)
8989
}
9090

9191
if allowPatchDelete {
9292
decoded, err = AppendDeletesTo(&manifests, decoded, i)
9393
if err != nil {
94-
return nil, err
94+
return nil, fmt.Errorf("error processing patch delete statements at line %d: %w", manifests.Line, err)
9595
}
9696

9797
if manifests.IsZero() {
@@ -100,22 +100,36 @@ func parse(r io.Reader, allowPatchDelete bool) (decoded []config.Document, err e
100100
}
101101

102102
for _, manifest := range manifests.Content {
103+
switch manifest.Kind { //nolint:exhaustive
104+
case yaml.MappingNode:
105+
// expected
106+
case yaml.ScalarNode:
107+
if manifest.Tag == "!!null" {
108+
// skip null documents
109+
continue
110+
}
111+
112+
fallthrough
113+
default:
114+
return nil, fmt.Errorf("expected a YAML document at line %d", manifest.Line)
115+
}
116+
103117
id := documentID{
104118
APIVersion: findValue(manifest, ManifestAPIVersionKey, false),
105119
Kind: cmp.Or(findValue(manifest, ManifestKindKey, false), "v1alpha1"),
106120
Name: findValue(manifest, "name", false),
107121
}
108122

109123
if _, ok := knownDocuments[id]; ok {
110-
return nil, fmt.Errorf("duplicate document %s/%s/%s is not allowed", id.APIVersion, id.Kind, id.Name)
124+
return nil, fmt.Errorf("duplicate document %s/%s/%s is not allowed (line %d)", id.APIVersion, id.Kind, id.Name, manifest.Line)
111125
}
112126

113127
knownDocuments[id] = struct{}{}
114128

115129
var target config.Document
116130

117131
if target, err = decode(manifest); err != nil {
118-
return nil, err
132+
return nil, fmt.Errorf("error decoding document %s/%s/%s (line %d): %w", id.APIVersion, id.Kind, id.Name, manifest.Line, err)
119133
}
120134

121135
decoded = append(decoded, target)

‎pkg/machinery/config/configloader/internal/decoder/decoder_test.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,22 @@ func TestDecoder(t *testing.T) {
120120
kind: mock
121121
apiVersion: v1alpha1
122122
test: true
123+
`),
124+
expected: []config.Document{
125+
&Mock{
126+
Test: true,
127+
},
128+
},
129+
expectedErr: "",
130+
},
131+
{
132+
name: "empty docs",
133+
source: []byte(`---
134+
kind: mock
135+
apiVersion: v1alpha1
136+
test: true
137+
---
138+
---
123139
`),
124140
expected: []config.Document{
125141
&Mock{
@@ -135,7 +151,7 @@ apiVersion: v1alpha2
135151
test: true
136152
`),
137153
expected: nil,
138-
expectedErr: "missing kind",
154+
expectedErr: "error decoding document v1alpha2/v1alpha1/ (line 2): missing kind",
139155
},
140156
{
141157
name: "empty kind",
@@ -145,7 +161,7 @@ apiVersion: v1alpha2
145161
test: true
146162
`),
147163
expected: nil,
148-
expectedErr: "missing kind",
164+
expectedErr: "error decoding document v1alpha2/v1alpha1/ (line 2): missing kind",
149165
},
150166
{
151167
name: "tab instead of spaces",
@@ -167,7 +183,7 @@ test: true
167183
extra: fail
168184
`),
169185
expected: nil,
170-
expectedErr: "unknown keys found during decoding:\nextra: fail\n",
186+
expectedErr: "error decoding document v1alpha1/mock/ (line 2): unknown keys found during decoding:\nextra: fail\n",
171187
},
172188
{
173189
name: "extra fields in map",
@@ -180,7 +196,7 @@ map:
180196
extra: me
181197
`),
182198
expected: nil,
183-
expectedErr: "unknown keys found during decoding:\nmap:\n first:\n extra: me\n",
199+
expectedErr: "error decoding document v1alpha2/mock/ (line 2): unknown keys found during decoding:\nmap:\n first:\n extra: me\n",
184200
},
185201
{
186202
name: "extra fields in slice",
@@ -194,7 +210,7 @@ slice:
194210
fields: here
195211
`),
196212
expected: nil,
197-
expectedErr: "unknown keys found during decoding:\nslice:\n - fields: here\n more: extra\n not: working\n",
213+
expectedErr: "error decoding document v1alpha2/mock/ (line 2): unknown keys found during decoding:\nslice:\n - fields: here\n more: extra\n not: working\n",
198214
},
199215
{
200216
name: "extra zero fields in map",
@@ -207,7 +223,7 @@ map:
207223
b: {}
208224
`),
209225
expected: nil,
210-
expectedErr: "unknown keys found during decoding:\nmap:\n second:\n a:\n b: {}\n",
226+
expectedErr: "error decoding document v1alpha2/mock/ (line 2): unknown keys found during decoding:\nmap:\n second:\n a:\n b: {}\n",
211227
},
212228
{
213229
name: "valid nested",
@@ -306,7 +322,7 @@ config:
306322
- content: MONITOR ${upsmonHost} 1 remote pass foo
307323
mountPath: /usr/local/etc/nut/upsmon.conf
308324
`),
309-
expectedErr: "\"ExtensionServiceConfig\" \"\": not registered",
325+
expectedErr: "error decoding document /ExtensionServiceConfig/ (line 2): \"ExtensionServiceConfig\" \"\": not registered",
310326
},
311327
}
312328

0 commit comments

Comments
 (0)