Optimize build + CI workflow: per-arch parallel builds, Trivy gating, SBOM#37
Conversation
json is bundled in PHP since 8.0 and cannot be disabled, so passing it to install-php-extensions is a no-op. Removing it slightly speeds the extension install step and avoids implying it's optional.
Replace the separate `RUN chmod +x` layers and the `find … sed -i 's/\r$//'` build-time line-ending fix with two cleaner alternatives: - `COPY --chmod=0755` sets the executable bit at copy time, removing a whole image layer per script. - `.gitattributes` pins LF for shell scripts, s6-overlay files, and Dockerfiles so CRLF can't sneak in via a Windows checkout in the first place. s6's supervision tree refuses CRLF; pinning at the VCS layer is more reliable than scrubbing during build.
Drop a comment block above the libmemcachedutil2 / libssl3 dummy-package synthesis explaining what the t64 transition is and the specific condition under which this hack can be removed: when upstream install-php-extensions binaries link against the t64 SONAMEs directly. This is the most opaque piece of Dockerfile.v2 and the one most likely to bite a future maintainer.
The publish job previously declared its own inline matrix (variant × php-version × php-type × php-base, with the same exclude rules and a hand-rolled list of 12 v2/trixie includes). It duplicated the logic already in the setup job and made the two trivially easy to drift. Have setup emit both a test_matrix (narrowed on PRs) and a publish_matrix (always the full set) from a single `build_matrix` shell function. Both jobs consume their respective output. Net: -45 lines, one source of truth, adding a php-version requires one edit instead of two. Also moves the github.event_name interpolation into an env: block per workflow-injection guidance.
Enable sbom: true on the build-push-action so each pushed manifest gets a Software Bill of Materials attestation alongside the existing provenance attestation (mode=max). Supports supply-chain auditing of which OS packages / PHP extensions shipped in a given tag.
For v2/trixie builds the workflow previously created bookworm-aliased manifest tags in a follow-up `docker buildx imagetools create` step. Compute those alias tags up-front in the vars step and pass them as additional entries in the build-push-action `tags:` input instead. Benefits: - One less workflow step and three fewer registry API roundtrips per v2/trixie image (the manifest is published with all tags directly). - Bookworm-aliased tags now inherit the same provenance + SBOM attestations attached by build-push-action; the imagetools-create variant didn't carry them over. - Move matrix interpolations out of run: shell into env: per the workflow-injection hardening guidance.
Previously Trivy ran in the publish job *after* the image had already been pushed to all three registries — a CRITICAL finding would ship to users before anyone saw it, and the SARIF upload told us about it after the fact. Move the scan into build-and-test against the locally-loaded amd64 test image and set exit-code: 1 so a CRITICAL or HIGH finding fails the job. publish depends on build-and-test, so vulnerable images can no longer reach the registries. Notes: - ignore-unfixed: true suppresses advisories without an upstream fix (otherwise we'd be permanently red on every base-image release). - security-events: write added to build-and-test so the SARIF upload to GitHub Code Scanning still works. - Upload step runs `if: always()` so scan results land in Security even when the gate fails — that's the whole point of the report.
Replace the prior two-job pipeline (build-and-test amd64, then publish multi-arch from scratch via QEMU) with a per-arch build matrix that pushes by digest and a merge step that assembles final manifests. Changes: - setup now emits a build_matrix with an arch axis. On PRs only amd64 is built; on push/schedule/dispatch all three are built in parallel. Each entry carries its own runner/platform/qemu fields so the matrix selects the correct GitHub runner without YAML duplication. - arm64 builds run natively on `ubuntu-24.04-arm` instead of through QEMU on amd64 — typically 3–5× faster for a non-trivial extension compile workload. amd64 stays on ubuntu-latest. arm/v7 still uses QEMU since no free native armv7 runner exists. - amd64 jobs continue to load + smoke-test + Trivy-scan the image; on main they additionally push the same image by digest to the GHCR staging registry (cache-hit-driven second build of the same content). arm64 / arm/v7 jobs push directly by digest with no smoke tests. - publish-merge replaces the prior publish job. It downloads the per-arch digest artifacts for a given (variant, php-version, php-type, php-base) tuple and runs `docker buildx imagetools create` three times — once per registry — to assemble the manifest lists with the primary tag plus the v2/trixie→bookworm compatibility tag. Net wins: - Eliminates the redundant amd64 rebuild that publish used to do. - arm64 throughput dominated by native execution, not emulation. - Build-and-test feedback for PRs is unchanged: amd64 only. Hardening: - All `matrix.*` interpolations in run: scripts moved to env: blocks. - Cache scope is now per-arch so concurrent matrix jobs don't compete on the same gha cache key. - Trivy SARIF gets an explicit `category:` so per-tuple uploads don't overwrite each other in the GitHub Security tab.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Optimizes the Docker build pipeline and CI workflow by parallelizing per-architecture builds (with native arm64 runners), moving Trivy scanning to gate publish, attaching SBOMs to pushed images, and simplifying Dockerfile layers.
Changes:
- Refactor CI workflow to build per-arch in parallel, gate publishing on Trivy results, and merge multi-arch manifests via
imagetools createagainst GHCR-staged digests. - Replace separate
RUN chmod/ CRLFsedsteps withCOPY --chmod=and a new.gitattributesenforcing LF line endings; drop redundantjsonextension; add explanatory comment for Trixie t64 dummy packages. - Generate
build_matrixandpublish_matrixfrom a single shell function in the setup job, narrowing the PR test matrix to amd64-only.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.github/workflows/docker-ci.yml |
Splits build/publish into per-arch build + manifest-merge jobs; adds Trivy gate, SBOM, per-arch caches, and PR-only amd64 narrowing. |
Dockerfile.v2 |
Uses COPY --chmod, removes redundant json extension, and documents the Trixie t64 equivs workaround. |
Dockerfile.v1 |
Removes redundant json extension and switches retry.sh install to COPY --chmod. |
.gitattributes |
New file enforcing LF endings for shell scripts, s6 service files, and Dockerfiles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Trivy vulnerability scan | ||
| if: matrix.arch == 'amd64' | ||
| uses: aquasecurity/trivy-action@master | ||
| with: | ||
| scan-type: image | ||
| image-ref: test-${{ steps.vars.outputs.TAG }} | ||
| format: 'sarif' | ||
| severity: 'CRITICAL,HIGH' | ||
| ignore-unfixed: true | ||
| exit-code: '1' |
| - name: Trivy vulnerability scan | ||
| if: matrix.arch == 'amd64' | ||
| uses: aquasecurity/trivy-action@master |
|
|
||
| # Push the per-arch image to the staging registry (GHCR) by digest. | ||
| # The merge job will pull these digests and assemble final manifests | ||
| # on Docker Hub, GHCR, and Quay with the proper tags. |
| sbom: true | ||
| cache-from: type=gha,scope=${{ steps.vars.outputs.CACHE_SCOPE }} | ||
| cache-to: type=gha,mode=max,scope=${{ steps.vars.outputs.CACHE_SCOPE }} | ||
| outputs: type=image,name=${{ env.STAGING_NAME }},push-by-digest=true,name-canonical=true,push=true |
| - name: Inspect final manifest | ||
| run: | | ||
| echo "::group::Creating bookworm compatibility tags for trixie-built v2 image" | ||
|
|
||
| # Replace 'trixie' with 'bookworm' in tag names to maintain backward compatibility | ||
| BOOKWORM_VERSION="${{ matrix.php-version }}-${{ matrix.php-type }}-bookworm" | ||
|
|
||
| # Create manifest aliases pointing trixie-built images to bookworm tags | ||
| docker buildx imagetools create -t \ | ||
| docker.io/${{ secrets.DOCKERHUB_USERNAME }}/php-docker:${BOOKWORM_VERSION}-v2 \ | ||
| ${{ steps.vars.outputs.DOCKERHUB_TAG }} | ||
|
|
||
| docker buildx imagetools create -t \ | ||
| ghcr.io/kingpin/php-docker:${BOOKWORM_VERSION}-v2 \ | ||
| ${{ steps.vars.outputs.GHCR_TAG }} | ||
|
|
||
| docker buildx imagetools create -t \ | ||
| quay.io/kingpinx1/php-docker:${BOOKWORM_VERSION}-v2 \ | ||
| ${{ steps.vars.outputs.QUAY_TAG }} | ||
|
|
||
| echo "✅ Created bookworm compatibility tags pointing to trixie image" | ||
| echo "::endgroup::" | ||
|
|
||
| - name: Run Trivy vulnerability scanner | ||
| uses: aquasecurity/trivy-action@master | ||
| with: | ||
| scan-type: image | ||
| image-ref: ${{ steps.vars.outputs.DOCKERHUB_TAG }} | ||
| format: 'sarif' | ||
| severity: 'CRITICAL,HIGH' | ||
| output: 'trivy-results-${{ matrix.variant }}-${{ matrix.php-version }}-${{ matrix.php-type }}-${{ matrix.php-base }}.sarif' | ||
|
|
||
| - name: Upload Trivy results | ||
| uses: github/codeql-action/upload-sarif@v4 | ||
| with: | ||
| sarif_file: 'trivy-results-${{ matrix.variant }}-${{ matrix.php-version }}-${{ matrix.php-type }}-${{ matrix.php-base }}.sarif' | ||
| set -euo pipefail | ||
| VERSION="${PHP_VERSION}-${PHP_TYPE}-${PHP_BASE}" | ||
| if [ "$VARIANT" = "v2" ]; then SUFFIX="-v2"; else SUFFIX=""; fi | ||
| docker buildx imagetools inspect "docker.io/${DOCKERHUB_USERNAME}/php-docker:${VERSION}${SUFFIX}" |
Debian branches already run `apt-get -y upgrade` to pull base image security patches at build time, but the Alpine branches only ran `apk update` followed by `apk add` — leaving base layer packages at whatever versions the upstream php:*-alpine tag was published with. When the ECR-hosted base image lags upstream Alpine point releases (e.g. 3.22.1 vs 3.23.x), Trivy's CRITICAL/HIGH gate trips on fixed CVEs that an `apk upgrade` would patch immediately. Add `apk upgrade --no-cache` between `apk update` and `apk add` in both Dockerfile.v1 and Dockerfile.v2, mirroring the Debian behavior. Verified locally: v1-8.2-cli-alpine now reports 0 CRITICAL/HIGH CVEs.
The post-publish inspect step only verified the Docker Hub primary tag. If `imagetools create` produced a malformed manifest on GHCR or Quay, or the v2/trixie -> bookworm alias tag failed to land, the job would still report success. Iterate over the same three registries and tag list used in the create step so any partial publish fails the job.
Summary
Targeted optimization pass on the Dockerfiles and CI workflow. Eight atomic commits covering nine items from the recent review:
Dockerfile (verified locally —
./extras/test-build.sh both 8.5-fpm-alpinepasses, s6 init runs cleanly end-to-end)build: drop redundant json from install-php-extensions— json has been bundled in PHP since 8.0; passing it to install-php-extensions is a no-op.build: use COPY --chmod and .gitattributes for line endings— replaces separateRUN chmodlayers withCOPY --chmod=0755and pins LF endings in.gitattributesso the build-timesed -i 's/\r$//'dance is no longer needed.docs(dockerfile): explain Trixie t64 equivs workaround— comment block explaining what thelibmemcachedutil2/libssl3dummy packages are for and the specific condition under which they can be removed (upstream install-php-extensions linking against t64 SONAMEs).CI workflow
ci: generate publish matrix from setup job— drops the inline 90-line publish matrix; setup now emits bothtest_matrix(narrowed on PRs) andpublish_matrix(always full set) from onebuild_matrix()shell function. Adding a php-version is now one edit instead of two.ci: attach SBOM to published images—sbom: trueon build-push-action alongside the existingprovenance: mode=max.ci: inline bookworm-compat tags in build-push tags list— the v2/trixie → bookworm alias is now emitted in the same buildx invocation instead of a follow-updocker buildx imagetools createstep. The aliased tags now also carry the same provenance + SBOM attestations.ci: run Trivy in build-and-test to gate publish— Trivy previously ran after push; an unfixed CRITICAL or HIGH finding would ship to users before anyone saw it. Moved into build-and-test against the locally-loaded image withseverity: CRITICAL,HIGH,exit-code: 1, andignore-unfixed: true.publishdepends onbuild, so vulnerable images can no longer reach the registries.ci: per-arch parallel builds with native arm64 runners— biggest change. Per-arch matrix (amd64 / arm64 / armv7) where arm64 runs natively onubuntu-24.04-arminstead of through QEMU. Each arch build pushes to GHCR by digest; a newpublish-mergejob assembles final manifests on all three registries withdocker buildx imagetools create. Eliminates the prior redundant amd64 rebuild between test and publish. Cache scope is now per-arch so concurrent matrix jobs don't fight on the same gha cache key.Test plan
./extras/test-build.sh both 8.5-fpm-alpinesucceeds for v1 and v2.docker run … php /app/php_test.php) end-to-end.ubuntu-24.04-armrunner is available for this repo.docker/build-push-action@v7outputs: type=image,…,push-by-digest=true,…works withprovenance: mode=max+sbom: true.actions/download-artifact@v4pattern +merge-multiple: truecollects the per-arch digests as expected.publish-mergeproduces multi-arch manifests on Docker Hub, GHCR, and Quay with the correct primary + bookworm-compat tags, and that the v2/trixie alias still resolves on the bookworm tag for existing consumers.category:values in the Security tab (no overwriting).