From a506ce7f476172b36a613584925bedee66a6e3fd Mon Sep 17 00:00:00 2001 From: Matthieu Vachon Date: Thu, 21 May 2026 20:45:06 +0000 Subject: [PATCH 1/2] fix(ci): skip Docker login, push and Scout steps for fork PRs The Docker build workflow already had a correct push condition but the registry login and Docker Scout steps ran unconditionally. For fork PRs the pushed image does not exist so Scout would fail; this change guards both steps with the same same-repo condition so fork PRs only build the image without pushing or scanning. --- .dev/todo/ci-docker-push-fork-pr.md | 73 +++++++++++++++++++++++++++++ .github/workflows/docker.yml | 2 + docs/release-notes/change-log.md | 6 +++ 3 files changed, 81 insertions(+) create mode 100644 .dev/todo/ci-docker-push-fork-pr.md diff --git a/.dev/todo/ci-docker-push-fork-pr.md b/.dev/todo/ci-docker-push-fork-pr.md new file mode 100644 index 000000000..8d6cf5ca8 --- /dev/null +++ b/.dev/todo/ci-docker-push-fork-pr.md @@ -0,0 +1,73 @@ +# Update GitHub CI Docker Image Push for Same-Repo PRs Only + +mode: bug +state: review +root_git: ../substreams.worktrees/fix/ci-docker-push-fork-pr +worktree: ../substreams.worktrees/fix/ci-docker-push-fork-pr +branch: fix/ci-docker-push-fork-pr +target_branch: develop + +> **Resume protocol:** read **Dev Feedback** and the **State Tracker** below first, then jump to the +> step marked `Current`. Ensure that you are in the correct worktree and branch according to preamble here. Update current with Developer feedback and update the tracker after every meaningful change. +> Do not mutate completed steps; append a new entry instead. + +--- + +## Initial Description + +Update GitHub CI so that Docker images are pushed only if pull request is made within same repo (essentially building the Docker image but omitting the push when running for forks PR). + +## Dev Feedback + +## Spec & Implementation + +### Root Cause Analysis + +The `.github/workflows/docker.yml` workflow already had a correct condition on the "Build and push Docker image" step (line 79): + +```yaml +push: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} +``` + +However, two steps were still running unconditionally for fork PRs: + +1. **"Log in to the Container registry"** — The login is unnecessary for fork PRs since no push will occur. While the login itself might succeed with a read-only GITHUB_TOKEN, it is wasteful and could cause confusion. + +2. **"Docker Scout"** — This step tries to pull the just-pushed image from the registry by SHA and scan it for CVEs. For fork PRs, no image was pushed, so Docker Scout would fail trying to find an image that doesn't exist in the registry. + +### Fix Applied + +Both steps now have the same condition guard as the push flag: + +```yaml +if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} +``` + +This ensures: +- **Non-PR triggers** (push to `develop`, tag pushes): all steps run as before — login, build+push, Docker Scout. +- **Same-repo PRs**: all steps run — login, build+push, Docker Scout with PR comment writing. +- **Fork PRs**: login is skipped, image is built but not pushed, Docker Scout is skipped. + +The `write-comment` condition on Docker Scout was already correct (only writes comments for same-repo PRs). + +### Files Changed + +- `.github/workflows/docker.yml` — Added `if:` conditions to "Log in to the Container registry" and "Docker Scout" steps. +- `docs/release-notes/change-log.md` — Added Unreleased section with the fix entry. + +## State Tracker + +**Last Updated:** 2026-05-21 +**Current Step:** Step 2 — Implementation Complete, Ready for Review +**Status:** In review + +### Step 1 — Begin Implementation (completed) +- Explored `.github/workflows/docker.yml` +- Identified that "Log in to the Container registry" and "Docker Scout" steps ran unconditionally for fork PRs +- The push condition on the build step was already correct + +### Step 2 — Implementation Complete (current) +- Added `if:` condition to "Log in to the Container registry" step +- Added `if:` condition to "Docker Scout" step +- Updated `docs/release-notes/change-log.md` with Unreleased section +- Committed changes to `fix/ci-docker-push-fork-pr` branch diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index c22f7b26d..1b8e57719 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -41,6 +41,7 @@ jobs: uses: docker/setup-buildx-action@v3 - name: Log in to the Container registry + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} uses: docker/login-action@v3 with: registry: ${{ env.REGISTRY }} @@ -88,6 +89,7 @@ jobs: - name: Docker Scout id: docker-scout + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} uses: docker/scout-action@v1 with: dockerhub-user: ${{ secrets.DOCKER_SCOUT_USER }} diff --git a/docs/release-notes/change-log.md b/docs/release-notes/change-log.md index 8977fb67e..aa6854c32 100644 --- a/docs/release-notes/change-log.md +++ b/docs/release-notes/change-log.md @@ -9,6 +9,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- CI: Docker image login, build and push are now skipped for fork PRs; image is still built (without push) to validate the Dockerfile. + ## v1.18.5 ### Server From 6269be80f0e538862fd3cdf5002f7ad6527380c1 Mon Sep 17 00:00:00 2001 From: Matthieu Vachon Date: Fri, 22 May 2026 17:17:21 -0400 Subject: [PATCH 2/2] Remove .dev plan file --- .dev/todo/ci-docker-push-fork-pr.md | 73 ----------------------------- 1 file changed, 73 deletions(-) delete mode 100644 .dev/todo/ci-docker-push-fork-pr.md diff --git a/.dev/todo/ci-docker-push-fork-pr.md b/.dev/todo/ci-docker-push-fork-pr.md deleted file mode 100644 index 8d6cf5ca8..000000000 --- a/.dev/todo/ci-docker-push-fork-pr.md +++ /dev/null @@ -1,73 +0,0 @@ -# Update GitHub CI Docker Image Push for Same-Repo PRs Only - -mode: bug -state: review -root_git: ../substreams.worktrees/fix/ci-docker-push-fork-pr -worktree: ../substreams.worktrees/fix/ci-docker-push-fork-pr -branch: fix/ci-docker-push-fork-pr -target_branch: develop - -> **Resume protocol:** read **Dev Feedback** and the **State Tracker** below first, then jump to the -> step marked `Current`. Ensure that you are in the correct worktree and branch according to preamble here. Update current with Developer feedback and update the tracker after every meaningful change. -> Do not mutate completed steps; append a new entry instead. - ---- - -## Initial Description - -Update GitHub CI so that Docker images are pushed only if pull request is made within same repo (essentially building the Docker image but omitting the push when running for forks PR). - -## Dev Feedback - -## Spec & Implementation - -### Root Cause Analysis - -The `.github/workflows/docker.yml` workflow already had a correct condition on the "Build and push Docker image" step (line 79): - -```yaml -push: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} -``` - -However, two steps were still running unconditionally for fork PRs: - -1. **"Log in to the Container registry"** — The login is unnecessary for fork PRs since no push will occur. While the login itself might succeed with a read-only GITHUB_TOKEN, it is wasteful and could cause confusion. - -2. **"Docker Scout"** — This step tries to pull the just-pushed image from the registry by SHA and scan it for CVEs. For fork PRs, no image was pushed, so Docker Scout would fail trying to find an image that doesn't exist in the registry. - -### Fix Applied - -Both steps now have the same condition guard as the push flag: - -```yaml -if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} -``` - -This ensures: -- **Non-PR triggers** (push to `develop`, tag pushes): all steps run as before — login, build+push, Docker Scout. -- **Same-repo PRs**: all steps run — login, build+push, Docker Scout with PR comment writing. -- **Fork PRs**: login is skipped, image is built but not pushed, Docker Scout is skipped. - -The `write-comment` condition on Docker Scout was already correct (only writes comments for same-repo PRs). - -### Files Changed - -- `.github/workflows/docker.yml` — Added `if:` conditions to "Log in to the Container registry" and "Docker Scout" steps. -- `docs/release-notes/change-log.md` — Added Unreleased section with the fix entry. - -## State Tracker - -**Last Updated:** 2026-05-21 -**Current Step:** Step 2 — Implementation Complete, Ready for Review -**Status:** In review - -### Step 1 — Begin Implementation (completed) -- Explored `.github/workflows/docker.yml` -- Identified that "Log in to the Container registry" and "Docker Scout" steps ran unconditionally for fork PRs -- The push condition on the build step was already correct - -### Step 2 — Implementation Complete (current) -- Added `if:` condition to "Log in to the Container registry" step -- Added `if:` condition to "Docker Scout" step -- Updated `docs/release-notes/change-log.md` with Unreleased section -- Committed changes to `fix/ci-docker-push-fork-pr` branch