ci(docker): also publish -shadow image variants

[MegaRedHand]

What

Publish a second, Shadow-simulator-compatible set of Docker images alongside the regular ones. Every tag now gets a -shadow twin:

Regular	Shadow
latest	latest-shadow
unstable	unstable-shadow
sha-<sha>	sha-<sha>-shadow
any dispatch tag	<tag>-shadow

How

• Adds a variant dimension (default + shadow) to the build matrix, cross-producted with arch (amd64/arm64) → 4 build jobs.
• The shadow variant passes the same build args as the shadow-docker-build Makefile target: SHADOW=1, FEATURES=shadow-integration, --no-default-features, and LOCKED= (unlocked, since the Shadow build injects an uncommitted quinn-udp patch).
• GHA build cache scope is now keyed by arch-variant so the two variants don't clobber each other's cache.
• The manifest job loops over both suffixes, emitting per-tag and sha-<sha> multi-arch manifests for each set.

Notes / assumptions

• Both arches are built for the shadow set (full symmetry with the regular set). If Shadow only ever runs on linux/amd64, we can drop the arm64 shadow build by trimming one matrix line to roughly halve the added CI cost.
• This ~doubles the build work per publish (4 builds vs 2); shadow builds are heavier than default.

Draft for review of the arch-scope decision before merging.

[github-actions]

🤖 Kimi Code Review

The PR correctly implements a matrix strategy to build Shadow-simulator-compatible images alongside standard ones. No critical bugs detected, but several points warrant attention:

Verification Required:

1. Dockerfile Build Args (.github/workflows/docker_publish.yaml:77-82): Ensure the repository's Dockerfile actually accepts the build arguments SHADOW, FEATURES, NO_DEFAULT_FEATURES, and LOCKED. If these are not defined as ARG instructions in the Dockerfile, the shadow builds will silently ignore them or fail depending on the base image strictness.

2. Environment Variable Definition (.github/workflows/docker_publish.yaml:116): The variable SHORT_SHA is referenced but not shown in the diff. Verify it is defined in the workflow's env: section (e.g., SHORT_SHA: ${{ github.sha }}), otherwise the manifest job will fail with an unbound variable error.

Positive Observations:

• Cache Isolation (.github/workflows/docker_publish.yaml:93-94): Correctly updates the GitHub Actions cache scope to include the variant name (${{ matrix.arch.name }}-${{ matrix.variant.name }}), preventing cache collision between standard and Shadow builds.
• Security: Proper use of environment variables (SUFFIX, ARCH) in the shell script at line 69-70 prevents potential injection vulnerabilities from matrix values.
• Idiomatic Matrix Usage: The refactoring from include: to Cartesian product matrices (arch × variant) is cleaner and scales correctly if additional variants are added later.

Minor Suggestion:

• Consider validating that LOCKED= (empty value) in the shadow variant is intentional. If the Dockerfile uses ARG LOCKED=--locked as a default, this explicitly unsets it, which may be the desired behavior for Shadow builds, but should be documented.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Codex Code Review

No findings.

The change is limited to CI packaging, not consensus/state-transition code. In .github/workflows/docker_publish.yaml, the new arch x variant matrix is internally consistent: tag generation in lines 68-80 matches manifest assembly in lines 118-132, and the shadow build args in lines 47-51 still match the existing shadow Docker path in Makefile and Dockerfile.

Residual risk: I couldn’t execute the GitHub Actions workflow here, so the only unverified part is runtime behavior of the docker/build-push-action input at lines 87-90. One manual workflow_dispatch run that publishes both a default and -shadow tag would close that gap.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[greptile-apps]

Greptile Summary

This PR extends the Docker publish workflow to build and publish Shadow-simulator-compatible image variants alongside the regular ones. Every tag now gets a -shadow twin by adding a variant dimension to the GHA matrix, cross-producted with the existing arch dimension (4 total build jobs). The manifest assembly job is updated to loop over both suffixes and create multi-arch manifests for each.

• The matrix restructuring correctly replaces the old include: list with arch and variant cross-product entries, and all references (matrix.arch.name, matrix.arch.runner, matrix.variant.suffix, etc.) are wired properly throughout.
• GHA cache scopes are keyed by arch-variant so the two variants no longer share a cache, preventing cross-contamination between default and shadow build layers.
• The shadow build args (SHADOW=1, FEATURES=shadow-integration, NO_DEFAULT_FEATURES=--no-default-features, LOCKED=) mirror the existing shadow-docker-build Makefile target, so the image behaviour should match what the team already tested locally.

