diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 097fa65b3e..e2083c3ad2 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -3109,11 +3109,13 @@ jobs: # (`rate-limit-wait-seconds` on check-contract and aggregate-results), 45 s # of own-gate retries, 7 s of status-write retries and the 90 s # `rerun-wait-seconds` wait, about four and a half minutes, plus up to - # 15 s of retry sleeps for each of the hold-lift step's three reads, the + # 15 s of retry sleeps for each of the hold-lift step's two reads, the # review-lane step's four and the lanes-hold step's eight, about four - # minutes, plus the cost-hold step's reads: three for most pull requests, + # minutes, plus the cost-hold step's reads: one for most pull requests, # which stop at the label events; for a lane-labeled one whose body cites a - # decision, about ten, and two more per review run since the window opened. + # decision, about eight, and two more per review run since the window + # opened, plus the base-config checkout's fetch (seconds; its retries + # replace the contents reads' 15 s each). timeout-minutes: 11 permissions: # `yield-to-full-run` lists this workflow's runs on the head SHA, and @@ -3163,12 +3165,15 @@ jobs: BROKER_PATHS='.github/actions/resolve-config/package.json .github/actions/resolve-config/package-lock.json' for attempt in 1 2 3; do - if files=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --paginate --jq '.[] | .filename, (.previous_filename // empty)'); then + if pages=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --paginate); then break fi [ "$attempt" -lt 3 ] || { echo "::error::could not read the files of #$PR_NUMBER; re-run this run."; exit 1; } sleep $((attempt * 5)) done + files=$(jq -r '.[] | .filename, (.previous_filename // empty)' <<<"$pages") + # The review-lane step reads the same list from here instead of again. + printf '%s\n' "$pages" >"$RUNNER_TEMP/pr-files.json" touched= for path in $BROKER_PATHS; do if grep -qxF "$path" <<<"$files"; then touched="$touched $path"; fi @@ -3252,6 +3257,32 @@ jobs: msg="draft: lanes not run; the ready_for_review run lints and tests this SHA" echo "::notice::$msg" echo "$msg" >>"$GITHUB_STEP_SUMMARY" + # THE HOLD STEPS READ THEIR CONFIG FROM THE BASE COMMIT, ONCE. The tracker + # config names the human-gated label and the trusted-actor list names who + # may lift a hold; both are read at the base so the pull request cannot + # rewrite its own hold. A sparse checkout of the two files at the base SHA + # is a git fetch, not a REST request: as of actions/checkout v7.0.1 its + # API calls (default branch, object format, the stale-merge tracepoint, the + # no-git archive download) all return early for a 40-hex `ref` on a runner + # with git; recheck on a checkout bump. It checks out no pull request + # code: the ref is the base commit, nothing here is executed, and no + # credential is persisted. + # The steps below read BASE_CONFIG: empty when this step did not succeed, + # which each of them treats as an unreadable config; a file missing from + # a checkout that succeeded is a file the base commit does not have. + - name: Check out the base commit's hold config + id: base_config + if: ${{ !cancelled() && (github.event_name == 'pull_request' || github.event_name == 'merge_group') }} + continue-on-error: true + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }} + path: base-config + persist-credentials: false + sparse-checkout: | + /.work-item-tracker.json + /.github/standards/trusted-actors/trusted-actors.json + sparse-checkout-cone-mode: false # A COST HOLD THE OWNER ALREADY DECIDED COMES OFF. A review lane re-holds # every new head, so its `cost` hold went back on after the owner had # recorded the cost decision. This step clears a hold only when all @@ -3294,6 +3325,7 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} LANES_APP_ID: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }} + BASE_CONFIG: ${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }} run: | keep() { echo "The cost hold stays for a person: $1." @@ -3314,15 +3346,18 @@ jobs: [[ $BASE_SHA =~ ^[0-9a-f]{40}$ && $HEAD_SHA =~ ^[0-9a-f]{40}$ && $PR_NUMBER =~ ^[1-9][0-9]*$ ]] || keep "the event names no commit or pull request" [[ $LANES_APP_ID =~ ^[1-9][0-9]*$ ]] || keep "AUTOMATION_LANES_APP_SENDER_ID is not set" repo=$GITHUB_REPOSITORY - tracker=$(read_api -H 'Accept: application/vnd.github.raw+json' "repos/$repo/contents/.work-item-tracker.json?ref=$BASE_SHA") && rc=0 || rc=$? - [ "$rc" -ne 1 ] || keep "the tracker config is unreadable" + [ -d "$BASE_CONFIG" ] || keep "the tracker config is unreadable" + tracker= + if [ -e "$BASE_CONFIG/.work-item-tracker.json" ]; then + tracker=$(cat "$BASE_CONFIG/.work-item-tracker.json") || keep "the tracker config is unreadable" + fi label=needs-human - if [ "$rc" -eq 0 ] && [ -n "$tracker" ]; then + if [ -n "$tracker" ]; then label=$(jq -er '[objects | .config | objects | .role_labels | objects | .["human-gated"] | strings | gsub("^\\s+|\\s+$"; "")][0] // "" | if . == "" then "needs-human" else . end' <<<"$tracker") || keep "the tracker config is invalid" fi - actors=$(read_api -H 'Accept: application/vnd.github.raw+json' "repos/$repo/contents/.github/standards/trusted-actors/trusted-actors.json?ref=$BASE_SHA") || keep "the trusted-actor list is unreadable" + actors=$(cat "$BASE_CONFIG/.github/standards/trusted-actors/trusted-actors.json") || keep "the trusted-actor list is unreadable" humans=$(jq -ec '[.actors[] | select(.kind == "human") | .id | numbers] | select(length > 0)' <<<"$actors") || keep "the trusted-actor list names no person" events=$(read_api "repos/$repo/issues/$PR_NUMBER/events?per_page=100" --paginate) || keep "the events are unreadable" # The window opens at the newest removal by a trusted person. @@ -3458,21 +3493,17 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} BASE_SHA: ${{ github.event.pull_request.base.sha }} + BASE_CONFIG: ${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }} run: | if [[ ! $BASE_SHA =~ ^[0-9a-f]{40}$ ]]; then echo "::error::the base commit '$BASE_SHA' is not a commit SHA." exit 1 fi - for attempt in 1 2 3; do - if tracker=$(gh api -H 'Accept: application/vnd.github.raw+json' "repos/$GITHUB_REPOSITORY/contents/.work-item-tracker.json?ref=$BASE_SHA" 2>"$RUNNER_TEMP/tracker.err"); then - break - elif grep -q 'HTTP 404' "$RUNNER_TEMP/tracker.err"; then - tracker= - break - fi - [ "$attempt" -lt 3 ] || { echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run."; exit 1; } - sleep $((attempt * 5)) - done + tracker= + if [ ! -d "$BASE_CONFIG" ] || { [ -e "$BASE_CONFIG/.work-item-tracker.json" ] && ! tracker=$(cat "$BASE_CONFIG/.work-item-tracker.json"); }; then + echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run." + exit 1 + fi human_gated=needs-human if [ -n "$tracker" ]; then human_gated=$(jq -er '[objects | .config | objects | .role_labels | objects @@ -3533,6 +3564,7 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} BASE_SHA: ${{ github.event.pull_request.base.sha }} HUMAN_GATED: ${{ steps.human_gated.outputs.label }} + BASE_CONFIG: ${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }} run: | if [[ ! $BASE_SHA =~ ^[0-9a-f]{40}$ ]]; then echo "::error::the base commit '$BASE_SHA' is not a commit SHA." @@ -3568,13 +3600,10 @@ jobs: echo "No review lane has held #$PR_NUMBER." exit 0 fi - for attempt in 1 2 3; do - if actors=$(gh api -H 'Accept: application/vnd.github.raw+json' "repos/$GITHUB_REPOSITORY/contents/.github/standards/trusted-actors/trusted-actors.json?ref=$BASE_SHA"); then - break - fi - [ "$attempt" -lt 3 ] || { echo "::error::could not read the trusted-actor list at $BASE_SHA; re-run this run."; exit 1; } - sleep $((attempt * 5)) - done + if [ ! -d "$BASE_CONFIG" ] || ! actors=$(cat "$BASE_CONFIG/.github/standards/trusted-actors/trusted-actors.json"); then + echo "::error::could not read the trusted-actor list at $BASE_SHA; re-run this run." + exit 1 + fi humans=$(jq -er '[.actors[] | select(.kind == "human") | .id | numbers | tostring] | select(length > 0) | join(" ")' <<<"$actors") || { echo "::error::the trusted-actor list at $BASE_SHA names no person." exit 1 @@ -3656,13 +3685,19 @@ jobs: exit 1 fi pr=$(read_api "#$PR_NUMBER" "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER") || exit 1 - for attempt in 1 2 3; do - if files=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --paginate --jq '.[] | [.filename, (.previous_filename // "")] | @tsv'); then - break - fi - [ "$attempt" -lt 3 ] || { echo "::error::could not read the files of #$PR_NUMBER; re-run this run."; exit 1; } - sleep $((attempt * 5)) - done + # The broker step above leaves the file list it read; read it again + # only when that step could not. + if [ -s "$RUNNER_TEMP/pr-files.json" ]; then + files=$(jq -r '.[] | [.filename, (.previous_filename // "")] | @tsv' "$RUNNER_TEMP/pr-files.json") + else + for attempt in 1 2 3; do + if files=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --paginate --jq '.[] | [.filename, (.previous_filename // "")] | @tsv'); then + break + fi + [ "$attempt" -lt 3 ] || { echo "::error::could not read the files of #$PR_NUMBER; re-run this run."; exit 1; } + sleep $((attempt * 5)) + done + fi if [ "$(grep -c . <<<"$files")" -lt "$(jq '.changed_files // 0' <<<"$pr")" ]; then echo "::error::GitHub listed fewer files than #$PR_NUMBER changes, so its hold-paths cannot be checked; a person merges it." exit 1 @@ -3748,6 +3783,7 @@ jobs: env: GH_TOKEN: ${{ github.token }} BASE_SHA: ${{ github.event.merge_group.base_sha }} + BASE_CONFIG: ${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }} run: | pr=$(sed -nE 's#^(refs/heads/)?gh-readonly-queue/.+/pr-([0-9]+)-[0-9a-f]{40}$#\2#p' <<<"$GITHUB_REF") if [ -z "$pr" ]; then @@ -3758,16 +3794,11 @@ jobs: echo "::error::the base commit '$BASE_SHA' is not a commit SHA." exit 1 fi - for attempt in 1 2 3; do - if tracker=$(gh api -H 'Accept: application/vnd.github.raw+json' "repos/$GITHUB_REPOSITORY/contents/.work-item-tracker.json?ref=$BASE_SHA" 2>"$RUNNER_TEMP/tracker.err"); then - break - elif grep -q 'HTTP 404' "$RUNNER_TEMP/tracker.err"; then - tracker= - break - fi - [ "$attempt" -lt 3 ] || { echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run."; exit 1; } - sleep $((attempt * 5)) - done + tracker= + if [ ! -d "$BASE_CONFIG" ] || { [ -e "$BASE_CONFIG/.work-item-tracker.json" ] && ! tracker=$(cat "$BASE_CONFIG/.work-item-tracker.json"); }; then + echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run." + exit 1 + fi human_gated=needs-human if [ -n "$tracker" ]; then human_gated=$(jq -er '[objects | .config | objects | .role_labels | objects diff --git a/scripts/ci-broker-pin.test.sh b/scripts/ci-broker-pin.test.sh index daa9829a10..05b9d277c3 100755 --- a/scripts/ci-broker-pin.test.sh +++ b/scripts/ci-broker-pin.test.sh @@ -1,8 +1,9 @@ #!/usr/bin/env bash # Runs the `Require a broker deploy PR for a resolve-config dependency change` step body of # .github/workflows/pr-require-checks.yml's ci-status under the shell Actions -# gives a step with no `shell:` (bash -e), against a stub `gh` that applies the -# step's own --jq filter to a pull-request files fixture. +# gives a step with no `shell:` (bash -e), against a stub `gh` that prints a +# pull-request files fixture. The step leaves the pages it read under +# RUNNER_TEMP for the review-lane step. # # The lanes token broker in melodic-software/azure-iac loads resolve-config's # modules from main's tip but imports ajv and yaml from its own deployment, and @@ -55,20 +56,15 @@ else fail "step is not inside ci-status before the aggregate (lines: $order)" fi -# The stub logs its arguments, fails the first STUB_FAILS calls, then applies -# the --jq argument to STUB_JSON, as gh does to each page. +# The stub logs its arguments, fails the first STUB_FAILS calls, then prints +# STUB_JSON, one page. mkdir -p "$TMP_ROOT/bin" cat >"$TMP_ROOT/bin/gh" <<'EOF' #!/usr/bin/env bash printf '%s\n' "$*" >>"$STUB_LOG" calls=$(wc -l <"$STUB_LOG") [[ "$calls" -gt "${STUB_FAILS:-0}" ]] || exit 1 -filter= -while [[ $# -gt 0 ]]; do - [[ "$1" == --jq ]] && { filter="$2"; shift; } - shift -done -jq -r "$filter" <<<"$STUB_JSON" +printf '%s\n' "$STUB_JSON" EOF printf '#!/usr/bin/env bash\n' >"$TMP_ROOT/bin/sleep" chmod +x "$TMP_ROOT/bin/gh" "$TMP_ROOT/bin/sleep" @@ -87,15 +83,20 @@ files() { expect() { local label="$1" want_rc="$2" want_calls="$3" out rc calls : >"$TMP_ROOT/log" + rm -f "$TMP_ROOT/pr-files.json" out="$(PATH="$TMP_ROOT/bin:$PATH" STUB_LOG="$TMP_ROOT/log" STUB_JSON="$4" PR_BODY="$5" STUB_FAILS="$6" \ - PR_NUMBER=8313 GITHUB_REPOSITORY=o/r bash -e "$TMP_ROOT/body.sh" 2>&1)" && rc=0 || rc=$? + PR_NUMBER=8313 GITHUB_REPOSITORY=o/r RUNNER_TEMP="$TMP_ROOT" bash -e "$TMP_ROOT/body.sh" 2>&1)" && rc=0 || rc=$? calls=$(wc -l <"$TMP_ROOT/log") if [[ "$rc" -ne "$want_rc" ]]; then fail "$label: expected rc=$want_rc got rc=$rc out='$out'" elif [[ "$calls" -ne "$want_calls" ]]; then fail "$label: expected $want_calls gh call(s) got $calls" - elif [[ "$want_calls" -gt 0 && "$(sort -u "$TMP_ROOT/log")" != "api repos/o/r/pulls/8313/files?per_page=100 --paginate --jq "* ]]; then + elif [[ "$want_calls" -gt 0 && "$(sort -u "$TMP_ROOT/log")" != "api repos/o/r/pulls/8313/files?per_page=100 --paginate" ]]; then fail "$label: unexpected gh call '$(sort -u "$TMP_ROOT/log")'" + elif [[ "$want_calls" -gt "$6" ]] && ! jq -se --argjson want "$(jq -sc . <<<"$4")" '. == $want' "$TMP_ROOT/pr-files.json" >/dev/null 2>&1; then + fail "$label: the pages read were not left for the review-lane step: '$(cat "$TMP_ROOT/pr-files.json" 2>&1)'" + elif [[ "$want_calls" -le "$6" && -e "$TMP_ROOT/pr-files.json" ]]; then + fail "$label: an unread file list left a pr-files.json" elif [[ -n "${7:-}" && "$out" != *"$7"* ]]; then fail "$label: output lacks '$7': '$out'" else @@ -117,6 +118,8 @@ expect "files the broker does not load need no line" 0 1 \ expect "a same-named file elsewhere needs no line" 0 1 "$(files "package.json" "$RC/sub/package.json" "$RC/package.json.orig")" "" 0 expect "a rename away from a dependency path fails" 1 1 \ "[{\"filename\":\"$RC/old/package.json\",\"previous_filename\":\"$RC/package.json\",\"status\":\"renamed\"}]" "" 0 "$RC/package.json" +expect "a dependency path on the second page fails" 1 1 "$(files README.md)$(files "$RC/package.json")" "" 0 "$RC/package.json" +expect "a dependency path on the second page passes with the line" 0 1 "$(files README.md)$(files "$RC/package.json")" "$LINE" 0 expect "a CRLF body line counts" 0 1 "$(files "$RC/package.json")" $'Closes #1\r\n'"$LINE"$'\r\n## Summary\r' 0 expect "trailing blanks after the number count" 0 1 "$(files "$RC/package.json")" "$LINE " 0 expect "the line inside other text does not count" 1 1 "$(files "$RC/package.json")" "see $LINE" 0 diff --git a/scripts/ci-hold-cost-decision.test.sh b/scripts/ci-hold-cost-decision.test.sh index 57f3effc32..87a2882f2f 100755 --- a/scripts/ci-hold-cost-decision.test.sh +++ b/scripts/ci-hold-cost-decision.test.sh @@ -49,7 +49,8 @@ else fail "step lost its cost_hold id" fi # shellcheck disable=SC2016 # the ${{ }} text is workflow source, not shell -for env in 'BASE_SHA: ${{ github.event.pull_request.base.sha }}' 'HEAD_SHA: ${{ github.event.pull_request.head.sha }}'; do +for env in 'BASE_SHA: ${{ github.event.pull_request.base.sha }}' 'HEAD_SHA: ${{ github.event.pull_request.head.sha }}' \ + "BASE_CONFIG: \${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }}"; do if [[ "$step" == *"$env"* ]]; then ok "step env carries $env"; else fail "step env lacks $env"; fi done order="$(grep -nE "^ ci-status:$|^ - name: ($NAME|Fail a pull request carrying the human-gated label|Fail a review-lane hold an untrusted account lifted)$" "$WORKFLOW" | cut -d: -f1 | paste -sd' ')" @@ -67,7 +68,10 @@ else fi # The stub logs its arguments and answers from STUB_* variables; a read whose -# name is in STUB_FAIL fails. Runs, jobs and annotations come from JSON maps +# name is in STUB_FAIL fails. The base commit's config is a fixture directory: +# STUB_TRACKER (absent when unset) and STUB_ACTORS; a `tracker` failure leaves +# BASE_CONFIG empty, as a failed checkout does, and an `actors` failure leaves +# the trusted-actor list out of it. Runs, jobs and annotations come from JSON maps # keyed by workflow file, run id and check-run id. mkdir -p "$TMP_ROOT/bin" cat >"$TMP_ROOT/bin/gh" <<'EOF' @@ -78,12 +82,6 @@ from() { jq -ce --arg k "$2" '.[$k] // error' <<<"$1" || { echo 'gh: Not Found ( # The path segment after /<1>/ in the api path argument. key() { local a; for a in "${@:2}"; do [[ "$a" == */"$1"/* ]] && { a="${a##*/"$1"/}"; printf '%s' "${a%%[/?]*}"; return; }; done; } case "$*" in -*/contents/.work-item-tracker.json*) - failing tracker - [[ -n "${STUB_TRACKER+set}" ]] || { echo 'gh: Not Found (HTTP 404)' >&2; exit 1; } - printf '%s\n' "$STUB_TRACKER" - ;; -*/contents/.github/standards/trusted-actors/*) failing actors; printf '%s\n' "$STUB_ACTORS" ;; *'-X DELETE'*/labels/*) failing delete ;; */issues/6419/labels*) failing labels; [[ -z "$STUB_LABELS" ]] || tr ',' '\n' <<<"$STUB_LABELS" ;; */issues/6419/events*) failing events; printf '%s\n' "$STUB_EVENTS" ;; @@ -165,18 +163,25 @@ lifted() { # expect