Skip to content

Commit 5db2e80

Browse files
authored
Add entrypoint and command keys for service containers (#343)
Introduce service-container-mapping schema definition with entrypoint and command properties, gated behind allowServiceContainerCommand feature flag. Job containers remain unaffected.
1 parent 83de320 commit 5db2e80

File tree

10 files changed

+313
-8
lines changed

10 files changed

+313
-8
lines changed

expressions/src/features.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ describe("FeatureFlags", () => {
5555
"missingInputsQuickfix",
5656
"blockScalarChompingWarning",
5757
"allowCaseFunction",
58-
"allowCopilotRequestsPermission"
58+
"allowCopilotRequestsPermission",
59+
"allowServiceContainerCommand"
5960
]);
6061
});
6162
});

expressions/src/features.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ export interface ExperimentalFeatures {
4040
* @default false
4141
*/
4242
allowCopilotRequestsPermission?: boolean;
43+
44+
/**
45+
* Enable `entrypoint` and `command` keys in service containers (`jobs.<job_id>.services.*`).
46+
* @default false
47+
*/
48+
allowServiceContainerCommand?: boolean;
4349
}
4450

4551
/**
@@ -55,7 +61,8 @@ const allFeatureKeys: ExperimentalFeatureKey[] = [
5561
"missingInputsQuickfix",
5662
"blockScalarChompingWarning",
5763
"allowCaseFunction",
58-
"allowCopilotRequestsPermission"
64+
"allowCopilotRequestsPermission",
65+
"allowServiceContainerCommand"
5966
];
6067

6168
export class FeatureFlags {

languageservice/src/complete.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,3 +1015,38 @@ jobs:
10151015
expect(labels).not.toContain("copilot-requests");
10161016
});
10171017
});
1018+
1019+
describe("service container command/entrypoint completion", () => {
1020+
it("suggests entrypoint and command in service container", async () => {
1021+
const input = `on: push
1022+
jobs:
1023+
build:
1024+
runs-on: ubuntu-latest
1025+
services:
1026+
redis:
1027+
image: redis
1028+
|`;
1029+
const result = await complete(...getPositionFromCursor(input));
1030+
1031+
expect(result).not.toBeUndefined();
1032+
const labels = result.map(x => x.label);
1033+
expect(labels).toContain("entrypoint");
1034+
expect(labels).toContain("command");
1035+
});
1036+
1037+
it("does not suggest entrypoint and command in job container", async () => {
1038+
const input = `on: push
1039+
jobs:
1040+
build:
1041+
runs-on: ubuntu-latest
1042+
container:
1043+
image: node:20
1044+
|`;
1045+
const result = await complete(...getPositionFromCursor(input));
1046+
1047+
expect(result).not.toBeUndefined();
1048+
const labels = result.map(x => x.label);
1049+
expect(labels).not.toContain("entrypoint");
1050+
expect(labels).not.toContain("command");
1051+
});
1052+
});

languageservice/src/complete.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ export async function complete(
116116
config,
117117
{
118118
fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0,
119-
errorPolicy: ErrorPolicy.TryConversion
119+
errorPolicy: ErrorPolicy.TryConversion,
120+
featureFlags: config?.featureFlags
120121
},
121122
true
122123
);
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
import {FeatureFlags} from "@actions/expressions";
2+
import {registerLogger} from "./log.js";
3+
import {createDocument} from "./test-utils/document.js";
4+
import {TestLogger} from "./test-utils/logger.js";
5+
import {clearCache} from "./utils/workflow-cache.js";
6+
import {validate, ValidationConfig} from "./validate.js";
7+
8+
registerLogger(new TestLogger());
9+
10+
const configWithFlag: ValidationConfig = {
11+
featureFlags: new FeatureFlags({allowServiceContainerCommand: true})
12+
};
13+
14+
beforeEach(() => {
15+
clearCache();
16+
});
17+
18+
describe("service container command/entrypoint", () => {
19+
describe("with feature flag enabled", () => {
20+
it("allows command in service container", async () => {
21+
const input = `
22+
on: push
23+
jobs:
24+
build:
25+
runs-on: ubuntu-latest
26+
services:
27+
redis:
28+
image: redis
29+
command: --port 6380
30+
steps:
31+
- run: echo hi
32+
`;
33+
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
34+
const commandErrors = result.filter(d => d.message.includes("command"));
35+
expect(commandErrors).toEqual([]);
36+
});
37+
38+
it("allows entrypoint in service container", async () => {
39+
const input = `
40+
on: push
41+
jobs:
42+
build:
43+
runs-on: ubuntu-latest
44+
services:
45+
redis:
46+
image: redis
47+
entrypoint: /usr/local/bin/redis-server
48+
steps:
49+
- run: echo hi
50+
`;
51+
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
52+
const entrypointErrors = result.filter(d => d.message.includes("entrypoint"));
53+
expect(entrypointErrors).toEqual([]);
54+
});
55+
56+
it("allows both command and entrypoint in service container", async () => {
57+
const input = `
58+
on: push
59+
jobs:
60+
build:
61+
runs-on: ubuntu-latest
62+
services:
63+
redis:
64+
image: redis
65+
entrypoint: /usr/local/bin/redis-server
66+
command: --port 6380
67+
steps:
68+
- run: echo hi
69+
`;
70+
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
71+
const relevantErrors = result.filter(d => d.message.includes("command") || d.message.includes("entrypoint"));
72+
expect(relevantErrors).toEqual([]);
73+
});
74+
75+
it("rejects command in job container even with flag enabled", async () => {
76+
const input = `
77+
on: push
78+
jobs:
79+
build:
80+
runs-on: ubuntu-latest
81+
container:
82+
image: node:20
83+
command: node
84+
steps:
85+
- run: echo hi
86+
`;
87+
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
88+
const commandErrors = result.filter(d => d.message.includes("command"));
89+
expect(commandErrors.length).toBeGreaterThan(0);
90+
});
91+
92+
it("rejects entrypoint in job container even with flag enabled", async () => {
93+
const input = `
94+
on: push
95+
jobs:
96+
build:
97+
runs-on: ubuntu-latest
98+
container:
99+
image: node:20
100+
entrypoint: /bin/bash
101+
steps:
102+
- run: echo hi
103+
`;
104+
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
105+
const entrypointErrors = result.filter(d => d.message.includes("entrypoint"));
106+
expect(entrypointErrors.length).toBeGreaterThan(0);
107+
});
108+
});
109+
110+
describe("with feature flag disabled", () => {
111+
it("rejects command in service container", async () => {
112+
const input = `
113+
on: push
114+
jobs:
115+
build:
116+
runs-on: ubuntu-latest
117+
services:
118+
redis:
119+
image: redis
120+
command: --port 6380
121+
steps:
122+
- run: echo hi
123+
`;
124+
const result = await validate(createDocument("wf.yaml", input));
125+
const commandErrors = result.filter(d => d.message.includes("command"));
126+
expect(commandErrors.length).toBeGreaterThan(0);
127+
});
128+
129+
it("rejects entrypoint in service container", async () => {
130+
const input = `
131+
on: push
132+
jobs:
133+
build:
134+
runs-on: ubuntu-latest
135+
services:
136+
redis:
137+
image: redis
138+
entrypoint: /usr/local/bin/redis-server
139+
steps:
140+
- run: echo hi
141+
`;
142+
const result = await validate(createDocument("wf.yaml", input));
143+
const entrypointErrors = result.filter(d => d.message.includes("entrypoint"));
144+
expect(entrypointErrors.length).toBeGreaterThan(0);
145+
});
146+
});
147+
});

languageservice/src/validate.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ async function validateWorkflow(textDocument: TextDocument, config?: ValidationC
8484
// Errors will be updated in the context. Attempt to do the conversion anyway in order to give the user more information
8585
const template = await getOrConvertWorkflowTemplate(result.context, result.value, textDocument.uri, config, {
8686
fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0,
87-
errorPolicy: ErrorPolicy.TryConversion
87+
errorPolicy: ErrorPolicy.TryConversion,
88+
featureFlags: config?.featureFlags
8889
});
8990

9091
// Validate expressions and value providers

workflow-parser/src/model/convert.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ export type WorkflowTemplateConverterOptions = {
4141

4242
/**
4343
* Feature flags for experimental features.
44-
* This option is not currently used but keeping it for future use.
4544
*/
4645
featureFlags?: FeatureFlags;
4746
};
@@ -62,6 +61,8 @@ export async function convertWorkflowTemplate(
6261
const result = {} as WorkflowTemplate;
6362
const opts = getOptionsWithDefaults(options);
6463

64+
context.state.featureFlags = opts.featureFlags;
65+
6566
if (context.errors.getErrors().length > 0 && opts.errorPolicy === ErrorPolicy.ReturnErrorsOnly) {
6667
result.errors = context.errors.getErrors().map(x => ({
6768
Message: x.message

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

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,91 @@ export function convertToJobContainer(context: TemplateContext, container: Templ
7070
}
7171
}
7272

73+
export function convertToServiceContainer(context: TemplateContext, container: TemplateToken): Container | undefined {
74+
let image: StringToken | undefined;
75+
let env: MappingToken | undefined;
76+
let ports: SequenceToken | undefined;
77+
let volumes: SequenceToken | undefined;
78+
let options: StringToken | undefined;
79+
let entrypoint: StringToken | undefined;
80+
let command: StringToken | undefined;
81+
82+
// Skip validation for expressions for now to match
83+
// behavior of the other parsers
84+
for (const [, token] of TemplateToken.traverse(container)) {
85+
if (token.isExpression) {
86+
return;
87+
}
88+
}
89+
90+
if (isString(container)) {
91+
image = container.assertString("container item");
92+
return {image: image};
93+
}
94+
95+
const mapping = container.assertMapping("container item");
96+
if (mapping)
97+
for (const item of mapping) {
98+
const key = item.key.assertString("container item key");
99+
const value = item.value;
100+
101+
switch (key.value) {
102+
case "image":
103+
image = value.assertString("container image");
104+
break;
105+
case "credentials":
106+
convertToJobCredentials(context, value);
107+
break;
108+
case "env":
109+
env = value.assertMapping("container env");
110+
for (const envItem of env) {
111+
envItem.key.assertString("container env value");
112+
}
113+
break;
114+
case "ports":
115+
ports = value.assertSequence("container ports");
116+
for (const port of ports) {
117+
port.assertString("container port");
118+
}
119+
break;
120+
case "volumes":
121+
volumes = value.assertSequence("container volumes");
122+
for (const volume of volumes) {
123+
volume.assertString("container volume");
124+
}
125+
break;
126+
case "options":
127+
options = value.assertString("container options");
128+
break;
129+
case "entrypoint":
130+
entrypoint = value.assertString("container entrypoint");
131+
break;
132+
case "command":
133+
command = value.assertString("container command");
134+
break;
135+
default:
136+
context.error(key, `Unexpected container item key: ${key.value}`);
137+
}
138+
}
139+
140+
if (!image) {
141+
context.error(container, "Container image cannot be empty");
142+
} else {
143+
return {image, env, ports, volumes, options, entrypoint, command};
144+
}
145+
}
146+
73147
export function convertToJobServices(context: TemplateContext, services: TemplateToken): Container[] | undefined {
74148
const serviceList: Container[] = [];
149+
const flags = context.state.featureFlags as import("@actions/expressions/features").FeatureFlags | undefined;
150+
const useServiceContainer = flags?.isEnabled("allowServiceContainerCommand") ?? false;
75151

76152
const mapping = services.assertMapping("services");
77153
for (const service of mapping) {
78154
service.key.assertString("service key");
79-
const container = convertToJobContainer(context, service.value);
155+
const container = useServiceContainer
156+
? convertToServiceContainer(context, service.value)
157+
: convertToJobContainer(context, service.value);
80158
if (container) {
81159
serviceList.push(container);
82160
}

workflow-parser/src/model/workflow-template.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ export type Container = {
7575
ports?: SequenceToken;
7676
volumes?: SequenceToken;
7777
options?: StringToken;
78+
entrypoint?: StringToken;
79+
command?: StringToken;
7880
};
7981

8082
export type Credential = {

0 commit comments

Comments
 (0)