Skip to content

Commit ba800d5

Browse files
ericscipleericsciple
committed
Validate empty container image aligned with Go new container validation
- container: '' is silent (null) for job containers, error for services - docker:// prefix stripped before empty check - Expression-aware: only skips validation when image itself is expression - Expression keys in container/services mappings handled gracefully - Removes legacy convertToJobCredentials (validation-only, no detailed field checking)
1 parent a92bf9e commit ba800d5

File tree

2 files changed

+174
-86
lines changed

2 files changed

+174
-86
lines changed

‎workflow-parser/src/model/converter/container.ts‎

Lines changed: 73 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,114 +1,107 @@
11
import {TemplateContext} from "../../templates/template-context.js";
2-
import {MappingToken, SequenceToken, StringToken, TemplateToken} from "../../templates/tokens/index.js";
2+
import {StringToken, TemplateToken} from "../../templates/tokens/index.js";
33
import {isString} from "../../templates/tokens/type-guards.js";
4-
import {Container, Credential} from "../workflow-template.js";
5-
6-
export function convertToJobContainer(context: TemplateContext, container: TemplateToken): Container | undefined {
7-
let image: StringToken | undefined;
8-
let env: MappingToken | undefined;
9-
let ports: SequenceToken | undefined;
10-
let volumes: SequenceToken | undefined;
11-
let options: StringToken | undefined;
12-
13-
// Skip validation for expressions for now to match
14-
// behavior of the other parsers
15-
for (const [, token] of TemplateToken.traverse(container)) {
16-
if (token.isExpression) {
17-
return;
18-
}
4+
import {Container} from "../workflow-template.js";
5+
6+
const DOCKER_URI_PREFIX = "docker://";
7+
8+
export function convertToJobContainer(
9+
context: TemplateContext,
10+
container: TemplateToken,
11+
isServiceContainer = false
12+
): Container | undefined {
13+
// Expression? Skip validation
14+
if (container.isExpression) {
15+
return;
1916
}
2017

18+
// Shorthand form
2119
if (isString(container)) {
22-
// Workflow uses shorthand syntax `container: image-name`
23-
image = container.assertString("container item");
20+
const image = container.assertString("container item");
2421
if (!image || image.value.length === 0) {
22+
// Error for service container, silent for job container
23+
if (isServiceContainer) {
24+
context.error(container, "Container image cannot be empty");
25+
}
26+
return;
27+
}
28+
29+
// Trim docker:// prefix and check if empty
30+
const trimmed = image.value.startsWith(DOCKER_URI_PREFIX)
31+
? image.value.substring(DOCKER_URI_PREFIX.length)
32+
: image.value;
33+
if (trimmed.length === 0) {
2534
context.error(container, "Container image cannot be empty");
2635
return;
2736
}
28-
return {image: image};
37+
38+
return {image};
2939
}
3040

41+
// Mapping form
3142
const mapping = container.assertMapping("container item");
32-
if (mapping)
43+
44+
let hasExpressionKey = false;
45+
let hasImageKey = false;
46+
47+
if (mapping) {
3348
for (const item of mapping) {
49+
// Key is expression?
50+
if (item.key.isExpression) {
51+
hasExpressionKey = true;
52+
continue;
53+
}
54+
3455
const key = item.key.assertString("container item key");
35-
const value = item.value;
36-
37-
switch (key.value) {
38-
case "image":
39-
image = value.assertString("container image");
40-
break;
41-
case "credentials":
42-
convertToJobCredentials(context, value);
43-
break;
44-
case "env":
45-
env = value.assertMapping("container env");
46-
for (const envItem of env) {
47-
envItem.key.assertString("container env value");
48-
}
49-
break;
50-
case "ports":
51-
ports = value.assertSequence("container ports");
52-
for (const port of ports) {
53-
port.assertString("container port");
54-
}
55-
break;
56-
case "volumes":
57-
volumes = value.assertSequence("container volumes");
58-
for (const volume of volumes) {
59-
volume.assertString("container volume");
56+
57+
if (key.value === "image") {
58+
hasImageKey = true;
59+
60+
// Expression? Skip
61+
if (item.value.isExpression) {
62+
continue;
63+
}
64+
65+
const imageStr = item.value.assertString("container image");
66+
if (imageStr) {
67+
// Trim docker:// prefix and check if empty
68+
const trimmed = imageStr.value.startsWith(DOCKER_URI_PREFIX)
69+
? imageStr.value.substring(DOCKER_URI_PREFIX.length)
70+
: imageStr.value;
71+
if (trimmed.length === 0) {
72+
context.error(item.value, "Container image cannot be empty");
6073
}
61-
break;
62-
case "options":
63-
options = value.assertString("container options");
64-
break;
65-
default:
66-
context.error(key, `Unexpected container item key: ${key.value}`);
74+
}
6775
}
6876
}
77+
}
6978

70-
if (!image || image.value.length === 0) {
79+
// Check for missing image key
80+
if (!hasImageKey && !hasExpressionKey) {
7181
context.error(container, "Container image cannot be empty");
72-
} else {
73-
return {image, env, ports, volumes, options};
7482
}
7583
}
7684

7785
export function convertToJobServices(context: TemplateContext, services: TemplateToken): Container[] | undefined {
86+
// Expression? Skip validation
87+
if (services.isExpression) {
88+
return;
89+
}
90+
7891
const serviceList: Container[] = [];
7992

8093
const mapping = services.assertMapping("services");
8194
for (const service of mapping) {
95+
// Key is expression?
96+
if (service.key.isExpression) {
97+
continue;
98+
}
99+
82100
service.key.assertString("service key");
83-
const container = convertToJobContainer(context, service.value);
101+
const container = convertToJobContainer(context, service.value, true);
84102
if (container) {
85103
serviceList.push(container);
86104
}
87105
}
88106
return serviceList;
89107
}
90-
91-
function convertToJobCredentials(context: TemplateContext, value: TemplateToken): Credential | undefined {
92-
const mapping = value.assertMapping("credentials");
93-
94-
let username: StringToken | undefined;
95-
let password: StringToken | undefined;
96-
97-
for (const item of mapping) {
98-
const key = item.key.assertString("credentials item");
99-
const value = item.value;
100-
101-
switch (key.value) {
102-
case "username":
103-
username = value.assertString("credentials username");
104-
break;
105-
case "password":
106-
password = value.assertString("credentials password");
107-
break;
108-
default:
109-
context.error(key, `credentials key ${key.value}`);
110-
}
111-
}
112-
113-
return {username, password};
114-
}

‎workflow-parser/testdata/reader/job-container-empty-image.yml‎

Lines changed: 101 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,61 +2,156 @@ include-source: false # Drop file/line/col from output
22
---
33
on: push
44
jobs:
5+
# Empty shorthand - silent for job container (matches Go new behavior)
56
build1:
67
runs-on: linux
78
container: ""
89
steps:
910
- run: echo hi
11+
# Empty mapping image - error
1012
build2:
1113
runs-on: linux
1214
container:
1315
image: ""
1416
steps:
1517
- run: echo hi
18+
# Empty service shorthand - error
1619
build3:
1720
runs-on: linux
1821
services:
1922
svc: ""
2023
steps:
2124
- run: echo hi
25+
# Empty service mapping image - error
2226
build4:
2327
runs-on: linux
2428
services:
2529
svc:
2630
image: ""
2731
steps:
2832
- run: echo hi
33+
# Null container - silent for job container
2934
build5:
3035
runs-on: linux
3136
container:
3237
steps:
3338
- run: echo hi
39+
# Null service - error
3440
build6:
3541
runs-on: linux
3642
services:
3743
svc:
3844
steps:
3945
- run: echo hi
46+
# Empty image with expression in other field - error
47+
build7:
48+
runs-on: linux
49+
container:
50+
image: ""
51+
env:
52+
FOO: ${{ secrets.X }}
53+
steps:
54+
- run: echo hi
55+
# Empty service image with expression credential - error
56+
build8:
57+
runs-on: linux
58+
services:
59+
svc:
60+
image: ""
61+
credentials:
62+
username: ${{ github.actor }}
63+
password: secret
64+
steps:
65+
- run: echo hi
66+
# docker:// prefix only - error
67+
build9:
68+
runs-on: linux
69+
container: docker://
70+
steps:
71+
- run: echo hi
72+
# docker:// prefix in mapping - error
73+
build10:
74+
runs-on: linux
75+
container:
76+
image: docker://
77+
steps:
78+
- run: echo hi
79+
# docker:// service shorthand - error
80+
build11:
81+
runs-on: linux
82+
services:
83+
svc: docker://
84+
steps:
85+
- run: echo hi
86+
# docker:// service mapping - error
87+
build12:
88+
runs-on: linux
89+
services:
90+
svc:
91+
image: docker://
92+
steps:
93+
- run: echo hi
94+
# Empty object (no image key) - error
95+
build13:
96+
runs-on: linux
97+
container: {}
98+
steps:
99+
- run: echo hi
100+
# Empty service object - error
101+
build14:
102+
runs-on: linux
103+
services:
104+
svc: {}
105+
steps:
106+
- run: echo hi
107+
# Null image value - error
108+
build15:
109+
runs-on: linux
110+
container:
111+
image:
112+
steps:
113+
- run: echo hi
40114
---
41115
{
42116
"errors": [
43117
{
44-
"Message": ".github/workflows/job-container-empty-image.yml (Line: 5, Col: 16): Container image cannot be empty"
118+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 13, Col: 14): Container image cannot be empty"
119+
},
120+
{
121+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 20, Col: 12): Container image cannot be empty"
122+
},
123+
{
124+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 28, Col: 16): Container image cannot be empty"
125+
},
126+
{
127+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 41, Col: 11): Container image cannot be empty"
128+
},
129+
{
130+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 48, Col: 14): Container image cannot be empty"
131+
},
132+
{
133+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 58, Col: 16): Container image cannot be empty"
134+
},
135+
{
136+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 67, Col: 16): Container image cannot be empty"
137+
},
138+
{
139+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 74, Col: 14): Container image cannot be empty"
45140
},
46141
{
47-
"Message": ".github/workflows/job-container-empty-image.yml (Line: 11, Col: 7): Container image cannot be empty"
142+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 81, Col: 12): Container image cannot be empty"
48143
},
49144
{
50-
"Message": ".github/workflows/job-container-empty-image.yml (Line: 17, Col: 12): Container image cannot be empty"
145+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 89, Col: 16): Container image cannot be empty"
51146
},
52147
{
53-
"Message": ".github/workflows/job-container-empty-image.yml (Line: 24, Col: 9): Container image cannot be empty"
148+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 95, Col: 16): Container image cannot be empty"
54149
},
55150
{
56-
"Message": ".github/workflows/job-container-empty-image.yml (Line: 29, Col: 15): Container image cannot be empty"
151+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 102, Col: 12): Container image cannot be empty"
57152
},
58153
{
59-
"Message": ".github/workflows/job-container-empty-image.yml (Line: 35, Col: 11): Container image cannot be empty"
154+
"Message": ".github/workflows/job-container-empty-image.yml (Line: 109, Col: 13): Container image cannot be empty"
60155
}
61156
]
62157
}

0 commit comments

Comments
 (0)