From 5db2e80f32d7bf62e18b3c858f9a23478b8a2e5d Mon Sep 17 00:00:00 2001 From: eric sciple Date: Tue, 31 Mar 2026 15:45:18 -0500 Subject: [PATCH] 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. --- expressions/src/features.test.ts | 3 +- expressions/src/features.ts | 9 +- languageservice/src/complete.test.ts | 35 +++++ languageservice/src/complete.ts | 3 +- ...validate.service-container-command.test.ts | 147 ++++++++++++++++++ languageservice/src/validate.ts | 3 +- workflow-parser/src/model/convert.ts | 3 +- .../src/model/converter/container.ts | 80 +++++++++- .../src/model/workflow-template.ts | 2 + workflow-parser/src/workflow-v1.0.json | 36 ++++- 10 files changed, 313 insertions(+), 8 deletions(-) create mode 100644 languageservice/src/validate.service-container-command.test.ts diff --git a/expressions/src/features.test.ts b/expressions/src/features.test.ts index 837b900..fb2850e 100644 --- a/expressions/src/features.test.ts +++ b/expressions/src/features.test.ts @@ -55,7 +55,8 @@ describe("FeatureFlags", () => { "missingInputsQuickfix", "blockScalarChompingWarning", "allowCaseFunction", - "allowCopilotRequestsPermission" + "allowCopilotRequestsPermission", + "allowServiceContainerCommand" ]); }); }); diff --git a/expressions/src/features.ts b/expressions/src/features.ts index d474d2d..4068a75 100644 --- a/expressions/src/features.ts +++ b/expressions/src/features.ts @@ -40,6 +40,12 @@ export interface ExperimentalFeatures { * @default false */ allowCopilotRequestsPermission?: boolean; + + /** + * Enable `entrypoint` and `command` keys in service containers (`jobs..services.*`). + * @default false + */ + allowServiceContainerCommand?: boolean; } /** @@ -55,7 +61,8 @@ const allFeatureKeys: ExperimentalFeatureKey[] = [ "missingInputsQuickfix", "blockScalarChompingWarning", "allowCaseFunction", - "allowCopilotRequestsPermission" + "allowCopilotRequestsPermission", + "allowServiceContainerCommand" ]; export class FeatureFlags { diff --git a/languageservice/src/complete.test.ts b/languageservice/src/complete.test.ts index aa9e483..fcdf659 100644 --- a/languageservice/src/complete.test.ts +++ b/languageservice/src/complete.test.ts @@ -1015,3 +1015,38 @@ jobs: expect(labels).not.toContain("copilot-requests"); }); }); + +describe("service container command/entrypoint completion", () => { + it("suggests entrypoint and command in service container", async () => { + const input = `on: push +jobs: + build: + runs-on: ubuntu-latest + services: + redis: + image: redis + |`; + const result = await complete(...getPositionFromCursor(input)); + + expect(result).not.toBeUndefined(); + const labels = result.map(x => x.label); + expect(labels).toContain("entrypoint"); + expect(labels).toContain("command"); + }); + + it("does not suggest entrypoint and command in job container", async () => { + const input = `on: push +jobs: + build: + runs-on: ubuntu-latest + container: + image: node:20 + |`; + const result = await complete(...getPositionFromCursor(input)); + + expect(result).not.toBeUndefined(); + const labels = result.map(x => x.label); + expect(labels).not.toContain("entrypoint"); + expect(labels).not.toContain("command"); + }); +}); diff --git a/languageservice/src/complete.ts b/languageservice/src/complete.ts index bbc364d..0ca1948 100644 --- a/languageservice/src/complete.ts +++ b/languageservice/src/complete.ts @@ -116,7 +116,8 @@ export async function complete( config, { fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0, - errorPolicy: ErrorPolicy.TryConversion + errorPolicy: ErrorPolicy.TryConversion, + featureFlags: config?.featureFlags }, true ); diff --git a/languageservice/src/validate.service-container-command.test.ts b/languageservice/src/validate.service-container-command.test.ts new file mode 100644 index 0000000..53537c9 --- /dev/null +++ b/languageservice/src/validate.service-container-command.test.ts @@ -0,0 +1,147 @@ +import {FeatureFlags} from "@actions/expressions"; +import {registerLogger} from "./log.js"; +import {createDocument} from "./test-utils/document.js"; +import {TestLogger} from "./test-utils/logger.js"; +import {clearCache} from "./utils/workflow-cache.js"; +import {validate, ValidationConfig} from "./validate.js"; + +registerLogger(new TestLogger()); + +const configWithFlag: ValidationConfig = { + featureFlags: new FeatureFlags({allowServiceContainerCommand: true}) +}; + +beforeEach(() => { + clearCache(); +}); + +describe("service container command/entrypoint", () => { + describe("with feature flag enabled", () => { + it("allows command in service container", async () => { + const input = ` +on: push +jobs: + build: + runs-on: ubuntu-latest + services: + redis: + image: redis + command: --port 6380 + steps: + - run: echo hi +`; + const result = await validate(createDocument("wf.yaml", input), configWithFlag); + const commandErrors = result.filter(d => d.message.includes("command")); + expect(commandErrors).toEqual([]); + }); + + it("allows entrypoint in service container", async () => { + const input = ` +on: push +jobs: + build: + runs-on: ubuntu-latest + services: + redis: + image: redis + entrypoint: /usr/local/bin/redis-server + steps: + - run: echo hi +`; + const result = await validate(createDocument("wf.yaml", input), configWithFlag); + const entrypointErrors = result.filter(d => d.message.includes("entrypoint")); + expect(entrypointErrors).toEqual([]); + }); + + it("allows both command and entrypoint in service container", async () => { + const input = ` +on: push +jobs: + build: + runs-on: ubuntu-latest + services: + redis: + image: redis + entrypoint: /usr/local/bin/redis-server + command: --port 6380 + steps: + - run: echo hi +`; + const result = await validate(createDocument("wf.yaml", input), configWithFlag); + const relevantErrors = result.filter(d => d.message.includes("command") || d.message.includes("entrypoint")); + expect(relevantErrors).toEqual([]); + }); + + it("rejects command in job container even with flag enabled", async () => { + const input = ` +on: push +jobs: + build: + runs-on: ubuntu-latest + container: + image: node:20 + command: node + steps: + - run: echo hi +`; + const result = await validate(createDocument("wf.yaml", input), configWithFlag); + const commandErrors = result.filter(d => d.message.includes("command")); + expect(commandErrors.length).toBeGreaterThan(0); + }); + + it("rejects entrypoint in job container even with flag enabled", async () => { + const input = ` +on: push +jobs: + build: + runs-on: ubuntu-latest + container: + image: node:20 + entrypoint: /bin/bash + steps: + - run: echo hi +`; + const result = await validate(createDocument("wf.yaml", input), configWithFlag); + const entrypointErrors = result.filter(d => d.message.includes("entrypoint")); + expect(entrypointErrors.length).toBeGreaterThan(0); + }); + }); + + describe("with feature flag disabled", () => { + it("rejects command in service container", async () => { + const input = ` +on: push +jobs: + build: + runs-on: ubuntu-latest + services: + redis: + image: redis + command: --port 6380 + steps: + - run: echo hi +`; + const result = await validate(createDocument("wf.yaml", input)); + const commandErrors = result.filter(d => d.message.includes("command")); + expect(commandErrors.length).toBeGreaterThan(0); + }); + + it("rejects entrypoint in service container", async () => { + const input = ` +on: push +jobs: + build: + runs-on: ubuntu-latest + services: + redis: + image: redis + entrypoint: /usr/local/bin/redis-server + steps: + - run: echo hi +`; + const result = await validate(createDocument("wf.yaml", input)); + const entrypointErrors = result.filter(d => d.message.includes("entrypoint")); + expect(entrypointErrors.length).toBeGreaterThan(0); + }); + }); +}); diff --git a/languageservice/src/validate.ts b/languageservice/src/validate.ts index 43b253e..a0c0bd8 100644 --- a/languageservice/src/validate.ts +++ b/languageservice/src/validate.ts @@ -84,7 +84,8 @@ async function validateWorkflow(textDocument: TextDocument, config?: ValidationC // Errors will be updated in the context. Attempt to do the conversion anyway in order to give the user more information const template = await getOrConvertWorkflowTemplate(result.context, result.value, textDocument.uri, config, { fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0, - errorPolicy: ErrorPolicy.TryConversion + errorPolicy: ErrorPolicy.TryConversion, + featureFlags: config?.featureFlags }); // Validate expressions and value providers diff --git a/workflow-parser/src/model/convert.ts b/workflow-parser/src/model/convert.ts index 73f0d71..89983cf 100644 --- a/workflow-parser/src/model/convert.ts +++ b/workflow-parser/src/model/convert.ts @@ -41,7 +41,6 @@ export type WorkflowTemplateConverterOptions = { /** * Feature flags for experimental features. - * This option is not currently used but keeping it for future use. */ featureFlags?: FeatureFlags; }; @@ -62,6 +61,8 @@ export async function convertWorkflowTemplate( const result = {} as WorkflowTemplate; const opts = getOptionsWithDefaults(options); + context.state.featureFlags = opts.featureFlags; + if (context.errors.getErrors().length > 0 && opts.errorPolicy === ErrorPolicy.ReturnErrorsOnly) { result.errors = context.errors.getErrors().map(x => ({ Message: x.message diff --git a/workflow-parser/src/model/converter/container.ts b/workflow-parser/src/model/converter/container.ts index 4c4bf79..f087866 100644 --- a/workflow-parser/src/model/converter/container.ts +++ b/workflow-parser/src/model/converter/container.ts @@ -70,13 +70,91 @@ export function convertToJobContainer(context: TemplateContext, container: Templ } } +export function convertToServiceContainer(context: TemplateContext, container: TemplateToken): Container | undefined { + let image: StringToken | undefined; + let env: MappingToken | undefined; + let ports: SequenceToken | undefined; + let volumes: SequenceToken | undefined; + let options: StringToken | undefined; + let entrypoint: StringToken | undefined; + let command: StringToken | undefined; + + // Skip validation for expressions for now to match + // behavior of the other parsers + for (const [, token] of TemplateToken.traverse(container)) { + if (token.isExpression) { + return; + } + } + + if (isString(container)) { + image = container.assertString("container item"); + return {image: image}; + } + + const mapping = container.assertMapping("container item"); + if (mapping) + for (const item of mapping) { + const key = item.key.assertString("container item key"); + const value = item.value; + + switch (key.value) { + case "image": + image = value.assertString("container image"); + break; + case "credentials": + convertToJobCredentials(context, value); + break; + case "env": + env = value.assertMapping("container env"); + for (const envItem of env) { + envItem.key.assertString("container env value"); + } + break; + case "ports": + ports = value.assertSequence("container ports"); + for (const port of ports) { + port.assertString("container port"); + } + break; + case "volumes": + volumes = value.assertSequence("container volumes"); + for (const volume of volumes) { + volume.assertString("container volume"); + } + break; + case "options": + options = value.assertString("container options"); + break; + case "entrypoint": + entrypoint = value.assertString("container entrypoint"); + break; + case "command": + command = value.assertString("container command"); + break; + default: + context.error(key, `Unexpected container item key: ${key.value}`); + } + } + + if (!image) { + context.error(container, "Container image cannot be empty"); + } else { + return {image, env, ports, volumes, options, entrypoint, command}; + } +} + export function convertToJobServices(context: TemplateContext, services: TemplateToken): Container[] | undefined { const serviceList: Container[] = []; + const flags = context.state.featureFlags as import("@actions/expressions/features").FeatureFlags | undefined; + const useServiceContainer = flags?.isEnabled("allowServiceContainerCommand") ?? false; const mapping = services.assertMapping("services"); for (const service of mapping) { service.key.assertString("service key"); - const container = convertToJobContainer(context, service.value); + const container = useServiceContainer + ? convertToServiceContainer(context, service.value) + : convertToJobContainer(context, service.value); if (container) { serviceList.push(container); } diff --git a/workflow-parser/src/model/workflow-template.ts b/workflow-parser/src/model/workflow-template.ts index 8224438..39821ce 100644 --- a/workflow-parser/src/model/workflow-template.ts +++ b/workflow-parser/src/model/workflow-template.ts @@ -75,6 +75,8 @@ export type Container = { ports?: SequenceToken; volumes?: SequenceToken; options?: StringToken; + entrypoint?: StringToken; + command?: StringToken; }; export type Credential = { diff --git a/workflow-parser/src/workflow-v1.0.json b/workflow-parser/src/workflow-v1.0.json index 4de1c20..f514407 100644 --- a/workflow-parser/src/workflow-v1.0.json +++ b/workflow-parser/src/workflow-v1.0.json @@ -2399,7 +2399,7 @@ ], "one-of": [ "non-empty-string", - "container-mapping" + "service-container-mapping" ] }, "container-registry-credentials": { @@ -2647,6 +2647,38 @@ "string": { "require-non-empty": true } + }, + "service-container-mapping": { + "mapping": { + "properties": { + "image": { + "type": "non-empty-string", + "description": "The Docker image to use as the container. The value can be the Docker Hub image or a registry name." + }, + "options": { + "type": "string", + "description": "Additional Docker container resource options." + }, + "env": "container-env", + "ports": { + "type": "sequence-of-non-empty-string", + "description": "An array of ports to expose on the container." + }, + "volumes": { + "type": "sequence-of-non-empty-string", + "description": "An array of volumes for the container to use. You can use volumes to share data between services or other steps in a job. You can specify named Docker volumes, anonymous Docker volumes, or bind mounts on the host." + }, + "credentials": "container-registry-credentials", + "entrypoint": { + "type": "string", + "description": "Override the default ENTRYPOINT in the service container image." + }, + "command": { + "type": "string", + "description": "Override the default CMD in the service container image." + } + } + } } } -} \ No newline at end of file +}