Skip to content

ci: Use a matrix for publishing docker images #359

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

Closed
sdn4z wants to merge 1 commit into from
Closed

ci: Use a matrix for publishing docker images #359

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 21 additions & 7 deletions .github/workflows/publish.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ jobs:
permissions:
contents: read
packages: write
strategy:
Copy link
Contributor

@github-actions github-actions bot Oct 2, 2025

Choose a reason for hiding this comment

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

🦉 🎯 Correctness

Severity: HIGH 🔴

    strategy:
      matrix:
        include:
          - platform: linux/amd64
            cache-ref: buildcache-amd64
          - platform: linux/arm64
            cache-ref: buildcache-arm64

Adding a matrix strategy to this publishing job is incorrect. It will cause the job to run multiple times, once for each platform. Each run will build and push a single-architecture image, overwriting the tags from the previous runs. For example, the latest tag will end up pointing only to the linux/arm64 image, breaking the multi-platform support.

This job should run only once to build and push a single multi-platform manifest that supports all target architectures.

More information about this comment
  • File: .github/workflows/publish.yml
  • Line: 119
  • Relative line: 4
  • With suggestion: No
  • Suggestion ready for replacement: No

matrix:
include:
- platform: linux/amd64
cache-ref: buildcache-amd64
- platform: linux/arm64
cache-ref: buildcache-arm64
steps:
- name: Check out the repo
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
Expand All @@ -137,21 +144,28 @@ jobs:
uses: ./.github/actions/docker-build
with:
push: "true"
platforms: linux/amd64,linux/arm64
platforms: ${{ matrix.platform }}
dockerfile: ./Dockerfile
context: .
image-name: elementsinteractive/twyn
cache-from: type=registry,ref=ghcr.io/elementsinteractive/twyn:buildcache-amd64,ref=ghcr.io/elementsinteractive/twyn:buildcache-arm64
cache-from: type=registry,ref=ghcr.io/elementsinteractive/twyn:${{ matrix.cache-ref }}

- name: Delete old cache entries
env:
GH_TOKEN: ${{ github.token }}
run: |
# Get all versions of the container package
versions=$(gh api "orgs/elementsinteractive/packages/container/twyn/versions" --paginate)

# Extract version IDs that do NOT have any buildcache-* tags (buildcache-amd64, buildcache-arm64, etc.)
ids_to_delete=$(echo "$versions" | jq -r '.[] | select(.metadata.container.tags | map(test("^buildcache-")) | any | not) | .id')
# Get all versions (paginate + slurp into one array)
Copy link
Contributor

@github-actions github-actions bot Oct 2, 2025

Choose a reason for hiding this comment

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

🦉 🎯 Correctness

Severity: HIGH 🔴

          # Get all versions (paginate + slurp into one array)
          versions=$(gh api "orgs/elementsinteractive/packages/container/twyn/versions" --paginate | jq -s 'add')

          # Pattern for the current cache ref (GitHub Actions expression expands before script runs)
          cache_ref="${{ matrix.cache-ref }}"

          # Extract version IDs that do NOT have the cache_ref tag
          ids_to_delete=$(echo "$versions" | jq -r --arg pattern "$cache_ref" '
            .[] 
            | select(.metadata.container.tags | map(test($pattern)) | any | not) 
            | .id
          ')

This script has a critical flaw and will delete your production Docker images.

The jq filter select(.metadata.container.tags | map(test($pattern)) | any | not) selects all package versions that are not tagged with the current matrix job's cache tag (e.g., buildcache-amd64). This means it will target for deletion:

  1. The cache images for all other architectures (e.g., buildcache-arm64).
  2. The actual versioned images you just published (e.g., v1.2.3, latest), as they do not have buildcache-* tags.

Running this in a matrix also creates a race condition where both jobs will attempt to delete the same images. This step is destructive and needs to be completely re-evaluated.

It's worth noting that the original implementation also appeared to be incorrect, as it would delete all non-cache images. The goal of cache pruning needs to be clarified and implemented safely, likely in a separate job after the publish step is complete.

More information about this comment
  • File: .github/workflows/publish.yml
  • Line: 157
  • Relative line: 35
  • With suggestion: No
  • Suggestion ready for replacement: No

versions=$(gh api "orgs/elementsinteractive/packages/container/twyn/versions" --paginate | jq -s 'add')

# Pattern for the current cache ref (GitHub Actions expression expands before script runs)
cache_ref="${{ matrix.cache-ref }}"

# Extract version IDs that do NOT have the cache_ref tag
ids_to_delete=$(echo "$versions" | jq -r --arg pattern "$cache_ref" '
.[]
| select(.metadata.container.tags | map(test($pattern)) | any | not)
| .id
')

# Delete them
for id in $ids_to_delete; do
Expand Down