Confidence Score: 4/5

Safe to merge; the workflow correctly builds and publishes both regular and shadow image variants with properly isolated caches and manifests.

The matrix restructuring and cache-scope keying are correct. The only notable gap is that the manifest job hardcodes the suffix loop independently of the build matrix variant list, meaning a future third variant would get per-arch images built but no multi-arch manifest assembled without a manual edit to two separate places.

.github/workflows/docker_publish.yaml — specifically the suffix loop in the publish-manifest step, which is decoupled from the build matrix variant list.

Important Files Changed

Filename	Overview
.github/workflows/docker_publish.yaml	Adds a cross-product build matrix (arch × variant) to publish shadow-simulator-compatible image twins alongside regular images; manifest loop correctly combines per-arch layers but hardcodes the suffix list independently of the matrix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    trigger([Push to main / workflow_dispatch]) --> matrix

    subgraph matrix["build-image matrix (4 jobs)"]
        A1["amd64 × default\ntag-amd64"]
        A2["amd64 × shadow\ntag-shadow-amd64"]
        B1["arm64 × default\ntag-arm64"]
        B2["arm64 × shadow\ntag-shadow-arm64"]
    end

    matrix --> manifest["publish-manifest"]

    subgraph manifest["publish-manifest"]
        M1["suffix='':\ndocker buildx imagetools create\ntag, sha-SHORT\nfrom tag-amd64 + tag-arm64"]
        M2["suffix='-shadow':\ndocker buildx imagetools create\ntag-shadow, sha-SHORT-shadow\nfrom tag-shadow-amd64 + tag-shadow-arm64"]
    end

    M1 --> R1[("ghcr.io/…:tag\nghcr.io/…:sha-SHORT")]
    M2 --> R2[("ghcr.io/…:tag-shadow\nghcr.io/…:sha-SHORT-shadow")]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    trigger([Push to main / workflow_dispatch]) --> matrix

    subgraph matrix["build-image matrix (4 jobs)"]
        A1["amd64 × default\ntag-amd64"]
        A2["amd64 × shadow\ntag-shadow-amd64"]
        B1["arm64 × default\ntag-arm64"]
        B2["arm64 × shadow\ntag-shadow-arm64"]
    end

    matrix --> manifest["publish-manifest"]

    subgraph manifest["publish-manifest"]
        M1["suffix='':\ndocker buildx imagetools create\ntag, sha-SHORT\nfrom tag-amd64 + tag-arm64"]
        M2["suffix='-shadow':\ndocker buildx imagetools create\ntag-shadow, sha-SHORT-shadow\nfrom tag-shadow-amd64 + tag-shadow-arm64"]
    end

    M1 --> R1[("ghcr.io/…:tag\nghcr.io/…:sha-SHORT")]
    M2 --> R2[("ghcr.io/…:tag-shadow\nghcr.io/…:sha-SHORT-shadow")]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/workflows/docker_publish.yaml:119-133
**Hardcoded suffix loop decoupled from build matrix**

The manifest job iterates `for suffix in "" "-shadow"` rather than deriving suffixes from the build matrix. If a third variant is added to the `variant:` list in `build-image`, the build will push per-arch images for it but the manifest step will silently skip creating the multi-arch manifest for that variant. The two lists need to be kept in sync manually.

_{Reviews (1): Last reviewed commit: "ci(docker): also publish -shadow image v..." | Re-trigger Greptile}

[greptile-apps]

Hardcoded suffix loop decoupled from build matrix

The manifest job iterates for suffix in "" "-shadow" rather than deriving suffixes from the build matrix. If a third variant is added to the variant: list in build-image, the build will push per-arch images for it but the manifest step will silently skip creating the multi-arch manifest for that variant. The two lists need to be kept in sync manually.

Prompt To Fix With AI

This is a comment left during a code review.
Path: .github/workflows/docker_publish.yaml
Line: 119-133

Comment:
**Hardcoded suffix loop decoupled from build matrix**

