diff --git a/.changeset/validate-generated-workflow-yaml.md b/.changeset/validate-generated-workflow-yaml.md new file mode 100644 index 000000000..29a47fc79 --- /dev/null +++ b/.changeset/validate-generated-workflow-yaml.md @@ -0,0 +1,5 @@ +--- +"radius": patch +--- + +Validate generated verify, deploy, and delete workflows as YAML before they can be committed, surfacing malformed upstream templates or quote-bearing environment and application-file values as local generation errors instead of GitHub Actions HTTP 422 failures. diff --git a/.github/workflows/live-tests.yml b/.github/workflows/live-tests.yml new file mode 100644 index 000000000..6298822ed --- /dev/null +++ b/.github/workflows/live-tests.yml @@ -0,0 +1,67 @@ +# yaml-language-server: $schema=https://www.schemastore.org/github-workflow.json +--- +name: Live upstream tests + +# Runs the opt-in `*.live.test.ts` suites that fetch the real workflow templates +# from radius-project/radius at runtime and assert YAML validity plus the OIDC +# environment-binding contract. A failure means an upstream template change +# broke what the extension generates (e.g. the invalid-YAML deploy regression +# in #407). +# +# This is a SEPARATE workflow from the hermetic Build suite (build.yml), which +# stays fully offline and deterministic. These live suites are intentionally +# excluded from that suite and from the publish/release pipeline because they are +# non-hermetic: they depend on an external repo's moving `main` ref and on +# network access to raw.githubusercontent.com. +# +# They run on every pull request and every push to `main` so an upstream +# template regression is flagged immediately as a failed check, plus nightly as a +# catch-all in case no PR lands between an upstream change and the next run. +# +# Tradeoff to be aware of: because they fetch from radius-project/radius@main, a +# regression pushed upstream can turn an otherwise-good ai-extensions PR red until +# upstream is fixed. That is intentional here — surfacing the break early is the +# point — but a failure in this job should be triaged against the current +# upstream templates, not assumed to be caused by the PR's own changes. + +on: + pull_request: + push: + branches: [main] + schedule: + # 07:00 UTC daily (~midnight US Pacific), as a catch-all between pushes. + - cron: "0 7 * * *" + workflow_dispatch: + +permissions: {} + +jobs: + live: + name: Live upstream template tests + runs-on: ubuntu-latest + permissions: + contents: read + env: + # Enables the `describe.skipIf(!LIVE)` blocks in the workflow live tests + # (workflow-yaml.live.test.ts and oidc-environment-contract.live.test.ts). + RUN_LIVE_WORKFLOW_TESTS: "1" + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install pnpm + uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6.0.10 + + - name: Setup Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version-file: .node-version + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Run live upstream workflow tests + run: pnpm test -- src/workflows/workflow-yaml.live.test.ts src/workflows/oidc-environment-contract.live.test.ts diff --git a/packages/adapter-canvas/build.mjs b/packages/adapter-canvas/build.mjs index fee2b1b48..78b6f44ef 100644 --- a/packages/adapter-canvas/build.mjs +++ b/packages/adapter-canvas/build.mjs @@ -31,6 +31,10 @@ import { homedir } from "node:os"; const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(__dirname, "../.."); +const yamlBrowserEntry = resolve( + dirname(fileURLToPath(import.meta.resolve("yaml"))), + "../browser/index.js" +); // .node-version is the one place the supported Node major is declared; deriving // the compile target from it keeps them from drifting apart. @@ -351,6 +355,9 @@ const options = { format: "esm", platform: "node", target, + // yaml's Node entry is CommonJS and leaves a dynamic require("process") in + // the ESM bundle. Its browser entry is equivalent pure ESM parser code. + alias: { yaml: yamlBrowserEntry }, // The runtime is known to be Node, so emit UTF-8 rather than \uXXXX escapes. charset: "utf8", minify: true, diff --git a/packages/adapter-canvas/package.json b/packages/adapter-canvas/package.json index 16c65baa8..0f56227c9 100644 --- a/packages/adapter-canvas/package.json +++ b/packages/adapter-canvas/package.json @@ -26,7 +26,8 @@ }, "dependencies": { "@radius-project/adapter-shared": "workspace:*", - "@radius-project/core": "workspace:*" + "@radius-project/core": "workspace:*", + "yaml": "catalog:" }, "devDependencies": { "@github/copilot-sdk": "catalog:", diff --git a/packages/adapter-canvas/src/infra.test.ts b/packages/adapter-canvas/src/infra.test.ts index 137df6264..0ef0046c5 100644 --- a/packages/adapter-canvas/src/infra.test.ts +++ b/packages/adapter-canvas/src/infra.test.ts @@ -118,6 +118,42 @@ const STALE_AZURE_VERIFY = const STALE_AWS_VERIFY = "name: verify\njobs:\n v:\n run: echo STALE ${{ vars.AWS_ROLE_ARN }}\n"; +describe("generated workflow YAML validation", () => { + beforeEach(() => { + h.upstream = { ...BASE_UPSTREAM }; + }); + + it.each([ + { + name: "deploy environment", + generate: () => generateDeployWorkflow("it's-prod", ".radius/app.bicep"), + context: 'deploy workflow "run-rad-commands.yml"' + }, + { + name: "deploy app file", + generate: () => generateDeployWorkflow("prod", ".radius/app's.bicep"), + context: 'deploy workflow "run-rad-commands-azure.yml"' + }, + { + name: "delete environment", + generate: () => generateDeleteWorkflow("it's-prod"), + context: 'delete workflow "delete-application.yml"' + }, + { + name: "verify environment", + generate: () => generateVerifyWorkflow("it's-prod", "azure"), + context: 'verify workflow "verify-azure.yml"' + } + ])( + "rejects an invalid $name scalar before commit", + async ({ generate, context }) => { + await expect(generate()).rejects.toThrow( + `Generated ${context} is invalid YAML` + ); + } + ); +}); + describe("GHCR verification probe", () => { it("checks push permission with a non-mutating upload session", () => { const workflow = configureVerifyGhcrProbe( diff --git a/packages/adapter-canvas/src/infra.ts b/packages/adapter-canvas/src/infra.ts index f9191cda8..b525d7152 100644 --- a/packages/adapter-canvas/src/infra.ts +++ b/packages/adapter-canvas/src/infra.ts @@ -22,6 +22,7 @@ import { DELETE_AWS_FILE } from "@radius-project/core"; import type { DeployWorkflowOptions } from "@radius-project/core"; +import { parse as parseYaml } from "yaml"; import { fetchFileFromRepoResult, fetchFileFromRepo, @@ -84,6 +85,17 @@ function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } +function assertValidWorkflowYaml(workflow: string, context: string): void { + try { + parseYaml(workflow); + } catch (error) { + throw new Error( + `Generated ${context} is invalid YAML: ${errorMessage(error)}`, + { cause: error } + ); + } +} + export { DEPLOY_DISPATCHER_FILE, DEPLOY_AZURE_FILE, DEPLOY_AWS_FILE }; export { DELETE_APP_DISPATCHER_FILE, DELETE_AZURE_FILE, DELETE_AWS_FILE }; @@ -139,9 +151,11 @@ export async function generateVerifyWorkflow( if (!fileName) throw new Error(`No verify template for provider "${provider}".`); const upstream = await fetchRadiusTemplate(fileName); - return configureVerifyGhcrProbe( + const workflow = configureVerifyGhcrProbe( coreGenerateVerifyWorkflow(env, platform, upstream) ); + assertValidWorkflowYaml(workflow, `verify workflow "${fileName}"`); + return workflow; } /** @@ -238,6 +252,9 @@ export async function generateDeployWorkflow( generated[DEPLOY_DISPATCHER_FILE] ); } + for (const [file, workflow] of Object.entries(generated)) { + assertValidWorkflowYaml(workflow, `deploy workflow "${file}"`); + } return generated; } @@ -271,6 +288,9 @@ export async function generateDeleteWorkflow( generated[DELETE_APP_DISPATCHER_FILE] ); } + for (const [file, workflow] of Object.entries(generated)) { + assertValidWorkflowYaml(workflow, `delete workflow "${file}"`); + } return generated; } diff --git a/packages/core/package.json b/packages/core/package.json index 0d09b6d2e..afc35a6da 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -22,6 +22,7 @@ "lint": "eslint \"src/**/*.ts\" vitest.config.ts" }, "devDependencies": { - "vitest": "catalog:" + "vitest": "catalog:", + "yaml": "catalog:" } } diff --git a/packages/core/src/workflows/deploy.test.ts b/packages/core/src/workflows/deploy.test.ts index 2a4c4c6e6..d2896fa88 100644 --- a/packages/core/src/workflows/deploy.test.ts +++ b/packages/core/src/workflows/deploy.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { parse as parseYaml } from "yaml"; import { DEFAULT_TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS, DEFAULT_TARGET_CLUSTER_ARCH_MODE, @@ -182,3 +183,120 @@ describe("generateDeployWorkflow", () => { ); }); }); + +// Fixtures that FAITHFULLY MIRROR the quoting of the real radius-project/radius +// deploy templates (`.github/extension/run-rad-commands*.yml`), unlike the +// inline fixtures above which use double quotes throughout. Reproducing the real +// mix is what lets these tests catch the issue #407 class of bug: +// - plain values are injected into SINGLE-quoted scalars (`APP_FILE`, `ENV`) +// and into unquoted GHA expressions (`APP_IMAGE`, dispatcher `environment`); +// - the arch placeholders are injected into DOUBLE-quoted scalars, because the +// value core injects is itself a GHA expression whose default is +// single-quoted (`${{ vars.X || 'detect' }}`). A single-quoted scalar there +// nests the quotes and produces invalid YAML (the #407 regression). +const REALISTIC_TEMPLATES = { + [DEPLOY_DISPATCHER_FILE]: `name: deploy +on: + workflow_dispatch: + inputs: + environment: + default: '{{ENV}}' +jobs: + detect: + runs-on: ubuntu-latest + environment: \${{ inputs.environment || '{{ENV}}' }} + env: + TARGET_CLUSTER_ARCH_MODE: "{{TARGET_CLUSTER_ARCH_MODE}}" + TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS: "{{TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS}}" + steps: + - run: echo \${{ github.sha }} +`, + [DEPLOY_AZURE_FILE]: `name: deploy-azure +env: + APP_FILE: '{{APP_FILE}}' + APP_IMAGE: \${{ inputs.image || github.sha || 'latest' }} + TARGET_CLUSTER_ARCH_MODE: "{{TARGET_CLUSTER_ARCH_MODE}}" + TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS: "{{TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS}}" +jobs: + deploy: + runs-on: ubuntu-latest + steps: + - uses: radius-project/radius/.github/extension/actions/run-rad-commands@{{RADIUS_REF}} +`, + [DEPLOY_AWS_FILE]: `name: deploy-aws +env: + APP_FILE: '{{APP_FILE}}' + APP_IMAGE: \${{ inputs.image || github.sha || 'latest' }} + TARGET_CLUSTER_ARCH_MODE: "{{TARGET_CLUSTER_ARCH_MODE}}" + TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS: "{{TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS}}" +jobs: + deploy: + runs-on: ubuntu-latest + steps: + - uses: radius-project/radius/.github/extension/actions/run-rad-commands@{{RADIUS_REF}} +` +}; + +describe("generateDeployWorkflow YAML validity", () => { + // Positive guard: with the real templates' quoting and the default arch vars + // (single-quoted GHA expressions), every rendered file must parse as valid + // YAML. Prior tests only did `.toContain(...)` substring checks and never + // parsed the output, so they stayed green against invalid YAML. + it("renders valid YAML for every file when arch scalars are double-quoted", () => { + const files = generateDeployWorkflow( + "prod", + ".radius/app.bicep", + REALISTIC_TEMPLATES + ); + + for (const [name, body] of Object.entries(files)) { + expect( + () => parseYaml(body), + `${name} should parse as valid YAML` + ).not.toThrow(); + } + }); + + // The injected GHA expression (with its single-quoted default) must survive + // substitution intact inside the double-quoted scalar. + it("preserves the injected GHA arch expressions as YAML string values", () => { + const files = generateDeployWorkflow( + "prod", + ".radius/app.bicep", + REALISTIC_TEMPLATES + ); + const azure = parseYaml(files[DEPLOY_AZURE_FILE]) as { + env: Record; + }; + + expect(azure.env.TARGET_CLUSTER_ARCH_MODE).toBe( + `\${{ vars.${RADIUS_BUILD_ARCH_MODE_VAR} || '${DEFAULT_TARGET_CLUSTER_ARCH_MODE}' }}` + ); + expect(azure.env.TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS).toBe( + `\${{ vars.${RADIUS_BUILD_PLATFORMS_VAR} || '${DEFAULT_TARGET_CLUSTER_ARCH_FALLBACK_PLATFORMS}' }}` + ); + // The plain single-quoted scalars stay intact too. + expect(azure.env.APP_FILE).toBe(".radius/app.bicep"); + }); + + // Negative regression guard: this is exactly the #407 bug. When the arch + // scalar is SINGLE-quoted (as the radius templates were before #12721), the + // single-quoted default inside the injected GHA expression nests and YAML + // terminates the scalar early, so the rendered file is invalid YAML. This test + // locks in WHY the scalar must be double-quoted: if someone reintroduces the + // single-quoted scalar (or an injected value that nests quotes), it fails. + it("produces invalid YAML when an arch scalar is single-quoted (issue #407)", () => { + const broken = { + ...REALISTIC_TEMPLATES, + [DEPLOY_AZURE_FILE]: REALISTIC_TEMPLATES[DEPLOY_AZURE_FILE].replace( + 'TARGET_CLUSTER_ARCH_MODE: "{{TARGET_CLUSTER_ARCH_MODE}}"', + "TARGET_CLUSTER_ARCH_MODE: '{{TARGET_CLUSTER_ARCH_MODE}}'" + ) + }; + // Substitution still succeeds (no unresolved placeholder), so generation + // does not throw — the breakage only surfaces when the YAML is parsed. + const files = generateDeployWorkflow("prod", ".radius/app.bicep", broken); + + expect(() => parseYaml(files[DEPLOY_AZURE_FILE])).toThrow(); + }); +}); diff --git a/packages/core/src/workflows/oidc-environment-contract.live.test.ts b/packages/core/src/workflows/oidc-environment-contract.live.test.ts index 19c59bcaf..8b89196ac 100644 --- a/packages/core/src/workflows/oidc-environment-contract.live.test.ts +++ b/packages/core/src/workflows/oidc-environment-contract.live.test.ts @@ -15,10 +15,10 @@ // no template copy, so it would only assert against a fixture we wrote here. // // This hits the network and depends on an external repo's moving ref, so it is -// NOT part of the default hermetic suite: it runs only when -// RUN_LIVE_WORKFLOW_TESTS is set (e.g. locally or in a scheduled job), never in -// normal CI. Runtime day-to-day protection is the verify-credentials flow, which -// fails loudly if the FIC does not match; this test catches the drift earlier. +// NOT part of the default hermetic suite. A separate non-required CI workflow +// sets RUN_LIVE_WORKFLOW_TESTS on pull requests, pushes to main, and nightly. +// Runtime day-to-day protection is the verify-credentials flow, which fails +// loudly if the FIC does not match; this test catches the drift earlier. import { describe, it, expect } from "vitest"; import { RADIUS_WORKFLOW_REPO, diff --git a/packages/core/src/workflows/workflow-yaml.live.test.ts b/packages/core/src/workflows/workflow-yaml.live.test.ts new file mode 100644 index 000000000..128aa0736 --- /dev/null +++ b/packages/core/src/workflows/workflow-yaml.live.test.ts @@ -0,0 +1,125 @@ +// Opt-in live regression test. Fetches the CURRENT workflow templates from +// radius-project/radius `.github/extension/` and asserts that, once the +// extension fills them, every generated file parses as valid YAML. +// +// Why this matters: the extension renders templates fetched from +// radius-project/radius at RUNTIME (RADIUS_REF, currently `main`) — it bundles +// no copy. `generateDeployWorkflow` injects GitHub Actions expressions whose +// string defaults are single-quoted (`${{ vars.RADIUS_BUILD_ARCH_MODE || +// 'detect' }}`). If an upstream template wraps such a placeholder in a +// single-quoted YAML scalar, the injected quotes nest and the rendered file +// becomes invalid YAML — every deploy dispatch then fails upstream with +// `HTTP 422: failed to parse workflow`. That is exactly issue #407 +// (radius-project/ai-extensions#407), caused by radius-project/radius#12640 and +// fixed by radius-project/radius#12721. The hermetic tests in deploy.test.ts use +// inline fixtures, so only a LIVE render of the real templates catches an +// upstream quoting (or any other YAML) regression. +// +// Coverage is not limited to the deploy workflows: every generator the +// extension ships (deploy, delete, verify) is rendered from its real upstream +// templates and asserted to produce valid YAML, so a malformed scalar, +// indentation, or quoting change in ANY of those upstream templates is caught. +// +// This hits the network and depends on an external repo's moving ref, so it is +// NOT part of the default hermetic suite. A separate non-required CI workflow +// sets RUN_LIVE_WORKFLOW_TESTS on pull requests, pushes to main, and nightly. +import { describe, it, expect } from "vitest"; +import { parse as parseYaml } from "yaml"; +import { azure } from "../platforms/azure.js"; +import { aws } from "../platforms/aws.js"; +import { + RADIUS_WORKFLOW_REPO, + RADIUS_WORKFLOW_DIR, + RADIUS_REF, + DEPLOY_DISPATCHER_FILE, + DEPLOY_AZURE_FILE, + DEPLOY_AWS_FILE, + generateDeployWorkflow +} from "./deploy.js"; +import { + DELETE_RADIUS_REF, + DELETE_APP_DISPATCHER_FILE, + DELETE_AZURE_FILE, + DELETE_AWS_FILE, + generateDeleteWorkflow +} from "./delete.js"; +import { + VERIFY_AZURE_FILE, + VERIFY_AWS_FILE, + generateVerifyWorkflow +} from "./verify.js"; + +const LIVE = !!process.env.RUN_LIVE_WORKFLOW_TESTS; + +async function fetchWorkflow(file: string, ref: string): Promise { + const url = `https://raw.githubusercontent.com/${RADIUS_WORKFLOW_REPO}/${ref}/${RADIUS_WORKFLOW_DIR}/${file}`; + const res = await fetch(url); + if (!res.ok) { + throw new Error(`failed to fetch ${url}: ${res.status} ${res.statusText}`); + } + return res.text(); +} + +async function fetchTemplates( + files: string[], + ref: string +): Promise> { + const bodies = await Promise.all(files.map((f) => fetchWorkflow(f, ref))); + const templates: Record = {}; + files.forEach((f, i) => { + templates[f] = bodies[i]; + }); + return templates; +} + +function assertAllValidYaml( + generated: Record, + ref: string +): void { + for (const [name, body] of Object.entries(generated)) { + expect( + () => parseYaml(body), + `${name} rendered from upstream ${ref} should parse as valid YAML` + ).not.toThrow(); + } +} + +describe.skipIf(!LIVE)( + "live workflow YAML validity (opt-in: set RUN_LIVE_WORKFLOW_TESTS)", + () => { + // The deploy workflows are the ones that inject single-quoted GHA arch + // expressions, so this is the case that would have caught radius#12640. + it("renders valid YAML from the current upstream deploy templates", async () => { + const templates = await fetchTemplates( + [DEPLOY_DISPATCHER_FILE, DEPLOY_AZURE_FILE, DEPLOY_AWS_FILE], + RADIUS_REF + ); + const generated = generateDeployWorkflow( + "prod", + ".radius/app.bicep", + templates + ); + assertAllValidYaml(generated, RADIUS_REF); + }, 30_000); + + it("renders valid YAML from the current upstream delete templates", async () => { + const templates = await fetchTemplates( + [DELETE_APP_DISPATCHER_FILE, DELETE_AZURE_FILE, DELETE_AWS_FILE], + DELETE_RADIUS_REF + ); + const generated = generateDeleteWorkflow("prod", templates); + assertAllValidYaml(generated, DELETE_RADIUS_REF); + }, 30_000); + + it("renders valid YAML from the current upstream verify templates", async () => { + for (const [platform, file] of [ + [azure, VERIFY_AZURE_FILE], + [aws, VERIFY_AWS_FILE] + ] as const) { + const template = await fetchWorkflow(file, RADIUS_REF); + const rendered = generateVerifyWorkflow("prod", platform, template); + assertAllValidYaml({ [file]: rendered }, RADIUS_REF); + } + }, 30_000); + } +); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 062420f29..499b94701 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -24,6 +24,9 @@ catalogs: vitest: specifier: ^4.1.10 version: 4.1.10 + yaml: + specifier: ^2.9.0 + version: 2.9.0 overrides: read-yaml-file@<2: ^2.1.0 @@ -92,6 +95,9 @@ importers: '@radius-project/core': specifier: workspace:* version: link:../core + yaml: + specifier: 'catalog:' + version: 2.9.0 devDependencies: '@github/copilot-sdk': specifier: 'catalog:' @@ -154,6 +160,9 @@ importers: vitest: specifier: 'catalog:' version: 4.1.10(@types/node@24.13.3)(@vitest/browser-playwright@4.1.10)(@vitest/coverage-v8@4.1.10)(vite@8.2.0(@types/node@24.13.3)(esbuild@0.28.2)(yaml@2.9.0)) + yaml: + specifier: 'catalog:' + version: 2.9.0 plugins/radius: dependencies: diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index e57cf1032..cc2c7e1fb 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -27,5 +27,9 @@ catalog: "@vitest/coverage-v8": ^4.1.10 typescript: ^7.0.2 vitest: ^4.1.10 + # YAML parser used only by @radius-project/core tests to assert generated + # workflow files parse as valid YAML (dev-only; core ships no runtime deps). + # Caret range compatible with the 2.9.0 already resolved in the lockfile. + yaml: ^2.9.0 minimumReleaseAge: 11520 # 8 days