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
32 changes: 2 additions & 30 deletions .github/workflows/image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,31 +28,17 @@ jobs:
platforms: linux/${{ matrix.platform }}
dockerfile_path: './Dockerfile'
build_args: TASKBROKER_GIT_REVISION=${{ github.sha }}
ghcr: ${{ github.event.pull_request.head.repo.fork == false }}
ghcr: true
Copy link

Choose a reason for hiding this comment

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

Bug: Hardcoding ghcr: true causes 403 Forbidden errors when pushing images from fork pull requests due to insufficient GITHUB_TOKEN permissions.
Severity: CRITICAL | Confidence: 1.00

🔍 Detailed Analysis

The change to ghcr: true at .github/workflows/image.yml:31 forces the action to attempt pushing images to GHCR for all pull requests. For pull requests originating from forks, the GITHUB_TOKEN lacks the necessary permissions to push to the upstream organization's GHCR registry, resulting in a 403 Forbidden error. This prevents the CI workflow from completing successfully for external contributions.

💡 Suggested Fix

Reintroduce the conditional ghcr: ${{ github.event.pull_request.head.repo.fork == false }} to prevent pushing to GHCR for fork pull requests. Restore the artifact saving/loading mechanism for PRs.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .github/workflows/image.yml#L31

Potential issue: The change to `ghcr: true` at `.github/workflows/image.yml:31` forces
the action to attempt pushing images to GHCR for all pull requests. For pull requests
originating from forks, the `GITHUB_TOKEN` lacks the necessary permissions to push to
the upstream organization's GHCR registry, resulting in a 403 Forbidden error. This
prevents the CI workflow from completing successfully for external contributions.

Did we get this right? 👍 / 👎 to inform future reviews.

tag_suffix: -${{ matrix.platform }}
publish_on_pr: true
google_ar: false
tag_nightly: false
tag_latest: false
tags: ${{ github.event_name == 'pull_request' && 'ghcr.io/getsentry/taskbroker:nightly' || ''}}
outputs: ${{ github.event_name == 'pull_request' && 'type=docker' || ''}}

- name: Save Docker image to artifact
if: ${{ github.event_name == 'pull_request' }}
run: docker save -o /tmp/taskbroker.tar ghcr.io/getsentry/taskbroker:nightly

- name: Upload artifact for self-hosted e2e
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
if: ${{ github.event_name == 'pull_request' }}
with:
name: taskbroker-${{ matrix.platform }}
path: /tmp/taskbroker.tar
if-no-files-found: error
retention-days: 1

assemble-taskbroker-image:
runs-on: ubuntu-latest
needs: [build]
if: ${{ (github.ref_name == 'main' || startsWith(github.ref_name, 'releases/')) && github.event_name != 'pull_request' }}
Copy link

Choose a reason for hiding this comment

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

Bug: E2E job skips for all pull requests because its dependency assemble-taskbroker-image is skipped, and no if: always() is present.
Severity: CRITICAL | Confidence: 1.00

🔍 Detailed Analysis

The assemble-taskbroker-image job at .github/workflows/image.yml:41 is configured with an if condition that causes it to be skipped for all pull requests. The self-hosted-end-to-end job has a needs: [assemble-taskbroker-image] dependency but lacks an if: always() condition or status check. Consequently, when assemble-taskbroker-image is skipped, the self-hosted-end-to-end job is also skipped by default, preventing e2e tests from running for any pull request.

💡 Suggested Fix

Modify the self-hosted-end-to-end job to handle skipped dependencies, possibly by adding if: always() or reintroducing the artifact mechanism for PRs.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .github/workflows/image.yml#L41

Potential issue: The `assemble-taskbroker-image` job at `.github/workflows/image.yml:41`
is configured with an `if` condition that causes it to be skipped for all pull requests.
The `self-hosted-end-to-end` job has a `needs: [assemble-taskbroker-image]` dependency
but lacks an `if: always()` condition or status check. Consequently, when
`assemble-taskbroker-image` is skipped, the `self-hosted-end-to-end` job is also skipped
by default, preventing e2e tests from running for any pull request.

Did we get this right? 👍 / 👎 to inform future reviews.

permissions:
contents: read
packages: write
Expand All @@ -67,7 +53,6 @@ jobs:
uses: docker/setup-buildx-action@b5ca514318bd6ebac0fb2aedd5d36ec1b5c232a2 # v3.10.0

- name: Create multiplatform manifests
if: ${{ (github.ref_name == 'main' || startsWith(github.ref_name, 'releases/')) && github.event_name != 'pull_request' }}
run: |
docker buildx imagetools create \
--tag ghcr.io/getsentry/taskbroker:${{ github.event.pull_request.head.sha || github.sha }} \
Expand Down Expand Up @@ -106,19 +91,6 @@ jobs:
timeout-minutes: 30

steps:
- name: Download artifact for pull request
uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0
if: ${{ github.event_name == 'pull_request' }}
with:
name: taskbroker-amd64
path: /tmp/

- name: Load Docker image from tar file
if: ${{ github.event_name == 'pull_request' }}
run: |
docker image load --input /tmp/taskbroker.tar
docker image tag ghcr.io/getsentry/taskbroker:nightly ghcr.io/getsentry/taskbroker:${{ github.sha }}

- name: Run Sentry self-hosted e2e CI
uses: getsentry/self-hosted@master
with:
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# recent enough version of protobuf-compiler
FROM rust:1-bookworm AS build

RUN apt-get update && apt-get upgrade -y
RUN apt-get update && apt-get upgrade -y
RUN apt-get install -y cmake pkg-config libssl-dev librdkafka-dev protobuf-compiler

RUN USER=root cargo new --bin taskbroker
Expand Down
Loading