Skip to content

feat: added multi arch builds support - #131

Open
bdimitrov-netzine wants to merge 1 commit into
mainfrom
feat/multi-arch-docker
Open

feat: added multi arch builds support#131
bdimitrov-netzine wants to merge 1 commit into
mainfrom
feat/multi-arch-docker

Conversation

@bdimitrov-netzine

@bdimitrov-netzine bdimitrov-netzine commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adding build support for arm64

Closes #

Surface areas touched

  • Consensus protocol (primary / worker / network / state-sync)
  • Execution / EVM
  • JSON-RPC (eth_*, rayls_*, faucet)
  • Middleware (orchestrator / processor / bridge)
  • Infrastructure (types / storage / config / network-cli)
  • On-chain contracts (rayls-contracts/)
  • Operations (etc/, scripts, Docker, compose)
  • CI / build (.github/workflows/, Makefile)
  • Documentation only (doc/, in-crate READMEs, root docs)
  • Tests only

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bdimitrov-netzine's task in 2m 12s —— View job


PR Review: feat: added multi arch builds support

  • Gather context and read changed files
  • Review .github/workflows/build-docker.yml
  • Review .github/workflows/promote-docker.yml
  • Post comprehensive review feedback

Overall Assessment

The approach is solid and follows the canonical multi-arch Docker pattern correctly: native runners per arch → push by digest → merge manifest list. Avoiding QEMU for Rust release builds is the right call. The backward-compatibility handling in promote-docker.yml is clean. A few things worth addressing before merge:


Issues

1. printf format-string with variable data (build-docker.yml:254–256)

- name: Create manifest list and push tag
  working-directory: ${{ runner.temp }}/digests
  run: |
    docker buildx imagetools create \
      --tag "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME:$TAG" \
      $(printf -- "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME@sha256:%s " *)

$ECR_PRIVATE_REGISTRY and $IMAGE_NAME are embedded in the printf format string. Both values come from secrets/vars (not user input), so no injection risk in practice. However, if either value ever contains %, printf will misinterpret it as a format specifier. A safer and arguably clearer pattern:

images=()
for f in *; do
  images+=("$ECR_PRIVATE_REGISTRY/$IMAGE_NAME@sha256:$f")
done
docker buildx imagetools create \
  --tag "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME:$TAG" \
  "${images[@]}"

Fix this →


2. Trivy doesn't specify --platform for multi-arch scans (build-docker.yml:295–303)

- name: Run Trivy vulnerability scanner
  uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0
  with:
    image-ref: ${{ env.ECR_PRIVATE_REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.meta.outputs.image-tag }}

Each security-scan leg runs on a native runner (amd64/arm64), which means the Docker daemon defaults to pulling its own platform's image from the manifest list. This should cause Trivy to scan the right platform image on each runner. However, it relies on implicit behavior: Trivy resolves the platform via the Docker daemon's default. If Trivy or the ECR client bypasses the daemon's platform preference, both scan legs could end up scanning the same arch.

Consider adding an explicit --platform env var or using image-ref with @sha256:<arch-digest> to make the intent unambiguous. At minimum, confirm in the job summary that the correct arch image was scanned.


3. Unverified assumption: ubuntu-24.04-arm is available (build-docker.yml:152)

- platform: linux/arm64
  runner: ubuntu-24.04-arm

ubuntu-24.04-arm is a GitHub-hosted ARM runner. This runner type requires GitHub Team/Enterprise or is in public preview for certain repo types. If your organization does not have access, the arm build leg will silently queue forever or fail with a confusing error. Confirm this runner is available in raylsnetwork before merging.


Minor Notes

meta job inconsistent expression styles (build-docker.yml:131–133): The ECR create step uses ${{ env.IMAGE_NAME }} (GitHub expression syntax) while all other shell steps in the same file use $IMAGE_NAME (shell variable). Both work since GitHub injects workflow-level env vars into the shell environment, but the inconsistency is worth cleaning up for readability.

