-
Notifications
You must be signed in to change notification settings - Fork 29
fix: Multi arch. docker images and binary mismatch #382
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.
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
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request adds multi-architecture (amd64 and arm64) build support to CI/CD pipelines and Dockerfiles. QEMU setup is added to workflows, separate build steps for each architecture are introduced in CI with validation, and Dockerfile build constraints are simplified by removing platform directives and image digests. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant QEMU
participant Build as Build Steps
participant Test as Validation
participant Scan as Image Scan
CI->>QEMU: Setup QEMU (amd64, arm64)
QEMU-->>CI: Ready
par Build Phase
CI->>Build: Build x86 container (-amd64 tag)
Build-->>CI: amd64 image ready
CI->>Build: Build arm64 container (-arm64 tag)
Build-->>CI: arm64 image ready
end
CI->>Test: Test image & binary architectures
Test->>Test: Verify amd64 arch consistency
Test->>Test: Verify arm64 arch consistency
alt Mismatch detected
Test-->>CI: ❌ Validation failed
else Success
Test-->>CI: ✓ All architectures valid
end
CI->>Scan: Scan validated images
Scan-->>CI: Scan complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span multiple workflow files and Dockerfiles with a mix of new conditional logic (architecture validation step) and straightforward simplifications (removing platform directives). The multi-arch validation logic requires careful review, but the alterations are localized and follow a consistent pattern. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci.yaml(1 hunks).github/workflows/release-docker.yml(1 hunks)Dockerfile.development(2 hunks)Dockerfile.production(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: clippy
- GitHub Check: test
- GitHub Check: msrv
- GitHub Check: Redirect rules - openzeppelin-monitor
- GitHub Check: Header rules - openzeppelin-monitor
- GitHub Check: Pages changed - openzeppelin-monitor
- GitHub Check: Analyze (rust)
- GitHub Check: semgrep/ci
🔇 Additional comments (6)
Dockerfile.production (2)
2-2: Platform directives removed to enable multi-arch builds; verify digest strategy.Removing
--platform=${BUILDPLATFORM}is correct for QEMU-based cross-platform builds, as this directive forces the build to use the host platform and blocks cross-compilation. However, removing image digests (@sha256:...) may impact reproducibility. The PR objectives indicate these are outdated, but ensure your reproducibility strategy is captured elsewhere (e.g., lock files, SBOMs generated in CI).Also applies to: 15-15
12-12: Optimization:--profile releaseadded during binary build.The addition of
--profile releaseincreases optimization during the cargo install phase, which will improve runtime performance of the binary..github/workflows/release-docker.yml (1)
67-70: QEMU setup correctly positioned before Buildx.The QEMU setup step with correct platform specification (
linux/amd64,linux/arm64) is properly ordered before the Buildx setup, enabling multi-platform image builds on the GitHub Actions runner.Dockerfile.development (1)
2-2: Consistent removal of platform directives mirrors production Dockerfile.The removal of
--platform=${BUILDPLATFORM}from both base images aligns with the production Dockerfile changes and supports the multi-platform build strategy.Also applies to: 17-17
.github/workflows/ci.yaml (2)
245-248: QEMU setup correct; separate arch builds enable validation.QEMU setup with proper platform specification precedes Buildx, and separate build steps for amd64 and arm64 with explicit tags enable downstream architecture validation.
302-307: Clarify: Scan step uses untagged image reference.The scan step at line 305 references
openzeppelin-monitor-dev:${{ github.sha }}without an architecture tag (compare to the explicit-amd64and-arm64tags from the build steps). Verify this is intentional (e.g., if the tag without suffix resolves to amd64 by default) or if the step should specify which architecture image to scan.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #382 +/- ##
=====================================
Coverage 96.5% 96.5%
=====================================
Files 76 76
Lines 27116 27116
=====================================
Hits 26177 26177
Misses 939 939
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
05d0663 to
b50f786
Compare
|
I'd like to keep the pinned hash and add |
095746e to
a7c914f
Compare
Summary
Fixes #380
Summary by CodeRabbit
New Features
Chores