Skip to content

Commit 671f92d

Browse files
committed
Add validation for empty container image
Related PR: - actions/runner#4220 Relaxing schema non-empty-string for container/service image and moving to custom validation. This matches current production behavior which allows empty string at runtime, but not parse time.
1 parent fb5c6e4 commit 671f92d

File tree

5 files changed

+437
-61
lines changed

5 files changed

+437
-61
lines changed
Lines changed: 314 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,314 @@
1+
/* eslint-disable @typescript-eslint/no-non-null-assertion */
2+
import {nullTrace} from "../../test-utils/null-trace.js";
3+
import {parseWorkflow} from "../../workflows/workflow-parser.js";
4+
import {convertWorkflowTemplate, ErrorPolicy} from "../convert.js";
5+
6+
async function getErrors(content: string): Promise<string[]> {
7+
const result = parseWorkflow({name: "wf.yaml", content}, nullTrace);
8+
const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
9+
errorPolicy: ErrorPolicy.TryConversion
10+
});
11+
return (template.errors ?? []).map((e: {Message: string}) => e.Message);
12+
}
13+
14+
function expectNoContainerErrors(errors: string[]): void {
15+
const containerErrors = errors.filter(e => e.includes("Container image"));
16+
expect(containerErrors).toHaveLength(0);
17+
}
18+
19+
function expectContainerError(errors: string[], count = 1): void {
20+
const containerErrors = errors.filter(e => e.includes("Container image cannot be empty"));
21+
expect(containerErrors).toHaveLength(count);
22+
}
23+
24+
describe("container image validation", () => {
25+
describe("shorthand form", () => {
26+
it("container: '' is silent for job container", async () => {
27+
const errors = await getErrors(`on: push
28+
jobs:
29+
build:
30+
runs-on: linux
31+
container: ''
32+
steps:
33+
- run: echo hi`);
34+
expectNoContainerErrors(errors);
35+
});
36+
37+
it("container: docker:// errors", async () => {
38+
const errors = await getErrors(`on: push
39+
jobs:
40+
build:
41+
runs-on: linux
42+
container: docker://
43+
steps:
44+
- run: echo hi`);
45+
expectContainerError(errors);
46+
});
47+
48+
it("container: valid-image passes", async () => {
49+
const errors = await getErrors(`on: push
50+
jobs:
51+
build:
52+
runs-on: linux
53+
container: ubuntu:16.04
54+
steps:
55+
- run: echo hi`);
56+
expectNoContainerErrors(errors);
57+
});
58+
});
59+
60+
describe("mapping form", () => {
61+
it("container image: '' errors", async () => {
62+
const errors = await getErrors(`on: push
63+
jobs:
64+
build:
65+
runs-on: linux
66+
container:
67+
image: ''
68+
steps:
69+
- run: echo hi`);
70+
expectContainerError(errors);
71+
});
72+
73+
it("container image: docker:// errors", async () => {
74+
const errors = await getErrors(`on: push
75+
jobs:
76+
build:
77+
runs-on: linux
78+
container:
79+
image: docker://
80+
steps:
81+
- run: echo hi`);
82+
expectContainerError(errors);
83+
});
84+
85+
it("container: {} (empty object, missing image) errors", async () => {
86+
const errors = await getErrors(`on: push
87+
jobs:
88+
build:
89+
runs-on: linux
90+
container: {}
91+
steps:
92+
- run: echo hi`);
93+
expectContainerError(errors);
94+
});
95+
96+
it("container image: null errors", async () => {
97+
const errors = await getErrors(`on: push
98+
jobs:
99+
build:
100+
runs-on: linux
101+
container:
102+
image:
103+
steps:
104+
- run: echo hi`);
105+
expectContainerError(errors);
106+
});
107+
108+
it("empty image with expression in other field still errors", async () => {
109+
const errors = await getErrors(`on: push
110+
jobs:
111+
build:
112+
runs-on: linux
113+
container:
114+
image: ''
115+
options: \${{ matrix.opts }}
116+
steps:
117+
- run: echo hi`);
118+
expectContainerError(errors);
119+
});
120+
});
121+
122+
describe("services shorthand", () => {
123+
it("services svc: '' errors", async () => {
124+
const errors = await getErrors(`on: push
125+
jobs:
126+
build:
127+
runs-on: linux
128+
services:
129+
svc: ''
130+
steps:
131+
- run: echo hi`);
132+
expectContainerError(errors);
133+
});
134+
135+
it("services svc: docker:// errors", async () => {
136+
const errors = await getErrors(`on: push
137+
jobs:
138+
build:
139+
runs-on: linux
140+
services:
141+
svc: docker://
142+
steps:
143+
- run: echo hi`);
144+
expectContainerError(errors);
145+
});
146+
});
147+
148+
describe("services mapping", () => {
149+
it("services svc image: '' errors", async () => {
150+
const errors = await getErrors(`on: push
151+
jobs:
152+
build:
153+
runs-on: linux
154+
services:
155+
svc:
156+
image: ''
157+
steps:
158+
- run: echo hi`);
159+
expectContainerError(errors);
160+
});
161+
162+
it("services svc image: docker:// errors", async () => {
163+
const errors = await getErrors(`on: push
164+
jobs:
165+
build:
166+
runs-on: linux
167+
services:
168+
svc:
169+
image: docker://
170+
steps:
171+
- run: echo hi`);
172+
expectContainerError(errors);
173+
});
174+
175+
it("services svc: {} (empty object) errors", async () => {
176+
const errors = await getErrors(`on: push
177+
jobs:
178+
build:
179+
runs-on: linux
180+
services:
181+
svc: {}
182+
steps:
183+
- run: echo hi`);
184+
expectContainerError(errors);
185+
});
186+
187+
it("empty image with expression sibling service still errors", async () => {
188+
const errors = await getErrors(`on: push
189+
jobs:
190+
build:
191+
runs-on: linux
192+
services:
193+
svc1:
194+
image: ''
195+
svc2: \${{ matrix.svc }}
196+
steps:
197+
- run: echo hi`);
198+
expectContainerError(errors);
199+
});
200+
});
201+
202+
describe("expression safety", () => {
203+
it("container: expression skips validation", async () => {
204+
const errors = await getErrors(`on: push
205+
jobs:
206+
build:
207+
runs-on: linux
208+
container: \${{ matrix.container }}
209+
steps:
210+
- run: echo hi`);
211+
expectNoContainerErrors(errors);
212+
});
213+
214+
it("container image: expression skips validation", async () => {
215+
const errors = await getErrors(`on: push
216+
jobs:
217+
build:
218+
runs-on: linux
219+
container:
220+
image: \${{ matrix.image }}
221+
options: --privileged
222+
steps:
223+
- run: echo hi`);
224+
expectNoContainerErrors(errors);
225+
});
226+
227+
it("container with expression key skips validation", async () => {
228+
const errors = await getErrors(`on: push
229+
jobs:
230+
build:
231+
runs-on: linux
232+
container:
233+
\${{ vars.KEY }}: ubuntu
234+
steps:
235+
- run: echo hi`);
236+
expectNoContainerErrors(errors);
237+
});
238+
239+
it("services: expression skips validation", async () => {
240+
const errors = await getErrors(`on: push
241+
jobs:
242+
build:
243+
runs-on: linux
244+
services: \${{ fromJSON(inputs.services) }}
245+
steps:
246+
- run: echo hi`);
247+
expectNoContainerErrors(errors);
248+
});
249+
250+
it("services with expression alias key skips validation", async () => {
251+
const errors = await getErrors(`on: push
252+
jobs:
253+
build:
254+
runs-on: linux
255+
services:
256+
\${{ matrix.alias }}: postgres
257+
steps:
258+
- run: echo hi`);
259+
expectNoContainerErrors(errors);
260+
});
261+
262+
it("services container with expression key skips validation", async () => {
263+
const errors = await getErrors(`on: push
264+
jobs:
265+
build:
266+
runs-on: linux
267+
services:
268+
db:
269+
\${{ vars.KEY }}: postgres
270+
steps:
271+
- run: echo hi`);
272+
expectNoContainerErrors(errors);
273+
});
274+
275+
it("container with all expression fields skips validation", async () => {
276+
const errors = await getErrors(`on: push
277+
jobs:
278+
build:
279+
runs-on: linux
280+
container:
281+
image: \${{ matrix.image }}
282+
options: \${{ matrix.options }}
283+
steps:
284+
- run: echo hi`);
285+
expectNoContainerErrors(errors);
286+
});
287+
288+
it("services svc: expression skips validation", async () => {
289+
const errors = await getErrors(`on: push
290+
jobs:
291+
build:
292+
runs-on: linux
293+
services:
294+
db: \${{ matrix.db }}
295+
steps:
296+
- run: echo hi`);
297+
expectNoContainerErrors(errors);
298+
});
299+
300+
it("services image: expression skips validation", async () => {
301+
const errors = await getErrors(`on: push
302+
jobs:
303+
build:
304+
runs-on: linux
305+
services:
306+
db:
307+
image: \${{ matrix.db_image }}
308+
options: --health-cmd pg_isready
309+
steps:
310+
- run: echo hi`);
311+
expectNoContainerErrors(errors);
312+
});
313+
});
314+
});

0 commit comments

Comments
 (0)