Skip to content

Commit f62a0e1

Browse files
authored
Remove allowServiceContainerCommand feature flag (#345)
Service container entrypoint/command support is now unconditional.
1 parent 9e1662f commit f62a0e1

File tree

5 files changed

+66
-90
lines changed

5 files changed

+66
-90
lines changed

AGENTS.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Agents
2+
3+
## Build
4+
5+
```
6+
npx lerna run build
7+
```
8+
9+
## Test
10+
11+
```
12+
npm -w @actions/expressions test
13+
npm -w @actions/workflow-parser test
14+
npm -w @actions/languageservice test
15+
```
16+
17+
## Format
18+
19+
Always run formatting before committing:
20+
21+
```
22+
npx prettier --write <changed files>
23+
```
24+
25+
Verify with:
26+
27+
```
28+
npm run format-check -ws
29+
```
30+
31+
## Feature flags
32+
33+
Feature flags are defined in `expressions/src/features.ts` (`ExperimentalFeatures` interface + `allFeatureKeys` array). They are plumbed through `ConvertOptions`, `CompletionConfig`, `ValidationConfig`, and `initializationOptions`. When a feature graduates to stable, remove its flag and make the behavior unconditional.

expressions/src/features.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ describe("FeatureFlags", () => {
5555
"missingInputsQuickfix",
5656
"blockScalarChompingWarning",
5757
"allowCaseFunction",
58-
"allowCopilotRequestsPermission",
59-
"allowServiceContainerCommand"
58+
"allowCopilotRequestsPermission"
6059
]);
6160
});
6261
});

expressions/src/features.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,6 @@ 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;
4943
}
5044

5145
/**
@@ -61,8 +55,7 @@ const allFeatureKeys: ExperimentalFeatureKey[] = [
6155
"missingInputsQuickfix",
6256
"blockScalarChompingWarning",
6357
"allowCaseFunction",
64-
"allowCopilotRequestsPermission",
65-
"allowServiceContainerCommand"
58+
"allowCopilotRequestsPermission"
6659
];
6760

6861
export class FeatureFlags {
Lines changed: 30 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,18 @@
1-
import {FeatureFlags} from "@actions/expressions";
21
import {registerLogger} from "./log.js";
32
import {createDocument} from "./test-utils/document.js";
43
import {TestLogger} from "./test-utils/logger.js";
54
import {clearCache} from "./utils/workflow-cache.js";
6-
import {validate, ValidationConfig} from "./validate.js";
5+
import {validate} from "./validate.js";
76

87
registerLogger(new TestLogger());
98

10-
const configWithFlag: ValidationConfig = {
11-
featureFlags: new FeatureFlags({allowServiceContainerCommand: true})
12-
};
13-
149
beforeEach(() => {
1510
clearCache();
1611
});
1712

1813
describe("service container command/entrypoint", () => {
19-
describe("with feature flag enabled", () => {
20-
it("allows command in service container", async () => {
21-
const input = `
14+
it("allows command in service container", async () => {
15+
const input = `
2216
on: push
2317
jobs:
2418
build:
@@ -30,13 +24,13 @@ jobs:
3024
steps:
3125
- run: echo hi
3226
`;
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-
});
27+
const result = await validate(createDocument("wf.yaml", input));
28+
const commandErrors = result.filter(d => d.message.includes("command"));
29+
expect(commandErrors).toEqual([]);
30+
});
3731

38-
it("allows entrypoint in service container", async () => {
39-
const input = `
32+
it("allows entrypoint in service container", async () => {
33+
const input = `
4034
on: push
4135
jobs:
4236
build:
@@ -48,13 +42,13 @@ jobs:
4842
steps:
4943
- run: echo hi
5044
`;
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-
});
45+
const result = await validate(createDocument("wf.yaml", input));
46+
const entrypointErrors = result.filter(d => d.message.includes("entrypoint"));
47+
expect(entrypointErrors).toEqual([]);
48+
});
5549

56-
it("allows both command and entrypoint in service container", async () => {
57-
const input = `
50+
it("allows both command and entrypoint in service container", async () => {
51+
const input = `
5852
on: push
5953
jobs:
6054
build:
@@ -67,13 +61,13 @@ jobs:
6761
steps:
6862
- run: echo hi
6963
`;
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-
});
64+
const result = await validate(createDocument("wf.yaml", input));
65+
const relevantErrors = result.filter(d => d.message.includes("command") || d.message.includes("entrypoint"));
66+
expect(relevantErrors).toEqual([]);
67+
});
7468

75-
it("rejects command in job container even with flag enabled", async () => {
76-
const input = `
69+
it("rejects command in job container", async () => {
70+
const input = `
7771
on: push
7872
jobs:
7973
build:
@@ -84,13 +78,13 @@ jobs:
8478
steps:
8579
- run: echo hi
8680
`;
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-
});
81+
const result = await validate(createDocument("wf.yaml", input));
82+
const commandErrors = result.filter(d => d.message.includes("command"));
83+
expect(commandErrors.length).toBeGreaterThan(0);
84+
});
9185

92-
it("rejects entrypoint in job container even with flag enabled", async () => {
93-
const input = `
86+
it("rejects entrypoint in job container", async () => {
87+
const input = `
9488
on: push
9589
jobs:
9690
build:
@@ -101,47 +95,8 @@ jobs:
10195
steps:
10296
- run: echo hi
10397
`;
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-
});
98+
const result = await validate(createDocument("wf.yaml", input));
99+
const entrypointErrors = result.filter(d => d.message.includes("entrypoint"));
100+
expect(entrypointErrors.length).toBeGreaterThan(0);
146101
});
147102
});

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,11 @@ export function convertToServiceContainer(context: TemplateContext, container: T
146146

147147
export function convertToJobServices(context: TemplateContext, services: TemplateToken): Container[] | undefined {
148148
const serviceList: Container[] = [];
149-
const flags = context.state.featureFlags as import("@actions/expressions/features").FeatureFlags | undefined;
150-
const useServiceContainer = flags?.isEnabled("allowServiceContainerCommand") ?? false;
151149

152150
const mapping = services.assertMapping("services");
153151
for (const service of mapping) {
154152
service.key.assertString("service key");
155-
const container = useServiceContainer
156-
? convertToServiceContainer(context, service.value)
157-
: convertToJobContainer(context, service.value);
153+
const container = convertToServiceContainer(context, service.value);
158154
if (container) {
159155
serviceList.push(container);
160156
}

0 commit comments

Comments
 (0)