chore: docker fixes #2
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
WalkthroughThis update introduces advanced Docker and cross-compilation support for a Rust project. It adds new Dockerfiles for cross-architecture builds, a Cross.toml configuration, and a comprehensive .dockerignore. The main Dockerfile is refactored for multi-stage builds and improved caching. GitHub Actions and Cargo profiles are updated to support these enhancements and optimize build performance. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Docker Build
participant Cross Dockerfile
participant Cargo
Developer->>GitHub Actions: Push/PR triggers workflow
GitHub Actions->>Docker Build: Start multi-stage build
Docker Build->>Cross Dockerfile: (If cross-compiling) Use appropriate cross image
Docker Build->>Cargo: Build dependencies & application (with profile.docker)
Docker Build->>Docker Build: Copy binary to runtime image
Docker Build->>GitHub Actions: Publish built image
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
Dockerfile.cross-x86_64 (2)
4-10: Shrink image size & make apt non-interactiveMinor but useful tweaks:
ARG DEBIAN_FRONTEND=noninteractiveavoids tzdata/locale prompts on some mirrors.--no-install-recommendsplus a singleapt-get cleanfurther trims layers.+ARG DEBIAN_FRONTEND=noninteractive RUN apt-get update && \ - apt-get install -y \ + apt-get install -y --no-install-recommends \ build-essential \ @@ - rm -rf /var/lib/apt/lists/* + apt-get clean && rm -rf /var/lib/apt/lists/*
12-19: Inconsistent env-var naming with ARM DockerfileHere you use the target-suffix form (
CFLAGS_x86_64_unknown_linux_gnu).
In the ARM file (see lines 23-25) the plainBINDGEN_EXTRA_CLANG_ARGSis used instead. Aligning on one style prevents surprises when adding further targets or CI matrix jobs.Dockerfile.cross-aarch64 (2)
4-13: Add--no-install-recommendsand consolidate cleanupSame optimisation comment as for the x86_64 Dockerfile – eliminates ~100 MB per layer on Debian/Ubuntu bases.
23-25: Variable naming divergence
BINDGEN_EXTRA_CLANG_ARGS(no target suffix) differs from the x86_64 file’s target-specific form.
Prefer the suffixed variant for consistency or leave a code comment explaining the difference.Cross.toml (1)
10-14: Consider passingRUSTFLAGSfor determinismIf you rely on reproducible builds (especially for release images) it’s handy to add:
[build.env] passthrough = ["RUST_LOG", "CARGO_TERM_COLOR", "RUSTFLAGS"]and set
RUSTFLAGS="-C target-cpu=native"(or similar) in CI.
Optional, but avoids different codegen on local vs CI..github/workflows/docker.yml (1)
55-65: Consider moving the environment exports into the MakefileThe long list of
export …statements clutters the workflow and couples CI logic to build logic. Encapsulating them inside thedocker-build-pushtarget (or a dedicated.envfile) keeps the workflow cleaner and avoids duplication across local/CI builds.Dockerfile (2)
57-60: Runtime stage installs dev packages – unnecessarily bloats the final image
libssl-dev,pkg-config, andstraceare build/debug utilities not needed at runtime. They add ~60 MB.- apt-get install -y ca-certificates libssl-dev pkg-config strace && \ + apt-get install -y ca-certificates libssl3 && \If you really need
stracefor debugging, consider keeping two tags (e.g.:debug,:slim) instead of shipping it to production.
61-64: Run the binary as non-root for better container securityThe current runtime image leaves the process running as
root, which increases the blast radius of any RCE. Add an unprivileged user:+RUN useradd -m --uid 10001 lumen && \ + mkdir -p /app && chown -R lumen:lumen /app /usr/local/bin/lumen USER lumenand keep the workdir/entrypoint unchanged.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.dockerignore(1 hunks).github/workflows/docker.yml(3 hunks)Cargo.toml(1 hunks)Cross.toml(1 hunks)Dockerfile(1 hunks)Dockerfile.cross-aarch64(1 hunks)Dockerfile.cross-x86_64(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and push
🔇 Additional comments (4)
Cross.toml (1)
4-9: Guard against.dockerignoreexclusionsThis config points to two Dockerfiles that are currently ignored by
.dockerignore(see earlier comment). Ensure the ignore pattern is fixed or move these Dockerfiles outside the build context; otherwisecrosswill bail out before compiling.Cargo.toml (1)
205-216:codegen-units = 1slows compile; confirm trade-offSetting
codegen-units = 1for bothreleaseand the newdockerprofile improves runtime performance but can add several minutes to compile time in CI, especially with the large dependency tree here.
If wall-clock build speed is more critical than the last 1-2 % runtime gain, consider leaving it at the default (16) for thedockerprofile.No change required—just double-check the constraint.
.github/workflows/docker.yml (1)
30-41: ```shell
#!/bin/bash
set -eecho "Searching for Makefile(s):"
find . -type f -iname 'Makefile*' -not -path '/node_modules/' -not -path '/dist/' -not -path '/build/'echo -e "\nSearching for 'docker-build-push' occurrences:"
rg -n 'docker-build-push' .echo -e "\nShowing content around 'docker-build-push' in matching files:"
files=$(rg -l 'docker-build-push' . || true)
if [ -n "$files" ]; then
for f in $files; do
echo -e "\n--- $f ---"
sed -n '1,200p' "$f"
done
fiecho -e "\nSearching for CC_aarch64_unknown_linux_gnu references in the repo:"
rg -n 'CC_aarch64_unknown_linux_gnu' .</details> <details> <summary>Dockerfile (1)</summary> `49-54`: **Copying the binary assumes host → target path that may not exist** `cargo build --profile $BUILD_PROFILE` places the artifact under `/app/target/$TARGET/$BUILD_PROFILE/` when `--target` is set (as it is when you cross-compile). Using `/app/target/$BUILD_PROFILE/lumen` will fail in those cases. Guard against this by computing the actual location: ```sh RUN BIN_PATH=$(cargo metadata --format-version 1 --no-deps | jq -r '.target_directory') && \ cp "$BIN_PATH/$BUILD_PROFILE/lumen" /lumen
| # Docker | ||
| .dockerignore | ||
| Dockerfile* | ||
| docker-compose*.yml | ||
|
|
There was a problem hiding this comment.
Excluding Dockerfile* breaks cross & multi-stage builds
Cross.toml relies on Dockerfile.cross-x86_64 and Dockerfile.cross-aarch64, both of which live inside the build context.
Because .dockerignore filters them out, a cross build (or a plain docker build -f Dockerfile.cross-… .) will fail with:
Cannot locate specified Dockerfile: Dockerfile.cross-x86_64
Remove the blanket ignore or add negative globs for the files you actually need:
# Docker
-.dockerignore
-Dockerfile*
-docker-compose*.yml
+# Keep cross-compilation Dockerfiles
+!Dockerfile.cross-*
+# Still ignore any other ad-hoc compose files
+.dockerignore
+docker-compose*.yml📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Docker | |
| .dockerignore | |
| Dockerfile* | |
| docker-compose*.yml | |
| # Docker | |
| # Keep cross-compilation Dockerfiles | |
| !Dockerfile.cross-* | |
| # Still ignore any other ad-hoc compose files | |
| .dockerignore | |
| docker-compose*.yml |
🤖 Prompt for AI Agents
In the .dockerignore file around lines 41 to 45, the pattern "Dockerfile*"
excludes all Dockerfiles including those needed for cross and multi-stage builds
like Dockerfile.cross-x86_64 and Dockerfile.cross-aarch64. To fix this, remove
the blanket ignore for "Dockerfile*" or add negative patterns to explicitly
include the required Dockerfiles so they are not filtered out during the build
context preparation.
| export LIBCLANG_PATH=/usr/lib/llvm-18/lib | ||
| export BINDGEN_EXTRA_CLANG_ARGS="-I/usr/include" | ||
| export CC_x86_64_unknown_linux_gnu=x86_64-linux-gnu-gcc | ||
| export CXX_x86_64_unknown_linux_gnu=x86_64-linux-gnu-g++ | ||
| export AR_x86_64_unknown_linux_gnu=x86_64-linux-gnu-ar | ||
| export CC_aarch64_unknown_linux_gnu=aarch64-linux-gnu-gcc | ||
| export CXX_aarch64_unknown_linux_gnu=aarch64-linux-gnu-g++ | ||
| export AR_aarch64_unknown_linux_gnu=aarch64-linux-gnu-ar | ||
| export PKG_CONFIG_ALLOW_CROSS=1 | ||
| make docker-build-push |
There was a problem hiding this comment.
Hard-coded LIBCLANG_PATH likely wrong on Ubuntu 24.04
You install the un-versioned libclang-dev, but export /usr/lib/llvm-18/lib which exists only when the versioned package libclang-18-dev is installed.
If the default LLVM on the runner drifts (e.g. to 19) this path breaks.
Prefer a version-agnostic approach:
-export LIBCLANG_PATH=/usr/lib/llvm-18/lib
+export LIBCLANG_PATH=$(llvm-config --libdir)…and drop the fixed major version from the apt-install list, or pin both sides to the same version explicitly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export LIBCLANG_PATH=/usr/lib/llvm-18/lib | |
| export BINDGEN_EXTRA_CLANG_ARGS="-I/usr/include" | |
| export CC_x86_64_unknown_linux_gnu=x86_64-linux-gnu-gcc | |
| export CXX_x86_64_unknown_linux_gnu=x86_64-linux-gnu-g++ | |
| export AR_x86_64_unknown_linux_gnu=x86_64-linux-gnu-ar | |
| export CC_aarch64_unknown_linux_gnu=aarch64-linux-gnu-gcc | |
| export CXX_aarch64_unknown_linux_gnu=aarch64-linux-gnu-g++ | |
| export AR_aarch64_unknown_linux_gnu=aarch64-linux-gnu-ar | |
| export PKG_CONFIG_ALLOW_CROSS=1 | |
| make docker-build-push | |
| export LIBCLANG_PATH=$(llvm-config --libdir) | |
| export BINDGEN_EXTRA_CLANG_ARGS="-I/usr/include" | |
| export CC_x86_64_unknown_linux_gnu=x86_64-linux-gnu-gcc | |
| export CXX_x86_64_unknown_linux_gnu=x86_64-linux-gnu-g++ | |
| export AR_x86_64_unknown_linux_gnu=x86_64-linux-gnu-ar | |
| export CC_aarch64_unknown_linux_gnu=aarch64-linux-gnu-gcc | |
| export CXX_aarch64_unknown_linux_gnu=aarch64-linux-gnu-g++ | |
| export AR_aarch64_unknown_linux_gnu=aarch64-linux-gnu-ar | |
| export PKG_CONFIG_ALLOW_CROSS=1 | |
| make docker-build-push |
🤖 Prompt for AI Agents
In .github/workflows/docker.yml around lines 56 to 65, the LIBCLANG_PATH is
hard-coded to /usr/lib/llvm-18/lib which only exists if the versioned package
libclang-18-dev is installed, causing breakage if the LLVM version changes. To
fix this, remove the fixed version from the apt install command and instead set
LIBCLANG_PATH dynamically or to a version-agnostic path that matches the
installed libclang-dev package, ensuring consistency between the installed
package and the exported path.
| FROM lukemathwalker/cargo-chef:latest-rust-1 AS chef | ||
| WORKDIR /app | ||
|
|
||
| # Build stage | ||
| FROM rust:1.81-slim AS builder | ||
| LABEL org.opencontainers.image.licenses="MIT OR Apache-2.0" | ||
|
|
||
| # Install build dependencies | ||
| RUN apt-get update && apt-get install -y \ | ||
| RUN apt-get update && \ | ||
| apt-get -y upgrade && \ | ||
| apt-get install -y \ | ||
| build-essential \ | ||
| pkg-config \ | ||
| libssl-dev \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
| clang-14 \ | ||
| libclang-14-dev \ | ||
| llvm-14-dev \ | ||
| libc6-dev \ | ||
| && ln -sf /usr/lib/llvm-14/lib/libclang.so /usr/lib/libclang.so |
There was a problem hiding this comment.
Base image & package versions are not pinned – breaks reproducible builds
lukemathwalker/cargo-chef:latest-rust-1floats; tomorrow’s build may use a newer Rust, LLVM, or Debian base.- Mixing un-versioned packages (
clang,llvm-dev) with version-locked ones (clang-14,libclang-14-dev) makes the layer inconsistent and may silently pull in duplicate tool-chains.
Recommend:
-FROM lukemathwalker/cargo-chef:latest-rust-1 AS chef
+FROM lukemathwalker/cargo-chef:0.7.8-rust-1.77.2@sha256:<digest> AS chefand drop the -14 suffixes or pin every LLVM/Clang package to the same major.
Also, apt-get -y upgrade inside containers is usually avoided—upgrading packages you have no control over inflates image size and hurts determinism.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FROM lukemathwalker/cargo-chef:latest-rust-1 AS chef | |
| WORKDIR /app | |
| # Build stage | |
| FROM rust:1.81-slim AS builder | |
| LABEL org.opencontainers.image.licenses="MIT OR Apache-2.0" | |
| # Install build dependencies | |
| RUN apt-get update && apt-get install -y \ | |
| RUN apt-get update && \ | |
| apt-get -y upgrade && \ | |
| apt-get install -y \ | |
| build-essential \ | |
| pkg-config \ | |
| libssl-dev \ | |
| && rm -rf /var/lib/apt/lists/* | |
| clang-14 \ | |
| libclang-14-dev \ | |
| llvm-14-dev \ | |
| libc6-dev \ | |
| && ln -sf /usr/lib/llvm-14/lib/libclang.so /usr/lib/libclang.so | |
| FROM lukemathwalker/cargo-chef:0.7.8-rust-1.77.2@sha256:<digest> AS chef | |
| WORKDIR /app | |
| LABEL org.opencontainers.image.licenses="MIT OR Apache-2.0" | |
| RUN apt-get update && \ | |
| apt-get -y upgrade && \ | |
| apt-get install -y \ | |
| build-essential \ | |
| pkg-config \ | |
| libssl-dev \ | |
| clang-14 \ | |
| libclang-14-dev \ | |
| llvm-14-dev \ | |
| libc6-dev \ | |
| && ln -sf /usr/lib/llvm-14/lib/libclang.so /usr/lib/libclang.so |
🤖 Prompt for AI Agents
In Dockerfile lines 1 to 16, the base image and package versions are not pinned,
causing non-reproducible builds and inconsistent LLVM/Clang package versions.
Fix this by specifying an exact version tag for the base image instead of using
'latest-rust-1'. Also, either remove the '-14' suffixes from all LLVM/Clang
packages to use default versions consistently or pin all LLVM/Clang packages to
the same major version to avoid mixing versions. Finally, remove the 'apt-get -y
upgrade' command to prevent unnecessary image size increase and maintain build
determinism.
Description
Type of Change
Related Issues
Fixes #(issue)
Checklist
Testing
Additional Notes
Summary by CodeRabbit