-
Notifications
You must be signed in to change notification settings - Fork 50
Add upstream bugfix note on PR merge #761
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ycedres
wants to merge
15
commits into
openSUSE:openSUSE/release/3006.0
Choose a base branch
from
ycedres:workflow-add-upstream-bugfix-note
base: openSUSE/release/3006.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
5dea5e3
Add upstream bugfix note on PR merge
ycedres 244b7d5
Add concurrency group to serialize workflow runs
ycedres 00e65be
Switch from pull_request to push event to support forked PRs
ycedres 1a115cf
Fix env-file injection prevention in upstream bugfix workflow
ycedres c7caf51
Fix retry logic on non-fast-forward push failures
ycedres ecc04fa
Handle commits from any branch when manually triggered
ycedres 117e118
Replace PR search with commits→pulls API endpoint
ycedres 8ae0755
Detect merge strategy and annotate appropriate commits
ycedres 6f71d3b
Replace fixed EOF delimiters with unique random delimiters
ycedres cd2a668
Accept both http and https for upstream PR URLs
ycedres 06b10c6
Use double quotes to avoid glob issues
ycedres 25c5859
Normalize URLs in workflow_dispatch and handle all-zero SHA
ycedres b7e197b
Validate note URL is a valid upstream Salt PR link
ycedres 3a99adc
Consider case of more than one parent for the merge commit
ycedres b36c86f
Simplify workflow as only squash-and-merge is allowed
ycedres File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| name: Upstream Bug-fix Notes | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| commit_sha: | ||
| description: 'The SHA of the commit in openSUSE/salt to add the note to (default: HEAD)' | ||
| required: false | ||
| default: 'HEAD' | ||
| upstream_ref: | ||
| description: 'The upstream Salt PR number or full link (e.g., 66240 or https://github.com/saltstack/salt/pull/66240)' | ||
| required: true | ||
| push: | ||
| branches: | ||
| - openSUSE/release/3006.0 | ||
|
|
||
| concurrency: | ||
| group: upstream-notes-push | ||
| cancel-in-progress: false | ||
|
|
||
| jobs: | ||
| manage-notes: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: write | ||
| pull-requests: read | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Configure Git | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
|
|
||
| - name: Fetch existing notes | ||
| run: | | ||
| git fetch origin refs/notes/*:refs/notes/* || true | ||
|
|
||
| - name: Process Manual Input | ||
| if: github.event_name == 'workflow_dispatch' | ||
| run: | | ||
| REF="${{ github.event.inputs.upstream_ref }}" | ||
| if [[ "$REF" =~ ^[0-9]+$ ]]; then | ||
| URL="https://github.com/saltstack/salt/pull/$REF" | ||
| else | ||
| URL="$REF" | ||
| if [[ "$URL" =~ ^http:// ]]; then | ||
| URL="https://${URL#http://}" | ||
| elif [[ ! "$URL" =~ ^https:// ]]; then | ||
| URL="https://$URL" | ||
|
ycedres marked this conversation as resolved.
|
||
| fi | ||
|
|
||
| if [[ ! "$URL" =~ ^https://github\.com/saltstack/salt/pull/[0-9]+$ ]]; then | ||
| echo "Error: upstream_ref must be a PR number or a valid saltstack/salt PR URL." | ||
| echo "Got: $URL" | ||
| echo "Expected format: https://github.com/saltstack/salt/pull/<number>" | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
ycedres marked this conversation as resolved.
ycedres marked this conversation as resolved.
|
||
|
|
||
| SHA="${{ github.event.inputs.commit_sha }}" | ||
|
|
||
| # actions/checkout@v4 only fetches the triggered ref's history | ||
| # In workflow_dispatch mode, if a user triggers from 'master' but | ||
| # provides a commit_sha from 'openSUSE/release/3006.0', that commit won't be available | ||
| # and git rev-parse will fail. | ||
| # Try to resolve first, and if it fails, fetch all branches to make the | ||
| # commit available. | ||
| if ! RESOLVED_SHA=$(git rev-parse --verify "$SHA" 2>/dev/null); then | ||
| echo "Commit $SHA not found locally. Attempting to fetch all branches..." | ||
| git fetch --all | ||
|
|
||
| if ! RESOLVED_SHA=$(git rev-parse --verify "$SHA" 2>/dev/null); then | ||
| echo "Error: Commit $SHA not found in repository after fetching all branches." | ||
| echo "Please verify the commit SHA exists and is pushed to the remote." | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
||
| echo "Resolved commit: $RESOLVED_SHA" | ||
|
|
||
| # Use unique random delimiters to prevent env-file injection. | ||
| # A fixed delimiter (EOF) is vulnerable if the value contains a line equal to "EOF", | ||
| # which would close the heredoc early and allow variable injection. | ||
| # Generate random delimiters per write to make collision practically impossible. | ||
| DELIMITER="ghadelimiter_${RANDOM}_${RANDOM}_${RANDOM}" | ||
| { | ||
| echo "URL<<${DELIMITER}" | ||
| echo "$URL" | ||
| echo "${DELIMITER}" | ||
| echo "COMMITS<<${DELIMITER}" | ||
| echo "$RESOLVED_SHA" | ||
| echo "${DELIMITER}" | ||
| } >> "$GITHUB_ENV" | ||
|
|
||
| - name: Process Merged PR | ||
| if: github.event_name == 'push' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| # We trigger on 'push' instead of 'pull_request' because: | ||
| # - pull_request event: GITHUB_TOKEN is read-only for PRs from forks (security measure) | ||
| # - push event: GITHUB_TOKEN has write permissions since code is already merged into trusted branch | ||
| # This ensures git notes can be pushed regardless of whether the PR originated from a fork. | ||
|
|
||
| PUSH_SHA="${{ github.event.after }}" | ||
|
|
||
| # Use the commits→pulls API for reliable, deterministic PR resolution. | ||
| # gh pr list --search uses GitHub's search index which may have indexing delays, | ||
| # while this API endpoint provides a direct lookup for commits associated with PRs. | ||
| PR_NUMBER=$(gh api "repos/${{ github.repository }}/commits/$PUSH_SHA/pulls" \ | ||
| --jq '[.[] | select(.merged_at != null)][0].number // empty' || echo "") | ||
|
|
||
| if [ -z "$PR_NUMBER" ]; then | ||
| echo "No merged PR found for commit $PUSH_SHA. Skipping notes." | ||
| exit 0 | ||
| fi | ||
|
ycedres marked this conversation as resolved.
|
||
|
|
||
| echo "Found merged PR #$PR_NUMBER" | ||
|
|
||
| BODY=$(gh pr view "$PR_NUMBER" --json body --jq '.body') | ||
|
|
||
| if [[ "$BODY" =~ (https?://)?github\.com/saltstack/salt/pull/([0-9]+) ]]; then | ||
| URL="${BASH_REMATCH[0]}" | ||
| if [[ "$URL" =~ ^http:// ]]; then | ||
| URL="https://${URL#http://}" | ||
| elif [[ ! "$URL" =~ ^https:// ]]; then | ||
| URL="https://$URL" | ||
| fi | ||
|
|
||
| # Since the repository only uses squash-and-merge, exactly one new commit ($PUSH_SHA) | ||
| # is added to the base branch representing the merged PR. | ||
| COMMITS="$PUSH_SHA" | ||
|
|
||
| DELIMITER="ghadelimiter_${RANDOM}_${RANDOM}_${RANDOM}" | ||
| { | ||
| echo "URL<<${DELIMITER}" | ||
| echo "$URL" | ||
| echo "${DELIMITER}" | ||
| echo "COMMITS<<${DELIMITER}" | ||
| echo "$COMMITS" | ||
| echo "${DELIMITER}" | ||
| } >> "$GITHUB_ENV" | ||
| else | ||
| echo "No upstream PR link found in PR #$PR_NUMBER description. Skipping notes." | ||
| exit 0 | ||
| fi | ||
|
|
||
| - name: Add Notes | ||
| if: env.URL != '' | ||
| run: | | ||
| NOTES_ADDED=false | ||
|
|
||
| for SHA in $COMMITS; do | ||
| echo "Adding note to $SHA: Upstream PR: $URL" | ||
|
|
||
| if ! git cat-file -e "$SHA" 2>/dev/null; then | ||
| echo "Warning: Commit $SHA not found. Skipping." | ||
| continue | ||
| fi | ||
|
|
||
| EXISTING_NOTE=$(git notes show "$SHA" 2>/dev/null || true) | ||
| if [[ "$EXISTING_NOTE" == *"$URL"* ]]; then | ||
| echo "Note already contains this URL for $SHA. Skipping." | ||
| else | ||
| git notes append -m "Upstream PR: $URL" "$SHA" | ||
| NOTES_ADDED=true | ||
| fi | ||
| done | ||
|
|
||
| echo "NOTES_ADDED=$NOTES_ADDED" >> "$GITHUB_ENV" | ||
|
|
||
| - name: Push notes | ||
| if: env.URL != '' && env.NOTES_ADDED == 'true' | ||
| run: | | ||
| MAX_RETRIES=3 | ||
| RETRY_COUNT=0 | ||
|
|
||
| while [ $RETRY_COUNT -lt $MAX_RETRIES ]; do | ||
| if git push origin refs/notes/commits; then | ||
| echo "Successfully pushed notes" | ||
| exit 0 | ||
| else | ||
| RETRY_COUNT=$((RETRY_COUNT + 1)) | ||
| if [ $RETRY_COUNT -lt $MAX_RETRIES ]; then | ||
| echo "Push failed, retrying ($RETRY_COUNT/$MAX_RETRIES)..." | ||
| # When concurrent workflows race: | ||
| # 1. Workflow A adds note and pushes successfully | ||
| # 2. Workflow B adds note and push fails (non-fast-forward) | ||
| # 3. If we fetch with refs/notes/*:refs/notes/*, we overwrite our local. | ||
| # ref with remote, discarding the note we just added in step 2. | ||
| # 4. Next push succeeds but loses note in workflow B. | ||
| # Instead, fetch without updating local ref, then merge both sets of notes. | ||
| git fetch origin refs/notes/commits | ||
| git notes merge -s cat_sort_uniq FETCH_HEAD || { | ||
| echo "Notes merge failed" | ||
| exit 1 | ||
| } | ||
| sleep 2 | ||
|
ycedres marked this conversation as resolved.
|
||
| else | ||
| echo "Failed to push notes after $MAX_RETRIES attempts" | ||
| exit 1 | ||
| fi | ||
| fi | ||
| done | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.