From 1c77297328c9ed014c842c2cf675c2da65c1c5e7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 18:45:30 +0000 Subject: [PATCH 1/6] feat(compile): support vmImage pool syntax for non-1ES targets Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/ae57b6df-ffaf-4666-9284-28c65c7f2ced Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- docs/front-matter.md | 7 +- docs/targets.md | 2 +- docs/template-markers.md | 11 +- src/compile/codemods/0002_pool_object_form.rs | 71 +++++++++++ src/compile/codemods/mod.rs | 3 + src/compile/common.rs | 114 +++++++++++++++--- src/compile/mod.rs | 6 +- src/compile/types.rs | 71 ++++++++--- src/data/base.yml | 6 +- src/data/job-base.yml | 6 +- src/data/stage-base.yml | 6 +- tests/compiler_tests.rs | 4 +- 12 files changed, 250 insertions(+), 57 deletions(-) create mode 100644 src/compile/codemods/0002_pool_object_form.rs diff --git a/docs/front-matter.md b/docs/front-matter.md index a53f0836..ee5cf01a 100644 --- a/docs/front-matter.md +++ b/docs/front-matter.md @@ -17,8 +17,11 @@ engine: copilot # Engine identifier. Defaults to copilot. Currently only 'copilo # model: claude-opus-4.7 # timeout-minutes: 30 workspace: repo # Optional: "root", "repo" (alias: "self"), or a checked-out repository alias. If not specified, defaults to "root" when no additional repositories are listed in `repos:`, and to "repo" when one or more additional repos are checked out. See "Workspace Defaults" below. -pool: AZS-1ES-L-MMS-ubuntu-22.04 # Agent pool name (string format). Defaults to AZS-1ES-L-MMS-ubuntu-22.04. -# pool: # Alternative object format (required for 1ES if specifying os) +pool: # Optional pool configuration + vmImage: ubuntu-latest # Microsoft-hosted (default for non-1ES targets) +# pool: # Self-hosted pool +# name: MySelfHostedPool +# pool: # 1ES pool format # name: AZS-1ES-L-MMS-ubuntu-22.04 # os: linux # Operating system: "linux" or "windows". Defaults to "linux". repos: # compact repository declarations (replaces repositories: + checkout:) diff --git a/docs/targets.md b/docs/targets.md index f75dbe73..47e146c6 100644 --- a/docs/targets.md +++ b/docs/targets.md @@ -42,7 +42,7 @@ The output contains the same 3-job chain (Agent → Detection → Execution) as `standalone`, with: - Job names prefixed with the agent name for uniqueness (e.g., `DailyReview_Agent`) - No triggers, pipeline name, or resource declarations (the parent pipeline owns those) -- Pool baked in from the front matter `pool:` field +- Pool baked in from the front matter `pool:` field (`vmImage` or `name`; defaults to `vmImage: ubuntu-latest`) Example front matter: ```yaml diff --git a/docs/template-markers.md b/docs/template-markers.md index 6c28551a..49ed9a08 100644 --- a/docs/template-markers.md +++ b/docs/template-markers.md @@ -152,13 +152,14 @@ Should be replaced with the engine's log directory path, generated by `Engine::l ## {{ pool }} -Should be replaced with the agent pool name from the `pool` front matter field. Defaults to `AZS-1ES-L-MMS-ubuntu-22.04` if not specified. +Used by the 1ES template as the pool **name** value (`name: {{ pool }}`). +Defaults to `AZS-1ES-L-MMS-ubuntu-22.04` when not specified. -The pool configuration accepts both string and object formats: -- **String format**: `pool: AZS-1ES-L-MMS-ubuntu-22.04` -- **Object format**: `pool: { name: AZS-1ES-L-MMS-ubuntu-22.04, os: linux }` +## {{ pool_block }} -The `os` field (defaults to "linux") is primarily used for 1ES target compatibility. +Used by non-1ES templates under `pool:` and expands to either: +- `vmImage: ` for Microsoft-hosted pools (default: `vmImage: ubuntu-latest`) +- `name: ` for self-hosted pools ## {{ setup_job }} diff --git a/src/compile/codemods/0002_pool_object_form.rs b/src/compile/codemods/0002_pool_object_form.rs new file mode 100644 index 00000000..93521541 --- /dev/null +++ b/src/compile/codemods/0002_pool_object_form.rs @@ -0,0 +1,71 @@ +//! `pool: ` → `pool: { name: }` +//! +//! Non-1ES targets now support both self-hosted (`name`) and +//! Microsoft-hosted (`vmImage`) pool syntax. This codemod rewrites the +//! legacy scalar shorthand into an explicit object form so sources are +//! unambiguous and easier to evolve. + +use anyhow::Result; +use serde_yaml::{Mapping, Value}; + +use super::{Codemod, CodemodContext}; + +pub static CODEMOD: Codemod = Codemod { + id: "pool_object_form", + summary: "pool: -> pool: { name: }", + introduced_in: "0.30.0", + apply: apply_codemod, +}; + +fn apply_codemod(fm: &mut Mapping, _ctx: &CodemodContext) -> Result { + let key = Value::String("pool".to_string()); + let Some(pool_value) = fm.get(&key).cloned() else { + return Ok(false); + }; + + let Value::String(name) = pool_value else { + // Already object-form (or invalid in another way) — no-op. + return Ok(false); + }; + + let mut mapped = Mapping::new(); + mapped.insert(Value::String("name".to_string()), Value::String(name)); + fm.insert(key, Value::Mapping(mapped)); + Ok(true) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn rewrites_scalar_pool_to_name_object() { + let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y\npool: MyPool").unwrap(); + let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + assert!(changed); + assert_eq!( + fm.get(Value::String("pool".into())).cloned(), + Some(serde_yaml::from_str::("name: MyPool").unwrap()) + ); + } + + #[test] + fn noops_when_pool_is_already_mapping() { + let mut fm: Mapping = + serde_yaml::from_str("name: x\ndescription: y\npool:\n vmImage: ubuntu-latest") + .unwrap(); + let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + assert!(!changed); + assert_eq!( + fm.get(Value::String("pool".into())).cloned(), + Some(serde_yaml::from_str::("vmImage: ubuntu-latest").unwrap()) + ); + } + + #[test] + fn noops_when_pool_absent() { + let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y").unwrap(); + let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + assert!(!changed); + } +} diff --git a/src/compile/codemods/mod.rs b/src/compile/codemods/mod.rs index b952ac28..9dc987ca 100644 --- a/src/compile/codemods/mod.rs +++ b/src/compile/codemods/mod.rs @@ -35,6 +35,8 @@ use serde_yaml::Mapping; mod helpers; #[path = "0001_repos_unified.rs"] mod m0001_repos_unified; +#[path = "0002_pool_object_form.rs"] +mod m0002_pool_object_form; #[allow(unused_imports)] // Re-exported for future codemods; only `take_key` is in-tree use. pub use helpers::{insert_no_overwrite, rename_key, take_key, ConflictPolicy}; @@ -85,6 +87,7 @@ pub struct Codemod { /// without harm. pub static CODEMODS: &[&'static Codemod] = &[ &m0001_repos_unified::CODEMOD, + &m0002_pool_object_form::CODEMOD, ]; /// Result of running the codemod registry on a single front-matter diff --git a/src/compile/common.rs b/src/compile/common.rs index 71d46370..1edf1d4f 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -4,7 +4,7 @@ use anyhow::{Context, Result}; use std::collections::{HashMap, HashSet}; use std::path::Path; -use super::types::{FrontMatter, OnConfig, PipelineParameter, Repository, ReposItem}; +use super::types::{CompileTarget, FrontMatter, OnConfig, PipelineParameter, PoolConfig, Repository, ReposItem}; use super::extensions::{CompilerExtension, Extension, McpgServerConfig, McpgGatewayConfig, McpgConfig, CompileContext}; use crate::compile::types::McpConfig; use crate::fuzzy_schedule; @@ -996,8 +996,51 @@ pub fn sanitize_filename(name: &str) -> String { .join("-") } -/// Default pool name -pub const DEFAULT_POOL: &str = "AZS-1ES-L-MMS-ubuntu-22.04"; +/// Default self-hosted pool for 1ES templates. +pub const DEFAULT_ONEES_POOL: &str = "AZS-1ES-L-MMS-ubuntu-22.04"; +/// Default Microsoft-hosted VM image for non-1ES templates. +pub const DEFAULT_VM_IMAGE_POOL: &str = "ubuntu-latest"; + +/// Resolve the `{{ pool }}` replacement used by the 1ES template (`name: {{ pool }}`). +fn resolve_onees_pool_name(pool: Option<&PoolConfig>) -> Result { + let Some(pool) = pool else { + return Ok(DEFAULT_ONEES_POOL.to_string()); + }; + + if let Some(name) = pool.name() { + return Ok(name.to_string()); + } + + if let Some(vm_image) = pool.vm_image() { + anyhow::bail!( + "target: 1es does not support `pool.vmImage` ('{}'); use `pool.name` for a 1ES pool", + vm_image + ); + } + + Ok(DEFAULT_ONEES_POOL.to_string()) +} + +/// Resolve the non-1ES pool block line (`name: ...` or `vmImage: ...`). +fn resolve_non_onees_pool_block(pool: Option<&PoolConfig>) -> Result { + let Some(pool) = pool else { + return Ok(format!("vmImage: {}", DEFAULT_VM_IMAGE_POOL)); + }; + + match pool { + PoolConfig::Name(name) => Ok(format!("name: {}", name)), + PoolConfig::Full(full) => match (full.name.as_deref(), full.vm_image.as_deref()) { + (Some(name), Some(vm_image)) => anyhow::bail!( + "pool cannot specify both `name` and `vmImage` (got name='{}', vmImage='{}')", + name, + vm_image + ), + (Some(name), None) => Ok(format!("name: {}", name)), + (None, Some(vm_image)) => Ok(format!("vmImage: {}", vm_image)), + (None, None) => Ok(format!("vmImage: {}", DEFAULT_VM_IMAGE_POOL)), + }, + } +} /// Derive a valid ADO identifier from the agent name for use as a job-name /// prefix and stage name. Converts to PascalCase, stripping non-alphanumeric @@ -1692,7 +1735,7 @@ pub fn validate_resolve_pr_thread_statuses(front_matter: &FrontMatter) -> Result /// last, conditioned on the gate if filters are active. pub fn generate_setup_job( setup_steps: &[serde_yaml::Value], - pool: &str, + pool_block: &str, pr_filters: Option<&super::types::PrFilters>, pipeline_filters: Option<&super::types::PipelineFilters>, extensions: &[super::extensions::Extension], @@ -1756,7 +1799,7 @@ pub fn generate_setup_job( r#"- job: Setup displayName: "Setup" pool: - name: {pool} + {pool_block} steps: - checkout: self "# @@ -1778,7 +1821,7 @@ pub fn generate_setup_job( /// Generate the teardown job YAML pub fn generate_teardown_job( teardown_steps: &[serde_yaml::Value], - pool: &str, + pool_block: &str, ) -> String { if teardown_steps.is_empty() { return String::new(); @@ -1791,12 +1834,12 @@ pub fn generate_teardown_job( displayName: "Teardown" dependsOn: Execution pool: - name: {} + {} steps: - checkout: self {} "#, - pool, steps_yaml + pool_block, steps_yaml ) } @@ -2593,12 +2636,17 @@ pub async fn compile_shared( let source_path = generate_source_path(input_path); let pipeline_path = generate_pipeline_path(output_path); - // 7. Pool name - let pool = front_matter - .pool - .as_ref() - .map(|p| p.name().to_string()) - .unwrap_or_else(|| DEFAULT_POOL.to_string()); + // 7. Pool settings + let pool = if front_matter.target == CompileTarget::OneES { + resolve_onees_pool_name(front_matter.pool.as_ref())? + } else { + DEFAULT_ONEES_POOL.to_string() + }; + let pool_block = if front_matter.target == CompileTarget::OneES { + format!("name: {}", pool) + } else { + resolve_non_onees_pool_block(front_matter.pool.as_ref())? + }; // 8. Setup/teardown jobs, parameters, prepare/finalize steps let pr_filters = front_matter.pr_filters(); @@ -2612,8 +2660,8 @@ pub async fn compile_shared( let has_pipeline_filters = pipeline_filters .map(|f| !super::filter_ir::lower_pipeline_filters(f).is_empty()) .unwrap_or(false); - let setup_job = generate_setup_job(&front_matter.setup, &pool, pr_filters, pipeline_filters, extensions, ctx)?; - let teardown_job = generate_teardown_job(&front_matter.teardown, &pool); + let setup_job = generate_setup_job(&front_matter.setup, &pool_block, pr_filters, pipeline_filters, extensions, ctx)?; + let teardown_job = generate_teardown_job(&front_matter.teardown, &pool_block); let has_memory = front_matter .tools .as_ref() @@ -2746,6 +2794,7 @@ pub async fn compile_shared( ("{{ compiler_version }}", compiler_version), ("{{ engine_install_steps }}", &engine_install_steps), ("{{ pool }}", &pool), + ("{{ pool_block }}", &pool_block), ("{{ setup_job }}", &setup_job), ("{{ teardown_job }}", &teardown_job), ("{{ prepare_steps }}", &prepare_steps), @@ -6118,7 +6167,11 @@ mod tests { fn test_generate_setup_job_empty_returns_empty() { let fm: FrontMatter = serde_yaml::from_str("name: t\ndescription: t").unwrap(); let ctx = CompileContext::for_test(&fm); - assert!(generate_setup_job(&[], "MyPool", None, None, &[], &ctx).unwrap().is_empty()); + assert!( + generate_setup_job(&[], "name: MyPool", None, None, &[], &ctx) + .unwrap() + .is_empty() + ); } #[test] @@ -6126,7 +6179,7 @@ mod tests { let fm: FrontMatter = serde_yaml::from_str("name: t\ndescription: t").unwrap(); let ctx = CompileContext::for_test(&fm); let step: serde_yaml::Value = serde_yaml::from_str("bash: echo setup").unwrap(); - let out = generate_setup_job(&[step], "MyPool", None, None, &[], &ctx).unwrap(); + let out = generate_setup_job(&[step], "name: MyPool", None, None, &[], &ctx).unwrap(); assert!(out.contains("- job: Setup"), "out: {out}"); assert!(out.contains("displayName: \"Setup\""), "out: {out}"); assert!(out.contains("name: MyPool"), "out: {out}"); @@ -6136,19 +6189,40 @@ mod tests { #[test] fn test_generate_teardown_job_empty_returns_empty() { - assert!(generate_teardown_job(&[], "MyPool").is_empty()); + assert!(generate_teardown_job(&[], "name: MyPool").is_empty()); } #[test] fn test_generate_teardown_job_with_steps() { let step: serde_yaml::Value = serde_yaml::from_str("bash: echo td").unwrap(); - let out = generate_teardown_job(&[step], "MyPool"); + let out = generate_teardown_job(&[step], "name: MyPool"); assert!(out.contains("- job: Teardown"), "out: {out}"); assert!(out.contains("dependsOn: Execution"), "out: {out}"); assert!(out.contains("name: MyPool"), "out: {out}"); assert!(out.contains("echo td"), "out: {out}"); } + #[test] + fn test_resolve_non_onees_pool_block_defaults_to_vm_image() { + let block = resolve_non_onees_pool_block(None).expect("pool block"); + assert_eq!(block, "vmImage: ubuntu-latest"); + } + + #[test] + fn test_resolve_non_onees_pool_block_from_name() { + let pool = PoolConfig::Name("SelfHostedPool".to_string()); + let block = resolve_non_onees_pool_block(Some(&pool)).expect("pool block"); + assert_eq!(block, "name: SelfHostedPool"); + } + + #[test] + fn test_resolve_non_onees_pool_block_from_vm_image() { + let pool_yaml = "name: x\ndescription: x\npool:\n vmImage: windows-latest"; + let fm: FrontMatter = serde_yaml::from_str(pool_yaml).expect("front matter"); + let block = resolve_non_onees_pool_block(fm.pool.as_ref()).expect("pool block"); + assert_eq!(block, "vmImage: windows-latest"); + } + #[test] fn test_generate_agentic_depends_on_empty_steps() { assert!(generate_agentic_depends_on(&[], false, false, &[]).is_empty()); diff --git a/src/compile/mod.rs b/src/compile/mod.rs index 90375dc3..59519e52 100644 --- a/src/compile/mod.rs +++ b/src/compile/mod.rs @@ -778,7 +778,8 @@ Body "#; let (fm, _) = parse_markdown(content).unwrap(); let pool = fm.pool.unwrap(); - assert_eq!(pool.name(), "my-custom-pool"); + assert_eq!(pool.name(), Some("my-custom-pool")); + assert_eq!(pool.vm_image(), None); assert_eq!(pool.os(), "linux"); // default } @@ -795,7 +796,8 @@ Body "#; let (fm, _) = parse_markdown(content).unwrap(); let pool = fm.pool.unwrap(); - assert_eq!(pool.name(), "my-custom-pool"); + assert_eq!(pool.name(), Some("my-custom-pool")); + assert_eq!(pool.vm_image(), None); assert_eq!(pool.os(), "windows"); } diff --git a/src/compile/types.rs b/src/compile/types.rs index b84770dd..ae0d9f4e 100644 --- a/src/compile/types.rs +++ b/src/compile/types.rs @@ -27,10 +27,18 @@ pub enum CompileTarget { /// /// Examples: /// ```yaml -/// # Simple string format (works for both targets) -/// pool: AZS-1ES-L-MMS-ubuntu-22.04 +/// # Simple legacy string format (self-hosted pool name) +/// pool: MySelfHostedPool /// -/// # Object format (required for 1ES if specifying os) +/// # Object format (recommended) +/// pool: +/// vmImage: ubuntu-latest # Microsoft-hosted +/// +/// # Object format (self-hosted) +/// pool: +/// name: MySelfHostedPool +/// +/// # 1ES object format /// pool: /// name: AZS-1ES-L-MMS-ubuntu-22.04 /// os: linux @@ -38,7 +46,7 @@ pub enum CompileTarget { #[derive(Debug, Deserialize, Clone)] #[serde(untagged)] pub enum PoolConfig { - /// Simple pool name string + /// Simple legacy pool name string (self-hosted) Name(String), /// Full pool configuration object Full(PoolConfigFull), @@ -46,20 +54,34 @@ pub enum PoolConfig { impl Default for PoolConfig { fn default() -> Self { - PoolConfig::Name("AZS-1ES-L-MMS-ubuntu-22.04".to_string()) + PoolConfig::Full(PoolConfigFull { + name: None, + vm_image: Some("ubuntu-latest".to_string()), + os: None, + }) } } impl PoolConfig { - /// Get the pool name - pub fn name(&self) -> &str { + /// Get the self-hosted pool name, if configured. + pub fn name(&self) -> Option<&str> { match self { - PoolConfig::Name(name) => name, - PoolConfig::Full(full) => &full.name, + PoolConfig::Name(name) => Some(name), + PoolConfig::Full(full) => full.name.as_deref(), } } - /// Get the OS (defaults to "linux" if not specified) + /// Get the Microsoft-hosted VM image, if configured. + pub fn vm_image(&self) -> Option<&str> { + match self { + PoolConfig::Name(_) => None, + PoolConfig::Full(full) => full.vm_image.as_deref(), + } + } + + /// Get the OS (defaults to "linux" if not specified). + /// + /// Primarily applicable to 1ES pool configuration. #[allow(dead_code)] pub fn os(&self) -> &str { match self { @@ -80,7 +102,10 @@ impl SanitizeConfigTrait for PoolConfig { #[derive(Debug, Deserialize, Clone, SanitizeConfig)] pub struct PoolConfigFull { - pub name: String, + #[serde(default)] + pub name: Option, + #[serde(default, rename = "vmImage")] + pub vm_image: Option, #[serde(default)] pub os: Option, } @@ -1259,7 +1284,8 @@ mod tests { let yaml = "pool: MyPool"; let fm: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); let pool: PoolConfig = serde_yaml::from_value(fm["pool"].clone()).unwrap(); - assert_eq!(pool.name(), "MyPool"); + assert_eq!(pool.name(), Some("MyPool")); + assert_eq!(pool.vm_image(), None); assert_eq!(pool.os(), "linux"); // default } @@ -1268,7 +1294,8 @@ mod tests { let yaml = "pool:\n name: WinPool\n os: windows"; let fm: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); let pool: PoolConfig = serde_yaml::from_value(fm["pool"].clone()).unwrap(); - assert_eq!(pool.name(), "WinPool"); + assert_eq!(pool.name(), Some("WinPool")); + assert_eq!(pool.vm_image(), None); assert_eq!(pool.os(), "windows"); } @@ -1277,14 +1304,26 @@ mod tests { let yaml = "pool:\n name: LinuxPool"; let fm: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); let pool: PoolConfig = serde_yaml::from_value(fm["pool"].clone()).unwrap(); - assert_eq!(pool.name(), "LinuxPool"); + assert_eq!(pool.name(), Some("LinuxPool")); + assert_eq!(pool.vm_image(), None); + assert_eq!(pool.os(), "linux"); + } + + #[test] + fn test_pool_config_object_vm_image_form() { + let yaml = "pool:\n vmImage: ubuntu-latest"; + let fm: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); + let pool: PoolConfig = serde_yaml::from_value(fm["pool"].clone()).unwrap(); + assert_eq!(pool.name(), None); + assert_eq!(pool.vm_image(), Some("ubuntu-latest")); assert_eq!(pool.os(), "linux"); } #[test] - fn test_pool_config_default() { + fn test_pool_config_default_is_vm_image() { let pool = PoolConfig::default(); - assert_eq!(pool.name(), "AZS-1ES-L-MMS-ubuntu-22.04"); + assert_eq!(pool.name(), None); + assert_eq!(pool.vm_image(), Some("ubuntu-latest")); assert_eq!(pool.os(), "linux"); } diff --git a/src/data/base.yml b/src/data/base.yml index bbaa6863..f5829957 100644 --- a/src/data/base.yml +++ b/src/data/base.yml @@ -20,7 +20,7 @@ jobs: {{ agentic_depends_on }} {{ job_timeout }} pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -396,7 +396,7 @@ jobs: displayName: "Detection" dependsOn: Agent pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -598,7 +598,7 @@ jobs: - Detection condition: and(succeeded(), eq(dependencies.Detection.outputs['threatAnalysis.SafeToProcess'], 'true')) pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} diff --git a/src/data/job-base.yml b/src/data/job-base.yml index 9d8659f3..27f5bfdf 100644 --- a/src/data/job-base.yml +++ b/src/data/job-base.yml @@ -7,7 +7,7 @@ jobs: {{ agentic_depends_on }} {{ job_timeout }} pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -383,7 +383,7 @@ jobs: displayName: "Detection" dependsOn: {{ stage_prefix }}_Agent pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -585,7 +585,7 @@ jobs: - {{ stage_prefix }}_Detection condition: and(succeeded(), eq(dependencies.{{ stage_prefix }}_Detection.outputs['threatAnalysis.SafeToProcess'], 'true')) pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} diff --git a/src/data/stage-base.yml b/src/data/stage-base.yml index 47fe6e5a..9daa6934 100644 --- a/src/data/stage-base.yml +++ b/src/data/stage-base.yml @@ -11,7 +11,7 @@ stages: {{ agentic_depends_on }} {{ job_timeout }} pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -387,7 +387,7 @@ stages: displayName: "Detection" dependsOn: {{ stage_prefix }}_Agent pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -589,7 +589,7 @@ stages: - {{ stage_prefix }}_Detection condition: and(succeeded(), eq(dependencies.{{ stage_prefix }}_Detection.outputs['threatAnalysis.SafeToProcess'], 'true')) pool: - name: {{ pool }} + {{ pool_block }} steps: {{ checkout_self }} {{ checkout_repositories }} diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs index f4967eb3..2a3ca950 100644 --- a/tests/compiler_tests.rs +++ b/tests/compiler_tests.rs @@ -96,10 +96,10 @@ fn assert_required_markers(content: &str) { /// and that no hardcoded pool name leaks into the template. fn assert_pool_config(content: &str) { // Must appear once per job: Agent, Detection, Execution. - let pool_marker_count = content.matches("name: {{ pool }}").count(); + let pool_marker_count = content.matches("{{ pool_block }}").count(); assert_eq!( pool_marker_count, 3, - "Template should use '{{ pool }}' marker exactly three times (once for each job)" + "Template should use '{{ pool_block }}' marker exactly three times (once for each job)" ); assert!( !content.contains("name: AZS-1ES-L-MMS-ubuntu-22.04"), From f347808b4fb06f063a9588c97a252a93d4b38e87 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 18:51:33 +0000 Subject: [PATCH 2/6] fix(compile): simplify non-1ES pool value wiring Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/ae57b6df-ffaf-4666-9284-28c65c7f2ced Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- src/compile/common.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/compile/common.rs b/src/compile/common.rs index 1edf1d4f..c466b75d 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -2637,15 +2637,12 @@ pub async fn compile_shared( let pipeline_path = generate_pipeline_path(output_path); // 7. Pool settings - let pool = if front_matter.target == CompileTarget::OneES { - resolve_onees_pool_name(front_matter.pool.as_ref())? - } else { - DEFAULT_ONEES_POOL.to_string() - }; - let pool_block = if front_matter.target == CompileTarget::OneES { - format!("name: {}", pool) - } else { - resolve_non_onees_pool_block(front_matter.pool.as_ref())? + let (pool, pool_block) = match front_matter.target { + CompileTarget::OneES => { + let onees_pool = resolve_onees_pool_name(front_matter.pool.as_ref())?; + (onees_pool.clone(), format!("name: {}", onees_pool)) + } + _ => (String::new(), resolve_non_onees_pool_block(front_matter.pool.as_ref())?), }; // 8. Setup/teardown jobs, parameters, prepare/finalize steps From f167cc581e899a1399e211dc76822b55d662bf5a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 19:10:40 +0000 Subject: [PATCH 3/6] refactor(compile): unify pool marker across templates Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/a32e578f-2dbe-490f-b52c-2380cf007de9 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- docs/template-markers.md | 13 ++-- src/compile/common.rs | 141 +++++++++++++++++++++++---------------- src/data/1es-base.yml | 3 +- src/data/base.yml | 6 +- src/data/job-base.yml | 6 +- src/data/stage-base.yml | 6 +- tests/compiler_tests.rs | 4 +- 7 files changed, 102 insertions(+), 77 deletions(-) diff --git a/docs/template-markers.md b/docs/template-markers.md index 49ed9a08..988f8675 100644 --- a/docs/template-markers.md +++ b/docs/template-markers.md @@ -152,14 +152,13 @@ Should be replaced with the engine's log directory path, generated by `Engine::l ## {{ pool }} -Used by the 1ES template as the pool **name** value (`name: {{ pool }}`). -Defaults to `AZS-1ES-L-MMS-ubuntu-22.04` when not specified. +Used by all templates under a `pool:` block and expands to: +- non-1ES targets: one line (`vmImage: ` or `name: `) +- 1ES target: two lines (`name: ` and `os: `) -## {{ pool_block }} - -Used by non-1ES templates under `pool:` and expands to either: -- `vmImage: ` for Microsoft-hosted pools (default: `vmImage: ubuntu-latest`) -- `name: ` for self-hosted pools +Defaults: +- non-1ES: `vmImage: ubuntu-latest` +- 1ES: `name: AZS-1ES-L-MMS-ubuntu-22.04` + `os: linux` ## {{ setup_job }} diff --git a/src/compile/common.rs b/src/compile/common.rs index c466b75d..c128c6da 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -1001,44 +1001,64 @@ pub const DEFAULT_ONEES_POOL: &str = "AZS-1ES-L-MMS-ubuntu-22.04"; /// Default Microsoft-hosted VM image for non-1ES templates. pub const DEFAULT_VM_IMAGE_POOL: &str = "ubuntu-latest"; -/// Resolve the `{{ pool }}` replacement used by the 1ES template (`name: {{ pool }}`). -fn resolve_onees_pool_name(pool: Option<&PoolConfig>) -> Result { - let Some(pool) = pool else { - return Ok(DEFAULT_ONEES_POOL.to_string()); - }; - - if let Some(name) = pool.name() { - return Ok(name.to_string()); - } - - if let Some(vm_image) = pool.vm_image() { - anyhow::bail!( - "target: 1es does not support `pool.vmImage` ('{}'); use `pool.name` for a 1ES pool", - vm_image - ); - } - - Ok(DEFAULT_ONEES_POOL.to_string()) -} - -/// Resolve the non-1ES pool block line (`name: ...` or `vmImage: ...`). -fn resolve_non_onees_pool_block(pool: Option<&PoolConfig>) -> Result { - let Some(pool) = pool else { - return Ok(format!("vmImage: {}", DEFAULT_VM_IMAGE_POOL)); - }; +/// Resolve the `{{ pool }}` replacement block. +/// +/// - For non-1ES targets, this is a single line under `pool:`: +/// `name: ...` or `vmImage: ...`. +/// - For 1ES targets, this is two lines under `parameters.pool:`: +/// `name: ...` and `os: ...`. +fn resolve_pool_block(target: CompileTarget, pool: Option<&PoolConfig>) -> Result { + match target { + CompileTarget::OneES => { + let (name, os) = match pool { + None => (DEFAULT_ONEES_POOL.to_string(), "linux".to_string()), + Some(PoolConfig::Name(name)) => (name.clone(), "linux".to_string()), + Some(PoolConfig::Full(full)) => { + if let (Some(name), Some(vm_image)) = + (full.name.as_deref(), full.vm_image.as_deref()) + { + anyhow::bail!( + "pool cannot specify both `name` and `vmImage` (got name='{}', vmImage='{}')", + name, + vm_image + ); + } + if let Some(vm_image) = full.vm_image.as_deref() { + anyhow::bail!( + "target: 1es does not support `pool.vmImage` ('{}'); use `pool.name` for a 1ES pool", + vm_image + ); + } + ( + full.name + .as_deref() + .unwrap_or(DEFAULT_ONEES_POOL) + .to_string(), + full.os.as_deref().unwrap_or("linux").to_string(), + ) + } + }; + Ok(format!("name: {name}\nos: {os}")) + } + _ => { + let Some(pool) = pool else { + return Ok(format!("vmImage: {}", DEFAULT_VM_IMAGE_POOL)); + }; - match pool { - PoolConfig::Name(name) => Ok(format!("name: {}", name)), - PoolConfig::Full(full) => match (full.name.as_deref(), full.vm_image.as_deref()) { - (Some(name), Some(vm_image)) => anyhow::bail!( - "pool cannot specify both `name` and `vmImage` (got name='{}', vmImage='{}')", - name, - vm_image - ), - (Some(name), None) => Ok(format!("name: {}", name)), - (None, Some(vm_image)) => Ok(format!("vmImage: {}", vm_image)), - (None, None) => Ok(format!("vmImage: {}", DEFAULT_VM_IMAGE_POOL)), - }, + match pool { + PoolConfig::Name(name) => Ok(format!("name: {}", name)), + PoolConfig::Full(full) => match (full.name.as_deref(), full.vm_image.as_deref()) { + (Some(name), Some(vm_image)) => anyhow::bail!( + "pool cannot specify both `name` and `vmImage` (got name='{}', vmImage='{}')", + name, + vm_image + ), + (Some(name), None) => Ok(format!("name: {}", name)), + (None, Some(vm_image)) => Ok(format!("vmImage: {}", vm_image)), + (None, None) => Ok(format!("vmImage: {}", DEFAULT_VM_IMAGE_POOL)), + }, + } + } } } @@ -1735,7 +1755,7 @@ pub fn validate_resolve_pr_thread_statuses(front_matter: &FrontMatter) -> Result /// last, conditioned on the gate if filters are active. pub fn generate_setup_job( setup_steps: &[serde_yaml::Value], - pool_block: &str, + pool: &str, pr_filters: Option<&super::types::PrFilters>, pipeline_filters: Option<&super::types::PipelineFilters>, extensions: &[super::extensions::Extension], @@ -1799,7 +1819,7 @@ pub fn generate_setup_job( r#"- job: Setup displayName: "Setup" pool: - {pool_block} + {pool} steps: - checkout: self "# @@ -1821,7 +1841,7 @@ pub fn generate_setup_job( /// Generate the teardown job YAML pub fn generate_teardown_job( teardown_steps: &[serde_yaml::Value], - pool_block: &str, + pool: &str, ) -> String { if teardown_steps.is_empty() { return String::new(); @@ -1839,7 +1859,7 @@ pub fn generate_teardown_job( - checkout: self {} "#, - pool_block, steps_yaml + pool, steps_yaml ) } @@ -2637,13 +2657,7 @@ pub async fn compile_shared( let pipeline_path = generate_pipeline_path(output_path); // 7. Pool settings - let (pool, pool_block) = match front_matter.target { - CompileTarget::OneES => { - let onees_pool = resolve_onees_pool_name(front_matter.pool.as_ref())?; - (onees_pool.clone(), format!("name: {}", onees_pool)) - } - _ => (String::new(), resolve_non_onees_pool_block(front_matter.pool.as_ref())?), - }; + let pool = resolve_pool_block(front_matter.target.clone(), front_matter.pool.as_ref())?; // 8. Setup/teardown jobs, parameters, prepare/finalize steps let pr_filters = front_matter.pr_filters(); @@ -2657,8 +2671,8 @@ pub async fn compile_shared( let has_pipeline_filters = pipeline_filters .map(|f| !super::filter_ir::lower_pipeline_filters(f).is_empty()) .unwrap_or(false); - let setup_job = generate_setup_job(&front_matter.setup, &pool_block, pr_filters, pipeline_filters, extensions, ctx)?; - let teardown_job = generate_teardown_job(&front_matter.teardown, &pool_block); + let setup_job = generate_setup_job(&front_matter.setup, &pool, pr_filters, pipeline_filters, extensions, ctx)?; + let teardown_job = generate_teardown_job(&front_matter.teardown, &pool); let has_memory = front_matter .tools .as_ref() @@ -2791,7 +2805,6 @@ pub async fn compile_shared( ("{{ compiler_version }}", compiler_version), ("{{ engine_install_steps }}", &engine_install_steps), ("{{ pool }}", &pool), - ("{{ pool_block }}", &pool_block), ("{{ setup_job }}", &setup_job), ("{{ teardown_job }}", &teardown_job), ("{{ prepare_steps }}", &prepare_steps), @@ -6200,26 +6213,40 @@ mod tests { } #[test] - fn test_resolve_non_onees_pool_block_defaults_to_vm_image() { - let block = resolve_non_onees_pool_block(None).expect("pool block"); + fn test_resolve_pool_block_non_onees_defaults_to_vm_image() { + let block = resolve_pool_block(CompileTarget::Standalone, None).expect("pool block"); assert_eq!(block, "vmImage: ubuntu-latest"); } #[test] - fn test_resolve_non_onees_pool_block_from_name() { + fn test_resolve_pool_block_non_onees_from_name() { let pool = PoolConfig::Name("SelfHostedPool".to_string()); - let block = resolve_non_onees_pool_block(Some(&pool)).expect("pool block"); + let block = resolve_pool_block(CompileTarget::Standalone, Some(&pool)).expect("pool block"); assert_eq!(block, "name: SelfHostedPool"); } #[test] - fn test_resolve_non_onees_pool_block_from_vm_image() { + fn test_resolve_pool_block_non_onees_from_vm_image() { let pool_yaml = "name: x\ndescription: x\npool:\n vmImage: windows-latest"; let fm: FrontMatter = serde_yaml::from_str(pool_yaml).expect("front matter"); - let block = resolve_non_onees_pool_block(fm.pool.as_ref()).expect("pool block"); + let block = resolve_pool_block(CompileTarget::Standalone, fm.pool.as_ref()).expect("pool block"); assert_eq!(block, "vmImage: windows-latest"); } + #[test] + fn test_resolve_pool_block_onees_default_includes_name_and_os() { + let block = resolve_pool_block(CompileTarget::OneES, None).expect("pool block"); + assert_eq!(block, "name: AZS-1ES-L-MMS-ubuntu-22.04\nos: linux"); + } + + #[test] + fn test_resolve_pool_block_onees_honors_os_from_object() { + let yaml = "name: x\ndescription: x\ntarget: 1es\npool:\n name: CustomPool\n os: windows"; + let fm: FrontMatter = serde_yaml::from_str(yaml).expect("front matter"); + let block = resolve_pool_block(CompileTarget::OneES, fm.pool.as_ref()).expect("pool block"); + assert_eq!(block, "name: CustomPool\nos: windows"); + } + #[test] fn test_generate_agentic_depends_on_empty_steps() { assert!(generate_agentic_depends_on(&[], false, false, &[]).is_empty()); diff --git a/src/data/1es-base.yml b/src/data/1es-base.yml index d482a8fe..92a9f94e 100644 --- a/src/data/1es-base.yml +++ b/src/data/1es-base.yml @@ -24,8 +24,7 @@ extends: template: v1/1ES.Unofficial.PipelineTemplate.yml@1ESPipelineTemplates parameters: pool: - name: {{ pool }} - os: linux + {{ pool }} sdl: sourceAnalysisPool: name: AZS-1ES-W-MMS2022 diff --git a/src/data/base.yml b/src/data/base.yml index f5829957..4007d9c3 100644 --- a/src/data/base.yml +++ b/src/data/base.yml @@ -20,7 +20,7 @@ jobs: {{ agentic_depends_on }} {{ job_timeout }} pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -396,7 +396,7 @@ jobs: displayName: "Detection" dependsOn: Agent pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -598,7 +598,7 @@ jobs: - Detection condition: and(succeeded(), eq(dependencies.Detection.outputs['threatAnalysis.SafeToProcess'], 'true')) pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} diff --git a/src/data/job-base.yml b/src/data/job-base.yml index 27f5bfdf..5c7eb82f 100644 --- a/src/data/job-base.yml +++ b/src/data/job-base.yml @@ -7,7 +7,7 @@ jobs: {{ agentic_depends_on }} {{ job_timeout }} pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -383,7 +383,7 @@ jobs: displayName: "Detection" dependsOn: {{ stage_prefix }}_Agent pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -585,7 +585,7 @@ jobs: - {{ stage_prefix }}_Detection condition: and(succeeded(), eq(dependencies.{{ stage_prefix }}_Detection.outputs['threatAnalysis.SafeToProcess'], 'true')) pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} diff --git a/src/data/stage-base.yml b/src/data/stage-base.yml index 9daa6934..26ab1e39 100644 --- a/src/data/stage-base.yml +++ b/src/data/stage-base.yml @@ -11,7 +11,7 @@ stages: {{ agentic_depends_on }} {{ job_timeout }} pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -387,7 +387,7 @@ stages: displayName: "Detection" dependsOn: {{ stage_prefix }}_Agent pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} @@ -589,7 +589,7 @@ stages: - {{ stage_prefix }}_Detection condition: and(succeeded(), eq(dependencies.{{ stage_prefix }}_Detection.outputs['threatAnalysis.SafeToProcess'], 'true')) pool: - {{ pool_block }} + {{ pool }} steps: {{ checkout_self }} {{ checkout_repositories }} diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs index 2a3ca950..a637a07f 100644 --- a/tests/compiler_tests.rs +++ b/tests/compiler_tests.rs @@ -96,10 +96,10 @@ fn assert_required_markers(content: &str) { /// and that no hardcoded pool name leaks into the template. fn assert_pool_config(content: &str) { // Must appear once per job: Agent, Detection, Execution. - let pool_marker_count = content.matches("{{ pool_block }}").count(); + let pool_marker_count = content.matches("{{ pool }}").count(); assert_eq!( pool_marker_count, 3, - "Template should use '{{ pool_block }}' marker exactly three times (once for each job)" + "Template should use '{{ pool }}' marker exactly three times (once for each job)" ); assert!( !content.contains("name: AZS-1ES-L-MMS-ubuntu-22.04"), From a4e68da50a0a54c2b46d687cefb6678a6dfe21c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 20:18:03 +0000 Subject: [PATCH 4/6] fix(codemods): translate legacy default pool for non-1es Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/43e0b1b8-982d-4e55-915d-95a8474898ab Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- src/compile/codemods/0002_pool_object_form.rs | 51 +++++++++++++++++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/src/compile/codemods/0002_pool_object_form.rs b/src/compile/codemods/0002_pool_object_form.rs index 93521541..e33c5b55 100644 --- a/src/compile/codemods/0002_pool_object_form.rs +++ b/src/compile/codemods/0002_pool_object_form.rs @@ -1,4 +1,4 @@ -//! `pool: ` → `pool: { name: }` +//! `pool: ` → explicit object form //! //! Non-1ES targets now support both self-hosted (`name`) and //! Microsoft-hosted (`vmImage`) pool syntax. This codemod rewrites the @@ -12,11 +12,20 @@ use super::{Codemod, CodemodContext}; pub static CODEMOD: Codemod = Codemod { id: "pool_object_form", - summary: "pool: -> pool: { name: }", + summary: "pool: -> pool object form (name/vmImage)", introduced_in: "0.30.0", apply: apply_codemod, }; +const LEGACY_DEFAULT_POOL_NAME: &str = "AZS-1ES-L-MMS-ubuntu-22.04"; +const NON_ONEES_DEFAULT_VM_IMAGE: &str = "ubuntu-latest"; + +fn is_1es_target(fm: &Mapping) -> bool { + fm.get(Value::String("target".to_string())) + .and_then(Value::as_str) + .is_some_and(|v| v.eq_ignore_ascii_case("1es")) +} + fn apply_codemod(fm: &mut Mapping, _ctx: &CodemodContext) -> Result { let key = Value::String("pool".to_string()); let Some(pool_value) = fm.get(&key).cloned() else { @@ -29,7 +38,16 @@ fn apply_codemod(fm: &mut Mapping, _ctx: &CodemodContext) -> Result { }; let mut mapped = Mapping::new(); - mapped.insert(Value::String("name".to_string()), Value::String(name)); + // Translate the historical default for non-1ES sources to the new + // non-1ES default shape. + if name == LEGACY_DEFAULT_POOL_NAME && !is_1es_target(fm) { + mapped.insert( + Value::String("vmImage".to_string()), + Value::String(NON_ONEES_DEFAULT_VM_IMAGE.to_string()), + ); + } else { + mapped.insert(Value::String("name".to_string()), Value::String(name)); + } fm.insert(key, Value::Mapping(mapped)); Ok(true) } @@ -68,4 +86,31 @@ mod tests { let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); assert!(!changed); } + + #[test] + fn translates_legacy_default_pool_for_non_onees() { + let mut fm: Mapping = + serde_yaml::from_str("name: x\ndescription: y\npool: AZS-1ES-L-MMS-ubuntu-22.04") + .unwrap(); + let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + assert!(changed); + assert_eq!( + fm.get(Value::String("pool".into())).cloned(), + Some(serde_yaml::from_str::("vmImage: ubuntu-latest").unwrap()) + ); + } + + #[test] + fn keeps_legacy_default_pool_name_for_onees() { + let mut fm: Mapping = serde_yaml::from_str( + "name: x\ndescription: y\ntarget: 1es\npool: AZS-1ES-L-MMS-ubuntu-22.04", + ) + .unwrap(); + let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + assert!(changed); + assert_eq!( + fm.get(Value::String("pool".into())).cloned(), + Some(serde_yaml::from_str::("name: AZS-1ES-L-MMS-ubuntu-22.04").unwrap()) + ); + } } From 2dda1abc82bdea3786c7c78114fcac9623599b84 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 20:25:36 +0000 Subject: [PATCH 5/6] fix(codemods): keep scalar pool migration as name mapping Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/ba1c7870-b00a-4daa-9761-7f9f369222a6 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- src/compile/codemods/0002_pool_object_form.rs | 36 ++----------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/src/compile/codemods/0002_pool_object_form.rs b/src/compile/codemods/0002_pool_object_form.rs index e33c5b55..d4ffa49f 100644 --- a/src/compile/codemods/0002_pool_object_form.rs +++ b/src/compile/codemods/0002_pool_object_form.rs @@ -17,15 +17,6 @@ pub static CODEMOD: Codemod = Codemod { apply: apply_codemod, }; -const LEGACY_DEFAULT_POOL_NAME: &str = "AZS-1ES-L-MMS-ubuntu-22.04"; -const NON_ONEES_DEFAULT_VM_IMAGE: &str = "ubuntu-latest"; - -fn is_1es_target(fm: &Mapping) -> bool { - fm.get(Value::String("target".to_string())) - .and_then(Value::as_str) - .is_some_and(|v| v.eq_ignore_ascii_case("1es")) -} - fn apply_codemod(fm: &mut Mapping, _ctx: &CodemodContext) -> Result { let key = Value::String("pool".to_string()); let Some(pool_value) = fm.get(&key).cloned() else { @@ -38,16 +29,7 @@ fn apply_codemod(fm: &mut Mapping, _ctx: &CodemodContext) -> Result { }; let mut mapped = Mapping::new(); - // Translate the historical default for non-1ES sources to the new - // non-1ES default shape. - if name == LEGACY_DEFAULT_POOL_NAME && !is_1es_target(fm) { - mapped.insert( - Value::String("vmImage".to_string()), - Value::String(NON_ONEES_DEFAULT_VM_IMAGE.to_string()), - ); - } else { - mapped.insert(Value::String("name".to_string()), Value::String(name)); - } + mapped.insert(Value::String("name".to_string()), Value::String(name)); fm.insert(key, Value::Mapping(mapped)); Ok(true) } @@ -88,26 +70,12 @@ mod tests { } #[test] - fn translates_legacy_default_pool_for_non_onees() { + fn rewrites_legacy_default_pool_to_name_object() { let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y\npool: AZS-1ES-L-MMS-ubuntu-22.04") .unwrap(); let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); assert!(changed); - assert_eq!( - fm.get(Value::String("pool".into())).cloned(), - Some(serde_yaml::from_str::("vmImage: ubuntu-latest").unwrap()) - ); - } - - #[test] - fn keeps_legacy_default_pool_name_for_onees() { - let mut fm: Mapping = serde_yaml::from_str( - "name: x\ndescription: y\ntarget: 1es\npool: AZS-1ES-L-MMS-ubuntu-22.04", - ) - .unwrap(); - let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); - assert!(changed); assert_eq!( fm.get(Value::String("pool".into())).cloned(), Some(serde_yaml::from_str::("name: AZS-1ES-L-MMS-ubuntu-22.04").unwrap()) From 8abb9b4a77be0fbaf367ed43b768647c0631af20 Mon Sep 17 00:00:00 2001 From: James Devine Date: Thu, 14 May 2026 21:47:42 +0100 Subject: [PATCH 6/6] fix(compile): version-gate legacy pool default injection in pool_object_form codemod When pool is absent from front matter and the binary version is at or above 0.30.0 (the release that changed the implicit default from the 1ES self-hosted pool to vmImage: ubuntu-latest), the codemod now injects pool: { name: AZS-1ES-L-MMS-ubuntu-22.04 } to prevent silent breakage of existing pipelines. - Add compiler_version field to CodemodContext - Gate absent-pool injection on version >= introduced_in - Add version_gte helper for semver comparison - Add tests for version-gated and version-ungated behaviour Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/compile/codemods/0001_repos_unified.rs | 10 +- src/compile/codemods/0002_pool_object_form.rs | 106 ++++++++++++++++-- src/compile/codemods/mod.rs | 29 +++-- 3 files changed, 125 insertions(+), 20 deletions(-) diff --git a/src/compile/codemods/0001_repos_unified.rs b/src/compile/codemods/0001_repos_unified.rs index 88511c32..b87e523b 100644 --- a/src/compile/codemods/0001_repos_unified.rs +++ b/src/compile/codemods/0001_repos_unified.rs @@ -242,14 +242,14 @@ mod tests { fn run(input: &str) -> Mapping { let mut m: Mapping = serde_yaml::from_str(input).unwrap(); - let changed = apply_codemod(&mut m, &CodemodContext {}).expect("apply"); + let changed = apply_codemod(&mut m, &CodemodContext::current()).expect("apply"); assert!(changed, "expected codemod to fire on input:\n{}", input); m } fn run_noop(input: &str) -> Mapping { let mut m: Mapping = serde_yaml::from_str(input).unwrap(); - let changed = apply_codemod(&mut m, &CodemodContext {}).expect("apply"); + let changed = apply_codemod(&mut m, &CodemodContext::current()).expect("apply"); assert!(!changed, "expected codemod to be a no-op on input:\n{}", input); m } @@ -258,7 +258,7 @@ mod tests { let mut m: Mapping = serde_yaml::from_str(input).unwrap(); format!( "{}", - apply_codemod(&mut m, &CodemodContext {}).unwrap_err() + apply_codemod(&mut m, &CodemodContext::current()).unwrap_err() ) } @@ -425,10 +425,10 @@ mod tests { checkout: [a]\n", ) .unwrap(); - let first = apply_codemod(&mut m, &CodemodContext {}).expect("first"); + let first = apply_codemod(&mut m, &CodemodContext::current()).expect("first"); assert!(first, "first run should fire"); let snapshot = m.clone(); - let second = apply_codemod(&mut m, &CodemodContext {}).expect("second"); + let second = apply_codemod(&mut m, &CodemodContext::current()).expect("second"); assert!(!second, "second run should be a no-op"); assert_eq!(m, snapshot, "second run must not mutate"); } diff --git a/src/compile/codemods/0002_pool_object_form.rs b/src/compile/codemods/0002_pool_object_form.rs index d4ffa49f..b78a1caa 100644 --- a/src/compile/codemods/0002_pool_object_form.rs +++ b/src/compile/codemods/0002_pool_object_form.rs @@ -4,23 +4,61 @@ //! Microsoft-hosted (`vmImage`) pool syntax. This codemod rewrites the //! legacy scalar shorthand into an explicit object form so sources are //! unambiguous and easier to evolve. +//! +//! When the pool field is **absent** and the compiler version is at or +//! above `INTRODUCED_IN` (the release that changed the implicit +//! default from the 1ES self-hosted pool to `vmImage: ubuntu-latest`), +//! the codemod pins the legacy default explicitly so existing +//! pipelines are not silently broken. use anyhow::Result; use serde_yaml::{Mapping, Value}; use super::{Codemod, CodemodContext}; +use crate::compile::common::DEFAULT_ONEES_POOL; + +/// Version where the pool default changed from the legacy self-hosted +/// pool to `vmImage: ubuntu-latest`. +const INTRODUCED_IN: &str = "0.30.0"; pub static CODEMOD: Codemod = Codemod { id: "pool_object_form", summary: "pool: -> pool object form (name/vmImage)", - introduced_in: "0.30.0", + introduced_in: INTRODUCED_IN, apply: apply_codemod, }; -fn apply_codemod(fm: &mut Mapping, _ctx: &CodemodContext) -> Result { +/// Simple major.minor.patch comparison. Returns true when `version` +/// is greater than or equal to `threshold`. +fn version_gte(version: &str, threshold: &str) -> bool { + let parse = |s: &str| -> (u32, u32, u32) { + let mut parts = s.split('.'); + let major = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0); + let minor = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0); + let patch = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0); + (major, minor, patch) + }; + parse(version) >= parse(threshold) +} + +fn apply_codemod(fm: &mut Mapping, ctx: &CodemodContext) -> Result { let key = Value::String("pool".to_string()); + let Some(pool_value) = fm.get(&key).cloned() else { - return Ok(false); + // Pool absent — only inject the legacy default when the + // compiler version is at or above the release that changed + // the implicit default. Older binaries still carry the old + // default in `resolve_pool_block`, so no rewrite is needed. + if !version_gte(ctx.compiler_version, INTRODUCED_IN) { + return Ok(false); + } + let mut mapped = Mapping::new(); + mapped.insert( + Value::String("name".to_string()), + Value::String(DEFAULT_ONEES_POOL.to_string()), + ); + fm.insert(key, Value::Mapping(mapped)); + return Ok(true); }; let Value::String(name) = pool_value else { @@ -38,10 +76,17 @@ fn apply_codemod(fm: &mut Mapping, _ctx: &CodemodContext) -> Result { mod tests { use super::*; + /// Build a context with an explicit version for testing. + fn ctx(version: &'static str) -> CodemodContext { + CodemodContext { + compiler_version: version, + } + } + #[test] fn rewrites_scalar_pool_to_name_object() { let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y\npool: MyPool").unwrap(); - let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + let changed = apply_codemod(&mut fm, &ctx("0.30.0")).expect("apply"); assert!(changed); assert_eq!( fm.get(Value::String("pool".into())).cloned(), @@ -54,7 +99,7 @@ mod tests { let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y\npool:\n vmImage: ubuntu-latest") .unwrap(); - let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + let changed = apply_codemod(&mut fm, &ctx("0.30.0")).expect("apply"); assert!(!changed); assert_eq!( fm.get(Value::String("pool".into())).cloned(), @@ -63,10 +108,31 @@ mod tests { } #[test] - fn noops_when_pool_absent() { + fn inserts_legacy_default_when_pool_absent_and_version_gte() { let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y").unwrap(); - let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + let changed = apply_codemod(&mut fm, &ctx("0.30.0")).expect("apply"); + assert!(changed); + assert_eq!( + fm.get(Value::String("pool".into())).cloned(), + Some(serde_yaml::from_str::("name: AZS-1ES-L-MMS-ubuntu-22.04").unwrap()) + ); + } + + #[test] + fn noops_when_pool_absent_and_version_below() { + let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y").unwrap(); + let changed = apply_codemod(&mut fm, &ctx("0.29.0")).expect("apply"); assert!(!changed); + assert!(!fm.contains_key(Value::String("pool".into()))); + } + + #[test] + fn idempotent_after_inserting_legacy_default() { + let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y").unwrap(); + let changed1 = apply_codemod(&mut fm, &ctx("0.30.0")).expect("first apply"); + assert!(changed1); + let changed2 = apply_codemod(&mut fm, &ctx("0.30.0")).expect("second apply"); + assert!(!changed2, "second run must be a no-op"); } #[test] @@ -74,11 +140,35 @@ mod tests { let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y\npool: AZS-1ES-L-MMS-ubuntu-22.04") .unwrap(); - let changed = apply_codemod(&mut fm, &CodemodContext {}).expect("apply"); + let changed = apply_codemod(&mut fm, &ctx("0.30.0")).expect("apply"); assert!(changed); assert_eq!( fm.get(Value::String("pool".into())).cloned(), Some(serde_yaml::from_str::("name: AZS-1ES-L-MMS-ubuntu-22.04").unwrap()) ); } + + #[test] + fn scalar_rewrite_applies_regardless_of_version() { + // Scalar → object rewrite is unconditional; only the + // absent-pool injection is version-gated. + let mut fm: Mapping = + serde_yaml::from_str("name: x\ndescription: y\npool: MyPool").unwrap(); + let changed = apply_codemod(&mut fm, &ctx("0.28.0")).expect("apply"); + assert!(changed); + assert_eq!( + fm.get(Value::String("pool".into())).cloned(), + Some(serde_yaml::from_str::("name: MyPool").unwrap()) + ); + } + + #[test] + fn version_gte_comparisons() { + assert!(version_gte("0.30.0", "0.30.0")); + assert!(version_gte("0.31.0", "0.30.0")); + assert!(version_gte("1.0.0", "0.30.0")); + assert!(version_gte("0.30.1", "0.30.0")); + assert!(!version_gte("0.29.0", "0.30.0")); + assert!(!version_gte("0.29.99", "0.30.0")); + } } diff --git a/src/compile/codemods/mod.rs b/src/compile/codemods/mod.rs index 9dc987ca..f3f22ac3 100644 --- a/src/compile/codemods/mod.rs +++ b/src/compile/codemods/mod.rs @@ -41,12 +41,26 @@ mod m0002_pool_object_form; #[allow(unused_imports)] // Re-exported for future codemods; only `take_key` is in-tree use. pub use helpers::{insert_no_overwrite, rename_key, take_key, ConflictPolicy}; -/// Forward-compatible context passed to every codemod. Currently -/// empty; we keep it in the signature so future codemods can be -/// given (e.g.) the source path without breaking the function -/// pointer type. +/// Forward-compatible context passed to every codemod. +/// +/// Carries ambient information (e.g. compiler version) so codemods +/// can condition their behaviour without hard-coding assumptions. #[non_exhaustive] -pub struct CodemodContext {} +pub struct CodemodContext { + /// Semantic version of the running `ado-aw` binary + /// (e.g. `"0.30.0"`). Codemods can compare this against their + /// `introduced_in` to decide when a default has changed. + pub compiler_version: &'static str, +} + +impl CodemodContext { + /// Build a context using the compile-time package version. + pub fn current() -> Self { + Self { + compiler_version: env!("CARGO_PKG_VERSION"), + } + } +} /// A single front-matter codemod. /// @@ -141,10 +155,11 @@ pub(crate) fn apply_codemods_with( fm: &mut Mapping, registry: &[&'static Codemod], ) -> Result { + let ctx = CodemodContext::current(); let mut applied: Vec = Vec::new(); for c in registry { - let changed = (c.apply)(fm, &CodemodContext {}) - .with_context(|| format!("codemod {} failed", c.id))?; + let changed = + (c.apply)(fm, &ctx).with_context(|| format!("codemod {} failed", c.id))?; if changed { applied.push(AppliedCodemod { id: c.id,