Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ jobs:

No inputs are required for this action.

| Input | Description | Default |
|-----------|------------------------------------------------------------------------------|----------------------------------------------------------|
| `ref-sha` | Commit targeted by bookkeeping refs; does not change the commit being built. | PR head for pull requests, otherwise the workflow commit |

The PR head is used because GitHub can reject ref creation targeting the synthetic merge commit with HTTP 403 even with
`contents: write`. The ref target is only needed to create the ref and is never read to allocate or reuse a number.

### Outputs

| Output | Description |
Expand Down
4 changes: 4 additions & 0 deletions get-build-number/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ outputs:
description: The build number, newly claimed or reused if this workflow run already claimed one
value: ${{ steps.export.outputs.BUILD_NUMBER }}
inputs:
ref-sha:
description: Commit targeted by bookkeeping refs. Defaults to the PR head for pull requests, otherwise the workflow commit.
default: ${{ github.event.pull_request.head.sha || github.sha }}
host-actions-root:
description: Path to the actions folder on the host (used when called from another local action)
default: ''
Expand Down Expand Up @@ -60,6 +63,7 @@ runs:
if: steps.from-env.outputs.skip != 'true'
shell: bash
env:
BUILD_NUMBER_REF_SHA: ${{ inputs.ref-sha }}
GITHUB_TOKEN: ${{ github.token }}
GH_TOKEN: ${{ github.token }}
LEGACY_PROPERTY_TOKEN: ${{ steps.secrets.outputs.vault && fromJSON(steps.secrets.outputs.vault).github_token || '' }}
Expand Down
5 changes: 3 additions & 2 deletions get-build-number/get_build_number.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
# 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.
# Every ref points at $BUILD_NUMBER_REF_SHA (or $GITHUB_SHA for direct script callers) because creation requires a valid target;
# that target is never read back. PR merge commits can be rejected by GitHub even when the PR head is accepted.
# 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.

Expand All @@ -31,7 +32,7 @@ RETRY_INTERVAL_SECONDS="${RETRY_INTERVAL_SECONDS:-1}" # between internal retries

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
gh api --method POST -H "$GH_API_VERSION_HEADER" "$REFS_API_URL" -f "ref=refs/${ref}" -f "sha=${BUILD_NUMBER_REF_SHA:-$GITHUB_SHA}" 2>&1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 [cross-pr] Conflicts with #360's ref-target rule; one side's tests and input break

Breaks when this PR is reopened and merged into master alongside #360, and both change the sha= argument of create_ref().

How

  1. This PR sends sha=${BUILD_NUMBER_REF_SHA:-$GITHUB_SHA} (get-build-number/get_build_number.sh:35). It is fed from a new ref-sha input that defaults to the PR head (get-build-number/action.yml:9-11,66).
  2. BUILD-12998 Fix build-number ref authorization for PR merge commits #360 changes the same line to sha=${REF_TARGET_SHA} (get-build-number/get_build_number.sh:36). That value is resolved once from commits/HEAD (the default-branch head). BUILD-12998 Fix build-number ref authorization for PR merge commits #360 has no ref-sha input and does not pass BUILD_NUMBER_REF_SHA.
  3. If BUILD-12998 Fix build-number ref authorization for PR merge commits #360's line wins, this PR's ref-sha input and README table describe an option that has no effect. Its new spec also fails, because the mocked gh returns 1 unless sha=${BUILD_NUMBER_REF_SHA} is sent (spec/get_build_number_spec.sh:31).
  4. If this PR's line wins, BUILD-12998 Fix build-number ref authorization for PR merge commits #360's default-head targeting and its specs that expect sha=<default-head> fail. Combining both also adds a commits/HEAD call that this PR's mock doesn't expect.

Fix

Compared 9ecc067 with #360 at 984ec6f

Was this helpful? React with 👍 / 👎

}

delete_ref() {
Expand Down
21 changes: 21 additions & 0 deletions spec/get_build_number_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,27 @@ remove_build_number_file() {
Describe 'get_build_number.sh'
BeforeEach 'remove_build_number_file'

It 'should target the supplied commit for every bookkeeping ref'
export BUILD_NUMBER_REF_SHA="0123456789abcdef0123456789abcdef01234567"
export GH_REF_CALLS_FILE="${TEMP_DIR}/gh_ref_calls.txt"
rm -f "$GH_REF_CALLS_FILE"
Mock gh
if [[ "$*" == *"--method POST"* ]]; then
echo "$*" >>"$GH_REF_CALLS_FILE"
[[ "$*" == *"sha=${BUILD_NUMBER_REF_SHA}"* ]] || return 1
elif [[ "$*" == *"matching-refs/build-number/"* ]]; then
echo 'refs/build-number/5'
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_REF_CALLS_FILE" should include "ref=refs/build-number-lock/global"
The contents of file "$GH_REF_CALLS_FILE" should include "ref=refs/build-number/6"
The contents of file "$GH_REF_CALLS_FILE" should include "ref=refs/build-runs/${GITHUB_RUN_ID}/6"
End

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"
Expand Down
Loading