Gate container image validation behind feature flag
Add containerImageValidation experimental feature flag that gates the new container image validation behind an opt-in toggle. When the flag is off (default), the legacy converter logic is used. When enabled, the improved validation with expression handling runs. The legacy code is duplicated to keep code paths fully isolated and make the eventual cleanup diff minimal — just delete the legacy functions and the flag guards. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -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<keyof ExperimentalFeatures, "all">;
|
||||
* 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;
|
||||
|
||||
@@ -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<WorkflowTemplateConverterOptions> = {
|
||||
const defaultOptions: Omit<Required<WorkflowTemplateConverterOptions>, "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<WorkflowTemplateConverterOptions> {
|
||||
function getOptionsWithDefaults(
|
||||
options: WorkflowTemplateConverterOptions
|
||||
): Omit<Required<WorkflowTemplateConverterOptions>, "featureFlags"> {
|
||||
return {
|
||||
maxReusableWorkflowDepth:
|
||||
options.maxReusableWorkflowDepth !== undefined
|
||||
|
||||
@@ -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<string[]> {
|
||||
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
|
||||
});
|
||||
|
||||
@@ -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};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user