No strategy.fail-fast on build matrix (build-docker.yml:146): The default is true, so if one arch fails, the other is cancelled. This is acceptable for builds (no point continuing if you can't make the manifest list), but it's worth leaving a comment to make the intent explicit, since security-scan explicitly sets fail-fast: false.


Positives

  • Native runners, no QEMU: Exactly the right choice for a Rust release build. QEMU emulation for cargo build --release would take hours.
  • Push-by-digest pattern: Correctly delays tagging until both arches are ready, ensuring the tag always resolves to a manifest list and never a single-arch image.
  • ECR repo creation in meta job: Correctly eliminates the create-repository race condition that would occur if both arch legs ran it concurrently.
  • Backward compat in promote-docker.yml: The two-pass label read (multi-arch first, single-arch fallback + <no value> guard) handles mixed fleets of old and new images gracefully.
  • All action SHAs pinned: Good security hygiene throughout.
  • retention-days: 1 on digest artifacts: Correct — these are transient, workflow-internal artifacts with no value after the run.

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Security Scan - Code

Severity: HIGH, CRITICAL

No vulnerabilities found

View scan results

Report Summary

┌─────────────────────────────────────────────┬────────────┬─────────────────┬───────────────────┐
│                   Target                    │    Type    │ Vulnerabilities │ Misconfigurations │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ Cargo.lock                                  │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/Cargo.lock      │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/fuzz/Cargo.lock │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/state-sum/Cargo.lock                    │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/tps/package-lock.json                   │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ rayls-contracts/package-lock.json           │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/chaos-network/Dockerfile                │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-network/Dockerfile               │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-replay/Dockerfile                │ dockerfile │        -        │         0         │
└─────────────────────────────────────────────┴────────────┴─────────────────┴───────────────────┘
Legend:
- '-': Not scanned
- '0': Clean (no security findings detected)

@bdimitrov-netzine
bdimitrov-netzine marked this pull request as ready for review August 24, 2026 08:28
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bdimitrov-netzine's task in 1m 46s —— View job


PR Review: feat: added multi arch builds support

  • Gather context and read changed files
  • Review .github/workflows/build-docker.yml
  • Review .github/workflows/promote-docker.yml
  • Post comprehensive review feedback

Overall Assessment

The approach is solid and follows the canonical multi-arch Docker pattern correctly: native runners per arch → push by digest → merge manifest list. Avoiding QEMU for Rust release builds is the right call. The backward-compatibility handling in promote-docker.yml is clean. A few things worth addressing before merge:


Issues

1. printf format-string with variable data (build-docker.yml:254–256)

- name: Create manifest list and push tag
  working-directory: ${{ runner.temp }}/digests
  run: |
    docker buildx imagetools create \
      --tag "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME:$TAG" \
      $(printf -- "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME@sha256:%s " *)

$ECR_PRIVATE_REGISTRY and $IMAGE_NAME are embedded in the printf format string. Both values come from secrets/vars (not user input), so no injection risk in practice. However, if either value ever contains %, printf will misinterpret it as a format specifier. A safer and clearer pattern avoids the issue entirely:

images=()
for f in *; do
  images+=("$ECR_PRIVATE_REGISTRY/$IMAGE_NAME@sha256:$f")
done
docker buildx imagetools create \
  --tag "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME:$TAG" \
  "${images[@]}"

Fix this →


2. Trivy doesn't specify --platform for multi-arch scans (build-docker.yml:295–303)

- name: Run Trivy vulnerability scanner
  uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0
  with:
    image-ref: ${{ env.ECR_PRIVATE_REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.meta.outputs.image-tag }}

Each security-scan leg runs on a native runner (amd64/arm64), which means the Docker daemon defaults to pulling its own platform's image from the manifest list. This should cause Trivy to scan the right platform image on each runner. However, it relies on implicit behavior: Trivy resolves the platform via the Docker daemon's default. If Trivy or the ECR client bypasses the daemon's platform preference, both scan legs could end up scanning the same arch.

Consider adding an explicit --platform env var or using image-ref with @sha256:<arch-digest> to make the intent unambiguous.


3. Unverified assumption: ubuntu-24.04-arm is available (build-docker.yml:152)

- platform: linux/arm64
  runner: ubuntu-24.04-arm

ubuntu-24.04-arm is a GitHub-hosted ARM runner. This runner type requires GitHub Team/Enterprise or is in public preview for certain repo types. If your organization does not have access, the arm build leg will silently queue forever or fail with a confusing error. Confirm this runner is available in raylsnetwork before merging.


Minor Notes

meta job inconsistent expression styles (build-docker.yml:131–133): The ECR create step uses ${{ env.IMAGE_NAME }} (GitHub expression syntax) while all other shell steps in the same file use $IMAGE_NAME (shell variable). Both work since GitHub injects workflow-level env vars into the shell environment, but the inconsistency is worth cleaning up for readability.

No strategy.fail-fast on build matrix (build-docker.yml:146): The default is true, so if one arch fails, the other is cancelled. This is acceptable for builds (no point continuing if you can't make the manifest list), but it's worth leaving a comment to make the intent explicit, since security-scan explicitly sets fail-fast: false.

id: login-ecr unused (build-docker.yml:177): The login step in the build job carries id: login-ecr but its outputs are never referenced downstream. Minor noise — consider removing the id.

Missing set -euo pipefail in the merge job's manifest-create step: the other multi-line shell scripts in this file don't have it either, but this is the most critical step (a silent failure here could push an incomplete manifest). Adding set -euo pipefail would make failures loud.


Positives

  • Native runners, no QEMU: Exactly the right choice for a Rust release build. QEMU emulation for cargo build --release would take hours.
  • Push-by-digest pattern: Correctly delays tagging until both arches are ready, ensuring the tag always resolves to a manifest list and never a single-arch image.
  • ECR repo creation in meta job: Correctly eliminates the create-repository race condition that would occur if both arch legs ran it concurrently.
  • Backward compat in promote-docker.yml: The two-pass label read (multi-arch first, single-arch fallback + <no value> guard) handles mixed fleets of old and new images gracefully.
  • All action SHAs pinned: Good security hygiene throughout.
  • retention-days: 1 on digest artifacts: Correct — these are transient, workflow-internal artifacts with no value after the run.

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bdimitrov-netzine's task in 2m 21s —— View job


PR Review: feat: added multi arch builds support

  • Gather context
  • Read changed workflow files
  • Review .github/workflows/build-docker.yml
  • Review .github/workflows/promote-docker.yml
  • Post comprehensive review feedback

Overall Assessment

The multi-arch implementation is architecturally sound — native runners per arch → push by digest → merge manifest list is the canonical pattern. The promote-docker.yml backward-compatibility handling for mixed old/new images is clean. Three issues from the prior review remain unaddressed; there are two new minor findings.

Note: I cannot directly modify .github/workflows/ files (GitHub App permissions). The findings below are review feedback.


Outstanding Issues (from prior review, still present)

1. printf format string with variable data — build-docker.yml:256

$(printf -- "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME@sha256:%s " *)

$ECR_PRIVATE_REGISTRY and $IMAGE_NAME are embedded in the format string. If either ever contains %, printf will misinterpret it as a format specifier. Both values come from secrets/vars so no active injection risk today, but it's fragile. Safer pattern:

images=()
for f in *; do
  images+=("$ECR_PRIVATE_REGISTRY/$IMAGE_NAME@sha256:$f")
done
docker buildx imagetools create \
  --tag "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME:$TAG" \
  "${images[@]}"

Fix this →


2. Trivy does not pin which arch it scans — build-docker.yml:298

image-ref: ${{ env.ECR_PRIVATE_REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.meta.outputs.image-tag }}

Each security-scan leg runs on a native runner, so the Docker daemon should pull its own platform's image from the manifest list. However, this is implicit behaviour: Trivy resolves the platform through the Docker daemon's default, and if Trivy or the ECR client bypasses that (e.g. via the manifest API directly), both legs could scan the same arch.

Consider anchoring each leg to its digest explicitly:

image-ref: ${{ env.ECR_PRIVATE_REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.meta.outputs.image-tag }}
# or: pass the per-arch digest from the merge job as an output and use @sha256:<digest>

At minimum, print the image's actual architecture in the summary step so you can verify each leg scanned the right image.


3. ubuntu-24.04-arm runner availability — build-docker.yml:152

- platform: linux/arm64
  runner: ubuntu-24.04-arm

GitHub-hosted ARM runners require GitHub Team/Enterprise or the public preview access. If raylsnetwork does not have access, the ARM leg will queue silently or fail with a confusing message. Confirm this runner is available in the org before merging (a successful dry-run on a test tag would confirm it).


New Findings

4. provenance: false is undocumented — build-docker.yml:194

provenance: false

Disabling SLSA provenance attestation is a deliberate trade-off (typically required because ECR does not support OCI attestation manifests stored as separate image layers, which buildx generates by default). The cache comment nearby explains the cache decision; the provenance line deserves a similar note so it's not accidentally re-enabled:

# provenance attestations produce a separate OCI manifest that ECR doesn't
# support cleanly; keeping it false avoids broken manifest list entries.
provenance: false

5. Commented-out idempotency guard creates a silent overwrite risk — promote-docker.yml:153–164

#- name: Fail if target tag already exists

Without this check, running the promotion workflow twice on the same version/target pair will silently overwrite the existing ECR tag. The git-tag job refuses to move a tag that already exists at a different commit, but ECR has no equivalent protection here. If this was intentionally left off (e.g. to allow re-promotions), a short comment explaining the decision would prevent someone from un-commenting it later expecting it to be safe to enable.


Minor Notes (unchanged from prior review)

  • Inconsistent expression styles (build-docker.yml:131): ${{ env.IMAGE_NAME }} in the ECR create step vs. $IMAGE_NAME elsewhere in the same file. Both work, cosmetic cleanup only.
  • No explicit strategy.fail-fast on build matrix (build-docker.yml:146): Defaults to true, which is acceptable, but security-scan explicitly sets fail-fast: false — noting the intent explicitly on build would be consistent.

Positives

  • Native runners, no QEMU: Correct call for a Rust release build.
  • Push-by-digest pattern: Correctly defers tagging until both arches are ready.
  • ECR repo creation in meta job: Eliminates the create-repository race that would occur if both arch legs ran it concurrently.
  • Backward compat in promote-docker.yml: Two-pass label read (multi-arch first, single-arch fallback + <no value> guard) handles mixed image fleets cleanly.
  • All action SHAs pinned: Good security hygiene.
  • retention-days: 1 on digest artifacts: Correct, these are transient artifacts.

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bdimitrov-netzine's task in 2m 23s —— View job


PR Review: feat: added multi arch builds support

  • Gather context and read changed files
  • Review .github/workflows/build-docker.yml
  • Review .github/workflows/promote-docker.yml
  • Check which prior issues have been resolved
  • Post comprehensive review feedback

Overall Assessment

This synchronize appears to be a rebase/force-push rather than a set of fixes — all five outstanding issues from the three prior review rounds remain unaddressed. The implementation itself is architecturally sound (native runners, push-by-digest, manifest merge). The Inspect pushed manifest step is a welcome addition since the last review pass. Before merging, the issues below should be resolved.


Outstanding Issues (all carried over from prior review — none addressed)

1. printf format-string with variable data — build-docker.yml:256

$(printf -- "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME@sha256:%s " *)

Both $ECR_PRIVATE_REGISTRY and $IMAGE_NAME are embedded in the printf format string. If either ever contains %, printf will misinterpret it as a format specifier. Values come from secrets/vars so there's no active injection risk today, but the pattern is fragile. Safer alternative:

images=()
for f in *; do
  images+=("$ECR_PRIVATE_REGISTRY/$IMAGE_NAME@sha256:$f")
done
docker buildx imagetools create \
  --tag "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME:$TAG" \
  "${images[@]}"

Fix this →


2. Trivy does not pin which arch it scans — build-docker.yml:298

image-ref: ${{ env.ECR_PRIVATE_REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.meta.outputs.image-tag }}

Each security-scan leg runs on a native runner, so the Docker daemon should pull the matching platform's image from the manifest list. However this is implicit: Trivy resolves the platform through the daemon's default, and if Trivy or the ECR client bypasses that (e.g. via the manifest API directly), both legs could scan the same arch. Consider anchoring each leg to its digest explicitly, or at minimum printing the scanned image's architecture in the Summary step to confirm each leg scanned the right image.


3. ubuntu-24.04-arm runner availability — build-docker.yml:152

- platform: linux/arm64
  runner: ubuntu-24.04-arm

GitHub-hosted ARM runners require GitHub Team/Enterprise or public preview access. If raylsnetwork does not have access, the ARM leg will queue silently or fail with an unhelpful error. A successful dry-run workflow on a test tag against this org would confirm availability before merging.


4. provenance: false is undocumented — build-docker.yml:194

provenance: false

The comment that follows (line 195–196) explains the outputs push-by-digest intent, not why provenance is disabled. Disabling SLSA provenance is typically required because ECR doesn't cleanly support OCI attestation manifests generated by buildx. A one-line comment on the provenance line itself prevents it from being accidentally re-enabled:

# Disabled: buildx provenance attestations produce a separate OCI manifest that ECR doesn't support cleanly.
provenance: false

Fix this →


5. Commented-out idempotency guard — promote-docker.yml:153–164

#- name: Fail if target tag already exists

Running promotion twice on the same version/target pair silently overwrites the existing ECR tag. The git-tag job refuses to move a tag that exists at a different commit, but ECR has no equivalent protection here. If the guard is intentionally off (to allow re-promotions), a short comment saying so prevents someone from un-commenting it expecting it to be safe:

# Idempotency guard intentionally omitted: re-promotions are allowed
# (the git-tag job is the durable guard against moving a published commit).

Minor Notes (unchanged from prior reviews)

  • Inconsistent expression styles (build-docker.yml:131): The ECR create-repository step uses ${{ env.IMAGE_NAME }} while all shell steps use $IMAGE_NAME. Cosmetic, but worth cleaning up for consistency.
  • id: login-ecr unused (build-docker.yml:177): The step id is declared but its outputs are never referenced. Minor noise — the id: can be removed.
  • Missing set -euo pipefail in the merge job's manifest-create step: A silent failure here could push an incomplete manifest list. The git-tag job already uses set -euo pipefail; adding it here would be consistent and catch failures loudly.
  • No explicit strategy.fail-fast on build matrix (build-docker.yml:146): Defaults to true, which is acceptable. security-scan sets fail-fast: false explicitly — noting the intent on build would be consistent.

New Positive Since Last Review

Inspect pushed manifest step (build-docker.yml:258–262): Good addition — shows the manifest structure in the run summary. If you want to validate that both arches are present (not just inspect), you could parse the output:

docker buildx imagetools inspect "$ECR_PRIVATE_REGISTRY/$IMAGE_NAME:$TAG" \
  --format '{{ range .Manifest.Manifests }}{{ println .Platform.OS "/" .Platform.Architecture }}{{ end }}'

This would surface a one-arch manifest immediately rather than letting it reach security-scan.


Positives (carried from prior reviews)

  • Native runners for each arch (no QEMU) — correct for Rust release builds.
  • Push-by-digest pattern ensures the tag always resolves to a manifest list, never a single-arch image.
  • ECR repo creation moved to meta job — eliminates the create-repository race.
  • Two-pass label read in promote-docker.yml handles mixed old/new image fleets cleanly.
  • All action SHAs pinned throughout.
  • retention-days: 1 on digest artifacts is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant