Add YAML-validity tests for all generated workflows (regression guard for #407) - #430
Conversation
…ard for #407) The deploy-workflow generator injects GitHub Actions expressions whose string defaults are single-quoted (${{ vars.RADIUS_BUILD_ARCH_MODE || 'detect' }}). When an upstream template wraps such a placeholder in a single-quoted YAML scalar, the injected quotes nest and the rendered file becomes invalid YAML, so every Azure/AWS deploy dispatch fails upstream with HTTP 422 (issue #407). The existing deploy.test.ts used inline, already-double-quoted fixtures and asserted only with .toContain(...) substring checks -- it never parsed the rendered output as YAML nor exercised the real templates, so it stayed green against invalid output. Add consumer-side regression coverage in @radius-project/core: - yaml devDependency (dev-only; core ships no runtime deps). - Positive test: fixtures mirroring the real templates' quoting render valid YAML for all three files, and the injected GHA arch expressions survive substitution intact. - Negative guard: a single-quoted arch scalar renders invalid YAML, locking in why the scalar must be double-quoted. - Opt-in live test (RUN_LIVE_WORKFLOW_TESTS) that fetches the real upstream templates from radius-project/radius@main, renders them, and asserts valid YAML -- the only test that would have caught the radius#12640 regression. Relates to #407; source fix radius-project/radius#12721; regression origin radius-project/radius#12640. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: sk593 <shruthikumar@microsoft.com>
…flow-yaml-quoting Signed-off-by: sk593 <shruthikumar@microsoft.com> # Conflicts: # pnpm-lock.yaml
Dependency ReviewThe following issues were found:
License Issuespackages/adapter-canvas/package.json
OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Pull request overview
Adds regression coverage in @radius-project/core to ensure generated deploy workflow files are valid YAML, specifically guarding against the nested-quote failure mode that caused deploy dispatch HTTP 422 failures in #407 when GitHub Actions expressions with single-quoted defaults were injected into single-quoted YAML scalars.
Changes:
- Add
yamlas a devDependency (via workspace catalog) so tests can parse rendered workflows as YAML. - Extend
deploy.test.tswith YAML-parse assertions (positive + explicit negative regression guard for the single-quoted arch scalar case). - Add an opt-in live test that fetches current upstream Radius templates at
radius-project/radius@main, renders them, and asserts YAML validity whenRUN_LIVE_WORKFLOW_TESTSis set.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds yaml to the workspace catalog for test-only YAML parsing. |
| pnpm-lock.yaml | Records the catalog/importer entry for yaml in the lockfile. |
| packages/core/package.json | Adds yaml as a devDependency for @radius-project/core. |
| packages/core/src/workflows/deploy.test.ts | Adds hermetic YAML-validity tests (including a negative regression guard reproducing #407). |
| packages/core/src/workflows/deploy-yaml.live.test.ts | Adds an opt-in live test that validates YAML parsing against upstream templates fetched from radius-project/radius. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
🦋 Changeset detectedLatest commit: 7c34dd1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The live regression test only rendered the three deploy workflows. Extend it to also render the delete (delete-application/azure/aws.yml) and verify (verify-azure/aws.yml) generators from their real upstream templates and assert every generated file parses as valid YAML, so a malformed scalar, indentation, or quoting change in ANY shipped workflow template is caught -- not just the deploy arch-quote bug (#407). Rename deploy-yaml.live.test.ts -> workflow-yaml.live.test.ts to reflect the broader scope. Still opt-in via RUN_LIVE_WORKFLOW_TESTS. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: sk593 <shruthikumar@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: sk593 <shruthikumar@microsoft.com>
|
No changeset needed. This PR is test-only plus a dev-only |
Fixes the failing 'Build plugin dist > Check formatting' CI step; deploy.test.ts and workflow-yaml.live.test.ts did not match the repo Prettier config. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: sk593 <shruthikumar@microsoft.com>
The `*.live.test.ts` suites (workflow-yaml + oidc-environment-contract) are opt-in via RUN_LIVE_WORKFLOW_TESTS and nothing ran them automatically, so the upstream-contract canary they provide never fired. Add a scheduled workflow (nightly + workflow_dispatch) that sets the flag and runs both live suites. They stay out of the per-PR Build suite on purpose: they fetch templates from radius-project/radius@main over the network, so a regression pushed upstream would otherwise turn unrelated ai-extensions PRs red. On a schedule, a failure instead flags an upstream template regression (like the invalid-YAML deploy bug in #407) for triage without blocking contributors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: sk593 <shruthikumar@microsoft.com>
Make the upstream-contract canary easily flagged: the live suites now run as a required check on pull_request and push to main (still nightly + on demand too), so an invalid-YAML or shape regression in the radius-project/radius templates surfaces as a failed check right away instead of only within 24h. They remain a separate workflow from the hermetic Build suite, which stays offline and deterministic. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: sk593 <shruthikumar@microsoft.com>
nicolejms
left a comment
There was a problem hiding this comment.
two design questions on where the YAML-validity guard belongs.
Parse final verify, deploy, and delete workflow output in the Canvas adapter after all rendering and transformations. Invalid upstream templates or quote-bearing ENV/APP_FILE inputs now fail locally with a file-specific error instead of being committed and rejected later by GitHub Actions with HTTP 422. Keep core dependency-free by locating runtime validation in adapter-canvas. Bundle yaml's browser ESM entry so the generated extension remains loadable, and cover invalid environment/app-file scalars across all workflow generators. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: sk593 <shruthikumar@microsoft.com>
Why
Every Azure/AWS deploy dispatch was failing upstream with HTTP 422 because the generated deploy workflow files were invalid YAML (issue #407). This PR closes both gaps that let that ship: tests now parse rendered workflow YAML, and production generation now rejects invalid YAML before it can be committed.
Root cause — nested quotes. The deploy generator injects GitHub Actions expressions whose string defaults are single-quoted (GHA requires single quotes for string literals):
When the upstream template wraps that placeholder in a single-quoted YAML scalar —
TARGET_CLUSTER_ARCH_MODE: '{{TARGET_CLUSTER_ARCH_MODE}}'— the injected single quotes nest, YAML terminates the scalar early, and the rendered file is invalid. Double-quoting the scalar fixes it (source fix: radius-project/radius#12721).Why current tests missed it.
packages/core/src/workflows/deploy.test.tsrendered inline fixtures that were already double-quoted and asserted only with.toContain(...)substring checks. It never parsed the rendered output as YAML or exercised the real upstream templates, so it stayed green while the committed Radius templates produced invalid YAML after substitution.What
yamlto parse rendered workflow output.adapter-canvasparses final verify, deploy, and delete workflows after all core rendering and adapter transformations. Invalid upstream templates or quote-bearingENV/APP_FILEvalues now fail locally with a file-specific error before any workflow is committed, instead of surfacing later as GitHub Actions HTTP 422. Validation stays in the adapter so@radius-project/coreretains no runtime dependencies; the extension build aliasesyamlto its pure-ESM browser parser entry so the generated artifact remains loadable.APP_FILE/ENV, double-quoted arch scalars) render valid YAML for all deploy files, and injected GHA arch expressions survive substitution intact.workflow-yaml.live.test.tsfetches templates fromradius-project/radius@main, renders deploy, delete, and verify workflows, and parses every result. The existing OIDC environment-contract live suite runs alongside it..github/workflows/live-tests.ymlruns the live suites on every PR, pushes tomain, nightly, and manually. It is intentionally separate from the hermetic Build pipeline and is not listed as a required status check in the activemainruleset, so upstream/network failures are visible without blocking unrelated merges.radiuspatch changeset for the new runtime validation behavior.Testing
main.References