The manifest job iterates `for suffix in "" "-shadow"` rather than deriving suffixes from the build matrix. If a third variant is added to the `variant:` list in `build-image`, the build will push per-arch images for it but the manifest step will silently skip creating the multi-arch manifest for that variant. The two lists need to be kept in sync manually.

How can I resolve this? If you propose a fix, please make it concise.

[github-actions]

🤖 Claude Code Review

Review: ci(docker): also publish -shadow image variants (PR 508)

This is a CI-only change (.github/workflows/docker_publish.yaml); no Rust/consensus code is touched, so this review focuses on workflow correctness and the security implications of the new publish path.

Correctness

The matrix restructuring, tag naming, and manifest creation all line up correctly:

• Build job tags come out as ${t}${SUFFIX}-${ARCH} (e.g. unstable-shadow-amd64).
• Manifest job references ${t}${suffix}-amd64/-arm64 for the same tags, so the multi-arch manifest creation will find the images the build job actually pushed.
• build_args for the shadow variant (SHADOW=1, FEATURES=shadow-integration, NO_DEFAULT_FEATURES=--no-default-features, LOCKED=) exactly mirrors the existing shadow-docker-build Makefile target and the corresponding ARGs in Dockerfile (lines 28-39, 49-52). Good consistency check — I verified this against the Makefile and Dockerfile directly.
• Cache scope is now ${arch}-${variant}, correctly avoiding cache collisions between the two variants (previously scope was arch-only, which would have been wrong once a second variant was introduced).
• Using env: (SUFFIX, ARCH) instead of inlining ${{ matrix.* }} directly into the run: bash block in the "Prepare tags" step is good practice, even though matrix values here are trusted, not user input.

Security / supply-chain (worth flagging explicitly, given this is consensus-layer software)

LOCKED= (i.e., unlocked builds) is now wired into the automated public publish pipeline, not just a local dev target. This was already true of make shadow-docker-build, but that was a manual, local-only build. This PR makes it run unattended on every push to main and on every manual dispatch, publishing the result to ghcr.io/lambdaclass/ethlambda:*-shadow for public consumption (e.g., by Shadow simulator users).

Building without --locked means the entire dependency graph — not just the quinn-udp patch — is re-resolved against semver ranges at build time rather than pinned to Cargo.lock. Two consequences:

1. Non-reproducibility: two shadow builds from the same commit can produce different binaries if any dependency publishes a new semver-compatible version in between.
2. Supply-chain exposure: a compromised or yanked-and-replaced crate published between builds could be silently pulled into a publicly distributed image without any corresponding commit/lockfile change to audit.

Given the reason for going unlocked is narrowly the local-path quinn-udp patch (shadow/cargo-patch.toml, not a registry dependency), consider whether the lockfile could instead be regenerated/committed for the shadow feature set (or the patch entry added to Cargo.lock in a separate lockfile used only for shadow builds) so the build can stay --locked. If that's not practical, it's still worth calling out in the Makefile/workflow comments that shadow images are explicitly non-reproducible builds, so consumers know not to treat them as equivalent-security artifacts to the regular tags.

CI cost (already flagged by the author)

The PR description already raises the arm64-shadow-build question — agreed that's worth resolving before merging out of draft. Since Shadow simulator only runs on Linux/amd64 (it's a syscall-interception-based network simulator), building and publishing *-shadow-arm64 seems unlikely to have any consumer. Recommend dropping the arm64 leg from the shadow variant via exclude, e.g.:

strategy:
  matrix:
    arch: [...]
    variant: [...]
    exclude:
      - arch: { name: arm64 }
        variant: { name: shadow }

That halves the added cost (3 builds instead of 4) and the manifest job would need a small conditional adjustment to only create a single-arch "manifest" (or just push the amd64 image directly) for the shadow tags.

Minor / non-blocking

• build_args: "" for the default variant results in one blank line inserted into build-args: — harmless (docker/build-push-action ignores empty lines) but slightly noisy in build logs.
• Consider adding a short comment near the exclude (if added) or near the variant matrix explaining why symmetry was chosen/dropped, since this decision is exactly what's under review in the draft.

Overall the workflow logic is sound and the tag/manifest wiring is correct — my main actionable ask is resolving the arch-scope question the PR itself raises, and flagging the unlocked-build supply-chain tradeoff now that it's automated into a public-facing pipeline.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt