diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 5a423022..9a60cfb5 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -3,7 +3,7 @@ on: pull_request: permissions: id-token: write - contents: read + contents: write jobs: pre-commit: runs-on: warp-custom-ubuntu-24-04 diff --git a/.github/workflows/test-build-number.yml b/.github/workflows/test-build-number.yml index 0670aa96..833e375e 100644 --- a/.github/workflows/test-build-number.yml +++ b/.github/workflows/test-build-number.yml @@ -14,7 +14,7 @@ jobs: runs-on: warp-custom-ubuntu-24-04 permissions: id-token: write - contents: read + contents: write outputs: BUILD_NUMBER: ${{ steps.get_build_number.outputs.BUILD_NUMBER }} steps: @@ -46,12 +46,12 @@ jobs: exit 1 fi - test-build-number-reuse-from-cache: + test-build-number-reuse-same-run: needs: test-build-number-generation runs-on: warp-custom-ubuntu-24-04 permissions: id-token: write - contents: read + contents: write steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: @@ -61,18 +61,17 @@ jobs: run: | echo "BUILD_NUMBER: ${BUILD_NUMBER}" if [[ "${BUILD_NUMBER}" != "${{ needs.test-build-number-generation.outputs.BUILD_NUMBER }}" ]]; then - echo -e "::error title=test-build-number-reuse::Build number '${BUILD_NUMBER}' does not match the previous job build number" \ - "'${{ needs.test-build-number-generation.outputs.BUILD_NUMBER }}' despite it is the same workflow run.\n" \ - "Prefer using the output from SonarSource/ci-github-actions/get-build-number instead of calling it from distinct jobs." - # exit 1 # flaky test + echo "::error title=test-build-number-reuse::Build number '${BUILD_NUMBER}' does not match the previous job build number" \ + "'${{ needs.test-build-number-generation.outputs.BUILD_NUMBER }}' despite it is the same workflow run." + exit 1 fi - test-build-number-reuse-from-cache-windows: + test-build-number-reuse-same-run-windows: needs: test-build-number-generation runs-on: github-windows-latest-s permissions: id-token: write - contents: read + contents: write steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: @@ -83,10 +82,9 @@ jobs: run: | echo "BUILD_NUMBER: ${BUILD_NUMBER}" if [[ "${BUILD_NUMBER}" != "${{ needs.test-build-number-generation.outputs.BUILD_NUMBER }}" ]]; then - echo -e "::error title=test-build-number-reuse-from-cache-windows::Build number '${BUILD_NUMBER}' does not match the previous" \ - "job build number '${{ needs.test-build-number-generation.outputs.BUILD_NUMBER }}' despite it is the same workflow run.\n" \ - "Prefer using the output from SonarSource/ci-github-actions/get-build-number instead of calling it from distinct jobs." - # exit 1 # flaky test + echo "::error title=test-build-number-reuse-same-run-windows::Build number '${BUILD_NUMBER}' does not match the previous" \ + "job build number '${{ needs.test-build-number-generation.outputs.BUILD_NUMBER }}' despite it is the same workflow run." + exit 1 fi test-build-number-reuse-from-env: @@ -94,7 +92,7 @@ jobs: runs-on: warp-custom-ubuntu-24-04 permissions: id-token: write - contents: read + contents: write env: BUILD_NUMBER: ${{ needs.test-build-number-generation.outputs.BUILD_NUMBER }} steps: @@ -119,13 +117,13 @@ jobs: if: always() needs: - test-build-number-generation - - test-build-number-reuse-from-cache - - test-build-number-reuse-from-cache-windows + - test-build-number-reuse-same-run + - test-build-number-reuse-same-run-windows - test-build-number-reuse-from-env runs-on: warp-custom-ubuntu-24-04 permissions: id-token: write - contents: read + contents: write steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - uses: ./config-npm diff --git a/.github/workflows/test-shell-scripts.yml b/.github/workflows/test-shell-scripts.yml index 6e0f5020..c888f1e8 100644 --- a/.github/workflows/test-shell-scripts.yml +++ b/.github/workflows/test-shell-scripts.yml @@ -14,7 +14,7 @@ jobs: runs-on: warp-custom-ubuntu-24-04 permissions: id-token: write - contents: read + contents: write steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: diff --git a/.github/workflows/test-update-release-channel.yml b/.github/workflows/test-update-release-channel.yml index 8bd3266a..3dbb2bfb 100644 --- a/.github/workflows/test-update-release-channel.yml +++ b/.github/workflows/test-update-release-channel.yml @@ -14,7 +14,7 @@ jobs: runs-on: sonar-xs permissions: id-token: write - contents: read + contents: write steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Update release channel (dry-run, happy path) diff --git a/README.md b/README.md index 7e4e5f56..13faa798 100644 --- a/README.md +++ b/README.md @@ -70,31 +70,33 @@ These badges show the status of workflows in dummy repositories that use (or sho ## `get-build-number` -Manage the build number in GitHub Actions. +Get a unique, strictly increasing build number for a repository, reusing one already claimed by the current workflow run when applicable. +It sets `BUILD_NUMBER` as both an environment variable and a GitHub Actions output. Safe to call from multiple jobs in the same workflow +run - only one of them claims a number, the others wait for it and reuse it - and from concurrent workflow runs (e.g. several GitHub +Stacked PRs opened at once), where no two runs will ever get the same number. A job waiting for another job's claim fails after a bounded +timeout (a few minutes) if that claim never completes, rather than claiming an independent number - see [Git References](#git-references) +below for how this is coordinated. -The build number is stored in the GitHub repository property named `build_number`. This action will reuse or increment the build number, -and set it as an environment variable named `BUILD_NUMBER`, and as a GitHub Actions output variable also named `BUILD_NUMBER`. - -The build number is unique per workflow run ID. It is not incremented on workflow reruns. - -During execution the action temporarily writes `.build_number.txt` at the repository root (for -`actions/cache`); the file is removed before the action completes. Do not track a file named -`.build_number.txt` in your repository. - -The action authenticates `gh` with a Vault-issued GitHub token. It sets both `GITHUB_TOKEN` and -`GH_TOKEN` for that step so a workflow-exported `GH_TOKEN` cannot shadow the Vault credential -(`gh` prefers `GH_TOKEN` over `GITHUB_TOKEN`). +During execution the action temporarily writes `.build_number.txt` at the repository root; the file is removed before the action +completes. Do not track a file named `.build_number.txt` in your repository. ### Requirements #### Required GitHub Permissions - `id-token: write` -- `contents: read` +- `contents: write` + +> **Breaking change:** this action used to require only `contents: read`. Claiming now needs `contents: write` to create the +> [Git references](#git-references) below - `contents: read` alone will fail with a 403 on every claim except the narrow case where this +> exact workflow run already has a marker to reuse (e.g. certain reruns), since that path is read-only. There is no working +> read-only/rerun-only mode: any run that needs a genuinely new number will fail until the caller's `permissions:` block is updated. #### Required Vault Permissions -- `build-number`: GitHub preset to read and write the build number property. This is built-in to the Vault `auth.github` permission. +- `build-number`: GitHub preset used to read the legacy `build_number` repository property, needed only for repositories that predate this + action's current design. Built-in to the Vault `auth.github` permission. This dependency will be dropped once no repository needs it + anymore. ### Usage @@ -104,7 +106,7 @@ jobs: runs-on: sonar-xs permissions: id-token: write - contents: read + contents: write steps: - uses: SonarSource/ci-github-actions/get-build-number@v1 ``` @@ -131,6 +133,44 @@ No inputs are required for this action. |----------------------|--------------------------| | `BUILD_NUMBER` | The current build number | +### Git References + +This action coordinates purely through Git references on the repository - no external state, cache, or database. Each reference is created +pointing at `$GITHUB_SHA` (the commit that triggered the claim); that target is never read back - only the reference's existence matters +(for `build-number` and `build-number-lock`), or its name, which encodes the claimed number (for `build-runs`). + +| Reference | Lifetime | Purpose | +| ------------------------------------- | -------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `refs/build-number/` | Deleted once superseded by the next claim | The atomic claim itself and the sole source of truth for uniqueness. Deleting the superseded ref is only safe because it happens while holding the exclusive `build-number-lock` below - without that, a claim stalled in flight for an older number could resurrect it after deletion, republishing a number already used elsewhere (a real bug found in review before this lock existed). At most one ref (rarely a couple, if a past deletion failed and self-heals on the next claim) exists at a time. | +| `refs/build-runs//` | Not automatically deleted - see [Inspecting current refs](#inspecting-current-refs) below | Marker recording which number a workflow run claimed. Checked first, and lock-free, so a rerun or another job in the same run reuses it without ever touching `build-number-lock`. A completed run can still be re-run much later, so "the run finished" does not make its marker safe to delete - only the run's own record being gone from GitHub (confirmed via a 404, never inferred from age) does, since that's what makes re-running it impossible. | +| `refs/build-number-lock/global` | Seconds: created, used, and deleted again within a single claim, not a persistent artifact | Exclusive, repository-wide lock serializing every new claim (not just those within one run) - what makes deleting a superseded `build-number` ref above safe. Assumes claims are infrequent and each is fast enough that the resulting wait queue stays short; every other claimant waits for either this lock or the marker above instead of racing independently. | + +### Known limitations + +- `refs/build-runs//` markers are never automatically deleted, so they accumulate for the lifetime of the + repository - see [Inspecting current refs](#inspecting-current-refs) below to check what currently exists. Not expected to + matter in practice for a long time; no automated cleanup is implemented today. + +### Inspecting current refs + +These refs aren't pulled by a normal `git fetch`/`git clone` (they live outside the default `refs/heads/*`/`refs/tags/*` +namespaces), but can be listed directly with `git ls-remote` or the GitHub API. + +Current build number - only one `refs/build-number/*` ref exists at a time, since each new claim deletes the one it supersedes: + +```shell +$ git ls-remote origin 'refs/build-number/*' +7345fe785a3fc8705e3d8204a28ca4f909628e7e refs/build-number/12106 +``` + +History - mapping a workflow run to the build number it claimed or reused, one `refs/build-runs//` ref per run: + +```shell +$ git ls-remote origin 'refs/build-runs/*' +3cfd2386fdd965247c0e465ff4774239036894ec refs/build-runs/32867023550/12093 +002e0fb9a88da75c89316635b1f0a62a6ad9aff4 refs/build-runs/32868621162/12095 +``` + --- ## `config-maven` @@ -177,7 +217,7 @@ By default, Maven caches `~/.m2/repository`. You can customize this behavior: #### Required GitHub Permissions - `id-token: write` -- `contents: read` +- `contents: write` #### Required Vault Permissions @@ -1182,7 +1222,7 @@ This action configures pip to pull packages from the internal JFrog Artifactory #### Required GitHub Permissions - `id-token: write` -- `contents: read` +- `contents: write` #### Required Vault Permissions @@ -1193,7 +1233,7 @@ This action configures pip to pull packages from the internal JFrog Artifactory ```yaml permissions: id-token: write - contents: read + contents: write steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: SonarSource/ci-github-actions/config-pip@v1 @@ -1292,7 +1332,7 @@ default = true #### Required GitHub Permissions - `id-token: write` -- `contents: read` +- `contents: write` #### Required Vault Permissions @@ -1307,7 +1347,7 @@ The `uv` tool must be pre-installed. Use of `mise` is recommended. ```yaml permissions: id-token: write - contents: read + contents: write steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: jdx/mise-action@1648a7812b9aeae629881980618f079932869151 # v4.0.1 diff --git a/get-build-number/action.yml b/get-build-number/action.yml index 6d7807af..cdbd1c16 100644 --- a/get-build-number/action.yml +++ b/get-build-number/action.yml @@ -3,7 +3,7 @@ name: Get build number description: GitHub Action to get the build number for a repository outputs: BUILD_NUMBER: - description: The build number, incremented or reused if already cached + description: The build number, newly claimed or reused if this workflow run already claimed one value: ${{ steps.export.outputs.BUILD_NUMBER }} inputs: host-actions-root: @@ -38,36 +38,31 @@ runs: if: env.BUILD_NUMBER != '' shell: bash run: | - echo "BUILD_NUMBER ${BUILD_NUMBER} provided from environment, skipping both increment and save to cache." + echo "BUILD_NUMBER ${BUILD_NUMBER} provided from environment, skipping increment." echo "${BUILD_NUMBER}" > "$BUILD_NUMBER_FILE" echo "skip=true" >> $GITHUB_OUTPUT - # Reuse current build number in case of rerun - - name: Get cached build number - if: steps.from-env.outputs.skip != 'true' - uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - id: current-build-number - with: - path: ${{ env.BUILD_NUMBER_FILE }} - key: build-number-${{ github.run_id }} - enableCrossOsArchive: true - - # Otherwise, increment the build number + # Vault is only needed to read the legacy build_number property during migration to refs/build-number/* (see README); fetched + # unconditionally for simplicity, since this whole dependency is temporary and slated for removal once migration is complete. + # continue-on-error: a repository with no build_number history (or no {REPO_OWNER_NAME_DASH}-build-number preset configured + # yet) has nothing to migrate - a failure here must not block claiming, only fall through to get_build_number.sh's own + # no-migration-token warning path (an empty LEGACY_PROPERTY_TOKEN below, same as if this step is skipped entirely). - uses: SonarSource/vault-action-wrapper@881045d830534a70ec3c7c275fa3714412c8ff6e # 3.6.1 id: secrets - if: steps.from-env.outputs.skip != 'true' && steps.current-build-number.outputs.cache-hit != 'true' + if: steps.from-env.outputs.skip != 'true' + continue-on-error: true with: secrets: development/github/token/{REPO_OWNER_NAME_DASH}-build-number token | github_token; - - name: Get new build number - if: steps.from-env.outputs.skip != 'true' && steps.current-build-number.outputs.cache-hit != 'true' + + # Reuses this run's own claim if one already exists (a rerun, or another job in the same run); otherwise claims a new one under + # an exclusive, repository-wide lock, released as soon as it's no longer needed - see get_build_number.sh. + - name: Get build number + if: steps.from-env.outputs.skip != 'true' shell: bash env: - # gh prefers GH_TOKEN over GITHUB_TOKEN. Set both so a workflow-exported - # GH_TOKEN (e.g. a bot token) cannot shadow the Vault build-number token. - GITHUB_TOKEN: ${{ steps.current-build-number.outputs.cache-hit != 'true' && - steps.secrets.outputs.vault && fromJSON(steps.secrets.outputs.vault).github_token || '' }} - GH_TOKEN: ${{ steps.current-build-number.outputs.cache-hit != 'true' && - steps.secrets.outputs.vault && fromJSON(steps.secrets.outputs.vault).github_token || '' }} + GITHUB_TOKEN: ${{ github.token }} + GH_TOKEN: ${{ github.token }} + LEGACY_PROPERTY_TOKEN: ${{ steps.secrets.outputs.vault && fromJSON(steps.secrets.outputs.vault).github_token || '' }} run: ${ACTION_PATH_GET_BUILD_NUMBER}/get_build_number.sh - name: Export build number @@ -83,14 +78,6 @@ runs: echo "BUILD_NUMBER=${BUILD_NUMBER}" >> "$GITHUB_ENV" echo "BUILD_NUMBER=${BUILD_NUMBER}" >> "$GITHUB_OUTPUT" - - name: Save build number to cache - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - if: steps.from-env.outputs.skip != 'true' && steps.current-build-number.outputs.cache-hit != 'true' - with: - path: ${{ env.BUILD_NUMBER_FILE }} - key: build-number-${{ github.run_id }} - enableCrossOsArchive: true - - name: Remove build number file from workspace if: always() shell: bash diff --git a/get-build-number/get_build_number.sh b/get-build-number/get_build_number.sh index 8a534bf6..eb38f35a 100755 --- a/get-build-number/get_build_number.sh +++ b/get-build-number/get_build_number.sh @@ -1,24 +1,192 @@ #!/bin/bash -# Get the build number for a GitHub repository and save the incremented value to .build_number.txt +# Get the build number for a GitHub repository and save it to .build_number.txt, reusing one already claimed by this workflow run. +# +# refs/build-number/: the atomic claim itself and the sole source of truth for uniqueness. Claiming a number and deleting the +# one it superseded both happen while holding build-number-lock below. +# refs/build-runs//: marker recording which number this workflow run claimed. Checked first, and lock-free, so reruns +# and other jobs in the same run reuse it without ever touching build-number-lock. +# refs/build-number-lock: exclusive, transient, repository-wide lock serializing every new claim, not just those within one run. +# Released immediately after use (success or failure) via the trap below, not held for a run's lifetime. +# +# Every ref points at $GITHUB_SHA purely because creation requires some valid target; that target is never read back. +# All ref reads/writes use the ambient GITHUB_TOKEN. LEGACY_PROPERTY_TOKEN (from Vault) is only used to read the legacy +# build_number property during migration - see README. set -euo pipefail -: "${GITHUB_REPOSITORY:?}" +: "${GITHUB_REPOSITORY:?}" "${GITHUB_SHA:?}" "${GITHUB_RUN_ID:?}" + GH_API_VERSION_HEADER="X-GitHub-Api-Version: 2022-11-28" BUILD_NUMBER_FILE="${BUILD_NUMBER_FILE:-.build_number.txt}" - -echo "Fetching build number from repository properties..." +REFS_API_URL="repos/${GITHUB_REPOSITORY}/git/refs" +MATCHING_REFS_API_URL="repos/${GITHUB_REPOSITORY}/git/matching-refs" PROPERTIES_API_URL="repos/${GITHUB_REPOSITORY}/properties/values" -BUILD_NUMBER=$(gh api -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" --jq '.[] | select(.property_name == "build_number") | .value') -echo "Current build number from repo: ${BUILD_NUMBER:=0}" -if ! [[ "$BUILD_NUMBER" =~ ^[0-9]+$ ]]; then - echo "::error title=Invalid build number::Build number '${BUILD_NUMBER}' is not a valid positive integer." >&2 +PROPS_JQ='.[] | select(.property_name == "build_number") | .value' +NUMBER_NS="build-number" +RUN_NS="build-runs/${GITHUB_RUN_ID}" +NUMBER_LOCK_REF="build-number-lock/global" # git/refs requires at least 3 slash-separated components; a bare name is rejected +LOCK_POLL_INTERVAL_SECONDS="${LOCK_POLL_INTERVAL_SECONDS:-3}" +LOCK_WAIT_MAX_ATTEMPTS="${LOCK_WAIT_MAX_ATTEMPTS:-40}" # ~2 minutes at the default interval +RETRY_INTERVAL_SECONDS="${RETRY_INTERVAL_SECONDS:-1}" # between internal retries of a single marker check or marker write + +create_ref() { + local ref="$1" + gh api --method POST -H "$GH_API_VERSION_HEADER" "$REFS_API_URL" -f "ref=refs/${ref}" -f "sha=${GITHUB_SHA}" 2>&1 +} + +delete_ref() { + local ref="$1" + gh api --method DELETE -H "$GH_API_VERSION_HEADER" "${REFS_API_URL}/${ref}" >/dev/null 2>&1 +} + +# Reports a fatal ref-creation failure, calling out the common "caller still has contents: read" cause by name instead of only +# dumping the raw API response, since that response alone reads as a generic permission error, not a fix. +fail_claim() { + local response="$1" + if [[ "$response" == *"Resource not accessible by integration"* ]]; then + echo "::error title=Build number claim failed::${response} This action requires 'contents: write' in the calling workflow's" \ + "permissions (contents: read is no longer sufficient)." >&2 + else + echo "::error title=Build number claim failed::${response}" >&2 + fi + exit 1 +} + +# A false "no marker" here would make this run claim a second, unnecessary number instead of reusing an existing one, so a +# single transient failure isn't enough to conclude that - retried once before falling back to "not yet published". +find_run_marker() { + local output check_attempt + for check_attempt in 1 2; do + if output=$(gh api -H "$GH_API_VERSION_HEADER" "${MATCHING_REFS_API_URL}/${RUN_NS}/" --jq '.[0].ref // empty' 2>&1); then + echo "$output" + return 0 + fi + ((check_attempt < 2)) && sleep "$RETRY_INTERVAL_SECONDS" + done + echo "::warning title=Marker check inconclusive::Could not confirm whether refs/${RUN_NS}/* exists after 2 attempts (${output});" \ + "proceeding as if not yet published." >&2 + return 0 +} + +# Exits 0 (and tells the caller to stop) if this run already has a claim; exits 1 on a malformed marker. +try_reuse_existing_claim() { + local marker existing + marker=$(find_run_marker) + [[ -n "$marker" ]] || return 1 + if ! [[ "$marker" =~ ^refs/${RUN_NS}/([0-9]+)$ ]]; then + echo "::error title=Build number claim failed::Unexpected ref format: ${marker}" >&2 + exit 1 + fi + existing="${BASH_REMATCH[1]}" + echo "Reusing build number ${existing}, already claimed by this workflow run (refs/${RUN_NS}/${existing})" + echo "${existing}" >"$BUILD_NUMBER_FILE" +} + +HELD_LOCK="" +release_lock() { + [[ -n "$HELD_LOCK" ]] || return 0 + delete_ref "$NUMBER_LOCK_REF" || echo "::warning title=Build number lock not released::Failed to delete refs/${NUMBER_LOCK_REF};" \ + "a later claim may have to wait out its timeout before proceeding." >&2 +} +trap release_lock EXIT + +echo "::group::Get build number" + +attempt=1 +while true; do + try_reuse_existing_claim && { echo "::endgroup::" && exit 0; } + + RESPONSE=$(create_ref "$NUMBER_LOCK_REF") && LOCK_STATUS=0 || LOCK_STATUS=$? + if [[ "$LOCK_STATUS" -eq 0 ]]; then + HELD_LOCK=1 + break + fi + if [[ "$RESPONSE" != *"Reference already exists"* ]]; then + fail_claim "$RESPONSE" + fi + + if ((attempt >= LOCK_WAIT_MAX_ATTEMPTS)); then + echo "::error title=Build number claim timed out::Waited ${LOCK_WAIT_MAX_ATTEMPTS} attempts for refs/${NUMBER_LOCK_REF} to" \ + "be released; the job holding it may have failed before completing its claim. If no claim is genuinely in progress, the" \ + "lock leaked (e.g. a runner killed without running its cleanup) and blocks every claim in this repository until removed:" \ + "gh api --method DELETE ${REFS_API_URL}/${NUMBER_LOCK_REF}" >&2 + exit 1 + fi + echo "::debug::refs/${NUMBER_LOCK_REF} already held; waiting (attempt $((attempt + 1))/${LOCK_WAIT_MAX_ATTEMPTS})" + sleep "$LOCK_POLL_INTERVAL_SECONDS" + attempt=$((attempt + 1)) +done + +# We now hold the lock, but this run's own marker may have appeared while we were waiting (another job in the same run). +try_reuse_existing_claim && { echo "::endgroup::" && exit 0; } + +echo "::debug::Scanning refs/${NUMBER_NS}/* for the highest claimed build number" +NUMBER_REFS=$(gh api --paginate -H "$GH_API_VERSION_HEADER" "${MATCHING_REFS_API_URL}/${NUMBER_NS}/" --jq '.[].ref') +MAX_CLAIMED=0 +STALE_REFS=() +if [[ -n "$NUMBER_REFS" ]]; then + while IFS= read -r ref; do + [[ "$ref" =~ ^refs/${NUMBER_NS}/([0-9]+)$ ]] || continue + n="${BASH_REMATCH[1]}" + STALE_REFS+=("${NUMBER_NS}/${n}") + # Force base-10: a leading-zero ref name like build-number/08 would otherwise be parsed as an (invalid) octal literal. + ((10#$n > MAX_CLAIMED)) && MAX_CLAIMED=$((10#$n)) + done <<<"$NUMBER_REFS" +fi + +if [[ "$MAX_CLAIMED" -eq 0 ]]; then + if [[ -z "${LEGACY_PROPERTY_TOKEN:-}" ]]; then + echo "::warning title=Legacy build number not checked::No refs/${NUMBER_NS}/* exist yet and no migration token is available;" \ + "starting from 1. If this repository has a legacy build_number property, its numbers will be reused." >&2 + else + echo "::debug::No refs/${NUMBER_NS}/* found yet; checking the legacy build_number property as a migration seed" + LEGACY_BUILD_NUMBER=$(GH_TOKEN="${LEGACY_PROPERTY_TOKEN}" gh api -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" --jq "$PROPS_JQ") + if [[ -n "$LEGACY_BUILD_NUMBER" ]]; then + if ! [[ "$LEGACY_BUILD_NUMBER" =~ ^[0-9]+$ ]]; then + echo "::error title=Invalid build number::Legacy build_number property '${LEGACY_BUILD_NUMBER}' is not a valid positive" \ + "integer." >&2 + exit 1 + fi + echo "Seeding from legacy build_number property: ${LEGACY_BUILD_NUMBER}" + # 10# forces base-10: a leading-zero property value would otherwise be parsed as an (invalid) octal literal. + MAX_CLAIMED=$((10#$LEGACY_BUILD_NUMBER + 1000)) # buffer against the legacy property still advancing elsewhere during migration + fi + fi +fi +echo "::debug::Highest known build number: ${MAX_CLAIMED}" + +CANDIDATE=$((MAX_CLAIMED + 1)) +RESPONSE=$(create_ref "${NUMBER_NS}/${CANDIDATE}") && CLAIM_STATUS=0 || CLAIM_STATUS=$? +if [[ "$CLAIM_STATUS" -ne 0 ]]; then + if [[ "$RESPONSE" == *"Reference already exists"* ]]; then + echo "::error title=Build number claim failed::refs/${NUMBER_NS}/${CANDIDATE} already exists, which should be impossible" \ + "while holding refs/${NUMBER_LOCK_REF}. Check for another caller writing build-number refs without this lock (e.g. an" \ + "older, un-migrated version of this action) or a manual/external ref creation." >&2 + exit 1 + fi + fail_claim "$RESPONSE" +fi +echo "Claimed build number ${CANDIDATE} (refs/${NUMBER_NS}/${CANDIDATE})" + +if ((${#STALE_REFS[@]} > 0)); then + for ref in "${STALE_REFS[@]}"; do + delete_ref "$ref" || echo "::warning title=Stale build number ref not deleted::Failed to delete refs/${ref}; harmless, just" \ + "clutter - a future claim will retry." >&2 + done +fi + +# Retried rather than a single attempt: a same-run job waiting on the lock reuses this marker as soon as it appears, so a +# transient failure here - left unretried - would make it claim its own, second number for this run instead of waiting further. +MARKER_WRITTEN="" +for marker_attempt in 1 2 3; do + create_ref "${RUN_NS}/${CANDIDATE}" >/dev/null && { MARKER_WRITTEN=1; break; } + ((marker_attempt < 3)) && sleep "$RETRY_INTERVAL_SECONDS" +done +if [[ -z "$MARKER_WRITTEN" ]]; then + echo "::error title=Build number claim failed::Failed to record refs/${RUN_NS}/${CANDIDATE} after 3 attempts; other jobs/reruns" \ + "of this workflow run waiting on refs/${NUMBER_LOCK_REF} would otherwise time out instead of reusing it." >&2 exit 1 fi -BUILD_NUMBER=$((BUILD_NUMBER + 1)) -gh api --method PATCH -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" \ - -f "properties[][property_name]=build_number" \ - -f "properties[][value]=${BUILD_NUMBER}" -echo "Incremented 'build_number' repository property to ${BUILD_NUMBER}" -echo "${BUILD_NUMBER}" > "$BUILD_NUMBER_FILE" +echo "::endgroup::" +echo "${CANDIDATE}" >"$BUILD_NUMBER_FILE" diff --git a/spec/get_build_number_spec.sh b/spec/get_build_number_spec.sh index 54222de8..8312a99c 100755 --- a/spec/get_build_number_spec.sh +++ b/spec/get_build_number_spec.sh @@ -2,6 +2,10 @@ eval "$(shellspec - -c) exit 1" export GITHUB_REPOSITORY="my org/my-repo" +export GITHUB_SHA="deadbeefcafef00dfeed" +export GITHUB_RUN_ID="123456789" +export LOCK_POLL_INTERVAL_SECONDS=0 +export RETRY_INTERVAL_SECONDS=0 TEMP_DIR="${SHELLSPEC_TMPBASE:-/tmp}" export BUILD_NUMBER_FILE="${TEMP_DIR}/build_number.txt" @@ -9,47 +13,391 @@ Mock gh echo "gh $*" End +remove_build_number_file() { + rm -f "$BUILD_NUMBER_FILE" + return 0 +} + Describe 'get_build_number.sh' - It 'should increment and return the build number' + BeforeEach 'remove_build_number_file' + + It 'should reuse an existing claim immediately, without touching the lock or scanning build-number refs' + export GH_LOCK_CALLS_FILE="${TEMP_DIR}/gh_lock_calls_reuse.txt" + rm -f "$GH_LOCK_CALLS_FILE" Mock gh - if [[ "$*" =~ "api --method PATCH" ]]; then + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo "refs/build-runs/${GITHUB_RUN_ID}/5" + elif [[ "$*" == *"build-number-lock"* ]]; then + echo "1" >>"$GH_LOCK_CALLS_FILE" + echo "gh $*" + else echo "gh $*" - elif [[ "$*" =~ "properties/values" ]]; then + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Reusing build number 5" + The contents of file "$BUILD_NUMBER_FILE" should equal "5" + The path "$GH_LOCK_CALLS_FILE" should not be file + End + + It 'should claim build number 1 and warn when there are no build-number refs and no migration token' + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo '' + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Claimed build number 1" + The output should not include "checking the legacy build_number property" + The stderr should include "::warning title=Legacy build number not checked::" + The contents of file "$BUILD_NUMBER_FILE" should equal "1" + End + + It 'should seed the starting candidate (with a safety buffer) from the legacy property when no build-number refs exist yet' + export LEGACY_PROPERTY_TOKEN="vault-issued-token-placeholder" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo '' + elif [[ "$*" == *"properties/values"* ]]; then echo '42' else echo "gh $*" fi End - # shellcheck disable=SC2317 -# preserve() { %preserve BUILD_NUMBER; } -# AfterRun preserve When run script get-build-number/get_build_number.sh - The line 1 should include "Fetching build number" - The line 2 should equal "Current build number from repo: 42" - The line 3 should include "43" - The path "$BUILD_NUMBER_FILE" should be file - The contents of file "$BUILD_NUMBER_FILE" should equal "43" -# The variable BUILD_NUMBER should equal "43" + The status should be success + The output should include "Seeding from legacy build_number property: 42" + The output should include "Claimed build number 1043" + The contents of file "$BUILD_NUMBER_FILE" should equal "1043" End - It 'should return an error if BUILD_NUMBER is invalid' + It 'should return an error if the legacy build number property is invalid' + export LEGACY_PROPERTY_TOKEN="vault-issued-token-placeholder" Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo '' + elif [[ "$*" == *"properties/values"* ]]; then echo 'notANumber' + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "::error title=Invalid build number::Legacy build_number property 'notANumber'" + End + + It 'should claim the next number after the highest existing build-number ref, ignoring gaps, and delete the superseded ones' + export GH_DELETE_CALLS_FILE="${TEMP_DIR}/gh_delete_calls_number.txt" + rm -f "$GH_DELETE_CALLS_FILE" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + printf 'refs/build-number/1\nrefs/build-number/2\nrefs/build-number/3\nrefs/build-number/5\n' + elif [[ "$*" == *"--method DELETE"* && "$*" == *"build-number/"* ]]; then + echo "$*" >>"$GH_DELETE_CALLS_FILE" + echo "gh $*" + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Claimed build number 6" + The contents of file "$BUILD_NUMBER_FILE" should equal "6" + The contents of file "$GH_DELETE_CALLS_FILE" should include "build-number/1" + The contents of file "$GH_DELETE_CALLS_FILE" should include "build-number/2" + The contents of file "$GH_DELETE_CALLS_FILE" should include "build-number/3" + The contents of file "$GH_DELETE_CALLS_FILE" should include "build-number/5" + End + + It 'should warn but still succeed when a superseded build-number ref fails to delete' + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo 'refs/build-number/5' + elif [[ "$*" == *"--method DELETE"* && "$*" == *"build-number/5"* ]]; then + exit 1 + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Claimed build number 6" + The stderr should include "::warning title=Stale build number ref not deleted::Failed to delete refs/build-number/5" + The contents of file "$BUILD_NUMBER_FILE" should equal "6" + End + + It 'should fail immediately if the candidate build-number ref unexpectedly already exists while holding the lock' + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo '' + elif [[ "$*" == *"ref=refs/build-number/1"* ]]; then + echo '{"message":"Reference already exists"}' >&2 + exit 1 + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "refs/build-number/1 already exists, which should be impossible while holding refs/build-number-lock" + End + + It 'should fail immediately on an unexpected API error while claiming, not treat it as a collision' + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo '' + elif [[ "$*" == *"ref=refs/build-number/"* ]]; then + echo '{"message":"Internal Server Error"}' >&2 + exit 1 + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "::error title=Build number claim failed::" + The stderr should include "Internal Server Error" + End + + It 'should call out the contents:write permission requirement when the API denies access' + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo '' + elif [[ "$*" == *"ref=refs/build-number/"* ]]; then + echo '{"message":"Resource not accessible by integration"}' >&2 + exit 1 + else + echo "gh $*" + fi End When run script get-build-number/get_build_number.sh The status should be failure - The line 2 should equal "Current build number from repo: notANumber" - The stderr should include "::error title=Invalid build number::Build number 'notANumber'" + The output should include "::group::Get build number" + The stderr should include "Resource not accessible by integration" + The stderr should include "requires 'contents: write'" End - It 'should handle empty build number' + It 'should fail if recording the run marker fails, since other jobs/reruns may be waiting on it' Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then echo '' + elif [[ "$*" == *"ref=refs/build-runs/"* ]]; then + exit 1 + elif [[ "$*" == *"ref=refs/build-number/"* ]]; then + echo "gh $*" + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "Claimed build number 1" + The stderr should include "::error title=Build number claim failed::Failed to record" + The path "$BUILD_NUMBER_FILE" should not be file + End + + It 'should release the lock even when the claim ultimately fails' + export GH_UNLOCK_CALLS_FILE="${TEMP_DIR}/gh_unlock_calls_failure.txt" + rm -f "$GH_UNLOCK_CALLS_FILE" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo '' + elif [[ "$*" == *"ref=refs/build-number/"* ]]; then + echo '{"message":"Internal Server Error"}' >&2 + exit 1 + elif [[ "$*" == *"--method DELETE"* && "$*" == *"build-number-lock"* ]]; then + echo "1" >>"$GH_UNLOCK_CALLS_FILE" + echo "gh $*" + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "::error title=Build number claim failed::" + The path "$GH_UNLOCK_CALLS_FILE" should be file + End + + It 'should wait for the lock holder to publish a marker, then reuse it' + export GH_MARKER_CALLS_FILE="${TEMP_DIR}/gh_marker_calls_poll.txt" + rm -f "$GH_MARKER_CALLS_FILE" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + count=$(($(cat "$GH_MARKER_CALLS_FILE" 2>/dev/null || echo 0) + 1)) + echo "$count" >"$GH_MARKER_CALLS_FILE" + if [[ "$count" -lt 3 ]]; then + echo '' + else + echo "refs/build-runs/${GITHUB_RUN_ID}/9" + fi + elif [[ "$*" == *"ref=refs/build-number-lock"* ]]; then + echo '{"message":"Reference already exists"}' >&2 + exit 1 + else + echo "gh $*" + fi End When run script get-build-number/get_build_number.sh The status should be success - The line 2 should equal "Current build number from repo: 0" - # Ignore empty line from second call to gh - The line 4 should include "1" + The output should include "Reusing build number 9" + The contents of file "$BUILD_NUMBER_FILE" should equal "9" + End + + It 'should silently absorb a single transient error checking the marker via its internal retry' + export GH_MARKER_CALLS_FILE="${TEMP_DIR}/gh_marker_calls_transient.txt" + rm -f "$GH_MARKER_CALLS_FILE" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + count=$(($(cat "$GH_MARKER_CALLS_FILE" 2>/dev/null || echo 0) + 1)) + echo "$count" >"$GH_MARKER_CALLS_FILE" + if [[ "$count" -eq 1 ]]; then + echo '{"message":"Internal Server Error"}' >&2 + exit 1 + else + echo "refs/build-runs/${GITHUB_RUN_ID}/11" + fi + elif [[ "$*" == *"ref=refs/build-number-lock"* ]]; then + echo '{"message":"Reference already exists"}' >&2 + exit 1 + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Reusing build number 11" + The stderr should not include "Marker check inconclusive" + The contents of file "$BUILD_NUMBER_FILE" should equal "11" + End + + It 'should warn once its internal retry is exhausted, then succeed reusing the marker found on the next poll' + export GH_MARKER_CALLS_FILE="${TEMP_DIR}/gh_marker_calls_inconclusive.txt" + rm -f "$GH_MARKER_CALLS_FILE" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + count=$(($(cat "$GH_MARKER_CALLS_FILE" 2>/dev/null || echo 0) + 1)) + echo "$count" >"$GH_MARKER_CALLS_FILE" + if [[ "$count" -le 2 ]]; then + echo '{"message":"Internal Server Error"}' >&2 + exit 1 + else + echo "refs/build-runs/${GITHUB_RUN_ID}/12" + fi + elif [[ "$*" == *"ref=refs/build-number-lock"* ]]; then + echo '{"message":"Reference already exists"}' >&2 + exit 1 + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Reusing build number 12" + The stderr should include "::warning title=Marker check inconclusive::" + The contents of file "$BUILD_NUMBER_FILE" should equal "12" + End + + It 'should fail after exhausting the wait if the lock holder never publishes a marker' + export LOCK_WAIT_MAX_ATTEMPTS=3 + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"ref=refs/build-number-lock"* ]]; then + echo '{"message":"Reference already exists"}' >&2 + exit 1 + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "::error title=Build number claim timed out::Waited 3 attempts" + End + + It 'should fail immediately on an unexpected error acquiring the lock, not treat it as a collision' + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"ref=refs/build-number-lock"* ]]; then + echo '{"message":"Internal Server Error"}' >&2 + exit 1 + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "::error title=Build number claim failed::" + The stderr should include "Internal Server Error" + End + + It 'should reuse the marker if it appears between winning the lock and scanning build-number refs' + export GH_MARKER_CALLS_FILE="${TEMP_DIR}/gh_marker_calls_race.txt" + rm -f "$GH_MARKER_CALLS_FILE" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + count=$(($(cat "$GH_MARKER_CALLS_FILE" 2>/dev/null || echo 0) + 1)) + echo "$count" >"$GH_MARKER_CALLS_FILE" + if [[ "$count" -eq 1 ]]; then + echo '' + else + echo "refs/build-runs/${GITHUB_RUN_ID}/13" + fi + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo "::error title=test-bug::build-number should never be scanned in this scenario" >&2 + exit 1 + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Reusing build number 13" + The contents of file "$BUILD_NUMBER_FILE" should equal "13" + End + + It 'should fail if a marker ref has an unexpected format' + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo "refs/build-runs/${GITHUB_RUN_ID}/notanumber" + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "::error title=Build number claim failed::Unexpected ref format" End End