From a0abedd165af671e066fcd5aec00dbc00bf937f4 Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 19:04:04 +0000 Subject: [PATCH 01/14] Saving AI comments. --- .github/workflows/ado/templates/pr-package-build-stages.yml | 5 +++++ scripts/ci/components/compute_change_set.sh | 1 + 2 files changed, 6 insertions(+) diff --git a/.github/workflows/ado/templates/pr-package-build-stages.yml b/.github/workflows/ado/templates/pr-package-build-stages.yml index 1cc26e256fd..3ba2aa5a080 100644 --- a/.github/workflows/ado/templates/pr-package-build-stages.yml +++ b/.github/workflows/ado/templates/pr-package-build-stages.yml @@ -164,6 +164,7 @@ stages: # pipeline variables so the wiring stays visible in the YAML. - script: | set -euo pipefail + # AI: move build reason check to a template expression with 'variables['Build.Reason']' to fail fast. if ! git rev-parse --verify -q "HEAD^2" >/dev/null; then echo "##[error]HEAD is not a merge commit -- this pipeline must run as a PR build (Build.Reason=PullRequest)." exit 1 @@ -183,6 +184,7 @@ stages: done echo "Resolved range: base(merge-base)=$target_commit source=$source_commit" echo "##vso[task.setvariable variable=sourceCommit;isreadonly=true]$source_commit" + # AI: re-name to 'baseCommit' and 'base_commit', since this is not the target tip but the merge base (fork point). echo "##vso[task.setvariable variable=targetCommit;isreadonly=true]$target_commit" displayName: "Determine PR commit range" @@ -195,8 +197,11 @@ stages: # on PRs. The script self-prefixes AZLDEV_ALLOW_ROOT=1 internally. - script: | set -euo pipefail + # AI: use job variables for 'changedComponentsFile' and derive 'change_set_dir' from it, rather than hardcoding the staging path here. change_set_dir="$(Build.ArtifactStagingDirectory)/change-set" echo "##[group]Preparing change set" + # AI: extend the script below to allow optionally to set the names of the output files (changed-components.json, specs-diff.txt, render-set.txt) via parameters, rather than hardcoding them in the script. Then use the new parameters here to set the output file names, so the wrapper can control them if needed. + # AI: re-name the --X-commit parameters to --from-commit and --to-commit for clarity, since the script is computing the diff from the merge base (fork point) to the PR head. Once done, make sure the ordering of the passed commits is correct - seems wrong in case of this YAML. scripts/ci/components/compute_change_set.sh \ --output-dir "$change_set_dir" \ --source-commit "$SOURCE_COMMIT" \ diff --git a/scripts/ci/components/compute_change_set.sh b/scripts/ci/components/compute_change_set.sh index 32f11bf54c3..cb21bcbfbb2 100755 --- a/scripts/ci/components/compute_change_set.sh +++ b/scripts/ci/components/compute_change_set.sh @@ -64,6 +64,7 @@ changed_file="$output_dir/changed-components.json" specs_diff_file="$output_dir/specs-diff.txt" render_set_file="$output_dir/render-set.txt" +# @AI: replace below wrapper script with function, unless you find it used in multiple places. If replaced, check for necessary doc updates. "$script_dir/compute_changed.sh" \ --output-file "$changed_file" \ --source-commit "$source_commit" \ From 4c8dc5a308c7130af28d09f8617931a11da34044 Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 13:33:41 -0700 Subject: [PATCH 02/14] ci: rename compute_change_set.sh commit args to --from/--to Rename the confusingly-named --source-commit/--target-commit flags to --from-commit (baseline / merge base) and --to-commit (newer commit), and add optional --changed-components-file / --specs-diff-file / --render-set-file output-name overrides (defaults unchanged). Update all three callers: pr-package-build-stages.yml, common-steps.yml, and the check-rendered-specs GitHub Actions gate. Ordering preserved: from -> to (base -> head). Resolves the two AI: comments on the compute_change_set.sh call site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ado/templates/pr-package-build-stages.yml | 10 +++--- .../ado/templates/steps/common-steps.yml | 5 +-- .github/workflows/check-rendered-specs.yml | 4 +-- scripts/ci/components/compute_change_set.sh | 35 +++++++++++++------ 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ado/templates/pr-package-build-stages.yml b/.github/workflows/ado/templates/pr-package-build-stages.yml index 3ba2aa5a080..98d5d3340b9 100644 --- a/.github/workflows/ado/templates/pr-package-build-stages.yml +++ b/.github/workflows/ado/templates/pr-package-build-stages.yml @@ -173,7 +173,7 @@ stages: # Baseline = the MERGE BASE (fork point) of the target-branch tip # (HEAD^1) and the PR head (HEAD^2), NOT the target tip itself. # See the block comment above for why; this is passed as the - # `--target-commit` (from) baseline to the change-set helper. + # `--from-commit` baseline to the change-set helper. target_commit="$(git merge-base HEAD^1 HEAD^2)" # PR-supplied data is untrusted: validate both SHAs before use. for sha in "$target_commit" "$source_commit"; do @@ -200,12 +200,12 @@ stages: # AI: use job variables for 'changedComponentsFile' and derive 'change_set_dir' from it, rather than hardcoding the staging path here. change_set_dir="$(Build.ArtifactStagingDirectory)/change-set" echo "##[group]Preparing change set" - # AI: extend the script below to allow optionally to set the names of the output files (changed-components.json, specs-diff.txt, render-set.txt) via parameters, rather than hardcoding them in the script. Then use the new parameters here to set the output file names, so the wrapper can control them if needed. - # AI: re-name the --X-commit parameters to --from-commit and --to-commit for clarity, since the script is computing the diff from the merge base (fork point) to the PR head. Once done, make sure the ordering of the passed commits is correct - seems wrong in case of this YAML. + # from = merge base (fork point), to = PR head; the helper diffs + # from -> to. See the "Determine PR commit range" step above. scripts/ci/components/compute_change_set.sh \ --output-dir "$change_set_dir" \ - --source-commit "$SOURCE_COMMIT" \ - --target-commit "$TARGET_COMMIT" + --from-commit "$TARGET_COMMIT" \ + --to-commit "$SOURCE_COMMIT" echo "##[endgroup]" echo "##vso[task.setvariable variable=changedComponentsFile;isreadonly=true]$change_set_dir/changed-components.json" env: diff --git a/.github/workflows/ado/templates/steps/common-steps.yml b/.github/workflows/ado/templates/steps/common-steps.yml index 03fbc386975..ac5dde3d6fc 100644 --- a/.github/workflows/ado/templates/steps/common-steps.yml +++ b/.github/workflows/ado/templates/steps/common-steps.yml @@ -181,10 +181,11 @@ steps: trap publish_artifact EXIT echo "##[group]Preparing change set" + # from = previous build's commit (baseline), to = this build's commit. scripts/ci/components/compute_change_set.sh \ --output-dir "$change_set_dir" \ - --source-commit "$SOURCE_COMMIT" \ - --target-commit "$TARGET_COMMIT" + --from-commit "$TARGET_COMMIT" \ + --to-commit "$SOURCE_COMMIT" echo "##[endgroup]" echo "##[group]Upload set (sourcesChange == true, changeType in {added, changed})" diff --git a/.github/workflows/check-rendered-specs.yml b/.github/workflows/check-rendered-specs.yml index 71184a8474a..1afb6dc0c5d 100644 --- a/.github/workflows/check-rendered-specs.yml +++ b/.github/workflows/check-rendered-specs.yml @@ -437,8 +437,8 @@ jobs: # current head. /scripts-components/compute_change_set.sh \ --output-dir /output/change-set \ - --source-commit "$PR_HEAD_SHA" \ - --target-commit "$PR_BASE_SHA" + --from-commit "$PR_BASE_SHA" \ + --to-commit "$PR_HEAD_SHA" # Components removed in this PR no longer have a comp.toml, so # `azldev component render` will not touch (or clean) their old diff --git a/scripts/ci/components/compute_change_set.sh b/scripts/ci/components/compute_change_set.sh index cb21bcbfbb2..7a656f8daf5 100755 --- a/scripts/ci/components/compute_change_set.sh +++ b/scripts/ci/components/compute_change_set.sh @@ -24,19 +24,32 @@ set -euo pipefail cd "$(git rev-parse --show-toplevel)" usage() { - echo "Usage: $0 --output-dir DIR --source-commit SHA --target-commit SHA" >&2 + echo "Usage: $0 --output-dir DIR --from-commit SHA --to-commit SHA \\" >&2 + echo " [--changed-components-file NAME] [--specs-diff-file NAME] [--render-set-file NAME]" >&2 + echo " --from-commit baseline commit (merge-base / fork point)" >&2 + echo " --to-commit newer commit (e.g. PR head)" >&2 exit 1 } +# Output file names default to the canonical set but can be overridden by a +# caller that needs to control them (e.g. to stage several change sets side by +# side). Only the names are configurable; they always land under --output-dir. +changed_components_file_name="changed-components.json" +specs_diff_file_name="specs-diff.txt" +render_set_file_name="render-set.txt" + while [[ $# -gt 0 ]]; do case "$1" in - --output-dir) output_dir="$2"; shift 2 ;; - --source-commit) source_commit="$2"; shift 2 ;; - --target-commit) target_commit="$2"; shift 2 ;; + --output-dir) output_dir="$2"; shift 2 ;; + --from-commit) from_commit="$2"; shift 2 ;; + --to-commit) to_commit="$2"; shift 2 ;; + --changed-components-file) changed_components_file_name="$2"; shift 2 ;; + --specs-diff-file) specs_diff_file_name="$2"; shift 2 ;; + --render-set-file) render_set_file_name="$2"; shift 2 ;; *) usage ;; esac done -[[ -z "${output_dir:-}" || -z "${source_commit:-}" || -z "${target_commit:-}" ]] && usage +[[ -z "${output_dir:-}" || -z "${from_commit:-}" || -z "${to_commit:-}" ]] && usage # Defensive guard: the script owns --output-dir exclusively for the duration # of the invocation (it does `rm -rf` below to clean up stale state). Refuse @@ -60,15 +73,15 @@ mkdir -p "$output_dir" script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -changed_file="$output_dir/changed-components.json" -specs_diff_file="$output_dir/specs-diff.txt" -render_set_file="$output_dir/render-set.txt" +changed_file="$output_dir/$changed_components_file_name" +specs_diff_file="$output_dir/$specs_diff_file_name" +render_set_file="$output_dir/$render_set_file_name" # @AI: replace below wrapper script with function, unless you find it used in multiple places. If replaced, check for necessary doc updates. "$script_dir/compute_changed.sh" \ --output-file "$changed_file" \ - --source-commit "$source_commit" \ - --target-commit "$target_commit" + --source-commit "$to_commit" \ + --target-commit "$from_commit" # azldev's renderedSpecsDir is absolute. Translate to repo-relative so it # matches git's output (`git diff --name-only` always emits repo-relative @@ -81,7 +94,7 @@ specs_dir="$(realpath --relative-to="$(pwd)" "$specs_dir_abs")" # `azldev component changed` misses). --no-renames prevents collapse of # delete+add into a rename entry, which would lose the old path; the # Python script filters out deleted/unknown components. -git diff --no-renames --name-only "$target_commit" "$source_commit" \ +git diff --no-renames --name-only "$from_commit" "$to_commit" \ -- "$specs_dir" > "$specs_diff_file" python3 "$script_dir/compute_render_set.py" \ From 7f4d36ef52df2cfeb6c3a331ad954860325acf6d Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 13:36:01 -0700 Subject: [PATCH 03/14] ci: inline compute_changed.sh into compute_change_set.sh compute_changed.sh had a single caller (compute_change_set.sh), so fold it in as a compute_changed() function and delete the standalone script. Behavior is unchanged: azldev component changed --from --to with the same supply-chain drift guard. Update the components README table and the common-steps.yml comment that referenced the removed script. Resolves the @AI: comment on the compute_changed.sh wrapper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ado/templates/steps/common-steps.yml | 2 +- scripts/ci/components/README.md | 3 +- scripts/ci/components/compute_change_set.sh | 22 ++++++++--- scripts/ci/components/compute_changed.sh | 38 ------------------- 4 files changed, 19 insertions(+), 46 deletions(-) delete mode 100755 scripts/ci/components/compute_changed.sh diff --git a/.github/workflows/ado/templates/steps/common-steps.yml b/.github/workflows/ado/templates/steps/common-steps.yml index ac5dde3d6fc..f772aa12064 100644 --- a/.github/workflows/ado/templates/steps/common-steps.yml +++ b/.github/workflows/ado/templates/steps/common-steps.yml @@ -160,7 +160,7 @@ steps: # attacker-supplied bytes ride into the lookaside under an # unchanged component identity, so we treat it as hostile and # fail closed (supply-chain drift tripwire). --include-unchanged - # (set inside compute_changed.sh) ensures the full component + # (set inside compute_change_set.sh) ensures the full component # list is available for downstream consumers. - script: | set -euo pipefail diff --git a/scripts/ci/components/README.md b/scripts/ci/components/README.md index 8f16d9a8ce7..5271597dc87 100644 --- a/scripts/ci/components/README.md +++ b/scripts/ci/components/README.md @@ -9,9 +9,8 @@ PR package-build check | Script | Purpose | | ------ | ------- | -| `compute_changed.sh` | Wraps `azldev component changed --from --to -O json`. | | `compute_render_set.py` | Computes the union of (azldev-flagged components) and (components with hand-edited rendered specs), then drops deleted entries. | -| `compute_change_set.sh` | Orchestrates the two above: writes `changed-components.json`, `specs-diff.txt`, and `render-set.txt` into a caller-chosen output directory. | +| `compute_change_set.sh` | Runs `azldev component changed --from --to ` inline, unions the result with `compute_render_set.py`, and writes `changed-components.json`, `specs-diff.txt`, and `render-set.txt` into a caller-chosen output directory. | ## Conventions diff --git a/scripts/ci/components/compute_change_set.sh b/scripts/ci/components/compute_change_set.sh index 7a656f8daf5..55696aa8c0b 100755 --- a/scripts/ci/components/compute_change_set.sh +++ b/scripts/ci/components/compute_change_set.sh @@ -77,11 +77,23 @@ changed_file="$output_dir/$changed_components_file_name" specs_diff_file="$output_dir/$specs_diff_file_name" render_set_file="$output_dir/$render_set_file_name" -# @AI: replace below wrapper script with function, unless you find it used in multiple places. If replaced, check for necessary doc updates. -"$script_dir/compute_changed.sh" \ - --output-file "$changed_file" \ - --source-commit "$to_commit" \ - --target-commit "$from_commit" +# Compute the set of changed components between two commits. Writes one JSON +# entry per component to $out_file with fields: component, changeType, +# sourcesChange. azldev hard-fails if any component has sourcesChange == true +# without a corresponding identity change (added/changed/deleted) -- a +# supply-chain drift guard. Inline AZLDEV_ALLOW_ROOT=1 so CI agents running as +# root work without lifting the restriction at step scope (see +# .github/instructions/ado-pipeline.instructions.md). +compute_changed() { + local out_file="$1" from="$2" to="$3" + mkdir -p "$(dirname "$out_file")" + AZLDEV_ALLOW_ROOT=1 azldev component changed --from "$from" --to "$to" \ + -a --include-unchanged -O json > "$out_file" + echo "Changed components (non-unchanged):" + jq -r '.[] | select(.changeType != "unchanged") | " \(.changeType)\t\(.component)"' "$out_file" | sort +} + +compute_changed "$changed_file" "$from_commit" "$to_commit" # azldev's renderedSpecsDir is absolute. Translate to repo-relative so it # matches git's output (`git diff --name-only` always emits repo-relative diff --git a/scripts/ci/components/compute_changed.sh b/scripts/ci/components/compute_changed.sh deleted file mode 100755 index 48f7f935f71..00000000000 --- a/scripts/ci/components/compute_changed.sh +++ /dev/null @@ -1,38 +0,0 @@ -#!/usr/bin/env bash -# Compute the set of changed components between source and target commits. -# -# Writes one JSON entry per component to , with fields: -# - component: name -# - changeType: 'added' | 'changed' | 'unchanged' | 'deleted' -# - sourcesChange: bool (rendered 'sources' file differs across commits) -# -# azldev hard-fails if any component has sourcesChange == true without a -# corresponding identity change (added/changed/deleted) -- supply-chain -# drift protection. -# -# Callers are responsible for log grouping and artifact publication. -# `azldev` is invoked with an inline `AZLDEV_ALLOW_ROOT=1` prefix so CI -# agents running as root work without callers having to lift the -# restriction at step scope (see -# .github/instructions/ado-pipeline.instructions.md). - -set -euo pipefail - -usage() { echo "Usage: $0 --output-file FILE --source-commit SHA --target-commit SHA" >&2; exit 1; } - -while [[ $# -gt 0 ]]; do - case "$1" in - --output-file) output_file="$2"; shift 2 ;; - --source-commit) source_commit="$2"; shift 2 ;; - --target-commit) target_commit="$2"; shift 2 ;; - *) usage ;; - esac -done -[[ -z "${output_file:-}" || -z "${source_commit:-}" || -z "${target_commit:-}" ]] && usage - -mkdir -p "$(dirname "$output_file")" - -AZLDEV_ALLOW_ROOT=1 azldev component changed --from "$target_commit" --to "$source_commit" -a --include-unchanged -O json > "$output_file" - -echo "Changed components (non-unchanged):" -jq -r '.[] | select(.changeType != "unchanged") | " \(.changeType)\t\(.component)"' "$output_file" | sort From ab613e6d45f51caa997df86c164b6400439558cf Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 13:43:27 -0700 Subject: [PATCH 04/14] ci: extract common-steps into granular parameterized step templates Add reusable step templates under templates/steps/ so the post-merge and PR Control Tower pipelines can compose only the pieces they need instead of duplicating setup: ensure-full-history.yml unshallow guard install-deps.yml PipAuthenticate + azldev + python deps, with installMock / installAdoRequirements / normalizeGoGitConfig toggles commit-range-postmerge.yml previous-build delta (ADO Builds API) commit-range-pr.yml merge-commit-parent range (PR) prepare-change-set.yml compute_change_set.sh + triage artifact, from/to variable names parameterized verify-locks.yml lock freshness check verify-rendered-specs.yml render --check-only drift check These are additive; nothing consumes them yet (wired in follow-up commits). Two AI: fixes land here, in commit-range-pr.yml: - build-reason check moved to a compile-time template expression on variables['Build.Reason'] so a mis-triggered run fails fast. - the merge-base is now named baseCommit / base_commit (it is the fork point, not the target tip). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../steps/commit-range-postmerge.yml | 63 +++++++++++++ .../ado/templates/steps/commit-range-pr.yml | 52 +++++++++++ .../templates/steps/ensure-full-history.yml | 20 ++++ .../ado/templates/steps/install-deps.yml | 93 +++++++++++++++++++ .../templates/steps/prepare-change-set.yml | 70 ++++++++++++++ .../ado/templates/steps/verify-locks.yml | 15 +++ .../templates/steps/verify-rendered-specs.yml | 42 +++++++++ 7 files changed, 355 insertions(+) create mode 100644 .github/workflows/ado/templates/steps/commit-range-postmerge.yml create mode 100644 .github/workflows/ado/templates/steps/commit-range-pr.yml create mode 100644 .github/workflows/ado/templates/steps/ensure-full-history.yml create mode 100644 .github/workflows/ado/templates/steps/install-deps.yml create mode 100644 .github/workflows/ado/templates/steps/prepare-change-set.yml create mode 100644 .github/workflows/ado/templates/steps/verify-locks.yml create mode 100644 .github/workflows/ado/templates/steps/verify-rendered-specs.yml diff --git a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml new file mode 100644 index 00000000000..3af2a3cc9f3 --- /dev/null +++ b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml @@ -0,0 +1,63 @@ +# Microsoft Corporation +# +# Step template: resolve the (target, source) commit range for a POST-MERGE +# delta build. +# source = the commit that triggered this run (Build.SourceVersion). +# target = the previous CI build's commit on this branch, looked up via the +# ADO Builds API. Pairing each build with the one immediately before +# it keeps concurrent runs from overlapping and captures every commit +# a rebase merge appends. Runs after the dependency-install step +# because it imports the azure-devops SDK. See +# scripts/ci/ado/determine_commit_range.py for the full rationale and +# the first-run (source^1) fallback. +# +# Emits job-scope variables sourceCommit and targetCommit. + +steps: + - script: | + set -euo pipefail + # The helper prints the resolved range to stdout as two lines: + # sourceCommit= + # targetCommit= + # (all diagnostics go to stderr). We parse them and set the + # pipeline variables HERE so the variable wiring is visible in + # the YAML rather than hidden in the script. + range_output="$(python3 scripts/ci/ado/determine_commit_range.py \ + --definition-id "$SYSTEM_DEFINITIONID" \ + --current-build-id "$BUILD_BUILDID" \ + --branch "$DELTA_QUERY_BRANCH" \ + --source-commit "$BUILD_SOURCEVERSION")" + echo "$range_output" + + source_commit="$(sed -n 's/^sourceCommit=//p' <<< "$range_output")" + target_commit="$(sed -n 's/^targetCommit=//p' <<< "$range_output")" + if [[ -z "$source_commit" || -z "$target_commit" ]]; then + echo "##[error]determine_commit_range.py did not emit a source/target commit range." + exit 1 + fi + + # The helper emits baselineQueryFailed=true ONLY when the ADO Builds API + # query itself failed (bad SYSTEM_ACCESSTOKEN scope, network, SDK + # misconfig) -- as opposed to a successful query that simply found no + # prior build (a benign first run). A failed query silently degrades EVERY + # run to a single-commit delta, with the weekly true-up as the sole safety + # net, so raise a visible pipeline warning here rather than letting it pass + # unnoticed. + if [[ "$(sed -n 's/^baselineQueryFailed=//p' <<< "$range_output")" == "true" ]]; then + echo "##vso[task.logissue type=warning]Delta build could not query previous builds; degraded to a single-commit delta (target=source^1). Check the SYSTEM_ACCESSTOKEN scope and ADO Builds API connectivity. The weekly true-up job covers any gap." + fi + + # Mark the resolved range readonly: it is computed once here and only + # consumed downstream, so locking it prevents a later step from clobbering + # the commit range (matches changedComponentsFile / renderSetFile below). + echo "##vso[task.setvariable variable=sourceCommit;isreadonly=true]$source_commit" + echo "##vso[task.setvariable variable=targetCommit;isreadonly=true]$target_commit" + env: + SYSTEM_COLLECTIONURI: $(System.CollectionUri) + SYSTEM_TEAMPROJECT: $(System.TeamProject) + SYSTEM_ACCESSTOKEN: $(System.AccessToken) + SYSTEM_DEFINITIONID: $(System.DefinitionId) + BUILD_BUILDID: $(Build.BuildId) + DELTA_QUERY_BRANCH: $(Build.SourceBranch) + BUILD_SOURCEVERSION: $(Build.SourceVersion) + displayName: "Determine source and target commit range" diff --git a/.github/workflows/ado/templates/steps/commit-range-pr.yml b/.github/workflows/ado/templates/steps/commit-range-pr.yml new file mode 100644 index 00000000000..48b65e60ccc --- /dev/null +++ b/.github/workflows/ado/templates/steps/commit-range-pr.yml @@ -0,0 +1,52 @@ +# Microsoft Corporation +# +# Step template: resolve the PR commit range from the merge commit's parents. +# A PR-policy build checks out the MERGE commit (Build.SourceVersion): parent +# ^1 is the target-branch tip, parent ^2 is the PR head. The PR's true change +# set is the diff between their MERGE BASE (the fork point) and ^2 -- i.e. +# `git diff merge-base(^1,^2)..^2`, the same three-dot set GitHub's "Files +# changed" shows. We compare against the merge base, NOT the target tip: +# comparing against the tip additionally flags every component that landed on +# the target branch after the PR forked (the PR head still carries their +# pre-update state), which can balloon a 1-component PR into dozens. +# +# Emits job-scope variables sourceCommit (PR head, ^2) and baseCommit (the +# merge base). These feed prepare-change-set.yml (from=baseCommit, +# to=sourceCommit) and the trailing Control Tower call. + +steps: + # Fail fast, at compile time, when this is not a PR build: the merge-commit + # parent logic below only makes sense for a PullRequest trigger. Including the + # guard step is decided by a template expression on Build.Reason, so a + # mis-triggered run stops at its very first step. + - ${{ if ne(variables['Build.Reason'], 'PullRequest') }}: + - script: | + echo "##[error]This pipeline must run as a PR build (Build.Reason=PullRequest); got '$(Build.Reason)'." + exit 1 + displayName: "Guard: PR build only" + + - script: | + set -euo pipefail + # Defense in depth: a PullRequest trigger should always check out a merge + # commit, but verify the ^2 parent exists before relying on it. + if ! git rev-parse --verify -q "HEAD^2" >/dev/null; then + echo "##[error]HEAD is not a merge commit -- expected a PR merge build." + exit 1 + fi + source_commit="$(git rev-parse HEAD^2)" + # Baseline = the MERGE BASE (fork point) of the target-branch tip + # (HEAD^1) and the PR head (HEAD^2), NOT the target tip itself. + # See the block comment above for why; this is passed as the + # `--from-commit` baseline to the change-set helper. + base_commit="$(git merge-base HEAD^1 HEAD^2)" + # PR-supplied data is untrusted: validate both SHAs before use. + for sha in "$base_commit" "$source_commit"; do + if [[ ! "$sha" =~ ^[0-9a-f]{40}$ ]]; then + echo "##[error]invalid commit SHA: $sha" + exit 1 + fi + done + echo "Resolved range: base(merge-base)=$base_commit source=$source_commit" + echo "##vso[task.setvariable variable=sourceCommit;isreadonly=true]$source_commit" + echo "##vso[task.setvariable variable=baseCommit;isreadonly=true]$base_commit" + displayName: "Determine PR commit range" diff --git a/.github/workflows/ado/templates/steps/ensure-full-history.yml b/.github/workflows/ado/templates/steps/ensure-full-history.yml new file mode 100644 index 00000000000..edd55321822 --- /dev/null +++ b/.github/workflows/ado/templates/steps/ensure-full-history.yml @@ -0,0 +1,20 @@ +# Microsoft Corporation +# +# Step template: ensure the full git history is present, ONCE, before anything +# that needs it. The OneBranch checkout may be shallow (depth 1), but lock +# resolution, spec rendering (rpmautospec derives Release/changelog from +# `git log` over the lock files), and commit-range detection all require the +# complete history. Doing the unshallow here means the helper scripts can assume +# full history and never fetch themselves -- a `git fetch --depth=N` inside a +# script would re-shallow a full clone, which silently corrupts the rpmautospec +# Release calculation (and is a footgun when running the scripts locally). + +steps: + - script: | + set -euo pipefail + if [ "$(git rev-parse --is-shallow-repository)" = "true" ]; then + echo "##[group]Fetching full git history" + git fetch --unshallow + echo "##[endgroup]" + fi + displayName: "Ensure full git history" diff --git a/.github/workflows/ado/templates/steps/install-deps.yml b/.github/workflows/ado/templates/steps/install-deps.yml new file mode 100644 index 00000000000..226b78bb27c --- /dev/null +++ b/.github/workflows/ado/templates/steps/install-deps.yml @@ -0,0 +1,93 @@ +# Microsoft Corporation +# +# Step template: authenticate to the internal pip feed and install the host +# tools the Control Tower change-detection + submission steps need. azldev is +# always installed (its version comes from the committed .azldev-version and is +# validated before use); everything else is opt-in so each pipeline pulls only +# what it uses: +# installMock - mock + rpmautospec, needed by the post-merge +# pipelines' lock/render steps. The PR check does +# read-only change detection and does not need it. +# installAdoRequirements - the azure-devops SDK used by the post-merge +# commit-range helper (determine_commit_range.py). +# normalizeGoGitConfig - strip extensions.worktreeConfig so azldev's go-git +# can open the ADO-agent checkout (PR check only). + +parameters: + - name: installMock + type: boolean + default: false + - name: installAdoRequirements + type: boolean + default: false + - name: normalizeGoGitConfig + type: boolean + default: false + +steps: + - task: PipAuthenticate@1 + displayName: "Authenticate pip" + inputs: + artifactFeeds: "azl/ControlTowerFeed" + + - ${{ if parameters.normalizeGoGitConfig }}: + # azldev opens the repo with go-git, which rejects a config that declares + # the `worktreeconfig` extension while core.repositoryformatversion is still + # 0: "core.repositoryformatversion does not support extension: worktreeconfig". + # Native git tolerates this, and the ADO agent checkout leaves the extension + # set, so strip it before any azldev invocation. Each CI run is a fresh + # checkout so this is safe and self-contained. + # TODO: remove this step once azldev no longer needs the workaround (go-git + # v6 fixes the underlying bug): + # https://github.com/microsoft/azure-linux-dev-tools/issues/241 + - script: | + set -euo pipefail + if git config --get extensions.worktreeConfig >/dev/null 2>&1; then + echo "Removing extensions.worktreeConfig so go-git (azldev) can open the repo" + git config --unset-all extensions.worktreeConfig || true + fi + displayName: "Normalize git config for azldev (go-git)" + + - ${{ if parameters.installMock }}: + - script: | + set -euo pipefail + echo "##[group]Mock" + tdnf install -y mock mock-rpmautospec python3-chardet + sudo usermod -aG mock "$(whoami)" + echo "##[endgroup]" + displayName: "Install mock" + + - script: | + set -euo pipefail + echo "##[group]Azldev (host)" + # Only the version string comes from the checkout; reject a + # malformed/garbage value before it reaches `go install`. + AZLDEV_VERSION="$(tr -d '\n' < .azldev-version)" + if ! printf '%s' "$AZLDEV_VERSION" | grep -Eq '^[0-9A-Za-z._+-]+$'; then + echo "##[error].azldev-version is empty or has unexpected characters" + exit 1 + fi + echo "Installing azldev@${AZLDEV_VERSION}..." + go install "github.com/microsoft/azure-linux-dev-tools/cmd/azldev@${AZLDEV_VERSION}" + + go_bin_path="$(go env GOPATH)/bin" + echo "##vso[task.prependpath]$go_bin_path" + + "$go_bin_path/azldev" --version + echo "##[endgroup]" + displayName: "Install azldev" + + - script: | + set -euo pipefail + echo "##[group]Python dependencies (Control Tower client)" + pip install -r scripts/ci/control-tower/requirements.txt + echo "##[endgroup]" + displayName: "Install Control Tower Python client" + + - ${{ if parameters.installAdoRequirements }}: + - script: | + set -euo pipefail + echo "##[group]Python dependencies (ADO API client)" + pip install -r scripts/ci/ado/requirements.txt + echo "##[endgroup]" + displayName: "Install ADO API Python client" diff --git a/.github/workflows/ado/templates/steps/prepare-change-set.yml b/.github/workflows/ado/templates/steps/prepare-change-set.yml new file mode 100644 index 00000000000..3ee3cfffa2c --- /dev/null +++ b/.github/workflows/ado/templates/steps/prepare-change-set.yml @@ -0,0 +1,70 @@ +# Microsoft Corporation +# +# Step template: detect changed components and prepare the render set, then +# publish changed-components.json as a triage artifact on every exit path. +# Emits job-scope variables changedComponentsFile and renderSetFile for +# downstream consumers (the render-verify step and the trailing Control Tower +# call). +# +# The (from, to) commit range is read from job-scope variables named by the +# fromCommitVar / toCommitVar parameters, so the post-merge (targetCommit -> +# sourceCommit) and PR (baseCommit -> sourceCommit) pipelines share this +# template unchanged. compute_change_set.sh diffs from -> to. +# +# compute_change_set.sh hard-fails on the supply-chain drift tripwire +# (sourcesChange == true without an identity change in {added, changed, +# deleted}): that combination would let attacker-supplied bytes ride into the +# lookaside under an unchanged component identity, so it fails closed. +# +# Requires the job-scope variable `ob_outputDirectory` (the triage artifact is +# published there). + +parameters: + - name: fromCommitVar + type: string + default: targetCommit + - name: toCommitVar + type: string + default: sourceCommit + +steps: + - script: | + set -euo pipefail + change_set_dir="$(Build.ArtifactStagingDirectory)/change-set" + json_file="$change_set_dir/changed-components.json" + render_set_file="$change_set_dir/render-set.txt" + + # Publish the changed-components JSON for post-mortem triage on + # EVERY exit path, not just success -- if azldev hard-fails on a + # consistency tripwire the partial JSON is exactly what an + # operator needs to investigate. + publish_artifact() { + if [[ -s "$json_file" ]]; then + mkdir -p "$(ob_outputDirectory)/changed-components" + cp "$json_file" "$(ob_outputDirectory)/changed-components/" || true + fi + } + trap publish_artifact EXIT + + echo "##[group]Preparing change set" + scripts/ci/components/compute_change_set.sh \ + --output-dir "$change_set_dir" \ + --from-commit "$FROM_COMMIT" \ + --to-commit "$TO_COMMIT" + echo "##[endgroup]" + + echo "##[group]Upload set (sourcesChange == true, changeType in {added, changed})" + upload_count=$(jq -r '[.[] | select(.sourcesChange == true and (.changeType | IN("added", "changed")))] | length' "$json_file") + jq -r '.[] | select(.sourcesChange == true and (.changeType | IN("added", "changed"))) | .component' "$json_file" | sort + echo "Total: $upload_count component(s) to upload." + echo "##[endgroup]" + + # Publish the downstream pipeline variables now that the change set is + # ready (consumed by the render-verify step and the trailing Control + # Tower call). + echo "##vso[task.setvariable variable=changedComponentsFile;isreadonly=true]$json_file" + echo "##vso[task.setvariable variable=renderSetFile;isreadonly=true]$render_set_file" + env: + FROM_COMMIT: $(${{ parameters.fromCommitVar }}) + TO_COMMIT: $(${{ parameters.toCommitVar }}) + displayName: "Prepare change set" diff --git a/.github/workflows/ado/templates/steps/verify-locks.yml b/.github/workflows/ado/templates/steps/verify-locks.yml new file mode 100644 index 00000000000..9ab83958452 --- /dev/null +++ b/.github/workflows/ado/templates/steps/verify-locks.yml @@ -0,0 +1,15 @@ +# Microsoft Corporation +# +# Step template: verify lock files are current. --check-only validates without +# writing and exits nonzero if any lock would change. Publishes a lock-update +# diff to ob_outputDirectory for triage. +# +# Requires the job-scope variable `ob_outputDirectory`. + +steps: + - script: | + set -euo pipefail + scripts/ci/control-tower/verify_locks.sh \ + --output-file "$(Build.ArtifactStagingDirectory)/lock-update.json" \ + --publish-dir "$(ob_outputDirectory)" + displayName: "Verify lock files are up to date" diff --git a/.github/workflows/ado/templates/steps/verify-rendered-specs.yml b/.github/workflows/ado/templates/steps/verify-rendered-specs.yml new file mode 100644 index 00000000000..274b8adc91f --- /dev/null +++ b/.github/workflows/ado/templates/steps/verify-rendered-specs.yml @@ -0,0 +1,42 @@ +# Microsoft Corporation +# +# Step template: render the components flagged by the change set (azldev-flagged +# plus any with hand-edited spec files) in --check-only mode: azldev renders to +# a staging area and diffs against the on-disk output without writing. Exits +# nonzero if any component's rendered output would change, catching both stale +# renders and direct hand-edits. +# +# Consumes the job-scope variable renderSetFile set by prepare-change-set.yml. + +steps: + - script: | + set -euo pipefail + # A missing render-set file means "Prepare change set" did not complete; + # fail loud rather than silently skipping the render check. + if [[ ! -f "$RENDER_SET_FILE" ]]; then + echo "##[error]render-set file '$RENDER_SET_FILE' is missing -- 'Prepare change set' did not complete cleanly." + exit 1 + fi + # An empty render set just means no PR-scoped components changed. + if [[ ! -s "$RENDER_SET_FILE" ]]; then + echo "No changed components -- skipping render." + exit 0 + fi + echo "##[group]Render set" + sed 's/^/ - /' "$RENDER_SET_FILE" + echo "##[endgroup]" + echo "##[group]Specs rendering + verification" + # `-x` fails loudly if the arg list would split into multiple + # xargs invocations -- a multi-batch render-check would + # silently mask drift in the later batches. `-n 50000` is + # required for `-x` to have any effect (without `-n`/`-L`, + # GNU xargs never splits). `--` ends azldev option parsing so + # component names beginning with `-` (none today) are + # unambiguous. `env AZLDEV_ALLOW_ROOT=1` is inline per + # .github/instructions/ado-pipeline.instructions.md. + xargs -x -n 50000 -d '\n' env AZLDEV_ALLOW_ROOT=1 azldev component render --check-only -- \ + < "$RENDER_SET_FILE" + echo "##[endgroup]" + env: + RENDER_SET_FILE: $(renderSetFile) + displayName: "Verify rendered specs are up to date" From acd6951b6d687af651bb75813cc4d94f57135fcd Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 13:44:54 -0700 Subject: [PATCH 05/14] ci: recompose common-steps from the granular step templates Replace the inlined step bodies in common-steps.yml with composition of the new templates/steps/* templates. The post-merge pipelines (package-build, source-upload) that splice common-steps in are unchanged in effect: same steps, same order, same emitted variables (sourceCommit/targetCommit -> changedComponentsFile/renderSetFile). Note one intentional, safe hardening: the shared install-deps.yml validates .azldev-version against ^[0-9A-Za-z._+-]+$ before `go install`, which the old common-steps did not. The committed .azldev-version is well-formed, so this only rejects a genuinely malformed value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ado/templates/steps/common-steps.yml | 247 ++---------------- 1 file changed, 22 insertions(+), 225 deletions(-) diff --git a/.github/workflows/ado/templates/steps/common-steps.yml b/.github/workflows/ado/templates/steps/common-steps.yml index f772aa12064..04e6f774a16 100644 --- a/.github/workflows/ado/templates/steps/common-steps.yml +++ b/.github/workflows/ado/templates/steps/common-steps.yml @@ -1,19 +1,18 @@ # Microsoft Corporation # -# Shared step template for the Control Tower integration pipelines (package -# build + source upload). Spliced into each pipeline's single job via: +# Shared step set for the POST-MERGE Control Tower pipelines (package build + +# source upload). Spliced into each pipeline's single job via: # # steps: # - template: steps/common-steps.yml # - # -# Keeping these as STEPS (not a separate job/stage) is deliberate: the -# change-detection steps emit job-scoped pipeline variables (sourceCommit, -# targetCommit, changedComponentsFile, renderSetFile) and write the change-set -# files to disk. +# It composes the granular step templates in templates/steps/. Keeping these as +# STEPS (not a separate job/stage) is deliberate: the change-detection steps +# emit job-scoped pipeline variables (sourceCommit, targetCommit, +# changedComponentsFile, renderSetFile) and write the change-set files to disk. # Those flow freely to later steps in the SAME job but would need output -# variables + artifact upload/download to cross a job boundary. Splicing the -# common steps inline keeps that wiring trivial. +# variables + artifact upload/download to cross a job boundary. # # Contract for the including job: # * It MUST define the job-scope variable `ob_outputDirectory` (both stages @@ -21,223 +20,21 @@ # and prepare-change-set steps publish triage artifacts there. # * These steps assume normal step sequencing -- a failed step stops the ones # after it (the default; do NOT set step-level `continueOnError` on them). -# Later steps depend on earlier ones, e.g. the render step trusts that -# "Prepare change set" ran and set renderSetFile to a real path. A job-level -# `continueOnError` (which only affects how the JOB result is reported, not -# intra-job step flow) is fine -- the source-upload pipeline uses one. +# Later steps depend on earlier ones. A job-level `continueOnError` (which +# only affects how the JOB result is reported, not intra-job step flow) is +# fine -- the source-upload pipeline uses one. # -# The steps below are identical across both pipelines: ensure full git history, -# authenticate + install dependencies, resolve the (target, source) commit -# range, and validate (lock files + rendered specs). The pipeline-specific -# Control Tower call (package build or prcheck) is appended by the including -# stages template. +# The PR check composes these same granular templates directly, with its own +# commit-range-pr.yml and without the lock/render-verify steps (those are +# covered by the GitHub Actions PR gates). steps: - # Ensure the full git history is present, ONCE, before anything that needs it. - # The OneBranch checkout may be shallow (depth 1), but lock resolution, spec - # rendering (rpmautospec derives Release/changelog from `git log` over the - # lock files), and commit-range detection all require the complete history. - # Doing the unshallow here means the helper scripts can assume full history - # and never fetch themselves -- a `git fetch --depth=N` inside a script would - # re-shallow a full clone, which silently corrupts the rpmautospec Release - # calculation (and is a footgun when running the scripts locally). - - script: | - set -euo pipefail - if [ "$(git rev-parse --is-shallow-repository)" = "true" ]; then - echo "##[group]Fetching full git history" - git fetch --unshallow - echo "##[endgroup]" - fi - displayName: "Ensure full git history" - - - task: PipAuthenticate@1 - displayName: "Authenticate pip" - inputs: - artifactFeeds: "azl/ControlTowerFeed" - - - script: | - set -euo pipefail - - echo "##[group]Mock" - tdnf install -y mock mock-rpmautospec python3-chardet - sudo usermod -aG mock "$(whoami)" - echo "##[endgroup]" - - echo "##[group]Azldev" - AZLDEV_VERSION=$(cat .azldev-version) - echo "Installing azldev@${AZLDEV_VERSION}..." - go install "github.com/microsoft/azure-linux-dev-tools/cmd/azldev@${AZLDEV_VERSION}" - - go_bin_path="$(go env GOPATH)/bin" - echo "##vso[task.prependpath]$go_bin_path" - - "$go_bin_path/azldev" --version - echo "##[endgroup]" - - echo "##[group]Python dependencies" - pip install -r scripts/ci/control-tower/requirements.txt - pip install -r scripts/ci/ado/requirements.txt - echo "##[endgroup]" - displayName: "Install dependencies" - - # Resolve the (target, source) commit range for this post-merge build: - # source = the commit that triggered this run (Build.SourceVersion). - # target = the previous CI build's commit on this branch, looked up - # via the ADO Builds API. Pairing each build with the - # one immediately before it keeps concurrent runs from - # overlapping and captures every commit a rebase merge - # appends. Runs after the dependency-install step because - # it imports the azure-devops SDK. See - # scripts/ci/ado/determine_commit_range.py for the full - # rationale and the first-run (source^1) fallback. - - script: | - set -euo pipefail - # The helper prints the resolved range to stdout as two lines: - # sourceCommit= - # targetCommit= - # (all diagnostics go to stderr). We parse them and set the - # pipeline variables HERE so the variable wiring is visible in - # the YAML rather than hidden in the script. - range_output="$(python3 scripts/ci/ado/determine_commit_range.py \ - --definition-id "$SYSTEM_DEFINITIONID" \ - --current-build-id "$BUILD_BUILDID" \ - --branch "$DELTA_QUERY_BRANCH" \ - --source-commit "$BUILD_SOURCEVERSION")" - echo "$range_output" - - source_commit="$(sed -n 's/^sourceCommit=//p' <<< "$range_output")" - target_commit="$(sed -n 's/^targetCommit=//p' <<< "$range_output")" - if [[ -z "$source_commit" || -z "$target_commit" ]]; then - echo "##[error]determine_commit_range.py did not emit a source/target commit range." - exit 1 - fi - - # The helper emits baselineQueryFailed=true ONLY when the ADO Builds API - # query itself failed (bad SYSTEM_ACCESSTOKEN scope, network, SDK - # misconfig) -- as opposed to a successful query that simply found no - # prior build (a benign first run). A failed query silently degrades EVERY - # run to a single-commit delta, with the weekly true-up as the sole safety - # net, so raise a visible pipeline warning here rather than letting it pass - # unnoticed. - if [[ "$(sed -n 's/^baselineQueryFailed=//p' <<< "$range_output")" == "true" ]]; then - echo "##vso[task.logissue type=warning]Delta build could not query previous builds; degraded to a single-commit delta (target=source^1). Check the SYSTEM_ACCESSTOKEN scope and ADO Builds API connectivity. The weekly true-up job covers any gap." - fi - - # Mark the resolved range readonly: it is computed once here and only - # consumed downstream, so locking it prevents a later step from clobbering - # the commit range (matches changedComponentsFile / renderSetFile below). - echo "##vso[task.setvariable variable=sourceCommit;isreadonly=true]$source_commit" - echo "##vso[task.setvariable variable=targetCommit;isreadonly=true]$target_commit" - env: - SYSTEM_COLLECTIONURI: $(System.CollectionUri) - SYSTEM_TEAMPROJECT: $(System.TeamProject) - SYSTEM_ACCESSTOKEN: $(System.AccessToken) - SYSTEM_DEFINITIONID: $(System.DefinitionId) - BUILD_BUILDID: $(Build.BuildId) - DELTA_QUERY_BRANCH: $(Build.SourceBranch) - BUILD_SOURCEVERSION: $(Build.SourceVersion) - displayName: "Determine source and target commit range" - - # Verify lock files are current. --check-only validates without - # writing, exits nonzero if any lock would change. - - script: | - set -euo pipefail - scripts/ci/control-tower/verify_locks.sh \ - --output-file "$(Build.ArtifactStagingDirectory)/lock-update.json" \ - --publish-dir "$(ob_outputDirectory)" - displayName: "Verify lock files are up to date" - - # Detect changed components AND prepare the render set. The same - # changed-components JSON is consumed by the downstream - # render-verify step (below) and by the CT API calls (appended by - # the including stages template -- prcheck or package-build), so we - # compute it once here and publish it as a triage artifact on every - # exit path. - # - # azldev hard-fails if any component has sourcesChange == true - # without a corresponding identity change (changeType not in - # {added, changed, deleted}). That combination would let - # attacker-supplied bytes ride into the lookaside under an - # unchanged component identity, so we treat it as hostile and - # fail closed (supply-chain drift tripwire). --include-unchanged - # (set inside compute_change_set.sh) ensures the full component - # list is available for downstream consumers. - - script: | - set -euo pipefail - change_set_dir="$(Build.ArtifactStagingDirectory)/change-set" - json_file="$change_set_dir/changed-components.json" - render_set_file="$change_set_dir/render-set.txt" - - # Publish the changed-components JSON for post-mortem triage on - # EVERY exit path, not just success -- if azldev hard-fails on a - # consistency tripwire the partial JSON is exactly what an - # operator needs to investigate. - publish_artifact() { - if [[ -s "$json_file" ]]; then - mkdir -p "$(ob_outputDirectory)/changed-components" - cp "$json_file" "$(ob_outputDirectory)/changed-components/" || true - fi - } - trap publish_artifact EXIT - - echo "##[group]Preparing change set" - # from = previous build's commit (baseline), to = this build's commit. - scripts/ci/components/compute_change_set.sh \ - --output-dir "$change_set_dir" \ - --from-commit "$TARGET_COMMIT" \ - --to-commit "$SOURCE_COMMIT" - echo "##[endgroup]" - - echo "##[group]Upload set (sourcesChange == true, changeType in {added, changed})" - upload_count=$(jq -r '[.[] | select(.sourcesChange == true and (.changeType | IN("added", "changed")))] | length' "$json_file") - jq -r '.[] | select(.sourcesChange == true and (.changeType | IN("added", "changed"))) | .component' "$json_file" | sort - echo "Total: $upload_count component(s) to upload." - echo "##[endgroup]" - - # Publish the downstream pipeline variables now that the change set is - # ready (consumed by the render-verify step and the trailing Control - # Tower call). - echo "##vso[task.setvariable variable=changedComponentsFile;isreadonly=true]$json_file" - echo "##vso[task.setvariable variable=renderSetFile;isreadonly=true]$render_set_file" - env: - SOURCE_COMMIT: $(sourceCommit) - TARGET_COMMIT: $(targetCommit) - displayName: "Prepare change set" - - # Render the components flagged by the change set above - # (azldev-flagged plus any with hand-edited spec files) in - # --check-only mode: azldev renders to a staging area and diffs - # against the on-disk output without writing. Exits nonzero if - # any component's rendered output would change, catching both - # stale renders and direct hand-edits. - - script: | - set -euo pipefail - # A missing render-set file means "Prepare change set" did not complete; - # fail loud rather than silently skipping the render check. - if [[ ! -f "$RENDER_SET_FILE" ]]; then - echo "##[error]render-set file '$RENDER_SET_FILE' is missing -- 'Prepare change set' did not complete cleanly." - exit 1 - fi - # An empty render set just means no PR-scoped components changed. - if [[ ! -s "$RENDER_SET_FILE" ]]; then - echo "No changed components -- skipping render." - exit 0 - fi - echo "##[group]Render set" - sed 's/^/ - /' "$RENDER_SET_FILE" - echo "##[endgroup]" - echo "##[group]Specs rendering + verification" - # `-x` fails loudly if the arg list would split into multiple - # xargs invocations -- a multi-batch render-check would - # silently mask drift in the later batches. `-n 50000` is - # required for `-x` to have any effect (without `-n`/`-L`, - # GNU xargs never splits). `--` ends azldev option parsing so - # component names beginning with `-` (none today) are - # unambiguous. `env AZLDEV_ALLOW_ROOT=1` is inline per - # .github/instructions/ado-pipeline.instructions.md. - xargs -x -n 50000 -d '\n' env AZLDEV_ALLOW_ROOT=1 azldev component render --check-only -- \ - < "$RENDER_SET_FILE" - echo "##[endgroup]" - env: - RENDER_SET_FILE: $(renderSetFile) - displayName: "Verify rendered specs are up to date" + - template: ensure-full-history.yml + - template: install-deps.yml + parameters: + installMock: true + installAdoRequirements: true + - template: commit-range-postmerge.yml + - template: verify-locks.yml + - template: prepare-change-set.yml + - template: verify-rendered-specs.yml From 49c475302b7c2445cda134017baaaa09a3aa85b0 Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 13:47:20 -0700 Subject: [PATCH 06/14] ci: add Control Tower prcheck to the PR check and compose shared templates Rewrite the PR package-build stages template to: - compose the shared templates/steps/* templates (ensure-full-history, install-deps with normalizeGoGitConfig, commit-range-pr, prepare-change-set) instead of duplicating the setup inline; and - call Control Tower 'prcheck' BEFORE the scratch package build. prcheck uploads the missing lookaside sources for the changed components so they are present when the build fetches them, so it must run first. This folds the prcheck call (previously in sources-upload-stages.yml) into the active PR check. The scratch-build step is unchanged. Reusing the granular templates also lands the remaining template-local AI: fixes (build-reason template expression and baseCommit rename live in commit-range-pr.yml; the change_set_dir/changedComponentsFile derivation lives in prepare-change-set.yml). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ado/templates/pr-package-build-stages.yml | 180 ++++++------------ 1 file changed, 53 insertions(+), 127 deletions(-) diff --git a/.github/workflows/ado/templates/pr-package-build-stages.yml b/.github/workflows/ado/templates/pr-package-build-stages.yml index 98d5d3340b9..8a224383ac3 100644 --- a/.github/workflows/ado/templates/pr-package-build-stages.yml +++ b/.github/workflows/ado/templates/pr-package-build-stages.yml @@ -13,20 +13,21 @@ # 3. Resolve the PR commit range from the merge commit's parents # (^1 = target-branch tip, ^2 = PR head). # 4. Compute the changed-component set (shared compute_change_set.sh). -# 5. Submit a *scratch* Control Tower build of the PR head for exactly those +# 5. Call Control Tower 'prcheck' for those components: among other things it +# uploads the missing lookaside sources so they are present when the build +# below fetches them. +# 6. Submit a *scratch* Control Tower build of the PR head for exactly those # components (run_package_build.py --wait-for-completion). The build runs # in Control Tower's own sandbox; this pipeline WAITS for it to reach a # terminal state and fails the check if the build fails (or does not # finish within the poll timeout). NO PR-controlled code is built on the # CI agent -- only read-only change detection runs here. # -# It deliberately does NOT reuse templates/steps/common-steps.yml: that shared -# step set resolves the commit range via the *previous CI build* (the -# post-merge delta logic), which is wrong for a PR. A PR range comes from the -# merge commit's parents, computed inline below. Reusing common-steps would -# also pull in the lock/render verify steps (already covered by the GitHub -# Actions PR gates) and force a refactor of a file shared by two production -# pipelines. +# Setup + change detection are composed from the shared templates/steps/* +# templates (the same ones the post-merge pipeline uses via common-steps.yml), +# but with the PR commit-range template (merge-commit parents, not the +# previous-build delta) and WITHOUT the lock/render-verify steps -- those PR +# gates already run in GitHub Actions. # # Because it calls Control Tower, this pipeline needs the WIF service connection # and the Control Tower variable group (audience + base URL); the wrapper @@ -86,132 +87,57 @@ stages: - name: LinuxContainerImage value: ${{ parameters.containerImage }} steps: - # Full history: `azldev component changed` tree-diffs two commits and - # rpmautospec derives Release/changelog from `git log`. The CI - # checkout may be shallow (depth 1); unshallow once, up front. Never - # `git fetch --depth=N` afterwards — that re-shallows a full clone and - # silently corrupts the rpmautospec Release calculation. - - script: | - set -euo pipefail - if [ "$(git rev-parse --is-shallow-repository)" = "true" ]; then - echo "##[group]Fetching full git history" - git fetch --unshallow - echo "##[endgroup]" - fi - displayName: "Ensure full git history" + # Shared setup + change detection, composed from the granular step + # templates. install-deps skips mock and the ADO API client (neither + # is needed for read-only change detection) but strips the go-git + # worktreeConfig extension so azldev can open the agent checkout. + - template: steps/ensure-full-history.yml - - task: PipAuthenticate@1 - displayName: "Authenticate pip" - inputs: - artifactFeeds: "azl/ControlTowerFeed" - - # azldev opens the repo with go-git, which rejects a config that - # declares the `worktreeconfig` extension while - # core.repositoryformatversion is still 0: - # "core.repositoryformatversion does not support extension: worktreeconfig" - # Native git tolerates this, and the ADO agent checkout leaves the - # extension set, so strip it before any azldev invocation. Each CI run - # is a fresh checkout so this is safe and self-contained. - # TODO: remove this step once azldev no longer needs the workaround - # (go-git v6 fixes the underlying bug): - # https://github.com/microsoft/azure-linux-dev-tools/issues/241 - - script: | - set -euo pipefail - if git config --get extensions.worktreeConfig >/dev/null 2>&1; then - echo "Removing extensions.worktreeConfig so go-git (azldev) can open the repo" - git config --unset-all extensions.worktreeConfig || true - fi - displayName: "Normalize git config for azldev (go-git)" - - # Host deps for change detection + the Control Tower submission only: - # azldev (`azldev component changed` + git diff -- no mock, no build) - # and the Control Tower Python client. The build itself never runs on - # the agent; it runs asynchronously in Control Tower's own sandbox. - - script: | - set -euo pipefail - echo "##[group]Azldev (host, for change-set)" - # Only the version string comes from the PR checkout; reject a - # malformed/garbage value before it reaches `go install`. - AZLDEV_VERSION="$(tr -d '\n' < .azldev-version)" - if ! printf '%s' "$AZLDEV_VERSION" | grep -Eq '^[0-9A-Za-z._+-]+$'; then - echo "##[error].azldev-version is empty or has unexpected characters" - exit 1 - fi - echo "Installing azldev@${AZLDEV_VERSION}..." - go install "github.com/microsoft/azure-linux-dev-tools/cmd/azldev@${AZLDEV_VERSION}" + - template: steps/install-deps.yml + parameters: + normalizeGoGitConfig: true - go_bin_path="$(go env GOPATH)/bin" - echo "##vso[task.prependpath]$go_bin_path" + # PR commit range = merge-commit parents (baseCommit = merge base, + # sourceCommit = PR head). prepare-change-set diffs baseCommit -> + # sourceCommit. + - template: steps/commit-range-pr.yml - "$go_bin_path/azldev" --version - echo "##[endgroup]" + - template: steps/prepare-change-set.yml + parameters: + fromCommitVar: baseCommit - echo "##[group]Python dependencies (Control Tower client)" - pip install -r scripts/ci/control-tower/requirements.txt - echo "##[endgroup]" - displayName: "Install host dependencies" - - # Resolve the PR commit range. A PR-policy build checks out the MERGE - # commit (Build.SourceVersion): parent ^1 is the target-branch tip, - # parent ^2 is the PR head. The PR's true change set is the diff - # between their MERGE BASE (the fork point) and ^2 -- i.e. - # `git diff merge-base(^1,^2)..^2`, the same three-dot set GitHub's - # "Files changed" shows. We deliberately compare against the merge - # base, NOT the target tip: comparing against the tip additionally - # flags every component that landed on the target branch after the PR - # forked (the PR head still carries their pre-update state), which can - # balloon a 1-component PR into dozens. We read the range here and set - # pipeline variables so the wiring stays visible in the YAML. - - script: | - set -euo pipefail - # AI: move build reason check to a template expression with 'variables['Build.Reason']' to fail fast. - if ! git rev-parse --verify -q "HEAD^2" >/dev/null; then - echo "##[error]HEAD is not a merge commit -- this pipeline must run as a PR build (Build.Reason=PullRequest)." - exit 1 - fi - source_commit="$(git rev-parse HEAD^2)" - # Baseline = the MERGE BASE (fork point) of the target-branch tip - # (HEAD^1) and the PR head (HEAD^2), NOT the target tip itself. - # See the block comment above for why; this is passed as the - # `--from-commit` baseline to the change-set helper. - target_commit="$(git merge-base HEAD^1 HEAD^2)" - # PR-supplied data is untrusted: validate both SHAs before use. - for sha in "$target_commit" "$source_commit"; do - if [[ ! "$sha" =~ ^[0-9a-f]{40}$ ]]; then - echo "##[error]invalid commit SHA: $sha" - exit 1 - fi - done - echo "Resolved range: base(merge-base)=$target_commit source=$source_commit" - echo "##vso[task.setvariable variable=sourceCommit;isreadonly=true]$source_commit" - # AI: re-name to 'baseCommit' and 'base_commit', since this is not the target tip but the merge base (fork point). - echo "##vso[task.setvariable variable=targetCommit;isreadonly=true]$target_commit" - displayName: "Determine PR commit range" + # Call Control Tower 'prcheck' BEFORE the build. Among other things, + # prcheck uploads the missing lookaside sources for the changed + # components, so they are available when the scratch build below + # fetches them. It runs first for exactly that reason -- the build + # depends on the sources prcheck uploads. + - task: AzureCLI@2 + displayName: "Call Control Tower 'prcheck' API" + inputs: + azureSubscription: ${{ parameters.serviceConnection }} + scriptType: bash + scriptLocation: inlineScript + inlineScript: | + set -euo pipefail - # Compute the changed-component set with the shared, cross-pipeline - # single-source-of-truth helper (also used by the GitHub Actions PR - # gates). changed-components.json holds the per-component change - # records consumed by the Control Tower submit step below. - # compute_change_set.sh hard-fails on the supply-chain drift tripwire - # (sourcesChange without an identity change) -- a guard we want to keep - # on PRs. The script self-prefixes AZLDEV_ALLOW_ROOT=1 internally. - - script: | - set -euo pipefail - # AI: use job variables for 'changedComponentsFile' and derive 'change_set_dir' from it, rather than hardcoding the staging path here. - change_set_dir="$(Build.ArtifactStagingDirectory)/change-set" - echo "##[group]Preparing change set" - # from = merge base (fork point), to = PR head; the helper diffs - # from -> to. See the "Determine PR commit range" step above. - scripts/ci/components/compute_change_set.sh \ - --output-dir "$change_set_dir" \ - --from-commit "$TARGET_COMMIT" \ - --to-commit "$SOURCE_COMMIT" - echo "##[endgroup]" - echo "##vso[task.setvariable variable=changedComponentsFile;isreadonly=true]$change_set_dir/changed-components.json" + python3 scripts/ci/control-tower/run_prcheck.py \ + --api-audience "$API_AUDIENCE" \ + --api-base-url "$API_BASE_URL" \ + --build-reason "$CT_BUILD_REASON" \ + --changed-components-file "$CHANGED_COMPONENTS_FILE" \ + --source-commit "$SOURCE_COMMIT" \ + --repo-uri "$UPSTREAM_REPO_URL" env: + API_AUDIENCE: $(ApiAudience) + API_BASE_URL: $(ApiBaseAFDUrl) + # Non-reserved name: an `env:` override of the reserved BUILD_REASON var is silently ignored by the agent. + CT_BUILD_REASON: $(Build.Reason) + CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) - TARGET_COMMIT: $(targetCommit) - displayName: "Prepare change set" + # TODO: Base commit is not used yet. Will be needed once we move + # detection of affected components to CT. ADO task: 18816 + BASE_COMMIT: $(baseCommit) + UPSTREAM_REPO_URL: $(Build.Repository.Uri) # Submit a SCRATCH Control Tower build of the PR head for the changed # components. Scratch = throwaway: it never persists to a production From 782d66d972b23d3fc76b0652b67f8003a43e114e Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 13:49:34 -0700 Subject: [PATCH 07/14] ci: rename PR package-build check to the generic pr-check-ct pipeline Rename the template pair now that the PR check does prcheck + package build: pr-package-build.yml -> pr-check-ct.yml pr-package-build-stages.yml -> pr-check-ct-stages.yml Rename the stage/job PRPackageBuild -> PRCheckCT, repoint the wrapper @self template path and artifactBaseName (prcheckct), and refresh the header comments and the components README references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../{pr-package-build.yml => pr-check-ct.yml} | 19 ++++++++++--------- ...uild-stages.yml => pr-check-ct-stages.yml} | 13 +++++++------ scripts/ci/components/README.md | 6 +++--- 3 files changed, 20 insertions(+), 18 deletions(-) rename .github/workflows/ado/{pr-package-build.yml => pr-check-ct.yml} (87%) rename .github/workflows/ado/templates/{pr-package-build-stages.yml => pr-check-ct-stages.yml} (95%) diff --git a/.github/workflows/ado/pr-package-build.yml b/.github/workflows/ado/pr-check-ct.yml similarity index 87% rename from .github/workflows/ado/pr-package-build.yml rename to .github/workflows/ado/pr-check-ct.yml index f81f5a67110..d4bc9213d70 100644 --- a/.github/workflows/ado/pr-package-build.yml +++ b/.github/workflows/ado/pr-check-ct.yml @@ -1,16 +1,17 @@ # Microsoft Corporation # -# Wrapper pipeline — passed to ADO as the entry point for the PR package-build -# check. It submits a *scratch* Control Tower build of the components a pull -# request changes, WAITS for that build to finish, and fails the check if the -# build fails (or is rejected). The build runs in Control Tower's own sandbox — -# NO PR-controlled code is built on the CI agent (only read-only change -# detection runs there). +# Wrapper pipeline — passed to ADO as the entry point for the PR Control Tower +# check. It calls Control Tower 'prcheck' (which uploads the changed +# components' missing lookaside sources) and then submits a *scratch* Control +# Tower build of those components, WAITS for that build to finish, and fails +# the check if the build fails (or is rejected). The build runs in Control +# Tower's own sandbox — NO PR-controlled code is built on the CI agent (only +# read-only change detection runs there). # # This file owns all OneBranch-specific wiring (governed templates repo, # NonOfficial variant, featureFlags) and delegates the actual stages/jobs/steps # to the raw stages template at: -# .github/workflows/ado/templates/pr-package-build-stages.yml +# .github/workflows/ado/templates/pr-check-ct-stages.yml # # WHY SCRATCH + REVIEWER-GATED: building unmerged PR code is only safe because # (a) it is a *scratch* build (throwaway, never persisted to a production @@ -84,10 +85,10 @@ extends: enabled: false stages: - - template: /.github/workflows/ado/templates/pr-package-build-stages.yml@self + - template: /.github/workflows/ado/templates/pr-check-ct-stages.yml@self parameters: outputDirectory: $(Build.ArtifactStagingDirectory)/output - artifactBaseName: prpackagebuild + artifactBaseName: prcheckct containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 poolType: linux serviceConnection: CT-Endpoints-Access-ServiceConnection-DEV diff --git a/.github/workflows/ado/templates/pr-package-build-stages.yml b/.github/workflows/ado/templates/pr-check-ct-stages.yml similarity index 95% rename from .github/workflows/ado/templates/pr-package-build-stages.yml rename to .github/workflows/ado/templates/pr-check-ct-stages.yml index 8a224383ac3..02d84e0d5c5 100644 --- a/.github/workflows/ado/templates/pr-package-build-stages.yml +++ b/.github/workflows/ado/templates/pr-check-ct-stages.yml @@ -1,9 +1,10 @@ # Microsoft Corporation # -# Raw stages template for the PR package-build check. Wrapper-agnostic: declares -# the stages/jobs/steps and exposes the wrapper-coupled knobs as parameters. The -# wrapper at .github/workflows/ado/pr-package-build.yml supplies concrete -# values. See that wrapper for why this pipeline exists. +# Raw stages template for the PR Control Tower check (prcheck + scratch package +# build). Wrapper-agnostic: declares the stages/jobs/steps and exposes the +# wrapper-coupled knobs as parameters. The wrapper at +# .github/workflows/ado/pr-check-ct.yml supplies concrete values. See that +# wrapper for why this pipeline exists. # # What it does, per PR: # 1. Ensure full git history (rpmautospec + change detection need it). @@ -64,9 +65,9 @@ parameters: default: 21600 stages: - - stage: PRPackageBuild + - stage: PRCheckCT jobs: - - job: PRPackageBuild + - job: PRCheckCT # Fail-loud: a failed submission, an immediate Control Tower rejection, # or a build that fails (or never reaches a terminal state) turns the PR # check red. The build runs in Control Tower's own sandbox -- NOT on diff --git a/scripts/ci/components/README.md b/scripts/ci/components/README.md index 5271597dc87..14c1138abf0 100644 --- a/scripts/ci/components/README.md +++ b/scripts/ci/components/README.md @@ -4,8 +4,8 @@ Pipeline-agnostic shell + Python helpers consumed by the GitHub Actions PR gates (`.github/workflows/check-rendered-specs.yml`), the ADO Control Tower integration pipeline (`.github/workflows/ado/templates/sources-upload-stages.yml`), and the ADO -PR package-build check -(`.github/workflows/ado/templates/pr-package-build-stages.yml`). +PR Control Tower check +(`.github/workflows/ado/templates/pr-check-ct-stages.yml`). | Script | Purpose | | ------ | ------- | @@ -29,4 +29,4 @@ PR package-build check - `check-rendered-specs.yml` `render` job → `compute_change_set.sh` - `sources-upload-stages.yml` "Prepare change set" step → `compute_change_set.sh` -- `pr-package-build-stages.yml` "Prepare change set" step → `compute_change_set.sh` +- `pr-check-ct-stages.yml` (via `steps/prepare-change-set.yml`) → `compute_change_set.sh` From d65da3ec6d59b8ae70d22c1f27f64f2890df1df4 Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 13:52:25 -0700 Subject: [PATCH 08/14] ci: remove the standalone source-upload pipeline The Control Tower prcheck (source upload) call now lives in the PR check (pr-check-ct), so the standalone source-upload pipeline is redundant. Delete both files: .github/workflows/ado/sources-upload.yml .github/workflows/ado/templates/sources-upload-stages.yml Scrub the now-dangling references in the package-build wrapper/stages headers, common-steps.yml, the scripts/ci READMEs, and the ado-pipeline instructions (canonical pairing example and shared-sub-template note repointed to the still-live package-build pipeline and the granular templates/steps/*). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../instructions/ado-pipeline.instructions.md | 6 +- .github/workflows/ado/package-build.yml | 7 +- .github/workflows/ado/sources-upload.yml | 82 ---------------- .../ado/templates/package-build-stages.yml | 6 +- .../ado/templates/sources-upload-stages.yml | 93 ------------------- .../ado/templates/steps/common-steps.yml | 6 +- scripts/ci/ado/README.md | 6 +- scripts/ci/components/README.md | 12 +-- 8 files changed, 21 insertions(+), 197 deletions(-) delete mode 100644 .github/workflows/ado/sources-upload.yml delete mode 100644 .github/workflows/ado/templates/sources-upload-stages.yml diff --git a/.github/instructions/ado-pipeline.instructions.md b/.github/instructions/ado-pipeline.instructions.md index 5718e28098f..1143ed6f35a 100644 --- a/.github/instructions/ado-pipeline.instructions.md +++ b/.github/instructions/ado-pipeline.instructions.md @@ -35,11 +35,11 @@ Every ADO pipeline in this repo is split into **two YAML files**: `ob_*` job-scope variables and `LinuxContainerImage` still appear here (ADO requires them at job scope), but they are set from `${{ parameters.* }}`, so the raw author never has to know which OneBranch convention they satisfy. -File-pairing convention: a wrapper at `.github/workflows/ado/.yml` pairs with a raw stages template at `.github/workflows/ado/templates/-stages.yml`. **Multiple wrappers MAY share a single raw stages template** — that is in fact a primary motivation for the split: define several ADO pipelines (e.g. a DEV NonOfficial wrapper and a PROD Official wrapper, or per-environment variants) that all run the same stages/jobs/steps but differ in OneBranch variant, `featureFlags`, service connection, variable group, container image, etc. When wrappers share a raw template, name them so the relationship is obvious (e.g. `sources-upload-dev.yml` + `sources-upload-prod.yml` both pointing at `templates/sources-upload-stages.yml`). The variant choice cannot be hoisted into a shared sub-template because ADO requires `extends:` at the root of the entry pipeline — that is exactly why each wrapper exists. +File-pairing convention: a wrapper at `.github/workflows/ado/.yml` pairs with a raw stages template at `.github/workflows/ado/templates/-stages.yml`. **Multiple wrappers MAY share a single raw stages template** — that is in fact a primary motivation for the split: define several ADO pipelines (e.g. a DEV NonOfficial wrapper and a PROD Official wrapper, or per-environment variants) that all run the same stages/jobs/steps but differ in OneBranch variant, `featureFlags`, service connection, variable group, container image, etc. When wrappers share a raw template, name them so the relationship is obvious (e.g. `package-build-dev.yml` + `package-build-prod.yml` both pointing at `templates/package-build-stages.yml`). The variant choice cannot be hoisted into a shared sub-template because ADO requires `extends:` at the root of the entry pipeline — that is exactly why each wrapper exists. -See [.github/workflows/ado/sources-upload.yml](.github/workflows/ado/sources-upload.yml) and [.github/workflows/ado/templates/sources-upload-stages.yml](.github/workflows/ado/templates/sources-upload-stages.yml) for the canonical example. +See [.github/workflows/ado/package-build.yml](.github/workflows/ado/package-build.yml) and [.github/workflows/ado/templates/package-build-stages.yml](.github/workflows/ado/templates/package-build-stages.yml) for the canonical example. -Shared **step sub-templates** live under `.github/workflows/ado/templates/steps/.yml` and are spliced into a job's `steps:` via `- template: steps/.yml` (path relative to the including stages template). Use these to share step sequences across stages templates that differ only in a trailing pipeline-specific step. Splicing as **steps** (not a separate job/stage) keeps job-scoped pipeline variables and on-disk files flowing to the steps that follow — a separate job would force output variables + artifact upload/download. The including job must define any job-scope variables the shared steps reference (e.g. `ob_outputDirectory`). See [.github/workflows/ado/templates/steps/common-steps.yml](.github/workflows/ado/templates/steps/common-steps.yml), shared by the `package-build` and `sources-upload` pipelines. +Shared **step sub-templates** live under `.github/workflows/ado/templates/steps/.yml` and are spliced into a job's `steps:` via `- template: steps/.yml` (path relative to the including stages template). Use these to share step sequences across stages templates that differ only in a trailing pipeline-specific step. Splicing as **steps** (not a separate job/stage) keeps job-scoped pipeline variables and on-disk files flowing to the steps that follow — a separate job would force output variables + artifact upload/download. The including job must define any job-scope variables the shared steps reference (e.g. `ob_outputDirectory`). See the granular templates under [.github/workflows/ado/templates/steps/](.github/workflows/ado/templates/steps/) (e.g. `ensure-full-history.yml`, `install-deps.yml`, `prepare-change-set.yml`), shared by the `package-build` (via `common-steps.yml`) and `pr-check-ct` pipelines. ## OneBranch templates (MANDATORY — wrapper only) diff --git a/.github/workflows/ado/package-build.yml b/.github/workflows/ado/package-build.yml index 1ab1c27a03f..7f585ef8eb9 100644 --- a/.github/workflows/ado/package-build.yml +++ b/.github/workflows/ado/package-build.yml @@ -7,15 +7,14 @@ # .github/workflows/ado/templates/package-build-stages.yml # # Authenticates via Workload Identity Federation (OIDC) and calls the Control -# Tower APIs to run the v1 post-merge delta build (stopgap until source upload -# is reworked): +# Tower APIs to run the v1 post-merge delta build: # 1. Resolve the (target, source) commit range for this push from the # previous CI build (ADO Builds API). # 2. Submit official package builds for the components that changed across # that range, via Workload Identity Federation (OIDC) to Control Tower. # -# The setup + change-detection + validation steps are shared with the -# source-upload pipeline via templates/steps/common-steps.yml. +# The setup + change-detection + validation steps are composed from the shared +# templates/steps/* step templates via templates/steps/common-steps.yml. # # Helper scripts live under: # - scripts/ci/control-tower/ - (Control Tower-specific) diff --git a/.github/workflows/ado/sources-upload.yml b/.github/workflows/ado/sources-upload.yml deleted file mode 100644 index 1c3fc1c1849..00000000000 --- a/.github/workflows/ado/sources-upload.yml +++ /dev/null @@ -1,82 +0,0 @@ -# Microsoft Corporation -# -# Wrapper pipeline — passed to ADO as the entry point for the source-upload -# pipeline. This file owns all OneBranch-specific wiring (governed templates -# repo, Official vs NonOfficial variant, featureFlags) and delegates the actual -# stages/jobs/steps to the raw stages template at: -# .github/workflows/ado/templates/sources-upload-stages.yml -# -# Authenticates via Workload Identity Federation (OIDC) and calls the Control -# Tower 'prcheck' API to upload changed component sources. prcheck returns early -# (no upload) on pull-request triggers -- unmerged code should not consume -# capacity. -# -# The setup + change-detection + validation steps are shared with the -# package-build pipeline via templates/steps/common-steps.yml. -# -# Helper scripts live under: -# - scripts/ci/control-tower/ - (Control Tower-specific) -# - scripts/ci/ado/ - (ADO API: commit-range detection) -# - scripts/ci/components/ - (cross-pipeline azldev helpers shared with the GH Actions PR gates). -# -# Prerequisites (ADO / Azure Portal): -# 1. Entra ID App Registration with audience URI -# "api://" (see variable group below). -# 2. Federated identity credential on the app registration for the ADO -# service connection (issuer: https://vstoken.dev.azure.com/, -# subject: sc:////). -# 3. ARM service connection in ADO project settings using Workload Identity -# Federation (manual). -# 4. CI trigger configured (in ADO pipeline settings) to fire on pushes to -# the target branch. -# -# Variable Group (ADO Pipelines > Library): -# Name: "ControlTower-PRCheck" -# Required variables: -# - ApiAudience : Entra ID audience URI for the Control Tower app -# - ApiBaseAFDUrl : Base URL of the Control Tower service (Azure Front Door endpoint) - -# Trigger controlled by ADO branch policy — not YAML triggers. -trigger: none - -pr: none - -resources: - repositories: - - repository: templates - type: git - name: OneBranch.Pipelines/GovernedTemplates - ref: refs/heads/main - -extends: - template: v2/OneBranch.Official.CrossPlat.yml@templates - parameters: - featureFlags: - golang: - internalModuleProxy: - enabled: true - LinuxHostVersion: - Network: R1 - runOnHost: true - EnableCDPxPAT: false - - # https://aka.ms/obpipelines/sdl - globalSdl: - disableLegacyManifest: true - sbom: - enabled: false - tsa: - enabled: false - - stages: - - template: /.github/workflows/ado/templates/sources-upload-stages.yml@self - parameters: - outputDirectory: $(Build.ArtifactStagingDirectory)/output - artifactBaseName: sourceupload - containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 - poolType: linux - serviceConnection: CT-Endpoints-Access-ServiceConnection-DEV - variableGroup: ControlTower-PRCheck - # Must exceed the script's --poll-timeout-seconds (default 7200s = 120m) - # with enough headroom for setup steps and the final API call. - timeoutInMinutes: 150 diff --git a/.github/workflows/ado/templates/package-build-stages.yml b/.github/workflows/ado/templates/package-build-stages.yml index a3cfc9ce8a2..fb922e733eb 100644 --- a/.github/workflows/ado/templates/package-build-stages.yml +++ b/.github/workflows/ado/templates/package-build-stages.yml @@ -5,9 +5,9 @@ # OneBranch-coupled knobs as parameters. The wrapper at # .github/workflows/ado/package-build.yml supplies concrete values. # -# The setup + change-detection + validation steps are shared with the -# source-upload pipeline via templates/steps/common-steps.yml; this template -# appends only the package-build-specific Control Tower call. +# The setup + change-detection + validation steps come from the shared +# templates/steps/common-steps.yml (composed from templates/steps/*); this +# template appends only the package-build-specific Control Tower call. parameters: - name: outputDirectory diff --git a/.github/workflows/ado/templates/sources-upload-stages.yml b/.github/workflows/ado/templates/sources-upload-stages.yml deleted file mode 100644 index 501f5aff297..00000000000 --- a/.github/workflows/ado/templates/sources-upload-stages.yml +++ /dev/null @@ -1,93 +0,0 @@ -# Microsoft Corporation -# -# Raw stages template for the source-upload pipeline (Control Tower prcheck). -# OneBranch-agnostic: declares the stages/jobs/steps and exposes the -# OneBranch-coupled knobs as parameters. The wrapper at -# .github/workflows/ado/sources-upload.yml supplies concrete values. -# -# The setup + change-detection + validation steps are shared with the -# package-build pipeline via templates/steps/common-steps.yml; this template -# appends only the source-upload-specific Control Tower call (prcheck). -# -# NOTE: source upload is the original Control Tower flow. It runs prcheck, which -# returns early (no upload) on pull-request triggers -- unmerged code should not -# consume capacity. This pipeline is retained for when source upload returns to -# active duty; the v1 stopgap runs builds via package-build.yml instead. - -parameters: - - name: outputDirectory - type: string - - name: artifactBaseName - type: string - - name: containerImage - type: string - - name: poolType - type: string - default: linux - - name: serviceConnection - type: string - - name: variableGroup - type: string - - name: timeoutInMinutes - type: number - -stages: - - stage: SourceUpload - jobs: - - job: SourceUpload - # Non-blocking PR check: failing steps still render red error annotations - # and surface in the build-issues view, but the job resolves to - # SucceededWithIssues and the run to PartiallySucceeded, which the Azure - # Pipelines GitHub integration reports as success to GitHub PR checks and - # the merge queue. Errors and warnings remain visible to the user. - # TODO: Remove `continueOnError` once the pipeline is stable so failures - # block PRs and the merge queue. - # ADO task: 19179 - continueOnError: true - # Must exceed the script's --poll-timeout-seconds (run_prcheck.py default - # 7200s = 120m) with enough headroom for setup steps and the final API call. - timeoutInMinutes: ${{ parameters.timeoutInMinutes }} - pool: - type: ${{ parameters.poolType }} - variables: - - group: ${{ parameters.variableGroup }} - - name: ob_outputDirectory - value: ${{ parameters.outputDirectory }} - - name: ob_artifactBaseName - value: ${{ parameters.artifactBaseName }} - - name: LinuxContainerImage - value: ${{ parameters.containerImage }} - steps: - # Shared setup + change detection + validation (PipAuthenticate, - # install deps, determine commit range, verify locks, prepare change - # set, verify rendered specs). Produces the job-scope variables and - # change-set files the prcheck step below consumes. - - template: steps/common-steps.yml - - - task: AzureCLI@2 - displayName: "Call Control Tower 'prcheck' API" - inputs: - azureSubscription: ${{ parameters.serviceConnection }} - scriptType: bash - scriptLocation: inlineScript - inlineScript: | - set -euo pipefail - - python3 scripts/ci/control-tower/run_prcheck.py \ - --api-audience "$API_AUDIENCE" \ - --api-base-url "$API_BASE_URL" \ - --build-reason "$CT_BUILD_REASON" \ - --changed-components-file "$CHANGED_COMPONENTS_FILE" \ - --source-commit "$SOURCE_COMMIT" \ - --repo-uri "$UPSTREAM_REPO_URL" - env: - API_AUDIENCE: $(ApiAudience) - API_BASE_URL: $(ApiBaseAFDUrl) - # Non-reserved name: an `env:` override of the reserved BUILD_REASON var is silently ignored by the agent. - CT_BUILD_REASON: $(Build.Reason) - CHANGED_COMPONENTS_FILE: $(changedComponentsFile) - SOURCE_COMMIT: $(sourceCommit) - # TODO: Target commit is not used. Will be needed once we move detection of affected components to CT. - # ADO task: 18816 - TARGET_COMMIT: $(targetCommit) - UPSTREAM_REPO_URL: $(Build.Repository.Uri) diff --git a/.github/workflows/ado/templates/steps/common-steps.yml b/.github/workflows/ado/templates/steps/common-steps.yml index 04e6f774a16..be4bf21d009 100644 --- a/.github/workflows/ado/templates/steps/common-steps.yml +++ b/.github/workflows/ado/templates/steps/common-steps.yml @@ -1,7 +1,7 @@ # Microsoft Corporation # -# Shared step set for the POST-MERGE Control Tower pipelines (package build + -# source upload). Spliced into each pipeline's single job via: +# Shared step set for the POST-MERGE package-build Control Tower pipeline. +# Spliced into the pipeline's single job via: # # steps: # - template: steps/common-steps.yml @@ -22,7 +22,7 @@ # after it (the default; do NOT set step-level `continueOnError` on them). # Later steps depend on earlier ones. A job-level `continueOnError` (which # only affects how the JOB result is reported, not intra-job step flow) is -# fine -- the source-upload pipeline uses one. +# fine. # # The PR check composes these same granular templates directly, with its own # commit-range-pr.yml and without the lock/render-verify steps (those are diff --git a/scripts/ci/ado/README.md b/scripts/ci/ado/README.md index 2c857b3ce56..f70f8da0d07 100644 --- a/scripts/ci/ado/README.md +++ b/scripts/ci/ado/README.md @@ -40,6 +40,6 @@ found for the fallback) exits non-zero so the calling step fails. ## Callers -- `templates/steps/common-steps.yml` "Determine source and target commit range" - step → `determine_commit_range.py` (shared by the package-build and - source-upload pipelines). +- `templates/steps/commit-range-postmerge.yml` "Determine source and target + commit range" step → `determine_commit_range.py` (used by the post-merge + package-build pipeline). diff --git a/scripts/ci/components/README.md b/scripts/ci/components/README.md index 14c1138abf0..a29ecc76ec0 100644 --- a/scripts/ci/components/README.md +++ b/scripts/ci/components/README.md @@ -1,9 +1,9 @@ # Shared azldev component helpers Pipeline-agnostic shell + Python helpers consumed by the GitHub Actions -PR gates (`.github/workflows/check-rendered-specs.yml`), the ADO -Control Tower integration pipeline -(`.github/workflows/ado/templates/sources-upload-stages.yml`), and the ADO +PR gates (`.github/workflows/check-rendered-specs.yml`), the ADO post-merge +package-build pipeline +(`.github/workflows/ado/templates/package-build-stages.yml`), and the ADO PR Control Tower check (`.github/workflows/ado/templates/pr-check-ct-stages.yml`). @@ -21,12 +21,12 @@ PR Control Tower check `AZLDEV_ALLOW_ROOT=1` prefix per [`ado-pipeline.instructions.md`](../../../.github/instructions/ado-pipeline.instructions.md). Callers do **not** set this at step scope. -- **Single source of truth.** Both pipelines should call these scripts +- **Single source of truth.** Every consumer should call these scripts rather than re-implementing the change-set computation. A regression - here breaks both gates simultaneously, so changes need extra care. + here breaks every gate simultaneously, so changes need extra care. ## Callers - `check-rendered-specs.yml` `render` job → `compute_change_set.sh` -- `sources-upload-stages.yml` "Prepare change set" step → `compute_change_set.sh` +- `package-build-stages.yml` (via `steps/prepare-change-set.yml`) → `compute_change_set.sh` - `pr-check-ct-stages.yml` (via `steps/prepare-change-set.yml`) → `compute_change_set.sh` From 1d7e83dd4dde779ca0c1418c302ad191b66dc602 Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 16:59:40 -0700 Subject: [PATCH 09/14] ci: address PR #17885 review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - run_prcheck.py: remove the `build_reason == PullRequest` short-circuit so prcheck actually runs (and uploads sources) on PR triggers. (CT-side support for PR-triggered prcheck still to be verified — see session TODO.) - pr-check-ct-stages.yml: replace the `variableGroup` parameter with explicit `apiAudience` / `apiBaseAFDUrl` string params passed by the wrapper; drop the job-scope `- group:`. The wrapper now declares the `ControlTower-PRCheck` group at root and passes `$(ApiAudience)` / `$(ApiBaseAFDUrl)`. - Drop the redundant `CT_BUILD_REASON` env var in both AzureCLI steps; use the auto-provided `$BUILD_REASON` directly. - Drop the trailing "runs first…" sentence from the prcheck step comment. - Parameterize the names of job variables the step templates set (commit-range-pr: sourceCommitVar/baseCommitVar; commit-range-postmerge: sourceCommitVar/targetCommitVar; prepare-change-set: changedComponentsFileVar/renderSetFileVar), defaults = current names. - Document the `Var` / `OutputVar` variable-naming convention in ado-pipeline.instructions.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../instructions/ado-pipeline.instructions.md | 7 +++++ .github/workflows/ado/pr-check-ct.yml | 9 ++++++- .../ado/templates/pr-check-ct-stages.yml | 26 +++++++++---------- .../steps/commit-range-postmerge.yml | 12 +++++++-- .../ado/templates/steps/commit-range-pr.yml | 12 +++++++-- .../templates/steps/prepare-change-set.yml | 10 +++++-- scripts/ci/control-tower/run_prcheck.py | 4 --- 7 files changed, 55 insertions(+), 25 deletions(-) diff --git a/.github/instructions/ado-pipeline.instructions.md b/.github/instructions/ado-pipeline.instructions.md index 1143ed6f35a..c28ccf9fae2 100644 --- a/.github/instructions/ado-pipeline.instructions.md +++ b/.github/instructions/ado-pipeline.instructions.md @@ -41,6 +41,13 @@ See [.github/workflows/ado/package-build.yml](.github/workflows/ado/package-buil Shared **step sub-templates** live under `.github/workflows/ado/templates/steps/.yml` and are spliced into a job's `steps:` via `- template: steps/.yml` (path relative to the including stages template). Use these to share step sequences across stages templates that differ only in a trailing pipeline-specific step. Splicing as **steps** (not a separate job/stage) keeps job-scoped pipeline variables and on-disk files flowing to the steps that follow — a separate job would force output variables + artifact upload/download. The including job must define any job-scope variables the shared steps reference (e.g. `ob_outputDirectory`). See the granular templates under [.github/workflows/ado/templates/steps/](.github/workflows/ado/templates/steps/) (e.g. `ensure-full-history.yml`, `install-deps.yml`, `prepare-change-set.yml`), shared by the `package-build` (via `common-steps.yml`) and `pr-check-ct` pipelines. +**Parameterize the names of pipeline variables a template sets.** If a step template sets a job variable (`##vso[task.setvariable variable=...]`), expose `` through a parameter (with the conventional name as the default) so a caller composing several templates can rename it to avoid collisions. Use a consistent suffix scheme: + +- **Same-job variables** (default `##vso[task.setvariable]`, no `isoutput`): parameter `Var` (e.g. `sourceCommitVar`, `changedComponentsFileVar`). +- **Output variables** (`isoutput=true`, visible in other jobs/stages): parameter `OutputVar`, AND the setting task's `name:` must also be parameterized (a cross-job output variable is referenced as `.`, so the task name is part of the contract). + +Reading another template's variable follows the same shape: take a `Var` parameter and reference it as `$(${{ parameters.Var }})` in `env:` (e.g. `prepare-change-set.yml`'s `fromCommitVar`/`toCommitVar`). + ## OneBranch templates (MANDATORY — wrapper only) The rules in this section apply to the **wrapper** file. The raw stages template MUST NOT reference OneBranch at all. diff --git a/.github/workflows/ado/pr-check-ct.yml b/.github/workflows/ado/pr-check-ct.yml index d4bc9213d70..f45b9a66dd6 100644 --- a/.github/workflows/ado/pr-check-ct.yml +++ b/.github/workflows/ado/pr-check-ct.yml @@ -57,6 +57,12 @@ trigger: none pr: none +# Source the Control Tower API audience + base URL from the variable group here, +# in the wrapper (the env-specific entry point), and pass them to the stages +# template as parameters. The stages template stays variable-group-agnostic. +variables: + - group: ControlTower-PRCheck + resources: repositories: - repository: templates @@ -92,7 +98,8 @@ extends: containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 poolType: linux serviceConnection: CT-Endpoints-Access-ServiceConnection-DEV - variableGroup: ControlTower-PRCheck + apiAudience: $(ApiAudience) + apiBaseAFDUrl: $(ApiBaseAFDUrl) # Control Tower package target for the 4.0 branch. packageTarget: azl4 # This check WAITS for the Control Tower build to finish (pass/fail), diff --git a/.github/workflows/ado/templates/pr-check-ct-stages.yml b/.github/workflows/ado/templates/pr-check-ct-stages.yml index 02d84e0d5c5..4b7a94b20b9 100644 --- a/.github/workflows/ado/templates/pr-check-ct-stages.yml +++ b/.github/workflows/ado/templates/pr-check-ct-stages.yml @@ -46,7 +46,11 @@ parameters: default: linux - name: serviceConnection type: string - - name: variableGroup + # Control Tower API audience + base URL, passed by the wrapper (which owns the + # variable group / env-specific values). The template stays group-agnostic. + - name: apiAudience + type: string + - name: apiBaseAFDUrl type: string # Control Tower package target for builds submitted from this pipeline # (e.g. azl4 for the 4.0 branch, azl5 for 5.0). Bound per-branch by the @@ -80,7 +84,6 @@ stages: pool: type: ${{ parameters.poolType }} variables: - - group: ${{ parameters.variableGroup }} - name: ob_outputDirectory value: ${{ parameters.outputDirectory }} - name: ob_artifactBaseName @@ -110,8 +113,7 @@ stages: # Call Control Tower 'prcheck' BEFORE the build. Among other things, # prcheck uploads the missing lookaside sources for the changed # components, so they are available when the scratch build below - # fetches them. It runs first for exactly that reason -- the build - # depends on the sources prcheck uploads. + # fetches them. - task: AzureCLI@2 displayName: "Call Control Tower 'prcheck' API" inputs: @@ -124,15 +126,13 @@ stages: python3 scripts/ci/control-tower/run_prcheck.py \ --api-audience "$API_AUDIENCE" \ --api-base-url "$API_BASE_URL" \ - --build-reason "$CT_BUILD_REASON" \ + --build-reason "$BUILD_REASON" \ --changed-components-file "$CHANGED_COMPONENTS_FILE" \ --source-commit "$SOURCE_COMMIT" \ --repo-uri "$UPSTREAM_REPO_URL" env: - API_AUDIENCE: $(ApiAudience) - API_BASE_URL: $(ApiBaseAFDUrl) - # Non-reserved name: an `env:` override of the reserved BUILD_REASON var is silently ignored by the agent. - CT_BUILD_REASON: $(Build.Reason) + API_AUDIENCE: ${{ parameters.apiAudience }} + API_BASE_URL: ${{ parameters.apiBaseAFDUrl }} CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) # TODO: Base commit is not used yet. Will be needed once we move @@ -170,7 +170,7 @@ stages: python3 scripts/ci/control-tower/run_package_build.py \ --api-audience "$API_AUDIENCE" \ --api-base-url "$API_BASE_URL" \ - --build-reason "$CT_BUILD_REASON" \ + --build-reason "$BUILD_REASON" \ --changed-components-file "$CHANGED_COMPONENTS_FILE" \ --package-target "${{ parameters.packageTarget }}" \ --commit-sha "$SOURCE_COMMIT" \ @@ -178,10 +178,8 @@ stages: --wait-for-completion \ --poll-timeout-seconds ${{ parameters.pollTimeoutSeconds }} env: - API_AUDIENCE: $(ApiAudience) - API_BASE_URL: $(ApiBaseAFDUrl) - # Non-reserved name: an `env:` override of the reserved BUILD_REASON var is silently ignored by the agent. - CT_BUILD_REASON: $(Build.Reason) + API_AUDIENCE: ${{ parameters.apiAudience }} + API_BASE_URL: ${{ parameters.apiBaseAFDUrl }} CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) diff --git a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml index 3af2a3cc9f3..4e3982d107f 100644 --- a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml +++ b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml @@ -13,6 +13,14 @@ # # Emits job-scope variables sourceCommit and targetCommit. +parameters: + - name: sourceCommitVar + type: string + default: sourceCommit + - name: targetCommitVar + type: string + default: targetCommit + steps: - script: | set -euo pipefail @@ -50,8 +58,8 @@ steps: # Mark the resolved range readonly: it is computed once here and only # consumed downstream, so locking it prevents a later step from clobbering # the commit range (matches changedComponentsFile / renderSetFile below). - echo "##vso[task.setvariable variable=sourceCommit;isreadonly=true]$source_commit" - echo "##vso[task.setvariable variable=targetCommit;isreadonly=true]$target_commit" + echo "##vso[task.setvariable variable=${{ parameters.sourceCommitVar }};isreadonly=true]$source_commit" + echo "##vso[task.setvariable variable=${{ parameters.targetCommitVar }};isreadonly=true]$target_commit" env: SYSTEM_COLLECTIONURI: $(System.CollectionUri) SYSTEM_TEAMPROJECT: $(System.TeamProject) diff --git a/.github/workflows/ado/templates/steps/commit-range-pr.yml b/.github/workflows/ado/templates/steps/commit-range-pr.yml index 48b65e60ccc..2a26ef59d52 100644 --- a/.github/workflows/ado/templates/steps/commit-range-pr.yml +++ b/.github/workflows/ado/templates/steps/commit-range-pr.yml @@ -14,6 +14,14 @@ # merge base). These feed prepare-change-set.yml (from=baseCommit, # to=sourceCommit) and the trailing Control Tower call. +parameters: + - name: sourceCommitVar + type: string + default: sourceCommit + - name: baseCommitVar + type: string + default: baseCommit + steps: # Fail fast, at compile time, when this is not a PR build: the merge-commit # parent logic below only makes sense for a PullRequest trigger. Including the @@ -47,6 +55,6 @@ steps: fi done echo "Resolved range: base(merge-base)=$base_commit source=$source_commit" - echo "##vso[task.setvariable variable=sourceCommit;isreadonly=true]$source_commit" - echo "##vso[task.setvariable variable=baseCommit;isreadonly=true]$base_commit" + echo "##vso[task.setvariable variable=${{ parameters.sourceCommitVar }};isreadonly=true]$source_commit" + echo "##vso[task.setvariable variable=${{ parameters.baseCommitVar }};isreadonly=true]$base_commit" displayName: "Determine PR commit range" diff --git a/.github/workflows/ado/templates/steps/prepare-change-set.yml b/.github/workflows/ado/templates/steps/prepare-change-set.yml index 3ee3cfffa2c..38ae738b7e3 100644 --- a/.github/workflows/ado/templates/steps/prepare-change-set.yml +++ b/.github/workflows/ado/templates/steps/prepare-change-set.yml @@ -26,6 +26,12 @@ parameters: - name: toCommitVar type: string default: sourceCommit + - name: changedComponentsFileVar + type: string + default: changedComponentsFile + - name: renderSetFileVar + type: string + default: renderSetFile steps: - script: | @@ -62,8 +68,8 @@ steps: # Publish the downstream pipeline variables now that the change set is # ready (consumed by the render-verify step and the trailing Control # Tower call). - echo "##vso[task.setvariable variable=changedComponentsFile;isreadonly=true]$json_file" - echo "##vso[task.setvariable variable=renderSetFile;isreadonly=true]$render_set_file" + echo "##vso[task.setvariable variable=${{ parameters.changedComponentsFileVar }};isreadonly=true]$json_file" + echo "##vso[task.setvariable variable=${{ parameters.renderSetFileVar }};isreadonly=true]$render_set_file" env: FROM_COMMIT: $(${{ parameters.fromCommitVar }}) TO_COMMIT: $(${{ parameters.toCommitVar }}) diff --git a/scripts/ci/control-tower/run_prcheck.py b/scripts/ci/control-tower/run_prcheck.py index 20aaef5090d..f5f3982bca8 100644 --- a/scripts/ci/control-tower/run_prcheck.py +++ b/scripts/ci/control-tower/run_prcheck.py @@ -167,10 +167,6 @@ def main() -> None: print("Payload:") print(json.dumps(payload, indent=2)) - if args.build_reason == "PullRequest": - print("Skipping Control Tower call - pull request triggers are not supported, yet.") - return - if not components: print("No affected components detected between source and target commits; skipping Control Tower call.") return From a55d2f1623b4dedecd283e26700b404edcdd14a7 Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 18:18:20 -0700 Subject: [PATCH 10/14] ci: apply variableGroup/CT_BUILD_REASON + variable conventions uniformly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the second round of PR #17885 review comments and apply the same patterns across all ADO YAMLs (package-build as well as pr-check-ct): - Drop the `variableGroup` parameter from both stages templates; take `apiAudience`/`apiBaseAFDUrl` params and surface them (plus packageTarget / pollTimeoutSeconds) in the job `variables:` section so a caller may pass runtime $[ ] / $(macro) expressions. Both wrappers now declare the ControlTower-PRCheck group at root and pass $(ApiAudience)/$(ApiBaseAFDUrl). - Drop the redundant CT_BUILD_REASON everywhere; use the auto-provided $BUILD_REASON. - Route every pipeline parameter/variable used by a script through `env:` (setvariable name params, packageTarget, pollTimeoutSeconds, build reason) — no `${{ parameters }}`/`$(var)` inlined in script bodies. - Trim the wrapper variable-group comments to name only the extracted variables. AI instructions (ado-pipeline.instructions.md): - `OutputVarTask` for the output-variable task-name parameter. - New rule: pass pipeline params/vars into scripts through `env:`. - New rule: templates that own a `variables:` section surface runtime-consumed params as variables (runtime $[ ] expressions only evaluate there). - Update the worked wrapper/stages example to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../instructions/ado-pipeline.instructions.md | 28 +++++++++++++++---- .github/workflows/ado/package-build.yml | 7 ++++- .github/workflows/ado/pr-check-ct.yml | 4 +-- .../ado/templates/package-build-stages.yml | 26 +++++++++++------ .../ado/templates/pr-check-ct-stages.yml | 24 ++++++++++++---- .../steps/commit-range-postmerge.yml | 6 ++-- .../ado/templates/steps/commit-range-pr.yml | 9 ++++-- .../templates/steps/prepare-change-set.yml | 6 ++-- 8 files changed, 79 insertions(+), 31 deletions(-) diff --git a/.github/instructions/ado-pipeline.instructions.md b/.github/instructions/ado-pipeline.instructions.md index c28ccf9fae2..421f9bf3882 100644 --- a/.github/instructions/ado-pipeline.instructions.md +++ b/.github/instructions/ado-pipeline.instructions.md @@ -44,10 +44,14 @@ Shared **step sub-templates** live under `.github/workflows/ado/templates/steps/ **Parameterize the names of pipeline variables a template sets.** If a step template sets a job variable (`##vso[task.setvariable variable=...]`), expose `` through a parameter (with the conventional name as the default) so a caller composing several templates can rename it to avoid collisions. Use a consistent suffix scheme: - **Same-job variables** (default `##vso[task.setvariable]`, no `isoutput`): parameter `Var` (e.g. `sourceCommitVar`, `changedComponentsFileVar`). -- **Output variables** (`isoutput=true`, visible in other jobs/stages): parameter `OutputVar`, AND the setting task's `name:` must also be parameterized (a cross-job output variable is referenced as `.`, so the task name is part of the contract). +- **Output variables** (`isoutput=true`, visible in other jobs/stages): parameter `OutputVar` for the variable name, AND parameter `OutputVarTask` for the setting task's `name:` (a cross-job output variable is referenced as `.`, so the task name is part of the contract). Reading another template's variable follows the same shape: take a `Var` parameter and reference it as `$(${{ parameters.Var }})` in `env:` (e.g. `prepare-change-set.yml`'s `fromCommitVar`/`toCommitVar`). +**Pass pipeline parameters and variables into scripts through `env:`.** Do not inline `$(someVariable)` or `${{ parameters.X }}` inside an `inlineScript`/`script` body — map them in the step's `env:` block and reference the environment variable in the script. This keeps the script readable, avoids quoting/injection pitfalls, and makes the data flow explicit. This also covers values used to build a `##vso[...]` logging command: route the parameter through `env:` and reference `$ENV_VAR` in the `echo` rather than substituting `${{ parameters.X }}` inline. + +**Surface runtime-consumed parameters as `variables` when the template owns a `variables` section.** When a template defines its own `variables` section (i.e. it declares stages/jobs), convert the input parameters it consumes at runtime into entries in that `variables` section (`- name: foo` / `value: ${{ parameters.foo }}`) and reference them as `$(foo)`. A parameter value may be a runtime `$[ ]` (or `$(macro)`) expression, which is only evaluated inside the `variables` section — referencing `${{ parameters.foo }}` directly elsewhere would emit the literal, unevaluated string. Parameters used only in compile-time structural positions (e.g. `pool.type`, `timeoutInMinutes`, task `name:`/`azureSubscription:`) stay as parameters. + ## OneBranch templates (MANDATORY — wrapper only) The rules in this section apply to the **wrapper** file. The raw stages template MUST NOT reference OneBranch at all. @@ -246,6 +250,10 @@ Use these as a starting point for a new pipeline. Two files: a wrapper (NonOffic trigger: none pr: none +# Variables extracted from this group: ApiAudience, ApiBaseUrl. +variables: + - group: + resources: repositories: - repository: templates @@ -268,7 +276,8 @@ extends: containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 poolType: linux serviceConnection: - variableGroup: + apiAudience: $(ApiAudience) + apiBaseUrl: $(ApiBaseUrl) timeoutInMinutes: # explicit, conservative ``` @@ -287,7 +296,9 @@ parameters: default: linux - name: serviceConnection type: string - - name: variableGroup + - name: apiAudience + type: string + - name: apiBaseUrl type: string - name: timeoutInMinutes type: number @@ -300,13 +311,18 @@ stages: pool: type: ${{ parameters.poolType }} variables: - - group: ${{ parameters.variableGroup }} # audience URI, base URL, etc. - name: ob_outputDirectory value: ${{ parameters.outputDirectory }} - name: ob_artifactBaseName value: ${{ parameters.artifactBaseName }} - name: LinuxContainerImage value: ${{ parameters.containerImage }} + # Runtime-consumed params surfaced as variables (see the variable + # rules above); reference as $(apiAudience) / $(apiBaseUrl). + - name: apiAudience + value: ${{ parameters.apiAudience }} + - name: apiBaseUrl + value: ${{ parameters.apiBaseUrl }} steps: - task: PipAuthenticate@1 displayName: "Authenticate pip" @@ -325,8 +341,8 @@ stages: --api-audience "$API_AUDIENCE" \ --api-base-url "$API_BASE_URL" env: - API_AUDIENCE: $(ApiAudience) - API_BASE_URL: $(ApiBaseUrl) + API_AUDIENCE: $(apiAudience) + API_BASE_URL: $(apiBaseUrl) ``` Replace every `<...>` placeholder. diff --git a/.github/workflows/ado/package-build.yml b/.github/workflows/ado/package-build.yml index 7f585ef8eb9..ad625de6351 100644 --- a/.github/workflows/ado/package-build.yml +++ b/.github/workflows/ado/package-build.yml @@ -43,6 +43,10 @@ trigger: none pr: none +# Variables extracted from this group: ApiAudience, ApiBaseAFDUrl. +variables: + - group: ControlTower-PRCheck + resources: repositories: - repository: templates @@ -78,7 +82,8 @@ extends: containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 poolType: linux serviceConnection: CT-Endpoints-Access-ServiceConnection-DEV - variableGroup: ControlTower-PRCheck + apiAudience: $(ApiAudience) + apiBaseAFDUrl: $(ApiBaseAFDUrl) # Control Tower package target for the 4.0 branch. packageTarget: azl4 # Must exceed the script's --poll-timeout-seconds (default 600s = 10m) diff --git a/.github/workflows/ado/pr-check-ct.yml b/.github/workflows/ado/pr-check-ct.yml index f45b9a66dd6..866a57260f2 100644 --- a/.github/workflows/ado/pr-check-ct.yml +++ b/.github/workflows/ado/pr-check-ct.yml @@ -57,9 +57,7 @@ trigger: none pr: none -# Source the Control Tower API audience + base URL from the variable group here, -# in the wrapper (the env-specific entry point), and pass them to the stages -# template as parameters. The stages template stays variable-group-agnostic. +# Variables extracted from this group: ApiAudience, ApiBaseAFDUrl. variables: - group: ControlTower-PRCheck diff --git a/.github/workflows/ado/templates/package-build-stages.yml b/.github/workflows/ado/templates/package-build-stages.yml index fb922e733eb..b4636f41f63 100644 --- a/.github/workflows/ado/templates/package-build-stages.yml +++ b/.github/workflows/ado/templates/package-build-stages.yml @@ -21,7 +21,11 @@ parameters: default: linux - name: serviceConnection type: string - - name: variableGroup + # Control Tower API audience + base URL, passed by the wrapper (which owns the + # variable group / env-specific values). The template stays group-agnostic. + - name: apiAudience + type: string + - name: apiBaseAFDUrl type: string - name: timeoutInMinutes type: number @@ -47,13 +51,20 @@ stages: pool: type: ${{ parameters.poolType }} variables: - - group: ${{ parameters.variableGroup }} - name: ob_outputDirectory value: ${{ parameters.outputDirectory }} - name: ob_artifactBaseName value: ${{ parameters.artifactBaseName }} - name: LinuxContainerImage value: ${{ parameters.containerImage }} + # Runtime-consumed params surfaced as variables so a caller may pass + # runtime $[ ] / $(macro) expressions (evaluated here, not inline). + - name: apiAudience + value: ${{ parameters.apiAudience }} + - name: apiBaseAFDUrl + value: ${{ parameters.apiBaseAFDUrl }} + - name: packageTarget + value: ${{ parameters.packageTarget }} steps: # Shared setup + change detection + validation (PipAuthenticate, # install deps, determine commit range, verify locks, prepare change @@ -73,17 +84,16 @@ stages: python3 scripts/ci/control-tower/run_package_build.py \ --api-audience "$API_AUDIENCE" \ --api-base-url "$API_BASE_URL" \ - --build-reason "$CT_BUILD_REASON" \ + --build-reason "$BUILD_REASON" \ --changed-components-file "$CHANGED_COMPONENTS_FILE" \ - --package-target "${{ parameters.packageTarget }}" \ + --package-target "$PACKAGE_TARGET" \ --official-build \ --commit-sha "$SOURCE_COMMIT" \ --repo-uri "$UPSTREAM_REPO_URL" env: - API_AUDIENCE: $(ApiAudience) - API_BASE_URL: $(ApiBaseAFDUrl) - # Non-reserved name: an `env:` override of the reserved BUILD_REASON var is silently ignored by the agent. - CT_BUILD_REASON: $(Build.Reason) + API_AUDIENCE: $(apiAudience) + API_BASE_URL: $(apiBaseAFDUrl) + PACKAGE_TARGET: $(packageTarget) CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) diff --git a/.github/workflows/ado/templates/pr-check-ct-stages.yml b/.github/workflows/ado/templates/pr-check-ct-stages.yml index 4b7a94b20b9..d40ac6dd464 100644 --- a/.github/workflows/ado/templates/pr-check-ct-stages.yml +++ b/.github/workflows/ado/templates/pr-check-ct-stages.yml @@ -90,6 +90,16 @@ stages: value: ${{ parameters.artifactBaseName }} - name: LinuxContainerImage value: ${{ parameters.containerImage }} + # Runtime-consumed params surfaced as variables so a caller may pass + # runtime $[ ] / $(macro) expressions (evaluated here, not inline). + - name: apiAudience + value: ${{ parameters.apiAudience }} + - name: apiBaseAFDUrl + value: ${{ parameters.apiBaseAFDUrl }} + - name: packageTarget + value: ${{ parameters.packageTarget }} + - name: pollTimeoutSeconds + value: ${{ parameters.pollTimeoutSeconds }} steps: # Shared setup + change detection, composed from the granular step # templates. install-deps skips mock and the ADO API client (neither @@ -131,8 +141,8 @@ stages: --source-commit "$SOURCE_COMMIT" \ --repo-uri "$UPSTREAM_REPO_URL" env: - API_AUDIENCE: ${{ parameters.apiAudience }} - API_BASE_URL: ${{ parameters.apiBaseAFDUrl }} + API_AUDIENCE: $(apiAudience) + API_BASE_URL: $(apiBaseAFDUrl) CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) # TODO: Base commit is not used yet. Will be needed once we move @@ -172,14 +182,16 @@ stages: --api-base-url "$API_BASE_URL" \ --build-reason "$BUILD_REASON" \ --changed-components-file "$CHANGED_COMPONENTS_FILE" \ - --package-target "${{ parameters.packageTarget }}" \ + --package-target "$PACKAGE_TARGET" \ --commit-sha "$SOURCE_COMMIT" \ --repo-uri "$UPSTREAM_REPO_URL" \ --wait-for-completion \ - --poll-timeout-seconds ${{ parameters.pollTimeoutSeconds }} + --poll-timeout-seconds "$POLL_TIMEOUT_SECONDS" env: - API_AUDIENCE: ${{ parameters.apiAudience }} - API_BASE_URL: ${{ parameters.apiBaseAFDUrl }} + API_AUDIENCE: $(apiAudience) + API_BASE_URL: $(apiBaseAFDUrl) + PACKAGE_TARGET: $(packageTarget) + POLL_TIMEOUT_SECONDS: $(pollTimeoutSeconds) CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) diff --git a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml index 4e3982d107f..1afd21c9023 100644 --- a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml +++ b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml @@ -58,9 +58,11 @@ steps: # Mark the resolved range readonly: it is computed once here and only # consumed downstream, so locking it prevents a later step from clobbering # the commit range (matches changedComponentsFile / renderSetFile below). - echo "##vso[task.setvariable variable=${{ parameters.sourceCommitVar }};isreadonly=true]$source_commit" - echo "##vso[task.setvariable variable=${{ parameters.targetCommitVar }};isreadonly=true]$target_commit" + echo "##vso[task.setvariable variable=$SOURCE_COMMIT_VAR;isreadonly=true]$source_commit" + echo "##vso[task.setvariable variable=$TARGET_COMMIT_VAR;isreadonly=true]$target_commit" env: + SOURCE_COMMIT_VAR: ${{ parameters.sourceCommitVar }} + TARGET_COMMIT_VAR: ${{ parameters.targetCommitVar }} SYSTEM_COLLECTIONURI: $(System.CollectionUri) SYSTEM_TEAMPROJECT: $(System.TeamProject) SYSTEM_ACCESSTOKEN: $(System.AccessToken) diff --git a/.github/workflows/ado/templates/steps/commit-range-pr.yml b/.github/workflows/ado/templates/steps/commit-range-pr.yml index 2a26ef59d52..cfae3381c3f 100644 --- a/.github/workflows/ado/templates/steps/commit-range-pr.yml +++ b/.github/workflows/ado/templates/steps/commit-range-pr.yml @@ -29,7 +29,7 @@ steps: # mis-triggered run stops at its very first step. - ${{ if ne(variables['Build.Reason'], 'PullRequest') }}: - script: | - echo "##[error]This pipeline must run as a PR build (Build.Reason=PullRequest); got '$(Build.Reason)'." + echo "##[error]This pipeline must run as a PR build (Build.Reason=PullRequest); got '$BUILD_REASON'." exit 1 displayName: "Guard: PR build only" @@ -55,6 +55,9 @@ steps: fi done echo "Resolved range: base(merge-base)=$base_commit source=$source_commit" - echo "##vso[task.setvariable variable=${{ parameters.sourceCommitVar }};isreadonly=true]$source_commit" - echo "##vso[task.setvariable variable=${{ parameters.baseCommitVar }};isreadonly=true]$base_commit" + echo "##vso[task.setvariable variable=$SOURCE_COMMIT_VAR;isreadonly=true]$source_commit" + echo "##vso[task.setvariable variable=$BASE_COMMIT_VAR;isreadonly=true]$base_commit" + env: + SOURCE_COMMIT_VAR: ${{ parameters.sourceCommitVar }} + BASE_COMMIT_VAR: ${{ parameters.baseCommitVar }} displayName: "Determine PR commit range" diff --git a/.github/workflows/ado/templates/steps/prepare-change-set.yml b/.github/workflows/ado/templates/steps/prepare-change-set.yml index 38ae738b7e3..c25a9829f14 100644 --- a/.github/workflows/ado/templates/steps/prepare-change-set.yml +++ b/.github/workflows/ado/templates/steps/prepare-change-set.yml @@ -68,9 +68,11 @@ steps: # Publish the downstream pipeline variables now that the change set is # ready (consumed by the render-verify step and the trailing Control # Tower call). - echo "##vso[task.setvariable variable=${{ parameters.changedComponentsFileVar }};isreadonly=true]$json_file" - echo "##vso[task.setvariable variable=${{ parameters.renderSetFileVar }};isreadonly=true]$render_set_file" + echo "##vso[task.setvariable variable=$CHANGED_COMPONENTS_FILE_VAR;isreadonly=true]$json_file" + echo "##vso[task.setvariable variable=$RENDER_SET_FILE_VAR;isreadonly=true]$render_set_file" env: FROM_COMMIT: $(${{ parameters.fromCommitVar }}) TO_COMMIT: $(${{ parameters.toCommitVar }}) + CHANGED_COMPONENTS_FILE_VAR: ${{ parameters.changedComponentsFileVar }} + RENDER_SET_FILE_VAR: ${{ parameters.renderSetFileVar }} displayName: "Prepare change set" From 94b8223397e0cfa4a62bfcd552e61af6bcc8fc95 Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Wed, 1 Jul 2026 18:41:41 -0700 Subject: [PATCH 11/14] ci: restore BUILD_REASON caveat; drop caller-origin comments from templates Third round of PR #17885 review comments: - Restore a short note (both stages templates' env blocks) that Build.Reason is auto-exposed as $BUILD_REASON and must NOT be added to env: -- the agent drops env: overrides of that reserved predefined variable. Also documented as an exception to the "pass params through env:" rule in the ADO instructions. - Remove comments that assumed the caller's intent / parameter value origin (the "passed by the wrapper / owns the variable group" and "surfaced as variables so a caller may pass..." notes) from the stages and step templates; the calling convention lives in the ADO instructions, not in every YAML. Reworded install-deps.yml / prepare-change-set.yml param docs to be caller-agnostic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../instructions/ado-pipeline.instructions.md | 2 +- .../ado/templates/package-build-stages.yml | 10 +++---- .../ado/templates/pr-check-ct-stages.yml | 26 +++++++++---------- .../ado/templates/steps/install-deps.yml | 14 +++++----- .../templates/steps/prepare-change-set.yml | 5 ++-- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/.github/instructions/ado-pipeline.instructions.md b/.github/instructions/ado-pipeline.instructions.md index 421f9bf3882..d8e434f65b8 100644 --- a/.github/instructions/ado-pipeline.instructions.md +++ b/.github/instructions/ado-pipeline.instructions.md @@ -48,7 +48,7 @@ Shared **step sub-templates** live under `.github/workflows/ado/templates/steps/ Reading another template's variable follows the same shape: take a `Var` parameter and reference it as `$(${{ parameters.Var }})` in `env:` (e.g. `prepare-change-set.yml`'s `fromCommitVar`/`toCommitVar`). -**Pass pipeline parameters and variables into scripts through `env:`.** Do not inline `$(someVariable)` or `${{ parameters.X }}` inside an `inlineScript`/`script` body — map them in the step's `env:` block and reference the environment variable in the script. This keeps the script readable, avoids quoting/injection pitfalls, and makes the data flow explicit. This also covers values used to build a `##vso[...]` logging command: route the parameter through `env:` and reference `$ENV_VAR` in the `echo` rather than substituting `${{ parameters.X }}` inline. +**Pass pipeline parameters and variables into scripts through `env:`.** Do not inline `$(someVariable)` or `${{ parameters.X }}` inside an `inlineScript`/`script` body — map them in the step's `env:` block and reference the environment variable in the script. This keeps the script readable, avoids quoting/injection pitfalls, and makes the data flow explicit. This also covers values used to build a `##vso[...]` logging command: route the parameter through `env:` and reference `$ENV_VAR` in the `echo` rather than substituting `${{ parameters.X }}` inline. **Exception — reserved predefined variables:** ADO auto-exposes reserved predefined variables (e.g. `Build.Reason`) to scripts as their env equivalents (`$BUILD_REASON`) and silently drops an `env:` entry that shadows one, so reference the auto-exposed `$NAME` directly and do NOT add it to `env:` (a short inline comment noting this prevents a maintainer from "completing" the block). **Surface runtime-consumed parameters as `variables` when the template owns a `variables` section.** When a template defines its own `variables` section (i.e. it declares stages/jobs), convert the input parameters it consumes at runtime into entries in that `variables` section (`- name: foo` / `value: ${{ parameters.foo }}`) and reference them as `$(foo)`. A parameter value may be a runtime `$[ ]` (or `$(macro)`) expression, which is only evaluated inside the `variables` section — referencing `${{ parameters.foo }}` directly elsewhere would emit the literal, unevaluated string. Parameters used only in compile-time structural positions (e.g. `pool.type`, `timeoutInMinutes`, task `name:`/`azureSubscription:`) stay as parameters. diff --git a/.github/workflows/ado/templates/package-build-stages.yml b/.github/workflows/ado/templates/package-build-stages.yml index b4636f41f63..faab75b30c1 100644 --- a/.github/workflows/ado/templates/package-build-stages.yml +++ b/.github/workflows/ado/templates/package-build-stages.yml @@ -21,8 +21,6 @@ parameters: default: linux - name: serviceConnection type: string - # Control Tower API audience + base URL, passed by the wrapper (which owns the - # variable group / env-specific values). The template stays group-agnostic. - name: apiAudience type: string - name: apiBaseAFDUrl @@ -30,8 +28,7 @@ parameters: - name: timeoutInMinutes type: number # Control Tower package target for builds submitted from this pipeline - # (e.g. azl4 for the 4.0 branch, azl5 for 5.0). Bound per-branch by the - # wrapper so a branch's builds land in the correct target. + # (e.g. azl4 for the 4.0 branch, azl5 for 5.0). - name: packageTarget type: string @@ -57,8 +54,6 @@ stages: value: ${{ parameters.artifactBaseName }} - name: LinuxContainerImage value: ${{ parameters.containerImage }} - # Runtime-consumed params surfaced as variables so a caller may pass - # runtime $[ ] / $(macro) expressions (evaluated here, not inline). - name: apiAudience value: ${{ parameters.apiAudience }} - name: apiBaseAFDUrl @@ -97,3 +92,6 @@ stages: CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) + # Build.Reason is auto-exposed to the script as $BUILD_REASON; do + # not add a BUILD_REASON entry here -- the agent silently ignores + # env: overrides of that reserved predefined variable. diff --git a/.github/workflows/ado/templates/pr-check-ct-stages.yml b/.github/workflows/ado/templates/pr-check-ct-stages.yml index d40ac6dd464..9847290a5bb 100644 --- a/.github/workflows/ado/templates/pr-check-ct-stages.yml +++ b/.github/workflows/ado/templates/pr-check-ct-stages.yml @@ -31,8 +31,7 @@ # gates already run in GitHub Actions. # # Because it calls Control Tower, this pipeline needs the WIF service connection -# and the Control Tower variable group (audience + base URL); the wrapper -# supplies both as parameters. +# and the Control Tower API audience + base URL, supplied as parameters. parameters: - name: outputDirectory @@ -46,24 +45,19 @@ parameters: default: linux - name: serviceConnection type: string - # Control Tower API audience + base URL, passed by the wrapper (which owns the - # variable group / env-specific values). The template stays group-agnostic. - name: apiAudience type: string - name: apiBaseAFDUrl type: string # Control Tower package target for builds submitted from this pipeline - # (e.g. azl4 for the 4.0 branch, azl5 for 5.0). Bound per-branch by the - # wrapper so a branch's builds land in the correct target. + # (e.g. azl4 for the 4.0 branch, azl5 for 5.0). - name: packageTarget type: string - name: timeoutInMinutes type: number # Max seconds run_package_build.py waits for the Control Tower build to reach - # a terminal state. Keep below the job's timeoutInMinutes (above) so the - # script's own clear failure fires before ADO blunt-kills the job. Default - # 21600 = 6h (our worst-case build); the wrapper passes it alongside - # timeoutInMinutes so the two are raised together. + # a terminal state. Keep below the job's timeoutInMinutes so the script's own + # clear failure fires before ADO blunt-kills the job. Default 21600 = 6h. - name: pollTimeoutSeconds type: number default: 21600 @@ -90,8 +84,6 @@ stages: value: ${{ parameters.artifactBaseName }} - name: LinuxContainerImage value: ${{ parameters.containerImage }} - # Runtime-consumed params surfaced as variables so a caller may pass - # runtime $[ ] / $(macro) expressions (evaluated here, not inline). - name: apiAudience value: ${{ parameters.apiAudience }} - name: apiBaseAFDUrl @@ -149,6 +141,9 @@ stages: # detection of affected components to CT. ADO task: 18816 BASE_COMMIT: $(baseCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) + # Build.Reason is auto-exposed to the script as $BUILD_REASON; do + # not add a BUILD_REASON entry here -- the agent silently ignores + # env: overrides of that reserved predefined variable. # Submit a SCRATCH Control Tower build of the PR head for the changed # components. Scratch = throwaway: it never persists to a production @@ -175,8 +170,8 @@ stages: # --poll-timeout-seconds comes from the pollTimeoutSeconds # parameter (6h default = our worst-case build). Keep it below - # the job's timeoutInMinutes (wrapper) so the script's own clear - # failure fires before ADO blunt-kills the job. + # the job's timeoutInMinutes so the script's own clear failure + # fires before ADO blunt-kills the job. python3 scripts/ci/control-tower/run_package_build.py \ --api-audience "$API_AUDIENCE" \ --api-base-url "$API_BASE_URL" \ @@ -195,3 +190,6 @@ stages: CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) + # Build.Reason is auto-exposed to the script as $BUILD_REASON; do + # not add a BUILD_REASON entry here -- the agent silently ignores + # env: overrides of that reserved predefined variable. diff --git a/.github/workflows/ado/templates/steps/install-deps.yml b/.github/workflows/ado/templates/steps/install-deps.yml index 226b78bb27c..f5ebd2fbee7 100644 --- a/.github/workflows/ado/templates/steps/install-deps.yml +++ b/.github/workflows/ado/templates/steps/install-deps.yml @@ -3,15 +3,13 @@ # Step template: authenticate to the internal pip feed and install the host # tools the Control Tower change-detection + submission steps need. azldev is # always installed (its version comes from the committed .azldev-version and is -# validated before use); everything else is opt-in so each pipeline pulls only -# what it uses: -# installMock - mock + rpmautospec, needed by the post-merge -# pipelines' lock/render steps. The PR check does -# read-only change detection and does not need it. -# installAdoRequirements - the azure-devops SDK used by the post-merge -# commit-range helper (determine_commit_range.py). +# validated before use); everything else is opt-in: +# installMock - install mock + rpmautospec (needed for lock/render +# steps); leave off for change-detection-only jobs. +# installAdoRequirements - install the azure-devops SDK used by +# determine_commit_range.py (post-merge delta helper). # normalizeGoGitConfig - strip extensions.worktreeConfig so azldev's go-git -# can open the ADO-agent checkout (PR check only). +# can open the ADO-agent checkout. parameters: - name: installMock diff --git a/.github/workflows/ado/templates/steps/prepare-change-set.yml b/.github/workflows/ado/templates/steps/prepare-change-set.yml index c25a9829f14..800d3bb0256 100644 --- a/.github/workflows/ado/templates/steps/prepare-change-set.yml +++ b/.github/workflows/ado/templates/steps/prepare-change-set.yml @@ -7,9 +7,8 @@ # call). # # The (from, to) commit range is read from job-scope variables named by the -# fromCommitVar / toCommitVar parameters, so the post-merge (targetCommit -> -# sourceCommit) and PR (baseCommit -> sourceCommit) pipelines share this -# template unchanged. compute_change_set.sh diffs from -> to. +# fromCommitVar / toCommitVar parameters (defaults targetCommit -> sourceCommit). +# compute_change_set.sh diffs from -> to. # # compute_change_set.sh hard-fails on the supply-chain drift tripwire # (sourcesChange == true without an identity change in {added, changed, From 4144371553c3e95d9d266827adce27c94d606ba8 Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Thu, 2 Jul 2026 15:04:03 -0700 Subject: [PATCH 12/14] ci: get-changes-info refactor + ADO pipeline convention rules Address the fourth round of PR #17885 comments and back-apply the new ADO instruction rules across all touched files: - Rename common-steps.yml -> get-changes-info.yml and make it self-contained: a `mode` (pr|postmerge) parameter selects the commit-range template, and it runs full-history + install-deps(its own needs) + verify-locks + prepare-change-set + verify-rendered-specs, emitting the change-info vars via name params. The PR check now also verifies locks and rendered specs. - install-deps.yml is now granular (installAzldev / installControlTowerClient / installMock / installAdoRequirements / normalizeGoGitConfig); get-changes-info installs only what it needs, and each stages template installs the Control Tower client itself for its submission step. - Unify commit-range-postmerge on baseCommit (was targetCommit). - New instruction rules, applied everywhere: MIT copyright header on new files; alphabetical ordering of parameters/variables/env; "Sets job variables:" comment above a composed `- template:`; `OutputVarTask` for output-var task names; template comments describe only what the template does (no caller-intent / value-origin assumptions); prefer scriptPath over inlineScript for single-command tasks (with the AzureCLI-python exception documented). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../instructions/ado-pipeline.instructions.md | 65 ++++++--- .github/workflows/ado/package-build.yml | 12 +- .github/workflows/ado/pr-check-ct.yml | 10 +- .../ado/templates/package-build-stages.yml | 66 +++++----- .../ado/templates/pr-check-ct-stages.yml | 124 ++++++------------ .../steps/commit-range-postmerge.yml | 70 +++++----- .../ado/templates/steps/commit-range-pr.yml | 41 +++--- .../ado/templates/steps/common-steps.yml | 40 ------ .../templates/steps/ensure-full-history.yml | 14 +- .../ado/templates/steps/get-changes-info.yml | 74 +++++++++++ .../ado/templates/steps/install-deps.yml | 89 +++++++------ .../templates/steps/prepare-change-set.yml | 49 +++---- .../ado/templates/steps/verify-locks.yml | 9 +- .../templates/steps/verify-rendered-specs.yml | 40 +++--- scripts/ci/ado/determine_commit_range.py | 2 +- scripts/ci/control-tower/verify_locks.sh | 4 +- 16 files changed, 356 insertions(+), 353 deletions(-) delete mode 100644 .github/workflows/ado/templates/steps/common-steps.yml create mode 100644 .github/workflows/ado/templates/steps/get-changes-info.yml diff --git a/.github/instructions/ado-pipeline.instructions.md b/.github/instructions/ado-pipeline.instructions.md index d8e434f65b8..89f8350ac0e 100644 --- a/.github/instructions/ado-pipeline.instructions.md +++ b/.github/instructions/ado-pipeline.instructions.md @@ -39,7 +39,34 @@ File-pairing convention: a wrapper at `.github/workflows/ado/.yml` pairs w See [.github/workflows/ado/package-build.yml](.github/workflows/ado/package-build.yml) and [.github/workflows/ado/templates/package-build-stages.yml](.github/workflows/ado/templates/package-build-stages.yml) for the canonical example. -Shared **step sub-templates** live under `.github/workflows/ado/templates/steps/.yml` and are spliced into a job's `steps:` via `- template: steps/.yml` (path relative to the including stages template). Use these to share step sequences across stages templates that differ only in a trailing pipeline-specific step. Splicing as **steps** (not a separate job/stage) keeps job-scoped pipeline variables and on-disk files flowing to the steps that follow — a separate job would force output variables + artifact upload/download. The including job must define any job-scope variables the shared steps reference (e.g. `ob_outputDirectory`). See the granular templates under [.github/workflows/ado/templates/steps/](.github/workflows/ado/templates/steps/) (e.g. `ensure-full-history.yml`, `install-deps.yml`, `prepare-change-set.yml`), shared by the `package-build` (via `common-steps.yml`) and `pr-check-ct` pipelines. +Shared **step sub-templates** live under `.github/workflows/ado/templates/steps/.yml` and are spliced into a job's `steps:` via `- template: steps/.yml` (path relative to the including stages template). Use these to share step sequences across stages templates that differ only in a trailing pipeline-specific step. Splicing as **steps** (not a separate job/stage) keeps job-scoped pipeline variables and on-disk files flowing to the steps that follow — a separate job would force output variables + artifact upload/download. The including job must define any job-scope variables the shared steps reference (e.g. `ob_outputDirectory`). See the granular templates under [.github/workflows/ado/templates/steps/](.github/workflows/ado/templates/steps/) (e.g. `ensure-full-history.yml`, `install-deps.yml`, `prepare-change-set.yml`) and the composite `get-changes-info.yml`, shared by the `package-build` and `pr-check-ct` pipelines. + +**A template comment describes only what the template itself does** — not how a caller may or may not use it, and not where its parameter *values* originate. Assumptions about the caller (e.g. "passed by the wrapper", "owns the variable group", "the PR check does X") go stale and belong in the caller, not the shared template. The calling convention is documented here, once; do not repeat it in every YAML. + +**Copyright notice on every new file.** Any new file introduced (any language, not just ADO pipelines) starts with: + +```text +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +``` + +Adjust the comment syntax to the file type (e.g. `//` for C, `` for XML/HTML). + +**Sort lists alphabetically.** Within a file, keep `parameters`, `variables`, and step `env:` entries in alphabetical order by name (case-insensitive). This makes additions and diffs predictable and variables easy to find. Ordered sequences whose order is semantically load-bearing — job `steps:`, script lines — are NOT sorted. + +**Prefer a script file over `inlineScript` for single-command tasks.** A task whose body is one functionally-important command/script should reference a script file (e.g. `Bash@3` `filePath`, or `AzureCLI@2` `scriptLocation: scriptPath`) rather than embedding it inline. **Exception:** an `AzureCLI@2` step that exists only to establish the WIF az-login context for a non-shell script (e.g. `python3 some_script.py …` consumed by `DefaultAzureCredential`) keeps `inlineScript` — `scriptPath` runs the file *through* bash (ignoring any `#!` line), so honoring the rule would require a throwaway bash wrapper that adds indirection without value. + +**Comment which variables a composed template sets, above the `- template:` line.** When a template you invoke sets job/output variables that *this* file consumes, list them above the invocation in this fixed format (omit an empty section; list only the variables the caller actually uses): + +```yaml + # Sets job variables: + # - + # - + # + # Sets output job variables: + # - + - template: steps/commit-range-pr.yml +``` **Parameterize the names of pipeline variables a template sets.** If a step template sets a job variable (`##vso[task.setvariable variable=...]`), expose `` through a parameter (with the conventional name as the default) so a caller composing several templates can rename it to avoid collisions. Use a consistent suffix scheme: @@ -247,6 +274,9 @@ Use these as a starting point for a new pipeline. Two files: a wrapper (NonOffic ### Wrapper — `.github/workflows/ado/.yml` ```yaml +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + trigger: none pr: none @@ -271,35 +301,38 @@ extends: stages: - template: /.github/workflows/ado/templates/-stages.yml@self parameters: - outputDirectory: $(Build.ArtifactStagingDirectory)/output + apiAudience: $(ApiAudience) + apiBaseUrl: $(ApiBaseUrl) artifactBaseName: containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 + outputDirectory: $(Build.ArtifactStagingDirectory)/output poolType: linux serviceConnection: - apiAudience: $(ApiAudience) - apiBaseUrl: $(ApiBaseUrl) timeoutInMinutes: # explicit, conservative ``` ### Raw stages — `.github/workflows/ado/templates/-stages.yml` ```yaml +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + parameters: - - name: outputDirectory + - name: apiAudience + type: string + - name: apiBaseUrl type: string - name: artifactBaseName type: string - name: containerImage type: string + - name: outputDirectory + type: string - name: poolType type: string default: linux - name: serviceConnection type: string - - name: apiAudience - type: string - - name: apiBaseUrl - type: string - name: timeoutInMinutes type: number @@ -311,18 +344,16 @@ stages: pool: type: ${{ parameters.poolType }} variables: - - name: ob_outputDirectory - value: ${{ parameters.outputDirectory }} - - name: ob_artifactBaseName - value: ${{ parameters.artifactBaseName }} - - name: LinuxContainerImage - value: ${{ parameters.containerImage }} - # Runtime-consumed params surfaced as variables (see the variable - # rules above); reference as $(apiAudience) / $(apiBaseUrl). - name: apiAudience value: ${{ parameters.apiAudience }} - name: apiBaseUrl value: ${{ parameters.apiBaseUrl }} + - name: LinuxContainerImage + value: ${{ parameters.containerImage }} + - name: ob_artifactBaseName + value: ${{ parameters.artifactBaseName }} + - name: ob_outputDirectory + value: ${{ parameters.outputDirectory }} steps: - task: PipAuthenticate@1 displayName: "Authenticate pip" diff --git a/.github/workflows/ado/package-build.yml b/.github/workflows/ado/package-build.yml index ad625de6351..10327b4cf03 100644 --- a/.github/workflows/ado/package-build.yml +++ b/.github/workflows/ado/package-build.yml @@ -14,7 +14,7 @@ # that range, via Workload Identity Federation (OIDC) to Control Tower. # # The setup + change-detection + validation steps are composed from the shared -# templates/steps/* step templates via templates/steps/common-steps.yml. +# templates/steps/* step templates via templates/steps/get-changes-info.yml. # # Helper scripts live under: # - scripts/ci/control-tower/ - (Control Tower-specific) @@ -77,15 +77,15 @@ extends: stages: - template: /.github/workflows/ado/templates/package-build-stages.yml@self parameters: - outputDirectory: $(Build.ArtifactStagingDirectory)/output - artifactBaseName: packagebuild - containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 - poolType: linux - serviceConnection: CT-Endpoints-Access-ServiceConnection-DEV apiAudience: $(ApiAudience) apiBaseAFDUrl: $(ApiBaseAFDUrl) + artifactBaseName: packagebuild + containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 + outputDirectory: $(Build.ArtifactStagingDirectory)/output # Control Tower package target for the 4.0 branch. packageTarget: azl4 + poolType: linux + serviceConnection: CT-Endpoints-Access-ServiceConnection-DEV # Must exceed the script's --poll-timeout-seconds (default 600s = 10m) # with enough headroom for setup steps and the final API call. timeoutInMinutes: 25 diff --git a/.github/workflows/ado/pr-check-ct.yml b/.github/workflows/ado/pr-check-ct.yml index 866a57260f2..0186d103829 100644 --- a/.github/workflows/ado/pr-check-ct.yml +++ b/.github/workflows/ado/pr-check-ct.yml @@ -91,13 +91,11 @@ extends: stages: - template: /.github/workflows/ado/templates/pr-check-ct-stages.yml@self parameters: - outputDirectory: $(Build.ArtifactStagingDirectory)/output - artifactBaseName: prcheckct - containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 - poolType: linux - serviceConnection: CT-Endpoints-Access-ServiceConnection-DEV apiAudience: $(ApiAudience) apiBaseAFDUrl: $(ApiBaseAFDUrl) + artifactBaseName: prcheckct + containerImage: mcr.microsoft.com/onebranch/azurelinux/build:3.0 + outputDirectory: $(Build.ArtifactStagingDirectory)/output # Control Tower package target for the 4.0 branch. packageTarget: azl4 # This check WAITS for the Control Tower build to finish (pass/fail), @@ -107,4 +105,6 @@ extends: # script's own clear failure fires before ADO blunt-kills the job. # Raise both together if a legitimate build is being killed. pollTimeoutSeconds: 21600 + poolType: linux + serviceConnection: CT-Endpoints-Access-ServiceConnection-DEV timeoutInMinutes: 420 diff --git a/.github/workflows/ado/templates/package-build-stages.yml b/.github/workflows/ado/templates/package-build-stages.yml index faab75b30c1..cdeb8f92e4e 100644 --- a/.github/workflows/ado/templates/package-build-stages.yml +++ b/.github/workflows/ado/templates/package-build-stages.yml @@ -5,67 +5,69 @@ # OneBranch-coupled knobs as parameters. The wrapper at # .github/workflows/ado/package-build.yml supplies concrete values. # -# The setup + change-detection + validation steps come from the shared -# templates/steps/common-steps.yml (composed from templates/steps/*); this -# template appends only the package-build-specific Control Tower call. +# Per push: gather the change-set info (get-changes-info, mode=postmerge: full +# history, deps, commit range, lock + render verification, changed-component +# set), then submit an official Control Tower build for the changed components. parameters: - - name: outputDirectory + - name: apiAudience + type: string + - name: apiBaseAFDUrl type: string - name: artifactBaseName type: string - name: containerImage type: string + - name: outputDirectory + type: string + # Control Tower package target for builds submitted from this pipeline + # (e.g. azl4 for the 4.0 branch, azl5 for 5.0). + - name: packageTarget + type: string - name: poolType type: string default: linux - name: serviceConnection type: string - - name: apiAudience - type: string - - name: apiBaseAFDUrl - type: string - name: timeoutInMinutes type: number - # Control Tower package target for builds submitted from this pipeline - # (e.g. azl4 for the 4.0 branch, azl5 for 5.0). - - name: packageTarget - type: string stages: - stage: PackageBuild jobs: - job: PackageBuild - # This is a post-merge build: code has already merged, so there is no PR - # or merge queue to gate. The job fails loud -- any failing step turns - # the run red so submission/validation breakage is visible and alertable - # rather than masked as a partial success. The build itself runs - # asynchronously in Control Tower/Koji; this pipeline only submits it and - # confirms acceptance (see run_package_build.py). - # Must exceed the script's --poll-timeout-seconds (default 600s = 10m) - # with enough headroom for setup steps and the final API call. + # Post-merge build: code has already merged, so there is no PR or merge + # queue to gate. Fails loud -- any failing step turns the run red so + # submission/validation breakage is visible rather than masked. The + # build runs asynchronously in Control Tower/Koji; this pipeline only + # submits it and confirms acceptance (see run_package_build.py). timeoutInMinutes: ${{ parameters.timeoutInMinutes }} pool: type: ${{ parameters.poolType }} variables: - - name: ob_outputDirectory - value: ${{ parameters.outputDirectory }} - - name: ob_artifactBaseName - value: ${{ parameters.artifactBaseName }} - - name: LinuxContainerImage - value: ${{ parameters.containerImage }} - name: apiAudience value: ${{ parameters.apiAudience }} - name: apiBaseAFDUrl value: ${{ parameters.apiBaseAFDUrl }} + - name: LinuxContainerImage + value: ${{ parameters.containerImage }} + - name: ob_artifactBaseName + value: ${{ parameters.artifactBaseName }} + - name: ob_outputDirectory + value: ${{ parameters.outputDirectory }} - name: packageTarget value: ${{ parameters.packageTarget }} steps: - # Shared setup + change detection + validation (PipAuthenticate, - # install deps, determine commit range, verify locks, prepare change - # set, verify rendered specs). Produces the job-scope variables and - # change-set files the submit step below consumes. - - template: steps/common-steps.yml + # Sets job variables: + # - changedComponentsFile + # - sourceCommit + - template: steps/get-changes-info.yml + parameters: + mode: postmerge + + - template: steps/install-deps.yml + parameters: + installControlTowerClient: true - task: AzureCLI@2 displayName: "Submit package build to Control Tower" @@ -88,8 +90,8 @@ stages: env: API_AUDIENCE: $(apiAudience) API_BASE_URL: $(apiBaseAFDUrl) - PACKAGE_TARGET: $(packageTarget) CHANGED_COMPONENTS_FILE: $(changedComponentsFile) + PACKAGE_TARGET: $(packageTarget) SOURCE_COMMIT: $(sourceCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) # Build.Reason is auto-exposed to the script as $BUILD_REASON; do diff --git a/.github/workflows/ado/templates/pr-check-ct-stages.yml b/.github/workflows/ado/templates/pr-check-ct-stages.yml index 9847290a5bb..317e6c5c540 100644 --- a/.github/workflows/ado/templates/pr-check-ct-stages.yml +++ b/.github/workflows/ado/templates/pr-check-ct-stages.yml @@ -3,64 +3,43 @@ # Raw stages template for the PR Control Tower check (prcheck + scratch package # build). Wrapper-agnostic: declares the stages/jobs/steps and exposes the # wrapper-coupled knobs as parameters. The wrapper at -# .github/workflows/ado/pr-check-ct.yml supplies concrete values. See that -# wrapper for why this pipeline exists. +# .github/workflows/ado/pr-check-ct.yml supplies concrete values. # -# What it does, per PR: -# 1. Ensure full git history (rpmautospec + change detection need it). -# 2. Authenticate to the internal pip feed and install host deps: azldev (for -# change detection only -- no mock, no build) and the Control Tower Python -# client. -# 3. Resolve the PR commit range from the merge commit's parents -# (^1 = target-branch tip, ^2 = PR head). -# 4. Compute the changed-component set (shared compute_change_set.sh). -# 5. Call Control Tower 'prcheck' for those components: among other things it -# uploads the missing lookaside sources so they are present when the build -# below fetches them. -# 6. Submit a *scratch* Control Tower build of the PR head for exactly those -# components (run_package_build.py --wait-for-completion). The build runs -# in Control Tower's own sandbox; this pipeline WAITS for it to reach a -# terminal state and fails the check if the build fails (or does not -# finish within the poll timeout). NO PR-controlled code is built on the -# CI agent -- only read-only change detection runs here. -# -# Setup + change detection are composed from the shared templates/steps/* -# templates (the same ones the post-merge pipeline uses via common-steps.yml), -# but with the PR commit-range template (merge-commit parents, not the -# previous-build delta) and WITHOUT the lock/render-verify steps -- those PR -# gates already run in GitHub Actions. -# -# Because it calls Control Tower, this pipeline needs the WIF service connection -# and the Control Tower API audience + base URL, supplied as parameters. +# Per PR: gather the change-set info (get-changes-info, mode=pr: full history, +# deps, commit range, lock + render verification, changed-component set), then +# call Control Tower 'prcheck' (uploads the changed components' missing +# lookaside sources) and submit a *scratch* Control Tower build of the PR head +# for those components, waiting for it to finish. NO PR-controlled code is built +# on the CI agent -- only read-only change detection runs here. parameters: - - name: outputDirectory + - name: apiAudience + type: string + - name: apiBaseAFDUrl type: string - name: artifactBaseName type: string - name: containerImage type: string - - name: poolType - type: string - default: linux - - name: serviceConnection - type: string - - name: apiAudience - type: string - - name: apiBaseAFDUrl + - name: outputDirectory type: string # Control Tower package target for builds submitted from this pipeline # (e.g. azl4 for the 4.0 branch, azl5 for 5.0). - name: packageTarget type: string - - name: timeoutInMinutes - type: number # Max seconds run_package_build.py waits for the Control Tower build to reach # a terminal state. Keep below the job's timeoutInMinutes so the script's own # clear failure fires before ADO blunt-kills the job. Default 21600 = 6h. - name: pollTimeoutSeconds type: number default: 21600 + - name: poolType + type: string + default: linux + - name: serviceConnection + type: string + - name: timeoutInMinutes + type: number stages: - stage: PRCheckCT @@ -68,49 +47,39 @@ stages: - job: PRCheckCT # Fail-loud: a failed submission, an immediate Control Tower rejection, # or a build that fails (or never reaches a terminal state) turns the PR - # check red. The build runs in Control Tower's own sandbox -- NOT on - # this agent -- but this pipeline WAITS for it to finish - # (run_package_build.py --wait-for-completion). Size the timeout to - # cover the FULL build: it must exceed the script's pollTimeoutSeconds - # (6h default) so the script's own clear failure fires before ADO - # blunt-kills the job. + # check red. The build runs in Control Tower's own sandbox -- NOT on this + # agent -- but this pipeline WAITS for it to finish. Size the timeout to + # cover the FULL build: it must exceed pollTimeoutSeconds (6h default). timeoutInMinutes: ${{ parameters.timeoutInMinutes }} pool: type: ${{ parameters.poolType }} variables: - - name: ob_outputDirectory - value: ${{ parameters.outputDirectory }} - - name: ob_artifactBaseName - value: ${{ parameters.artifactBaseName }} - - name: LinuxContainerImage - value: ${{ parameters.containerImage }} - name: apiAudience value: ${{ parameters.apiAudience }} - name: apiBaseAFDUrl value: ${{ parameters.apiBaseAFDUrl }} + - name: LinuxContainerImage + value: ${{ parameters.containerImage }} + - name: ob_artifactBaseName + value: ${{ parameters.artifactBaseName }} + - name: ob_outputDirectory + value: ${{ parameters.outputDirectory }} - name: packageTarget value: ${{ parameters.packageTarget }} - name: pollTimeoutSeconds value: ${{ parameters.pollTimeoutSeconds }} steps: - # Shared setup + change detection, composed from the granular step - # templates. install-deps skips mock and the ADO API client (neither - # is needed for read-only change detection) but strips the go-git - # worktreeConfig extension so azldev can open the agent checkout. - - template: steps/ensure-full-history.yml - - - template: steps/install-deps.yml + # Sets job variables: + # - baseCommit + # - changedComponentsFile + # - sourceCommit + - template: steps/get-changes-info.yml parameters: - normalizeGoGitConfig: true + mode: pr - # PR commit range = merge-commit parents (baseCommit = merge base, - # sourceCommit = PR head). prepare-change-set diffs baseCommit -> - # sourceCommit. - - template: steps/commit-range-pr.yml - - - template: steps/prepare-change-set.yml + - template: steps/install-deps.yml parameters: - fromCommitVar: baseCommit + installControlTowerClient: true # Call Control Tower 'prcheck' BEFORE the build. Among other things, # prcheck uploads the missing lookaside sources for the changed @@ -135,11 +104,11 @@ stages: env: API_AUDIENCE: $(apiAudience) API_BASE_URL: $(apiBaseAFDUrl) - CHANGED_COMPONENTS_FILE: $(changedComponentsFile) - SOURCE_COMMIT: $(sourceCommit) # TODO: Base commit is not used yet. Will be needed once we move # detection of affected components to CT. ADO task: 18816 BASE_COMMIT: $(baseCommit) + CHANGED_COMPONENTS_FILE: $(changedComponentsFile) + SOURCE_COMMIT: $(sourceCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) # Build.Reason is auto-exposed to the script as $BUILD_REASON; do # not add a BUILD_REASON entry here -- the agent silently ignores @@ -149,16 +118,13 @@ stages: # components. Scratch = throwaway: it never persists to a production # repo, so building unmerged PR code is safe. Scratch is the default # (no --official-build); run_package_build.py additionally refuses an - # OFFICIAL build for a PR trigger. --wait-for-completion makes the - # script block until the build reaches a terminal state and fail the - # check on a build failure (or if it does not finish within - # --poll-timeout-seconds, 6h below -- our worst-case build). No PR - # code is built on this agent. + # OFFICIAL build for a PR trigger. --wait-for-completion blocks until + # the build reaches a terminal state and fails the check on a build + # failure (or on poll-timeout). No PR code is built on this agent. # # This step assumes the pipeline is wired as a REVIEWER-GATED check in - # ADO (see the wrapper header): it should not auto-run on every PR - # push, so that a maintainer eyeballs the diff before unmerged code is - # submitted for a build. + # ADO: it should not auto-run on every PR push, so a maintainer + # eyeballs the diff before unmerged code is submitted for a build. - task: AzureCLI@2 displayName: "Submit scratch build to Control Tower" inputs: @@ -168,10 +134,6 @@ stages: inlineScript: | set -euo pipefail - # --poll-timeout-seconds comes from the pollTimeoutSeconds - # parameter (6h default = our worst-case build). Keep it below - # the job's timeoutInMinutes so the script's own clear failure - # fires before ADO blunt-kills the job. python3 scripts/ci/control-tower/run_package_build.py \ --api-audience "$API_AUDIENCE" \ --api-base-url "$API_BASE_URL" \ @@ -185,9 +147,9 @@ stages: env: API_AUDIENCE: $(apiAudience) API_BASE_URL: $(apiBaseAFDUrl) + CHANGED_COMPONENTS_FILE: $(changedComponentsFile) PACKAGE_TARGET: $(packageTarget) POLL_TIMEOUT_SECONDS: $(pollTimeoutSeconds) - CHANGED_COMPONENTS_FILE: $(changedComponentsFile) SOURCE_COMMIT: $(sourceCommit) UPSTREAM_REPO_URL: $(Build.Repository.Uri) # Build.Reason is auto-exposed to the script as $BUILD_REASON; do diff --git a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml index 1afd21c9023..4ac24a9d682 100644 --- a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml +++ b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml @@ -1,25 +1,24 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # -# Step template: resolve the (target, source) commit range for a POST-MERGE -# delta build. -# source = the commit that triggered this run (Build.SourceVersion). -# target = the previous CI build's commit on this branch, looked up via the -# ADO Builds API. Pairing each build with the one immediately before -# it keeps concurrent runs from overlapping and captures every commit -# a rebase merge appends. Runs after the dependency-install step -# because it imports the azure-devops SDK. See -# scripts/ci/ado/determine_commit_range.py for the full rationale and -# the first-run (source^1) fallback. +# Step template: resolve the commit range for a post-merge delta build. +# source = the commit that triggered this run (Build.SourceVersion). +# base = the previous CI build's commit on this branch, from the ADO Builds +# API. Pairing each build with the one immediately before it keeps +# concurrent runs from overlapping and captures every commit a rebase +# merge appends. See scripts/ci/ado/determine_commit_range.py for the +# rationale and the first-run (source^1) fallback. # -# Emits job-scope variables sourceCommit and targetCommit. +# Sets job variables named by baseCommitVar (previous build) and sourceCommitVar +# (this build). parameters: + - name: baseCommitVar + type: string + default: baseCommit - name: sourceCommitVar type: string default: sourceCommit - - name: targetCommitVar - type: string - default: targetCommit steps: - script: | @@ -27,9 +26,8 @@ steps: # The helper prints the resolved range to stdout as two lines: # sourceCommit= # targetCommit= - # (all diagnostics go to stderr). We parse them and set the - # pipeline variables HERE so the variable wiring is visible in - # the YAML rather than hidden in the script. + # (all diagnostics go to stderr). We parse them and set the pipeline + # variables HERE so the wiring is visible in the YAML. range_output="$(python3 scripts/ci/ado/determine_commit_range.py \ --definition-id "$SYSTEM_DEFINITIONID" \ --current-build-id "$BUILD_BUILDID" \ @@ -38,36 +36,30 @@ steps: echo "$range_output" source_commit="$(sed -n 's/^sourceCommit=//p' <<< "$range_output")" - target_commit="$(sed -n 's/^targetCommit=//p' <<< "$range_output")" - if [[ -z "$source_commit" || -z "$target_commit" ]]; then + base_commit="$(sed -n 's/^targetCommit=//p' <<< "$range_output")" + if [[ -z "$source_commit" || -z "$base_commit" ]]; then echo "##[error]determine_commit_range.py did not emit a source/target commit range." exit 1 fi - # The helper emits baselineQueryFailed=true ONLY when the ADO Builds API - # query itself failed (bad SYSTEM_ACCESSTOKEN scope, network, SDK - # misconfig) -- as opposed to a successful query that simply found no - # prior build (a benign first run). A failed query silently degrades EVERY - # run to a single-commit delta, with the weekly true-up as the sole safety - # net, so raise a visible pipeline warning here rather than letting it pass - # unnoticed. + # baselineQueryFailed=true means the ADO Builds API query itself failed + # (bad SYSTEM_ACCESSTOKEN scope, network, SDK misconfig) -- not a benign + # first run. A failed query silently degrades every run to a single-commit + # delta, so raise a visible warning here. if [[ "$(sed -n 's/^baselineQueryFailed=//p' <<< "$range_output")" == "true" ]]; then - echo "##vso[task.logissue type=warning]Delta build could not query previous builds; degraded to a single-commit delta (target=source^1). Check the SYSTEM_ACCESSTOKEN scope and ADO Builds API connectivity. The weekly true-up job covers any gap." + echo "##vso[task.logissue type=warning]Delta build could not query previous builds; degraded to a single-commit delta (base=source^1). Check the SYSTEM_ACCESSTOKEN scope and ADO Builds API connectivity. The weekly true-up job covers any gap." fi - # Mark the resolved range readonly: it is computed once here and only - # consumed downstream, so locking it prevents a later step from clobbering - # the commit range (matches changedComponentsFile / renderSetFile below). + echo "##vso[task.setvariable variable=$BASE_COMMIT_VAR;isreadonly=true]$base_commit" echo "##vso[task.setvariable variable=$SOURCE_COMMIT_VAR;isreadonly=true]$source_commit" - echo "##vso[task.setvariable variable=$TARGET_COMMIT_VAR;isreadonly=true]$target_commit" env: + BASE_COMMIT_VAR: ${{ parameters.baseCommitVar }} + BUILD_BUILDID: $(Build.BuildId) + BUILD_SOURCEVERSION: $(Build.SourceVersion) + DELTA_QUERY_BRANCH: $(Build.SourceBranch) SOURCE_COMMIT_VAR: ${{ parameters.sourceCommitVar }} - TARGET_COMMIT_VAR: ${{ parameters.targetCommitVar }} - SYSTEM_COLLECTIONURI: $(System.CollectionUri) - SYSTEM_TEAMPROJECT: $(System.TeamProject) SYSTEM_ACCESSTOKEN: $(System.AccessToken) + SYSTEM_COLLECTIONURI: $(System.CollectionUri) SYSTEM_DEFINITIONID: $(System.DefinitionId) - BUILD_BUILDID: $(Build.BuildId) - DELTA_QUERY_BRANCH: $(Build.SourceBranch) - BUILD_SOURCEVERSION: $(Build.SourceVersion) - displayName: "Determine source and target commit range" + SYSTEM_TEAMPROJECT: $(System.TeamProject) + displayName: "Determine source and base commit range" diff --git a/.github/workflows/ado/templates/steps/commit-range-pr.yml b/.github/workflows/ado/templates/steps/commit-range-pr.yml index cfae3381c3f..db6393b604e 100644 --- a/.github/workflows/ado/templates/steps/commit-range-pr.yml +++ b/.github/workflows/ado/templates/steps/commit-range-pr.yml @@ -1,32 +1,27 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # -# Step template: resolve the PR commit range from the merge commit's parents. -# A PR-policy build checks out the MERGE commit (Build.SourceVersion): parent -# ^1 is the target-branch tip, parent ^2 is the PR head. The PR's true change -# set is the diff between their MERGE BASE (the fork point) and ^2 -- i.e. -# `git diff merge-base(^1,^2)..^2`, the same three-dot set GitHub's "Files -# changed" shows. We compare against the merge base, NOT the target tip: -# comparing against the tip additionally flags every component that landed on -# the target branch after the PR forked (the PR head still carries their -# pre-update state), which can balloon a 1-component PR into dozens. +# Step template: resolve a PR's commit range from the merge commit's parents. +# A PR-policy build checks out the MERGE commit: parent ^1 is the target-branch +# tip, parent ^2 is the PR head. The change set is the diff between their MERGE +# BASE (fork point) and ^2 -- the same three-dot set GitHub's "Files changed" +# shows. Comparing against the merge base rather than the target tip avoids +# flagging components that merely landed on the target branch after the fork. # -# Emits job-scope variables sourceCommit (PR head, ^2) and baseCommit (the -# merge base). These feed prepare-change-set.yml (from=baseCommit, -# to=sourceCommit) and the trailing Control Tower call. +# Sets job variables named by baseCommitVar (merge base) and sourceCommitVar +# (PR head). parameters: - - name: sourceCommitVar - type: string - default: sourceCommit - name: baseCommitVar type: string default: baseCommit + - name: sourceCommitVar + type: string + default: sourceCommit steps: # Fail fast, at compile time, when this is not a PR build: the merge-commit - # parent logic below only makes sense for a PullRequest trigger. Including the - # guard step is decided by a template expression on Build.Reason, so a - # mis-triggered run stops at its very first step. + # parent logic only makes sense for a PullRequest trigger. - ${{ if ne(variables['Build.Reason'], 'PullRequest') }}: - script: | echo "##[error]This pipeline must run as a PR build (Build.Reason=PullRequest); got '$BUILD_REASON'." @@ -42,10 +37,6 @@ steps: exit 1 fi source_commit="$(git rev-parse HEAD^2)" - # Baseline = the MERGE BASE (fork point) of the target-branch tip - # (HEAD^1) and the PR head (HEAD^2), NOT the target tip itself. - # See the block comment above for why; this is passed as the - # `--from-commit` baseline to the change-set helper. base_commit="$(git merge-base HEAD^1 HEAD^2)" # PR-supplied data is untrusted: validate both SHAs before use. for sha in "$base_commit" "$source_commit"; do @@ -55,9 +46,9 @@ steps: fi done echo "Resolved range: base(merge-base)=$base_commit source=$source_commit" - echo "##vso[task.setvariable variable=$SOURCE_COMMIT_VAR;isreadonly=true]$source_commit" echo "##vso[task.setvariable variable=$BASE_COMMIT_VAR;isreadonly=true]$base_commit" + echo "##vso[task.setvariable variable=$SOURCE_COMMIT_VAR;isreadonly=true]$source_commit" env: - SOURCE_COMMIT_VAR: ${{ parameters.sourceCommitVar }} BASE_COMMIT_VAR: ${{ parameters.baseCommitVar }} + SOURCE_COMMIT_VAR: ${{ parameters.sourceCommitVar }} displayName: "Determine PR commit range" diff --git a/.github/workflows/ado/templates/steps/common-steps.yml b/.github/workflows/ado/templates/steps/common-steps.yml deleted file mode 100644 index be4bf21d009..00000000000 --- a/.github/workflows/ado/templates/steps/common-steps.yml +++ /dev/null @@ -1,40 +0,0 @@ -# Microsoft Corporation -# -# Shared step set for the POST-MERGE package-build Control Tower pipeline. -# Spliced into the pipeline's single job via: -# -# steps: -# - template: steps/common-steps.yml -# - -# -# It composes the granular step templates in templates/steps/. Keeping these as -# STEPS (not a separate job/stage) is deliberate: the change-detection steps -# emit job-scoped pipeline variables (sourceCommit, targetCommit, -# changedComponentsFile, renderSetFile) and write the change-set files to disk. -# Those flow freely to later steps in the SAME job but would need output -# variables + artifact upload/download to cross a job boundary. -# -# Contract for the including job: -# * It MUST define the job-scope variable `ob_outputDirectory` (both stages -# templates set it from their `outputDirectory` parameter); the verify-locks -# and prepare-change-set steps publish triage artifacts there. -# * These steps assume normal step sequencing -- a failed step stops the ones -# after it (the default; do NOT set step-level `continueOnError` on them). -# Later steps depend on earlier ones. A job-level `continueOnError` (which -# only affects how the JOB result is reported, not intra-job step flow) is -# fine. -# -# The PR check composes these same granular templates directly, with its own -# commit-range-pr.yml and without the lock/render-verify steps (those are -# covered by the GitHub Actions PR gates). - -steps: - - template: ensure-full-history.yml - - template: install-deps.yml - parameters: - installMock: true - installAdoRequirements: true - - template: commit-range-postmerge.yml - - template: verify-locks.yml - - template: prepare-change-set.yml - - template: verify-rendered-specs.yml diff --git a/.github/workflows/ado/templates/steps/ensure-full-history.yml b/.github/workflows/ado/templates/steps/ensure-full-history.yml index edd55321822..aac374c264e 100644 --- a/.github/workflows/ado/templates/steps/ensure-full-history.yml +++ b/.github/workflows/ado/templates/steps/ensure-full-history.yml @@ -1,13 +1,9 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # -# Step template: ensure the full git history is present, ONCE, before anything -# that needs it. The OneBranch checkout may be shallow (depth 1), but lock -# resolution, spec rendering (rpmautospec derives Release/changelog from -# `git log` over the lock files), and commit-range detection all require the -# complete history. Doing the unshallow here means the helper scripts can assume -# full history and never fetch themselves -- a `git fetch --depth=N` inside a -# script would re-shallow a full clone, which silently corrupts the rpmautospec -# Release calculation (and is a footgun when running the scripts locally). +# Step template: unshallow the git checkout when it is shallow, so the full +# history is available to later steps. Fetches with --unshallow exactly once; +# a `git fetch --depth=N` would otherwise re-shallow a full clone. steps: - script: | diff --git a/.github/workflows/ado/templates/steps/get-changes-info.yml b/.github/workflows/ado/templates/steps/get-changes-info.yml new file mode 100644 index 00000000000..c79e4683b5c --- /dev/null +++ b/.github/workflows/ado/templates/steps/get-changes-info.yml @@ -0,0 +1,74 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +# +# Composite step template: gather a build's change-set information. Given a +# `mode` (pr | postmerge) it resolves the commit range, installs the host tools +# it needs (azldev + mock, plus the ADO API client in postmerge mode and the +# go-git config normalization in pr mode), verifies lock files, computes the +# changed-component set, and verifies rendered specs. +# +# Its install-deps covers only these internal needs. A caller that needs other +# host tools (e.g. the Control Tower client for a submission step) installs them +# with its own install-deps.yml step. +# +# Requires the job-scope variable ob_outputDirectory. Sets the change-info +# variables named by the *Var parameters. + +parameters: + - name: baseCommitVar + type: string + default: baseCommit + - name: changedComponentsFileVar + type: string + default: changedComponentsFile + - name: mode + type: string + values: + - pr + - postmerge + - name: renderSetFileVar + type: string + default: renderSetFile + - name: sourceCommitVar + type: string + default: sourceCommit + +steps: + - template: ensure-full-history.yml + + - template: install-deps.yml + parameters: + installAdoRequirements: ${{ eq(parameters.mode, 'postmerge') }} + installAzldev: true + installMock: true + normalizeGoGitConfig: ${{ eq(parameters.mode, 'pr') }} + + # Sets job variables: + # - baseCommit + # - sourceCommit + - ${{ if eq(parameters.mode, 'pr') }}: + - template: commit-range-pr.yml + parameters: + baseCommitVar: ${{ parameters.baseCommitVar }} + sourceCommitVar: ${{ parameters.sourceCommitVar }} + - ${{ if eq(parameters.mode, 'postmerge') }}: + - template: commit-range-postmerge.yml + parameters: + baseCommitVar: ${{ parameters.baseCommitVar }} + sourceCommitVar: ${{ parameters.sourceCommitVar }} + + - template: verify-locks.yml + + # Sets job variables: + # - changedComponentsFile + # - renderSetFile + - template: prepare-change-set.yml + parameters: + changedComponentsFileVar: ${{ parameters.changedComponentsFileVar }} + fromCommitVar: ${{ parameters.baseCommitVar }} + renderSetFileVar: ${{ parameters.renderSetFileVar }} + toCommitVar: ${{ parameters.sourceCommitVar }} + + - template: verify-rendered-specs.yml + parameters: + renderSetFileVar: ${{ parameters.renderSetFileVar }} diff --git a/.github/workflows/ado/templates/steps/install-deps.yml b/.github/workflows/ado/templates/steps/install-deps.yml index f5ebd2fbee7..17380e131f7 100644 --- a/.github/workflows/ado/templates/steps/install-deps.yml +++ b/.github/workflows/ado/templates/steps/install-deps.yml @@ -1,23 +1,28 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # -# Step template: authenticate to the internal pip feed and install the host -# tools the Control Tower change-detection + submission steps need. azldev is -# always installed (its version comes from the committed .azldev-version and is -# validated before use); everything else is opt-in: -# installMock - install mock + rpmautospec (needed for lock/render -# steps); leave off for change-detection-only jobs. -# installAdoRequirements - install the azure-devops SDK used by -# determine_commit_range.py (post-merge delta helper). -# normalizeGoGitConfig - strip extensions.worktreeConfig so azldev's go-git -# can open the ADO-agent checkout. +# Step template: authenticate to the internal pip feed, then install the +# requested host tools. Each tool is opt-in via a boolean parameter. azldev's +# version comes from the committed .azldev-version and is validated before use. parameters: - - name: installMock + # azure-devops SDK (scripts/ci/ado/requirements.txt). + - name: installAdoRequirements type: boolean default: false - - name: installAdoRequirements + # azldev CLI (change detection + spec render). + - name: installAzldev + type: boolean + default: false + # Control Tower client (scripts/ci/control-tower/requirements.txt). + - name: installControlTowerClient + type: boolean + default: false + # mock + mock-rpmautospec (spec render / lock work). + - name: installMock type: boolean default: false + # strip extensions.worktreeConfig so azldev's go-git can open the checkout. - name: normalizeGoGitConfig type: boolean default: false @@ -32,12 +37,10 @@ steps: # azldev opens the repo with go-git, which rejects a config that declares # the `worktreeconfig` extension while core.repositoryformatversion is still # 0: "core.repositoryformatversion does not support extension: worktreeconfig". - # Native git tolerates this, and the ADO agent checkout leaves the extension - # set, so strip it before any azldev invocation. Each CI run is a fresh - # checkout so this is safe and self-contained. - # TODO: remove this step once azldev no longer needs the workaround (go-git - # v6 fixes the underlying bug): - # https://github.com/microsoft/azure-linux-dev-tools/issues/241 + # Native git tolerates this and the ADO agent checkout leaves the extension + # set, so strip it. Each CI run is a fresh checkout so this is self-contained. + # TODO: remove once azldev no longer needs the workaround (go-git v6 fixes + # the underlying bug): https://github.com/microsoft/azure-linux-dev-tools/issues/241 - script: | set -euo pipefail if git config --get extensions.worktreeConfig >/dev/null 2>&1; then @@ -55,32 +58,34 @@ steps: echo "##[endgroup]" displayName: "Install mock" - - script: | - set -euo pipefail - echo "##[group]Azldev (host)" - # Only the version string comes from the checkout; reject a - # malformed/garbage value before it reaches `go install`. - AZLDEV_VERSION="$(tr -d '\n' < .azldev-version)" - if ! printf '%s' "$AZLDEV_VERSION" | grep -Eq '^[0-9A-Za-z._+-]+$'; then - echo "##[error].azldev-version is empty or has unexpected characters" - exit 1 - fi - echo "Installing azldev@${AZLDEV_VERSION}..." - go install "github.com/microsoft/azure-linux-dev-tools/cmd/azldev@${AZLDEV_VERSION}" + - ${{ if parameters.installAzldev }}: + - script: | + set -euo pipefail + echo "##[group]Azldev (host)" + # Only the version string comes from the checkout; reject a + # malformed/garbage value before it reaches `go install`. + AZLDEV_VERSION="$(tr -d '\n' < .azldev-version)" + if ! printf '%s' "$AZLDEV_VERSION" | grep -Eq '^[0-9A-Za-z._+-]+$'; then + echo "##[error].azldev-version is empty or has unexpected characters" + exit 1 + fi + echo "Installing azldev@${AZLDEV_VERSION}..." + go install "github.com/microsoft/azure-linux-dev-tools/cmd/azldev@${AZLDEV_VERSION}" - go_bin_path="$(go env GOPATH)/bin" - echo "##vso[task.prependpath]$go_bin_path" + go_bin_path="$(go env GOPATH)/bin" + echo "##vso[task.prependpath]$go_bin_path" - "$go_bin_path/azldev" --version - echo "##[endgroup]" - displayName: "Install azldev" + "$go_bin_path/azldev" --version + echo "##[endgroup]" + displayName: "Install azldev" - - script: | - set -euo pipefail - echo "##[group]Python dependencies (Control Tower client)" - pip install -r scripts/ci/control-tower/requirements.txt - echo "##[endgroup]" - displayName: "Install Control Tower Python client" + - ${{ if parameters.installControlTowerClient }}: + - script: | + set -euo pipefail + echo "##[group]Python dependencies (Control Tower client)" + pip install -r scripts/ci/control-tower/requirements.txt + echo "##[endgroup]" + displayName: "Install Control Tower Python client" - ${{ if parameters.installAdoRequirements }}: - script: | diff --git a/.github/workflows/ado/templates/steps/prepare-change-set.yml b/.github/workflows/ado/templates/steps/prepare-change-set.yml index 800d3bb0256..8a344647de4 100644 --- a/.github/workflows/ado/templates/steps/prepare-change-set.yml +++ b/.github/workflows/ado/templates/steps/prepare-change-set.yml @@ -1,36 +1,29 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # # Step template: detect changed components and prepare the render set, then # publish changed-components.json as a triage artifact on every exit path. -# Emits job-scope variables changedComponentsFile and renderSetFile for -# downstream consumers (the render-verify step and the trailing Control Tower -# call). +# compute_change_set.sh diffs from -> to and hard-fails on the supply-chain +# drift tripwire (sourcesChange == true without an identity change in +# {added, changed, deleted}). Requires the job-scope variable ob_outputDirectory. # -# The (from, to) commit range is read from job-scope variables named by the -# fromCommitVar / toCommitVar parameters (defaults targetCommit -> sourceCommit). -# compute_change_set.sh diffs from -> to. -# -# compute_change_set.sh hard-fails on the supply-chain drift tripwire -# (sourcesChange == true without an identity change in {added, changed, -# deleted}): that combination would let attacker-supplied bytes ride into the -# lookaside under an unchanged component identity, so it fails closed. -# -# Requires the job-scope variable `ob_outputDirectory` (the triage artifact is -# published there). +# Reads the (from, to) commit range from the variables named by fromCommitVar / +# toCommitVar; sets the variables named by changedComponentsFileVar and +# renderSetFileVar. parameters: - - name: fromCommitVar - type: string - default: targetCommit - - name: toCommitVar - type: string - default: sourceCommit - name: changedComponentsFileVar type: string default: changedComponentsFile + - name: fromCommitVar + type: string + default: baseCommit - name: renderSetFileVar type: string default: renderSetFile + - name: toCommitVar + type: string + default: sourceCommit steps: - script: | @@ -39,10 +32,9 @@ steps: json_file="$change_set_dir/changed-components.json" render_set_file="$change_set_dir/render-set.txt" - # Publish the changed-components JSON for post-mortem triage on - # EVERY exit path, not just success -- if azldev hard-fails on a - # consistency tripwire the partial JSON is exactly what an - # operator needs to investigate. + # Publish the changed-components JSON for post-mortem triage on EVERY exit + # path, not just success -- if azldev hard-fails on a consistency tripwire + # the partial JSON is exactly what an operator needs to investigate. publish_artifact() { if [[ -s "$json_file" ]]; then mkdir -p "$(ob_outputDirectory)/changed-components" @@ -64,14 +56,11 @@ steps: echo "Total: $upload_count component(s) to upload." echo "##[endgroup]" - # Publish the downstream pipeline variables now that the change set is - # ready (consumed by the render-verify step and the trailing Control - # Tower call). echo "##vso[task.setvariable variable=$CHANGED_COMPONENTS_FILE_VAR;isreadonly=true]$json_file" echo "##vso[task.setvariable variable=$RENDER_SET_FILE_VAR;isreadonly=true]$render_set_file" env: - FROM_COMMIT: $(${{ parameters.fromCommitVar }}) - TO_COMMIT: $(${{ parameters.toCommitVar }}) CHANGED_COMPONENTS_FILE_VAR: ${{ parameters.changedComponentsFileVar }} + FROM_COMMIT: $(${{ parameters.fromCommitVar }}) RENDER_SET_FILE_VAR: ${{ parameters.renderSetFileVar }} + TO_COMMIT: $(${{ parameters.toCommitVar }}) displayName: "Prepare change set" diff --git a/.github/workflows/ado/templates/steps/verify-locks.yml b/.github/workflows/ado/templates/steps/verify-locks.yml index 9ab83958452..4ebcf98ef8b 100644 --- a/.github/workflows/ado/templates/steps/verify-locks.yml +++ b/.github/workflows/ado/templates/steps/verify-locks.yml @@ -1,10 +1,9 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # # Step template: verify lock files are current. --check-only validates without -# writing and exits nonzero if any lock would change. Publishes a lock-update -# diff to ob_outputDirectory for triage. -# -# Requires the job-scope variable `ob_outputDirectory`. +# writing and exits nonzero if any lock would change; publishes a lock-update +# diff to ob_outputDirectory. steps: - script: | diff --git a/.github/workflows/ado/templates/steps/verify-rendered-specs.yml b/.github/workflows/ado/templates/steps/verify-rendered-specs.yml index 274b8adc91f..22928e19da2 100644 --- a/.github/workflows/ado/templates/steps/verify-rendered-specs.yml +++ b/.github/workflows/ado/templates/steps/verify-rendered-specs.yml @@ -1,23 +1,26 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # -# Step template: render the components flagged by the change set (azldev-flagged -# plus any with hand-edited spec files) in --check-only mode: azldev renders to -# a staging area and diffs against the on-disk output without writing. Exits -# nonzero if any component's rendered output would change, catching both stale -# renders and direct hand-edits. -# -# Consumes the job-scope variable renderSetFile set by prepare-change-set.yml. +# Step template: render the components in the render set (--check-only) and fail +# if any component's rendered output would change, catching stale renders and +# direct hand-edits. Reads the render-set path from the variable named by +# renderSetFileVar. + +parameters: + - name: renderSetFileVar + type: string + default: renderSetFile steps: - script: | set -euo pipefail - # A missing render-set file means "Prepare change set" did not complete; - # fail loud rather than silently skipping the render check. + # A missing render-set file means the change set was not prepared; fail + # loud rather than silently skipping the render check. if [[ ! -f "$RENDER_SET_FILE" ]]; then - echo "##[error]render-set file '$RENDER_SET_FILE' is missing -- 'Prepare change set' did not complete cleanly." + echo "##[error]render-set file '$RENDER_SET_FILE' is missing -- the change set was not prepared cleanly." exit 1 fi - # An empty render set just means no PR-scoped components changed. + # An empty render set just means no components changed. if [[ ! -s "$RENDER_SET_FILE" ]]; then echo "No changed components -- skipping render." exit 0 @@ -26,17 +29,16 @@ steps: sed 's/^/ - /' "$RENDER_SET_FILE" echo "##[endgroup]" echo "##[group]Specs rendering + verification" - # `-x` fails loudly if the arg list would split into multiple - # xargs invocations -- a multi-batch render-check would - # silently mask drift in the later batches. `-n 50000` is - # required for `-x` to have any effect (without `-n`/`-L`, - # GNU xargs never splits). `--` ends azldev option parsing so - # component names beginning with `-` (none today) are + # `-x` fails loudly if the arg list would split into multiple xargs + # invocations -- a multi-batch render-check would silently mask drift in + # the later batches. `-n 50000` is required for `-x` to have any effect + # (without `-n`/`-L`, GNU xargs never splits). `--` ends azldev option + # parsing so component names beginning with `-` (none today) are # unambiguous. `env AZLDEV_ALLOW_ROOT=1` is inline per # .github/instructions/ado-pipeline.instructions.md. xargs -x -n 50000 -d '\n' env AZLDEV_ALLOW_ROOT=1 azldev component render --check-only -- \ < "$RENDER_SET_FILE" echo "##[endgroup]" env: - RENDER_SET_FILE: $(renderSetFile) + RENDER_SET_FILE: $(${{ parameters.renderSetFileVar }}) displayName: "Verify rendered specs are up to date" diff --git a/scripts/ci/ado/determine_commit_range.py b/scripts/ci/ado/determine_commit_range.py index 00dd0b9077c..ed9185f91c6 100644 --- a/scripts/ci/ado/determine_commit_range.py +++ b/scripts/ci/ado/determine_commit_range.py @@ -1,6 +1,6 @@ """Resolve the ``(target, source)`` commit range for a post-merge delta build. -Strategy (see ``.github/workflows/ado/templates/steps/common-steps.yml``): +Strategy (see ``.github/workflows/ado/templates/steps/commit-range-postmerge.yml``): * ``source`` is the commit that triggered this run (``Build.SourceVersion``). * ``target`` is the ``sourceVersion`` of the immediately-preceding CI build of diff --git a/scripts/ci/control-tower/verify_locks.sh b/scripts/ci/control-tower/verify_locks.sh index 8dfe3d478b7..fe90da033ae 100755 --- a/scripts/ci/control-tower/verify_locks.sh +++ b/scripts/ci/control-tower/verify_locks.sh @@ -23,8 +23,8 @@ git config --unset extensions.worktreeConfig || true # NOTE: full git history (needed for lock resolution and rpmautospec Release # calculation) is ensured ONCE by the pipeline's "Ensure full git history" step -# (.github/workflows/ado/templates/steps/common-steps.yml) before this script -# runs. This script assumes it is present and does not fetch. +# (.github/workflows/ado/templates/steps/ensure-full-history.yml) before this +# script runs. This script assumes it is present and does not fetch. mkdir -p "$(dirname "$output_file")" From cb1059f3deac2aedbd6e3270ac97d8362a04376e Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Thu, 2 Jul 2026 17:19:29 -0700 Subject: [PATCH 13/14] ci: fifth round of PR #17885 review comments - Add the MIT copyright/license header to the ADO pipeline YAMLs that were missing it (both stages templates + both wrappers). - Route $(Build.ArtifactStagingDirectory) and $(ob_outputDirectory) through env: in verify-locks.yml and prepare-change-set.yml instead of inlining the macros in the script bodies. - compute_change_set.sh: reject unsafe --changed-components-file / --specs-diff-file / --render-set-file values (path separators / traversal) so they cannot escape --output-dir. - ADO instructions: drop the (repo-global) copyright rule from the ADO-specific file (still followed in this PR); generalize the inlineScript exception to "if scriptPath needs an artificial wrapper, use inlineScript". - commit-range-postmerge.yml: drop the parenthetical phrases from the header. - commit-range-pr.yml: restore the merge-base explanation comment above base_commit. Kept the compile-time Build.Reason guard (Copilot's "not available at compile time" concern is a false positive for this queue-time predefined variable). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/instructions/ado-pipeline.instructions.md | 11 +---------- .github/workflows/ado/package-build.yml | 3 ++- .github/workflows/ado/pr-check-ct.yml | 3 ++- .../workflows/ado/templates/package-build-stages.yml | 3 ++- .../workflows/ado/templates/pr-check-ct-stages.yml | 3 ++- .../ado/templates/steps/commit-range-postmerge.yml | 3 +-- .../ado/templates/steps/commit-range-pr.yml | 2 ++ .../ado/templates/steps/prepare-change-set.yml | 8 +++++--- .../workflows/ado/templates/steps/verify-locks.yml | 7 +++++-- scripts/ci/components/compute_change_set.sh | 12 ++++++++++++ 10 files changed, 34 insertions(+), 21 deletions(-) diff --git a/.github/instructions/ado-pipeline.instructions.md b/.github/instructions/ado-pipeline.instructions.md index 89f8350ac0e..fb04fc5bed9 100644 --- a/.github/instructions/ado-pipeline.instructions.md +++ b/.github/instructions/ado-pipeline.instructions.md @@ -43,18 +43,9 @@ Shared **step sub-templates** live under `.github/workflows/ado/templates/steps/ **A template comment describes only what the template itself does** — not how a caller may or may not use it, and not where its parameter *values* originate. Assumptions about the caller (e.g. "passed by the wrapper", "owns the variable group", "the PR check does X") go stale and belong in the caller, not the shared template. The calling convention is documented here, once; do not repeat it in every YAML. -**Copyright notice on every new file.** Any new file introduced (any language, not just ADO pipelines) starts with: - -```text -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. -``` - -Adjust the comment syntax to the file type (e.g. `//` for C, `` for XML/HTML). - **Sort lists alphabetically.** Within a file, keep `parameters`, `variables`, and step `env:` entries in alphabetical order by name (case-insensitive). This makes additions and diffs predictable and variables easy to find. Ordered sequences whose order is semantically load-bearing — job `steps:`, script lines — are NOT sorted. -**Prefer a script file over `inlineScript` for single-command tasks.** A task whose body is one functionally-important command/script should reference a script file (e.g. `Bash@3` `filePath`, or `AzureCLI@2` `scriptLocation: scriptPath`) rather than embedding it inline. **Exception:** an `AzureCLI@2` step that exists only to establish the WIF az-login context for a non-shell script (e.g. `python3 some_script.py …` consumed by `DefaultAzureCredential`) keeps `inlineScript` — `scriptPath` runs the file *through* bash (ignoring any `#!` line), so honoring the rule would require a throwaway bash wrapper that adds indirection without value. +**Prefer a script file over `inlineScript` for single-command tasks.** A task whose body is one functionally-important command/script should reference a script file (e.g. `Bash@3` `filePath`, or `AzureCLI@2` `scriptLocation: scriptPath`) rather than embedding it inline. **Exception:** if `scriptPath` cannot be used without an artificial wrapper, keep `inlineScript`. (For example, an `AzureCLI@2` step whose body is a non-shell command such as `python3 some_script.py …` — run for its WIF az-login context — cannot use `scriptPath`, which runs the file *through* bash and ignores any `#!` line; adding a throwaway bash wrapper just to satisfy the rule is not worth it.) **Comment which variables a composed template sets, above the `- template:` line.** When a template you invoke sets job/output variables that *this* file consumes, list them above the invocation in this fixed format (omit an empty section; list only the variables the caller actually uses): diff --git a/.github/workflows/ado/package-build.yml b/.github/workflows/ado/package-build.yml index 10327b4cf03..6eda717a931 100644 --- a/.github/workflows/ado/package-build.yml +++ b/.github/workflows/ado/package-build.yml @@ -1,4 +1,5 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # # Wrapper pipeline — passed to ADO as the entry point for the package-build # pipeline. This file owns all OneBranch-specific wiring (governed templates diff --git a/.github/workflows/ado/pr-check-ct.yml b/.github/workflows/ado/pr-check-ct.yml index 0186d103829..e0e4a81b0cb 100644 --- a/.github/workflows/ado/pr-check-ct.yml +++ b/.github/workflows/ado/pr-check-ct.yml @@ -1,4 +1,5 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # # Wrapper pipeline — passed to ADO as the entry point for the PR Control Tower # check. It calls Control Tower 'prcheck' (which uploads the changed diff --git a/.github/workflows/ado/templates/package-build-stages.yml b/.github/workflows/ado/templates/package-build-stages.yml index cdeb8f92e4e..4a4dd1cd63c 100644 --- a/.github/workflows/ado/templates/package-build-stages.yml +++ b/.github/workflows/ado/templates/package-build-stages.yml @@ -1,4 +1,5 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # # Raw stages template for the package-build pipeline (v1 post-merge delta # build). OneBranch-agnostic: declares the stages/jobs/steps and exposes the diff --git a/.github/workflows/ado/templates/pr-check-ct-stages.yml b/.github/workflows/ado/templates/pr-check-ct-stages.yml index 317e6c5c540..8306bbe5683 100644 --- a/.github/workflows/ado/templates/pr-check-ct-stages.yml +++ b/.github/workflows/ado/templates/pr-check-ct-stages.yml @@ -1,4 +1,5 @@ -# Microsoft Corporation +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. # # Raw stages template for the PR Control Tower check (prcheck + scratch package # build). Wrapper-agnostic: declares the stages/jobs/steps and exposes the diff --git a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml index 4ac24a9d682..49a181c20a6 100644 --- a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml +++ b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml @@ -9,8 +9,7 @@ # merge appends. See scripts/ci/ado/determine_commit_range.py for the # rationale and the first-run (source^1) fallback. # -# Sets job variables named by baseCommitVar (previous build) and sourceCommitVar -# (this build). +# Sets job variables named by baseCommitVar and sourceCommitVar. parameters: - name: baseCommitVar diff --git a/.github/workflows/ado/templates/steps/commit-range-pr.yml b/.github/workflows/ado/templates/steps/commit-range-pr.yml index db6393b604e..45cfdf42c79 100644 --- a/.github/workflows/ado/templates/steps/commit-range-pr.yml +++ b/.github/workflows/ado/templates/steps/commit-range-pr.yml @@ -37,6 +37,8 @@ steps: exit 1 fi source_commit="$(git rev-parse HEAD^2)" + # Baseline = the MERGE BASE (fork point) of the target-branch tip (HEAD^1) + # and the PR head (HEAD^2), NOT the target tip itself. base_commit="$(git merge-base HEAD^1 HEAD^2)" # PR-supplied data is untrusted: validate both SHAs before use. for sha in "$base_commit" "$source_commit"; do diff --git a/.github/workflows/ado/templates/steps/prepare-change-set.yml b/.github/workflows/ado/templates/steps/prepare-change-set.yml index 8a344647de4..326410c96a8 100644 --- a/.github/workflows/ado/templates/steps/prepare-change-set.yml +++ b/.github/workflows/ado/templates/steps/prepare-change-set.yml @@ -28,7 +28,7 @@ parameters: steps: - script: | set -euo pipefail - change_set_dir="$(Build.ArtifactStagingDirectory)/change-set" + change_set_dir="$ARTIFACT_STAGING_DIR/change-set" json_file="$change_set_dir/changed-components.json" render_set_file="$change_set_dir/render-set.txt" @@ -37,8 +37,8 @@ steps: # the partial JSON is exactly what an operator needs to investigate. publish_artifact() { if [[ -s "$json_file" ]]; then - mkdir -p "$(ob_outputDirectory)/changed-components" - cp "$json_file" "$(ob_outputDirectory)/changed-components/" || true + mkdir -p "$OB_OUTPUT_DIR/changed-components" + cp "$json_file" "$OB_OUTPUT_DIR/changed-components/" || true fi } trap publish_artifact EXIT @@ -59,8 +59,10 @@ steps: echo "##vso[task.setvariable variable=$CHANGED_COMPONENTS_FILE_VAR;isreadonly=true]$json_file" echo "##vso[task.setvariable variable=$RENDER_SET_FILE_VAR;isreadonly=true]$render_set_file" env: + ARTIFACT_STAGING_DIR: $(Build.ArtifactStagingDirectory) CHANGED_COMPONENTS_FILE_VAR: ${{ parameters.changedComponentsFileVar }} FROM_COMMIT: $(${{ parameters.fromCommitVar }}) + OB_OUTPUT_DIR: $(ob_outputDirectory) RENDER_SET_FILE_VAR: ${{ parameters.renderSetFileVar }} TO_COMMIT: $(${{ parameters.toCommitVar }}) displayName: "Prepare change set" diff --git a/.github/workflows/ado/templates/steps/verify-locks.yml b/.github/workflows/ado/templates/steps/verify-locks.yml index 4ebcf98ef8b..efdb11cb2e2 100644 --- a/.github/workflows/ado/templates/steps/verify-locks.yml +++ b/.github/workflows/ado/templates/steps/verify-locks.yml @@ -9,6 +9,9 @@ steps: - script: | set -euo pipefail scripts/ci/control-tower/verify_locks.sh \ - --output-file "$(Build.ArtifactStagingDirectory)/lock-update.json" \ - --publish-dir "$(ob_outputDirectory)" + --output-file "$ARTIFACT_STAGING_DIR/lock-update.json" \ + --publish-dir "$OB_OUTPUT_DIR" + env: + ARTIFACT_STAGING_DIR: $(Build.ArtifactStagingDirectory) + OB_OUTPUT_DIR: $(ob_outputDirectory) displayName: "Verify lock files are up to date" diff --git a/scripts/ci/components/compute_change_set.sh b/scripts/ci/components/compute_change_set.sh index 55696aa8c0b..b7988aa5e01 100755 --- a/scripts/ci/components/compute_change_set.sh +++ b/scripts/ci/components/compute_change_set.sh @@ -51,6 +51,18 @@ while [[ $# -gt 0 ]]; do done [[ -z "${output_dir:-}" || -z "${from_commit:-}" || -z "${to_commit:-}" ]] && usage +# The output-file-name params are joined onto --output-dir verbatim, so a value +# containing a path separator or traversal (e.g. `../foo`) would escape it. +# Reject anything that is not a plain file name. +for name in "$changed_components_file_name" "$specs_diff_file_name" "$render_set_file_name"; do + case "$name" in + */* | "" | . | ..) + echo "refusing unsafe output file name: '$name' (must be a plain file name)" >&2 + exit 2 + ;; + esac +done + # Defensive guard: the script owns --output-dir exclusively for the duration # of the invocation (it does `rm -rf` below to clean up stale state). Refuse # obviously-dangerous paths so a future caller passing an empty-after- From f2bfe9c86442c410baf09374dd0b799521ea15cc Mon Sep 17 00:00:00 2001 From: Pawel Winogrodzki Date: Thu, 2 Jul 2026 17:26:54 -0700 Subject: [PATCH 14/14] ci: audit PR against ADO instructions -- unmap auto-exposed predefined vars Audited every ADO YAML changed in this PR against .github/instructions/ado-pipeline.instructions.md. One violation of the reserved-predefined-variable rule found and fixed: commit-range-postmerge.yml mapped five auto-exposed predefined variables (Build.BuildId, Build.SourceVersion, System.CollectionUri, System.DefinitionId, System.TeamProject) in env:. Per the rule these are auto-exposed as $NAME and should not be mapped; the script already references them directly (and determine_commit_range.py reads System.CollectionUri/TeamProject from os.environ). Removed the five mappings and switched --branch to the auto-exposed $BUILD_SOURCEBRANCH; kept only the secret SYSTEM_ACCESSTOKEN (not auto-exposed) plus the two var-name parameters. All other changed files pass the audit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ado/templates/steps/commit-range-postmerge.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml index 49a181c20a6..98306e18b70 100644 --- a/.github/workflows/ado/templates/steps/commit-range-postmerge.yml +++ b/.github/workflows/ado/templates/steps/commit-range-postmerge.yml @@ -30,7 +30,7 @@ steps: range_output="$(python3 scripts/ci/ado/determine_commit_range.py \ --definition-id "$SYSTEM_DEFINITIONID" \ --current-build-id "$BUILD_BUILDID" \ - --branch "$DELTA_QUERY_BRANCH" \ + --branch "$BUILD_SOURCEBRANCH" \ --source-commit "$BUILD_SOURCEVERSION")" echo "$range_output" @@ -52,13 +52,12 @@ steps: echo "##vso[task.setvariable variable=$BASE_COMMIT_VAR;isreadonly=true]$base_commit" echo "##vso[task.setvariable variable=$SOURCE_COMMIT_VAR;isreadonly=true]$source_commit" env: + # System.AccessToken is a secret -- NOT auto-exposed -- so it must be + # mapped here. Every other predefined variable the script uses + # (Build.BuildId, Build.SourceBranch, Build.SourceVersion, + # System.CollectionUri, System.DefinitionId, System.TeamProject) is + # auto-exposed as $NAME and referenced directly, so it is not mapped here. BASE_COMMIT_VAR: ${{ parameters.baseCommitVar }} - BUILD_BUILDID: $(Build.BuildId) - BUILD_SOURCEVERSION: $(Build.SourceVersion) - DELTA_QUERY_BRANCH: $(Build.SourceBranch) SOURCE_COMMIT_VAR: ${{ parameters.sourceCommitVar }} SYSTEM_ACCESSTOKEN: $(System.AccessToken) - SYSTEM_COLLECTIONURI: $(System.CollectionUri) - SYSTEM_DEFINITIONID: $(System.DefinitionId) - SYSTEM_TEAMPROJECT: $(System.TeamProject) displayName: "Determine source and base commit range"