diff --git a/expressions/src/features.ts b/expressions/src/features.ts index b0a0770..4bc90b3 100644 --- a/expressions/src/features.ts +++ b/expressions/src/features.ts @@ -28,6 +28,13 @@ export interface ExperimentalFeatures { * @default false */ blockScalarChompingWarning?: boolean; + + /** + * Enable improved container image validation that handles + * expressions gracefully and validates empty/docker:// images. + * @default false + */ + containerImageValidation?: boolean; } /** @@ -39,7 +46,11 @@ export type ExperimentalFeatureKey = Exclude; * All known experimental feature keys. * This list must be kept in sync with the ExperimentalFeatures interface. */ -const allFeatureKeys: ExperimentalFeatureKey[] = ["missingInputsQuickfix", "blockScalarChompingWarning"]; +const allFeatureKeys: ExperimentalFeatureKey[] = [ + "missingInputsQuickfix", + "blockScalarChompingWarning", + "containerImageValidation" +]; export class FeatureFlags { private readonly features: ExperimentalFeatures; diff --git a/workflow-parser/src/model/convert.ts b/workflow-parser/src/model/convert.ts index afc14d4..7dbc946 100644 --- a/workflow-parser/src/model/convert.ts +++ b/workflow-parser/src/model/convert.ts @@ -1,3 +1,4 @@ +import {FeatureFlags} from "@actions/expressions"; import {TemplateContext} from "../templates/template-context.js"; import {TemplateToken, TemplateTokenError} from "../templates/tokens/template-token.js"; import {FileProvider} from "../workflows/file-provider.js"; @@ -37,9 +38,15 @@ export type WorkflowTemplateConverterOptions = { * By default, conversion will be skipped if there are errors in the {@link TemplateContext}. */ errorPolicy?: ErrorPolicy; + + /** + * Feature flags for experimental features. + * When not provided, all experimental features are disabled. + */ + featureFlags?: FeatureFlags; }; -const defaultOptions: Required = { +const defaultOptions: Omit, "featureFlags"> = { maxReusableWorkflowDepth: 4, fetchReusableWorkflowDepth: 0, errorPolicy: ErrorPolicy.ReturnErrorsOnly @@ -54,6 +61,11 @@ export async function convertWorkflowTemplate( const result = {} as WorkflowTemplate; const opts = getOptionsWithDefaults(options); + // Store feature flags in context for converter functions + if (options.featureFlags) { + context.state["featureFlags"] = options.featureFlags; + } + if (context.errors.getErrors().length > 0 && opts.errorPolicy === ErrorPolicy.ReturnErrorsOnly) { result.errors = context.errors.getErrors().map(x => ({ Message: x.message @@ -132,7 +144,9 @@ export async function convertWorkflowTemplate( return result; } -function getOptionsWithDefaults(options: WorkflowTemplateConverterOptions): Required { +function getOptionsWithDefaults( + options: WorkflowTemplateConverterOptions +): Omit, "featureFlags"> { return { maxReusableWorkflowDepth: options.maxReusableWorkflowDepth !== undefined diff --git a/workflow-parser/src/model/converter/container.test.ts b/workflow-parser/src/model/converter/container.test.ts index a57f379..47c435a 100644 --- a/workflow-parser/src/model/converter/container.test.ts +++ b/workflow-parser/src/model/converter/container.test.ts @@ -3,8 +3,12 @@ import {nullTrace} from "../../test-utils/null-trace.js"; import {parseWorkflow} from "../../workflows/workflow-parser.js"; import {convertWorkflowTemplate, ErrorPolicy} from "../convert.js"; +// Minimal FeatureFlags-compatible object for tests +const featureFlags = {isEnabled: (f: string) => f === "containerImageValidation"}; + async function getErrors(content: string): Promise { const result = parseWorkflow({name: "wf.yaml", content}, nullTrace); + result.context.state["featureFlags"] = featureFlags; const template = await convertWorkflowTemplate(result.context, result.value!, undefined, { errorPolicy: ErrorPolicy.TryConversion }); diff --git a/workflow-parser/src/model/converter/container.ts b/workflow-parser/src/model/converter/container.ts index 8ef9b27..0a1895d 100644 --- a/workflow-parser/src/model/converter/container.ts +++ b/workflow-parser/src/model/converter/container.ts @@ -1,8 +1,13 @@ +import {FeatureFlags} from "@actions/expressions"; import {TemplateContext} from "../../templates/template-context.js"; import {MappingToken, SequenceToken, StringToken, TemplateToken} from "../../templates/tokens/index.js"; import {isString} from "../../templates/tokens/type-guards.js"; import {Container, Credential} from "../workflow-template.js"; +function getFeatureFlags(context: TemplateContext): FeatureFlags | undefined { + return context.state["featureFlags"] as FeatureFlags | undefined; +} + const DOCKER_URI_PREFIX = "docker://"; function isEmptyImage(value: string): boolean { @@ -15,6 +20,11 @@ export function convertToJobContainer( container: TemplateToken, isServiceContainer = false ): Container | undefined { + // Feature flag guard — use legacy implementation when flag is off + if (!getFeatureFlags(context)?.isEnabled("containerImageValidation")) { + return convertToJobContainerLegacy(context, container); + } + if (container.isExpression) { return; } @@ -114,6 +124,11 @@ export function convertToJobContainer( } export function convertToJobServices(context: TemplateContext, services: TemplateToken): Container[] | undefined { + // Feature flag guard — use legacy implementation when flag is off + if (!getFeatureFlags(context)?.isEnabled("containerImageValidation")) { + return convertToJobServicesLegacy(context, services); + } + if (services.isExpression) { return; } @@ -169,3 +184,108 @@ function convertCredentials(context: TemplateContext, value: TemplateToken): Cre return {username, password}; } + +// ===== Legacy implementations (remove when containerImageValidation graduates) ===== + +function convertToJobContainerLegacy(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; + + 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": + convertToJobCredentialsLegacy(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; + 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}; + } +} + +function convertToJobServicesLegacy(context: TemplateContext, services: TemplateToken): Container[] | undefined { + const serviceList: Container[] = []; + + const mapping = services.assertMapping("services"); + for (const service of mapping) { + service.key.assertString("service key"); + const container = convertToJobContainerLegacy(context, service.value); + if (container) { + serviceList.push(container); + } + } + return serviceList; +} + +function convertToJobCredentialsLegacy(context: TemplateContext, value: TemplateToken): Credential | undefined { + const mapping = value.assertMapping("credentials"); + + let username: StringToken | undefined; + let password: StringToken | undefined; + + for (const item of mapping) { + const key = item.key.assertString("credentials item"); + const value = item.value; + + switch (key.value) { + case "username": + username = value.assertString("credentials username"); + break; + case "password": + password = value.assertString("credentials password"); + break; + default: + context.error(key, `credentials key ${key.value}`); + } + } + + return {username, password}; +}