Skip to content

[release-1.7] fix(ci): Make sure to checkout the PR branch before pushing the bundle diff generated - #1604

Merged
rm3l merged 1 commit into
redhat-developer:release-1.7from
openshift-cherrypick-robot:cherry-pick-1599-to-release-1.7
Sep 5, 2025
Merged

[release-1.7] fix(ci): Make sure to checkout the PR branch before pushing the bundle diff generated#1604
rm3l merged 1 commit into
redhat-developer:release-1.7from
openshift-cherrypick-robot:cherry-pick-1599-to-release-1.7

Conversation

@openshift-cherrypick-robot

@openshift-cherrypick-robot openshift-cherrypick-robot commented Sep 5, 2025

Copy link
Copy Markdown

This is an automated cherry-pick of #1599

/assign rm3l

Summary by Sourcery

Enable the CI workflow to checkout the pull request branch before committing and pushing regenerated bundle diffs

CI:

  • Fetch and checkout the head branch from the PR fork in the bundle diff job
  • Push regenerated manifest commits back to the correct PR branch repository

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 Security concerns

Credential handling:
Ensure pushes to the fork use GITHUB_TOKEN or a PAT with least privileges, and that the token has 'contents: write' permission for the workflow. Avoid leaking the token via command output; prefer using the actions/checkout-provided token or configuring the remote as 'https://${GITHUB_ACTOR}:${{ secrets.GITHUB_TOKEN }}@github.com/...'.

⚡ Recommended focus areas for review

Possible Permission Issue

Pushing to a fork via HTTPS requires a token/credentials. Ensure the workflow has appropriate permissions and uses an authenticated remote URL (e.g., GITHUB_TOKEN with write access) or a checked-out token to allow 'git push fork ...' to succeed.

git remote add fork "https://github.com/${{ github.event.pull_request.head.repo.full_name }}.git"
git fetch fork ${{ github.event.pull_request.head.ref }}
git checkout -B pr-branch fork/${{ github.event.pull_request.head.ref }}

git config user.name 'github-actions[bot]'
git config user.email 'github-actions[bot]@users.noreply.github.com'

git add -A .
git commit \
  -m "Regenerate bundle/installer manifests" \
  -m "Co-authored-by: $GITHUB_ACTOR <$GITHUB_ACTOR@users.noreply.github.com>"

git push fork pr-branch:${{ github.event.pull_request.head.ref }}
Remote Exists Edge Case

Adding the 'fork' remote unconditionally can fail if it already exists (e.g., on re-runs). Consider guarding with 'git remote get-url fork || git remote add fork ...' or using '--add' logic.

git remote add fork "https://github.com/${{ github.event.pull_request.head.repo.full_name }}.git"
git fetch fork ${{ github.event.pull_request.head.ref }}
git checkout -B pr-branch fork/${{ github.event.pull_request.head.ref }}
Branch Safety

Force-push behavior is undefined. If the PR branch advanced after checkout, a non-fast-forward push may fail. Decide whether to 'git pull --rebase' from 'fork/' before commit or use 'git push --force-with-lease' when appropriate to avoid flaky failures.

git checkout -B pr-branch fork/${{ github.event.pull_request.head.ref }}

git config user.name 'github-actions[bot]'
git config user.email 'github-actions[bot]@users.noreply.github.com'

git add -A .
git commit \
  -m "Regenerate bundle/installer manifests" \
  -m "Co-authored-by: $GITHUB_ACTOR <$GITHUB_ACTOR@users.noreply.github.com>"

git push fork pr-branch:${{ github.event.pull_request.head.ref }}

@rm3l rm3l left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Sep 5, 2025
@openshift-ci

openshift-ci Bot commented Sep 5, 2025

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved label Sep 5, 2025
@rm3l
rm3l merged commit 24632da into redhat-developer:release-1.7 Sep 5, 2025
6 checks passed
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Authenticate fork remote pushes

Pushing to the new 'fork' remote will fail without authentication. Include the
GitHub token in the remote URL so git can authenticate when fetching and
pushing. This prevents "Authentication failed" errors during the push.

.github/workflows/pr-bundle-diff-checks.yaml [63]

-git remote add fork "https://github.com/${{ github.event.pull_request.head.repo.full_name }}.git"
+git remote add fork "https://x-access-token:${{ github.token }}@github.com/${{ github.event.pull_request.head.repo.full_name }}.git"
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This is a critical fix, as the git push command to the fork's remote would fail due to missing authentication without the suggested github.token.

High
Stash changes before branch switch

Switching branches with uncommitted generated changes can fail with "would be
overwritten by checkout". Stash changes before checkout and re-apply them after
to mirror the previous autostash behavior. This ensures the commit step proceeds
reliably.

.github/workflows/pr-bundle-diff-checks.yaml [64-65]

+git stash --include-untracked --all
 git fetch fork ${{ github.event.pull_request.head.ref }}
 git checkout -B pr-branch fork/${{ github.event.pull_request.head.ref }}
+git stash pop || true
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies that uncommitted manifest changes could block the git checkout, and reintroduces the stashing logic that was present in the removed git pull --rebase --autostash command, preventing a potential workflow failure.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants