Monorepo for Tangled
spindle,appview: improve the reported wf parse errors, show in appview
Signed-off-by: dawn <dawn@tangled.org>
+363
-2
+8
appview/pages/templates/repo/pipelines/workflow.html
+8
appview/pages/templates/repo/pipelines/workflow.html
···
26
26
>Cancel</button>
27
27
</div>
28
28
{{ end }}
29
+
{{ with (index .Pipeline.Statuses .Workflow).Latest }}
30
+
{{ if .Error }}
31
+
<div class="mb-2 flex items-start gap-2 p-3 rounded border border-red-200 dark:border-red-900/60 bg-red-50/60 dark:bg-red-950/20 text-red-700 dark:text-red-400 text-sm">
32
+
{{ i "triangle-alert" "size-4 shrink-0 mt-0.5" }}
33
+
<span class="font-mono break-words whitespace-pre-wrap">{{- .Error -}}</span>
34
+
</div>
35
+
{{ end }}
36
+
{{ end }}
29
37
{{ block "logs" . }} {{ end }}
30
38
</div>
31
39
</section>
+232
spindle/engine/manifest.go
+232
spindle/engine/manifest.go
···
1
+
package engine
2
+
3
+
import (
4
+
"errors"
5
+
"fmt"
6
+
"reflect"
7
+
"strconv"
8
+
"strings"
9
+
10
+
"gopkg.in/yaml.v3"
11
+
"tangled.org/core/workflow"
12
+
)
13
+
14
+
// how many lines of context to show on above / below of an offending line.
15
+
const frameContext = 3
16
+
17
+
type manifestError struct {
18
+
line int
19
+
msg string
20
+
}
21
+
22
+
func (e *manifestError) Error() string { return e.msg }
23
+
24
+
// codeFrame renders the lines around `line` with a gutter and a `>` marker on
25
+
// the offending line, eg.
26
+
//
27
+
// 4 | image: alpine
28
+
// > 5 | registre:
29
+
// 6 | nixpkgs: github:nixos/nixpkgs/nixos-unstable
30
+
func codeFrame(raw string, line int) string {
31
+
lines := strings.Split(raw, "\n")
32
+
if line < 1 || line > len(lines) {
33
+
return ""
34
+
}
35
+
start := max(line-frameContext, 1)
36
+
end := min(line+frameContext, len(lines))
37
+
width := len(strconv.Itoa(end))
38
+
39
+
var b strings.Builder
40
+
for n := start; n <= end; n++ {
41
+
marker := " "
42
+
if n == line {
43
+
marker = "> "
44
+
}
45
+
fmt.Fprintf(&b, "%s%*d | %s\n", marker, width, n, lines[n-1])
46
+
}
47
+
return strings.TrimRight(b.String(), "\n")
48
+
}
49
+
50
+
var genericWorkflowKeys = ignoredKeys(reflect.TypeFor[workflow.Workflow]())
51
+
52
+
// ignoredKeys is the set of yaml keys we ignore on field checks for a struct.
53
+
// real, parseable keys come straight from the tags (via fieldsByYAMLName); on
54
+
// top of those we tolerate `yaml:"-"` fields by their conventional spelling.
55
+
// those have no yaml key of their own (the program fills them in itself, eg.
56
+
// `name` from the filename, `raw` from the file bytes), but users sometimes
57
+
// write one in the body anyway, and that's harmless rather than a typo.
58
+
func ignoredKeys(t reflect.Type) map[string]bool {
59
+
if t.Kind() == reflect.Pointer {
60
+
t = t.Elem()
61
+
}
62
+
keys := make(map[string]bool)
63
+
for k := range fieldsByYAMLName(t) {
64
+
keys[k] = true
65
+
}
66
+
for i := 0; i < t.NumField(); i++ {
67
+
f := t.Field(i)
68
+
if tag, _, _ := strings.Cut(f.Tag.Get("yaml"), ","); tag == "-" {
69
+
keys[strings.ToLower(f.Name)] = true
70
+
}
71
+
}
72
+
return keys
73
+
}
74
+
75
+
// this exists because yaml.v3 reports mismatches as "cannot unmarshal !!seq into
76
+
// map[string]interface {}", which is kind of confusing, even if it outputs a line.
77
+
// so we use reflection, walk the node tree alongside the schema type, and point
78
+
// at the field that's actually mis-shaped.
79
+
//
80
+
// returns nil when nothing is structurally wrong.
81
+
func DescribeManifestError(raw string, schema any) error {
82
+
var doc yaml.Node
83
+
if err := yaml.Unmarshal([]byte(raw), &doc); err != nil {
84
+
return nil
85
+
}
86
+
if len(doc.Content) == 0 {
87
+
return nil
88
+
}
89
+
err := checkNode(doc.Content[0], reflect.TypeOf(schema), "", genericWorkflowKeys)
90
+
var me *manifestError
91
+
if !errors.As(err, &me) {
92
+
return err // nil
93
+
}
94
+
if frame := codeFrame(raw, me.line); frame != "" {
95
+
return fmt.Errorf("%s\n\n%s", me.msg, frame)
96
+
}
97
+
return errors.New(me.msg)
98
+
}
99
+
100
+
// checkNode walks a yaml node against the type it's expected to decode into,
101
+
// recursing through structs, maps and slices. allowExtra names keys that are
102
+
// valid at this level despite not being in the struct (only the root uses it).
103
+
func checkNode(node *yaml.Node, t reflect.Type, path string, allowExtra map[string]bool) error {
104
+
if node.Kind == yaml.AliasNode && node.Alias != nil {
105
+
node = node.Alias
106
+
}
107
+
if t == nil {
108
+
return nil
109
+
}
110
+
if t.Kind() == reflect.Pointer {
111
+
t = t.Elem()
112
+
}
113
+
// `any` accepts anything (eg. registry values) so we can't check more
114
+
if t.Kind() == reflect.Interface {
115
+
return nil
116
+
}
117
+
// an empty value (eg. `registry:` with nothing under it) is harmless
118
+
if node.Kind == yaml.ScalarNode && (node.Tag == "!!null" || node.Value == "") {
119
+
return nil
120
+
}
121
+
122
+
want, ok := yamlKindForType(t)
123
+
if !ok {
124
+
return nil
125
+
}
126
+
if node.Kind != want {
127
+
return &manifestError{line: node.Line, msg: fmt.Sprintf(
128
+
"%s must be %s, but got %s (line %d)",
129
+
describePath(path), yamlKindName(want), yamlKindName(node.Kind), node.Line)}
130
+
}
131
+
132
+
switch t.Kind() {
133
+
case reflect.Struct:
134
+
fields := fieldsByYAMLName(t)
135
+
for i := 0; i+1 < len(node.Content); i += 2 {
136
+
key, val := node.Content[i], node.Content[i+1]
137
+
ft, ok := fields[key.Value]
138
+
if !ok {
139
+
// a struct has a fixed set of fields, so anything else is a typo.
140
+
// (maps, take arbitrary user-defined keys and don't count)
141
+
if allowExtra[key.Value] {
142
+
continue
143
+
}
144
+
return &manifestError{line: key.Line, msg: fmt.Sprintf(
145
+
"unknown field %s (line %d)",
146
+
describePath(joinKey(path, key.Value)), key.Line)}
147
+
}
148
+
if err := checkNode(val, ft, joinKey(path, key.Value), nil); err != nil {
149
+
return err
150
+
}
151
+
}
152
+
case reflect.Map:
153
+
for i := 0; i+1 < len(node.Content); i += 2 {
154
+
key, val := node.Content[i], node.Content[i+1]
155
+
if err := checkNode(val, t.Elem(), joinKey(path, key.Value), nil); err != nil {
156
+
return err
157
+
}
158
+
}
159
+
case reflect.Slice, reflect.Array:
160
+
for idx, val := range node.Content {
161
+
if err := checkNode(val, t.Elem(), fmt.Sprintf("%s[%d]", path, idx), nil); err != nil {
162
+
return err
163
+
}
164
+
}
165
+
}
166
+
return nil
167
+
}
168
+
169
+
// fieldsByYAMLName maps a struct's yaml keys to their field types, mirroring how
170
+
// yaml.v3 resolves keys: explicit tag name, else the lowercased field name.
171
+
func fieldsByYAMLName(t reflect.Type) map[string]reflect.Type {
172
+
fields := make(map[string]reflect.Type)
173
+
for i := 0; i < t.NumField(); i++ {
174
+
f := t.Field(i)
175
+
name, _, _ := strings.Cut(f.Tag.Get("yaml"), ",")
176
+
if name == "-" {
177
+
continue
178
+
}
179
+
if name == "" {
180
+
name = strings.ToLower(f.Name)
181
+
}
182
+
fields[name] = f.Type
183
+
}
184
+
return fields
185
+
}
186
+
187
+
func joinKey(path, key string) string {
188
+
if path == "" {
189
+
return key
190
+
}
191
+
return path + "." + key
192
+
}
193
+
194
+
func describePath(path string) string {
195
+
if path == "" {
196
+
return "the manifest"
197
+
}
198
+
return "`" + path + "`"
199
+
}
200
+
201
+
func yamlKindForType(t reflect.Type) (yaml.Kind, bool) {
202
+
switch t.Kind() {
203
+
case reflect.Pointer:
204
+
return yamlKindForType(t.Elem())
205
+
case reflect.Map, reflect.Struct:
206
+
return yaml.MappingNode, true
207
+
case reflect.Slice, reflect.Array:
208
+
return yaml.SequenceNode, true
209
+
case reflect.String, reflect.Bool,
210
+
reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
211
+
reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
212
+
reflect.Float32, reflect.Float64:
213
+
return yaml.ScalarNode, true
214
+
default:
215
+
return 0, false
216
+
}
217
+
}
218
+
219
+
func yamlKindName(k yaml.Kind) string {
220
+
switch k {
221
+
case yaml.MappingNode:
222
+
return "a mapping"
223
+
case yaml.SequenceNode:
224
+
return "a list"
225
+
case yaml.ScalarNode:
226
+
return "a scalar value"
227
+
case yaml.AliasNode:
228
+
return "an alias"
229
+
default:
230
+
return "an unknown value"
231
+
}
232
+
}
+112
spindle/engine/manifest_test.go
+112
spindle/engine/manifest_test.go
···
1
+
package engine
2
+
3
+
import (
4
+
"strings"
5
+
"testing"
6
+
)
7
+
8
+
type testManifest struct {
9
+
Image string `yaml:"image"`
10
+
Registry map[string]any `yaml:"registry"`
11
+
Dependencies []string `yaml:"dependencies"`
12
+
Nested map[string][]string `yaml:"nested"`
13
+
Steps []struct {
14
+
Name string `yaml:"name"`
15
+
} `yaml:"steps"`
16
+
}
17
+
18
+
func TestDescribeManifestError(t *testing.T) {
19
+
cases := []struct {
20
+
name string
21
+
raw string
22
+
want []string // substrings the message must contain
23
+
}{
24
+
{
25
+
name: "map field written as list",
26
+
raw: "registry:\n - nixpkgs: github:nixos/nixpkgs\n",
27
+
want: []string{"registry", "a mapping", "a list"},
28
+
},
29
+
{
30
+
name: "list field written as scalar",
31
+
raw: "dependencies: bun\n",
32
+
want: []string{"dependencies", "a list", "a scalar value"},
33
+
},
34
+
{
35
+
name: "scalar field written as mapping",
36
+
raw: "image:\n name: nixos\n",
37
+
want: []string{"image", "a scalar value", "a mapping"},
38
+
},
39
+
{
40
+
name: "nested map value mis-shaped",
41
+
raw: "nested:\n foo: bar\n", // foo should be a list of strings
42
+
want: []string{"nested.foo", "a list", "a scalar value"},
43
+
},
44
+
{
45
+
name: "field inside a list element mis-shaped",
46
+
raw: "steps:\n - name:\n x: y\n", // steps[0].name should be a scalar
47
+
want: []string{"steps[0].name", "a scalar value", "a mapping"},
48
+
},
49
+
{
50
+
name: "unknown top-level field (typo)",
51
+
raw: "dependancies:\n - bun\n",
52
+
want: []string{"unknown field", "dependancies"},
53
+
},
54
+
{
55
+
name: "unknown field inside a list element",
56
+
raw: "steps:\n - name: x\n cmd: y\n", // it's `command`, not `cmd`
57
+
want: []string{"unknown field", "steps[0].cmd"},
58
+
},
59
+
{
60
+
// `name` (filename-sourced, yaml:"-") must be tolerated so the real
61
+
// typo `registre` on a later line is the thing that surfaces.
62
+
name: "tolerated name does not mask a later typo",
63
+
raw: "name: mill\nengine: microvm\nregistre:\n - x: y\n",
64
+
want: []string{"unknown field", "registre"},
65
+
},
66
+
}
67
+
for _, tc := range cases {
68
+
t.Run(tc.name, func(t *testing.T) {
69
+
err := DescribeManifestError(tc.raw, testManifest{})
70
+
if err == nil {
71
+
t.Fatalf("expected an error, got nil")
72
+
}
73
+
for _, w := range tc.want {
74
+
if !strings.Contains(err.Error(), w) {
75
+
t.Errorf("error %q missing %q", err, w)
76
+
}
77
+
}
78
+
})
79
+
}
80
+
}
81
+
82
+
func TestDescribeManifestErrorNoFalsePositives(t *testing.T) {
83
+
cases := []string{
84
+
// well-formed manifest
85
+
"image: nixos\nregistry:\n nixpkgs: github:nixos/nixpkgs\ndependencies:\n - bun\n",
86
+
// empty value is harmless, not a mismatch
87
+
"image: nixos\nregistry:\n",
88
+
// generic workflow keys live in the same doc and aren't engine fields,
89
+
// but must not be flagged as unknown at the root
90
+
"engine: microvm\nwhen:\n - event: [push]\nclone:\n skip: true\nimage: nixos\n",
91
+
// `any` map values accept any shape, including nested lists/maps
92
+
"registry:\n k:\n - a\n - b\n",
93
+
// well-formed nested map-of-lists
94
+
"nested:\n foo:\n - a\n - b\n",
95
+
// user-defined map keys are data, never flagged as unknown fields
96
+
"nested:\n any-package-name:\n - a\n",
97
+
}
98
+
for _, raw := range cases {
99
+
if err := DescribeManifestError(raw, testManifest{}); err != nil {
100
+
t.Errorf("DescribeManifestError(%q) = %v, want nil", raw, err)
101
+
}
102
+
}
103
+
}
104
+
105
+
func TestDescribeManifestErrorPointerSchema(t *testing.T) {
106
+
// nixery passes a pointer to an anonymous struct
107
+
schema := &testManifest{}
108
+
err := DescribeManifestError("nested: oops\n", schema)
109
+
if err == nil || !strings.Contains(err.Error(), "nested") {
110
+
t.Fatalf("expected an error naming `nested`, got %v", err)
111
+
}
112
+
}
+3
spindle/engines/microvm/engine.go
+3
spindle/engines/microvm/engine.go
···
104
104
swf := &models.Workflow{}
105
105
var dwf manifestWorkflow
106
106
107
+
if err := engine.DescribeManifestError(twf.Raw, manifestWorkflow{}); err != nil {
108
+
return nil, err
109
+
}
107
110
if err := yaml.Unmarshal([]byte(twf.Raw), &dwf); err != nil {
108
111
return nil, err
109
112
}
+4
-2
spindle/engines/nixery/engine.go
+4
-2
spindle/engines/nixery/engine.go
···
91
91
Dependencies map[string][]string `yaml:"dependencies"`
92
92
Environment map[string]string `yaml:"environment"`
93
93
}{}
94
-
err := yaml.Unmarshal([]byte(twf.Raw), &dwf)
95
-
if err != nil {
94
+
if err := engine.DescribeManifestError(twf.Raw, dwf); err != nil {
95
+
return nil, err
96
+
}
97
+
if err := yaml.Unmarshal([]byte(twf.Raw), &dwf); err != nil {
96
98
return nil, err
97
99
}
98
100
+4
spindle/server.go
+4
spindle/server.go
···
427
427
for _, w := range tpl.Workflows {
428
428
if w != nil {
429
429
if _, ok := s.engs[w.Engine]; !ok {
430
+
s.l.Error("workflow failed: unknown engine",
431
+
"pipeline", pipelineId, "workflow", w.Name, "engine", w.Engine)
430
432
err = s.db.StatusFailed(models.WorkflowId{
431
433
PipelineId: pipelineId,
432
434
Name: w.Name,
···
446
448
447
449
ewf, err := s.engs[w.Engine].InitWorkflow(*w, tpl)
448
450
if err != nil {
451
+
s.l.Error("workflow failed: init workflow",
452
+
"pipeline", pipelineId, "workflow", w.Name, "engine", w.Engine, "err", err)
449
453
err = s.db.StatusFailed(models.WorkflowId{
450
454
PipelineId: pipelineId,
451
455
Name: w.Name,