Skip to content

Adding cloudflare preview builds #822

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 3, 2025
Merged

Adding cloudflare preview builds #822

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
60b2c62
Adding cloudflare preview builds
blckmn May 3, 2025
a416a99
Ensuring branch name is provided.
blckmn May 3, 2025
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
50 changes: 50 additions & 0 deletions .github/workflows/deploy-preview.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
name: 'Preview Deployment'
on:
pull_request_target:
branches:
- master

jobs:
deploy:
permissions:
actions: read
contents: read
deployments: write
pull-requests: write
Comment on lines +9 to +13
Copy link

@coderabbitai coderabbitai bot May 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add issues: write permission for PR comments
To allow the comment action to post deployment URLs on the pull request, include the issues: write permission under permissions.

 permissions:
   actions: read
   contents: read
   deployments: write
+  issues: write
   pull-requests: write
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
permissions:
actions: read
contents: read
deployments: write
pull-requests: write
permissions:
actions: read
contents: read
deployments: write
issues: write
pull-requests: write

Copy link
Member

Choose a reason for hiding this comment

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

adding this to my PR's as well

Copy link

Choose a reason for hiding this comment

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

@nerdCopter Great! Adding the issues: write permission will ensure your workflow can properly post comments on pull requests. Let me know if you need any other assistance with your GitHub Actions workflows.

✏️ Learnings added 
Learnt from: nerdCopter
PR: betaflight/blackbox-log-viewer#822
File: .github/workflows/deploy-preview.yml:9-13
Timestamp: 2025-05-14T16:07:59.143Z
Learning: nerdCopter prefers to include `issues: write` permission in GitHub Actions workflows that post comments on pull requests, in addition to the standard `pull-requests: write` permission.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- name: Checkout code
uses: actions/checkout@v4

Comment on lines +17 to +19
Copy link

@coderabbitai coderabbitai bot May 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Checkout PR head commit on pull_request_target
By default, actions/checkout in a pull_request_target event checks out the base branch. To build the PR changes, specify the head ref:

-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Checkout code
uses: actions/checkout@v4
- name: Checkout code
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}

Copy link
Member

Choose a reason for hiding this comment

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

it was caught here and ignored

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch information from the referenced PRs to understand the context
# First PR
echo "Fetching information from PR #4468 on betaflight/betaflight-configurator"
gh api repos/betaflight/betaflight-configurator/pulls/4468 --jq '.title, .html_url, .body'

# Second PR
echo "Fetching information from PR #834 on betaflight/blackbox-log-viewer"
gh api repos/betaflight/blackbox-log-viewer/pulls/834 --jq '.title, .html_url, .body'

# Look for discussions about pull_request_target in these PRs
echo "Looking for discussions about pull_request_target in PR #4468"
gh api repos/betaflight/betaflight-configurator/pulls/4468/comments --jq '.[].body' | grep -i "pull_request_target" || echo "No mentions found in comments"

echo "Looking for discussions about pull_request_target in PR #834"
gh api repos/betaflight/blackbox-log-viewer/pulls/834/comments --jq '.[].body' | grep -i "pull_request_target" || echo "No mentions found in comments"

# Look for review comments that might mention this issue
echo "Looking for review comments about pull_request_target in PR #4468"
gh api repos/betaflight/betaflight-configurator/pulls/4468/comments --jq '.[].body' | grep -i "checkout" || echo "No mentions of checkout in comments"

echo "Looking for review comments about pull_request_target in PR #834"
gh api repos/betaflight/blackbox-log-viewer/pulls/834/comments --jq '.[].body' | grep -i "checkout" || echo "No mentions of checkout in comments"

Length of output: 3278

🏁 Script executed:

#!/bin/bash
# Let's check if those PRs were actually merged or closed
echo "Checking status of PR #4468 on betaflight/betaflight-configurator"
gh api repos/betaflight/betaflight-configurator/pulls/4468 --jq '.state, .merged'

echo "Checking status of PR #834 on betaflight/blackbox-log-viewer"
gh api repos/betaflight/blackbox-log-viewer/pulls/834 --jq '.state, .merged'

# Let's also check if there were any discussions from the maintainers
echo "Looking for comments from maintainers on PR #4468"
gh api repos/betaflight/betaflight-configurator/pulls/4468/comments --jq '.[] | {user: .user.login, body: .body}' | grep -A 2 -i "blckmn\|nerdCopter"

echo "Looking for comments from maintainers on PR #834"
gh api repos/betaflight/blackbox-log-viewer/pulls/834/comments --jq '.[] | {user: .user.login, body: .body}' | grep -A 2 -i "blckmn\|nerdCopter"

# Let's see if there are any workflow files using pull_request_target in these repos
echo "Checking for workflow files using pull_request_target in blackbox-log-viewer"
gh api repos/betaflight/blackbox-log-viewer/contents/.github/workflows --jq '.[] | .name'

Length of output: 1256

@nerdCopter Thank you for providing that context. I see that both PRs you referenced have the same concern identified but remain open. I understand that the team is aware of the security implications with the current approach using pull_request_target and has made a deliberate decision about how to handle it. I'll respect that decision and won't flag this issue in future reviews.

✏️ Learnings added 
Learnt from: nerdCopter
PR: betaflight/blackbox-log-viewer#822
File: .github/workflows/deploy-preview.yml:17-19
Timestamp: 2025-05-14T16:08:27.243Z
Learning: For betaflight/blackbox-log-viewer repository, the team is aware of the security implications of using `pull_request_target` without specifying `persist-credentials: false` in the checkout action, and has chosen to proceed with the current approach. This concern should not be flagged in future reviews.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

- name: Cache node_modules
uses: actions/cache@v4
with:
path: node_modules/
key: node_modules-${{ runner.os }}-${{ hashFiles('yarn.lock') }}

- name: Install node.js
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'

- run: npm install yarn -g
- run: yarn install
- run: yarn build

- name: Deploy to Cloudflare
id: deploy
uses: cloudflare/wrangler-action@v3
with:
apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
command: pages deploy src/dist --project-name=${{ vars.CLOUDFLARE_PROJECT_NAME }} --branch ${{ github.head_ref }} --commit-dirty=true

- name: Add deployment comment
uses: thollander/actions-comment-pull-request@v3
with:
message: |
Preview URL: ${{ steps.deploy.outputs.deployment-url }}
reactions: eyes, rocket
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
reactions: eyes, rocket

Copy link
Member

Choose a reason for hiding this comment

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

@blckmn - this was requested by @nerdCopter

See betaflight/betaflight-configurator#4455 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this updates the single comment (recreation mode) issue.

I'm keeping the emojis though :)

comment-tag: 'Preview URL'
mode: recreate