A workspace with multiple sources set (e.g., both storage and pvc) was silently picking whichever matched first in the switch. Now parseTektonWorkflowConfig rejects workspaces with zero or multiple sources, and workspaces with empty names, at parse time with clear error messages.
+88
-1
Diff
round #0
+39
-1
provider_tekton.go
+39
-1
provider_tekton.go
···
89
89
}
90
90
cfg := doc.Tack.Tekton
91
91
if cfg.Pipeline == "" {
92
-
return nil, errors.New("workflow yaml: `tack.tekton.pipeline` is required")
92
+
return nil, errors.New(
93
+
"workflow yaml: `tack.tekton.pipeline` is required",
94
+
)
93
95
}
96
+
97
+
// Validate workspaces: each must have exactly one volume source
98
+
// so the resulting PipelineRun is unambiguous. Without this,
99
+
// a workspace with e.g. both `storage` and `pvc` set silently
100
+
// picks whichever the switch statement matches first.
101
+
for _, ws := range cfg.Workspaces {
102
+
if ws.Name == "" {
103
+
return nil, errors.New("workspace: name is required")
104
+
}
105
+
sources := 0
106
+
if ws.Storage != nil {
107
+
sources++
108
+
}
109
+
if ws.PVC != nil {
110
+
sources++
111
+
}
112
+
if ws.Secret != nil {
113
+
sources++
114
+
}
115
+
if ws.ConfigMap != nil {
116
+
sources++
117
+
}
118
+
if sources == 0 {
119
+
return nil, fmt.Errorf(
120
+
"workspace %q: no volume source specified", ws.Name,
121
+
)
122
+
}
123
+
if sources > 1 {
124
+
return nil, fmt.Errorf(
125
+
"workspace %q: multiple volume sources specified"+
126
+
" (pick one of storage, pvc, secret, config_map)",
127
+
ws.Name,
128
+
)
129
+
}
130
+
}
131
+
94
132
return &cfg, nil
95
133
}
96
134
+49
provider_tekton_test.go
+49
provider_tekton_test.go
···
5
5
"encoding/json"
6
6
"errors"
7
7
"log/slog"
8
+
"strings"
8
9
"testing"
9
10
"time"
10
11
···
40
41
}
41
42
}
42
43
44
+
func TestTektonWorkspaceValidation(t *testing.T) {
45
+
tests := []struct {
46
+
name string
47
+
yaml string
48
+
wantErr string
49
+
}{
50
+
{
51
+
name: "empty name",
52
+
yaml: "tack:\n tekton:\n pipeline: ci\n workspaces:\n - storage: 1Gi\n",
53
+
wantErr: "name is required",
54
+
},
55
+
{
56
+
name: "no source",
57
+
yaml: "tack:\n tekton:\n pipeline: ci\n workspaces:\n - name: scratch\n",
58
+
wantErr: "no volume source",
59
+
},
60
+
{
61
+
name: "multiple sources",
62
+
yaml: "tack:\n tekton:\n pipeline: ci\n workspaces:\n - name: data\n storage: 5Gi\n pvc: my-pvc\n",
63
+
wantErr: "multiple volume sources",
64
+
},
65
+
{
66
+
name: "valid single source",
67
+
yaml: "tack:\n tekton:\n pipeline: ci\n workspaces:\n - name: scratch\n storage: 1Gi\n",
68
+
},
69
+
}
70
+
for _, tt := range tests {
71
+
t.Run(tt.name, func(t *testing.T) {
72
+
_, err := parseTektonWorkflowConfig(tt.yaml)
73
+
if tt.wantErr == "" {
74
+
if err != nil {
75
+
t.Fatalf("unexpected error: %v", err)
76
+
}
77
+
return
78
+
}
79
+
if err == nil {
80
+
t.Fatalf("expected error containing %q", tt.wantErr)
81
+
}
82
+
if !strings.Contains(err.Error(), tt.wantErr) {
83
+
t.Fatalf(
84
+
"error %q does not contain %q",
85
+
err.Error(), tt.wantErr,
86
+
)
87
+
}
88
+
})
89
+
}
90
+
}
91
+
43
92
func TestTektonBuildPipelineRun(t *testing.T) {
44
93
cfg := &tektonWorkflowConfig{
45
94
Pipeline: "repo-ci",
History
1 round
0 comments
vincent.demeester.fr
submitted
#0
1 commit
expand
collapse
tekton: validate workspace configs reject ambiguous volume sources
A workspace with multiple sources set (e.g., both storage and pvc)
was silently picking whichever matched first in the switch. Now
parseTektonWorkflowConfig rejects workspaces with zero or multiple
sources, and workspaces with empty names, at parse time with clear
error messages.
no conflicts, ready to merge