feat(docker): update Dockerfile for Alpine base and add healthcheck s…#2
feat(docker): update Dockerfile for Alpine base and add healthcheck s…#2chakrihacker wants to merge 2 commits into
Conversation
Reviewer's GuideThis PR migrates the Docker build to an Alpine-based Bun image, streamlines dependency installation with a frozen lockfile, adds a healthcheck script with corresponding Docker HEALTHCHECK, and refines runtime configuration for nonroot execution and proper file copying. Flow diagram for Docker healthcheck processflowchart TD
Start([Docker HEALTHCHECK]) --> RunBun[/Run: bun healthcheck.js/]
RunBun --> CheckHealth[Fetch http://localhost:3000/health]
CheckHealth -->|HTTP 200 OK| Healthy[Exit 0]
CheckHealth -->|HTTP error or network error| Unhealthy[Exit 1]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe Dockerfile was updated to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @chakrihacker - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `Dockerfile:44` </location>
<code_context>
-CMD ["./server"]
+# Health check
+HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
+ CMD ["/usr/local/bin/bun", "/app/healthcheck.js"]
+
+EXPOSE 3000
</code_context>
<issue_to_address>
The healthcheck command assumes Bun is available in the runtime image.
The distroless/base image lacks Bun, so the healthcheck will fail unless Bun is added. Consider using a runtime image with Bun or a different healthcheck method.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ | ||
| CMD ["/usr/local/bin/bun", "/app/healthcheck.js"] |
There was a problem hiding this comment.
issue (bug_risk): The healthcheck command assumes Bun is available in the runtime image.
The distroless/base image lacks Bun, so the healthcheck will fail unless Bun is added. Consider using a runtime image with Bun or a different healthcheck method.
There was a problem hiding this comment.
do you have any suggestions, for healthcheck?
There was a problem hiding this comment.
Ask❓
do you have any suggestions, for healthcheck?
Answer:
Yes, I have several suggestions for the healthcheck given that the distroless/base image doesn't include Bun:
Option 1: Use curl/wget (requires adding them to the image)
# In the runtime stage, you'd need to use a different base image that includes curl
FROM gcr.io/distroless/base-debian11
# Or add curl to your build and copy it over
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD ["curl", "-f", "http://localhost:3000/health"]Option 2: Use a simple shell script with built-in tools
# Create a simple healthcheck script that uses basic tools
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD ["sh", "-c", "wget --no-verbose --tries=1 --spider http://localhost:3000/health || exit 1"]Option 3: Switch to a runtime image that includes Node.js/Bun
FROM oven/bun:alpine AS runtime
# This way your healthcheck.js will work as intended
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD ["bun", "/app/healthcheck.js"]Option 4: Copy Bun binary from build stage
COPY --from=build /usr/local/bin/bun /usr/local/bin/bun
# Then your current healthcheck would workRecommended approach: Option 3 (switch to bun:alpine runtime) since you're already using Bun and it maintains consistency. The distroless approach is great for security but creates this compatibility issue with your healthcheck script.
There was a problem hiding this comment.
Great question! Since the final runtime image (gcr.io/distroless/base) does not include Bun, the current healthcheck (/usr/local/bin/bun /app/healthcheck.js) will fail because Bun isn't available.
Here are a few suggestions for your healthcheck:
-
Use a simpler healthcheck with curl or wget:
- If you only need to check if your app is responding, you can use
curlorwgetin the healthcheck. However, the distroless image also doesn't include these tools by default. You'd need to switch to a minimal Alpine or Debian-based image that includes them, or add them in a custom image.
- If you only need to check if your app is responding, you can use
-
Custom minimal healthcheck binary:
- You could write a tiny Go or Rust binary that performs the healthcheck and copy it into the image. This is a common approach for distroless images.
-
Switch to a Bun-based runtime image:
- If you want to keep using a JS healthcheck, use a Bun-based image (like
oven/bun:alpine) for your runtime instead of distroless. This way, Bun will be available for the healthcheck script.
- If you want to keep using a JS healthcheck, use a Bun-based image (like
-
Remove the healthcheck or use Docker's basic TCP check:
- You can use Docker's built-in TCP healthcheck:
But again,
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ CMD nc -z localhost 3000 || exit 1
nc(netcat) is not available in distroless, so you'd need a base image that includes it.
- You can use Docker's built-in TCP healthcheck:
Summary:
- If you want to keep the image as small and secure as possible, consider a custom minimal binary for healthchecks.
- If you want a quick solution, switch to an Alpine-based runtime image and use
curlor keep Bun for the healthcheck.
Let me know which direction you'd like to take, and I can provide a more detailed example!
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Dockerfile(2 hunks)healthcheck.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
healthcheck.js (1)
src/index.ts (1)
uptime(43-49)
🔇 Additional comments (8)
healthcheck.js (1)
1-2: LGTM!Clear documentation and appropriate endpoint URL for health checking.
Dockerfile (7)
1-1: LGTM!Alpine base image reduces the final image size while maintaining functionality.
6-6: LGTM!Combining package file copying in a single command is more efficient and follows Docker best practices.
9-9: LGTM!The
--frozen-lockfileflag ensures reproducible builds by preventing automatic lockfile updates during installation.
14-15: LGTM!File copying optimizations look good. The trailing
./and healthcheck.js addition are appropriate.
36-36: LGTM!Adding the healthcheck script to the final image enables container health monitoring.
39-39: LGTM!Setting explicit non-root user improves container security by following the principle of least privilege.
47-49: LGTM!Moving EXPOSE before CMD and maintaining the existing command structure is appropriate.
| try { | ||
| const res = await fetch(url, { timeout: 2000 }); | ||
| if (res.ok) { | ||
| process.exit(0); // Healthy | ||
| } else { | ||
| process.exit(1); // HTTP error | ||
| } | ||
| } catch (err) { | ||
| process.exit(1); // Network error or timeout | ||
| } |
There was a problem hiding this comment.
Fix timeout implementation and consider runtime compatibility.
The current implementation has two issues:
- The
timeoutoption infetch()is not a standard Web API feature and may not work as expected in all runtimes. - Top-level await requires the script to be treated as an ES module or run in a compatible environment.
Apply this diff to fix the timeout implementation:
-// healthcheck.js
-const url = "http://localhost:3000/health";
-
-try {
- const res = await fetch(url, { timeout: 2000 });
- if (res.ok) {
- process.exit(0); // Healthy
- } else {
- process.exit(1); // HTTP error
- }
-} catch (err) {
- process.exit(1); // Network error or timeout
-}
+// healthcheck.js
+const url = "http://localhost:3000/health";
+
+(async () => {
+ try {
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 2000);
+
+ const res = await fetch(url, { signal: controller.signal });
+ clearTimeout(timeoutId);
+
+ if (res.ok) {
+ process.exit(0); // Healthy
+ } else {
+ process.exit(1); // HTTP error
+ }
+ } catch (err) {
+ process.exit(1); // Network error or timeout
+ }
+})();📝 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.
| try { | |
| const res = await fetch(url, { timeout: 2000 }); | |
| if (res.ok) { | |
| process.exit(0); // Healthy | |
| } else { | |
| process.exit(1); // HTTP error | |
| } | |
| } catch (err) { | |
| process.exit(1); // Network error or timeout | |
| } | |
| // healthcheck.js | |
| const url = "http://localhost:3000/health"; | |
| (async () => { | |
| try { | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => controller.abort(), 2000); | |
| const res = await fetch(url, { signal: controller.signal }); | |
| clearTimeout(timeoutId); | |
| if (res.ok) { | |
| process.exit(0); // Healthy | |
| } else { | |
| process.exit(1); // HTTP error | |
| } | |
| } catch (err) { | |
| process.exit(1); // Network error or timeout | |
| } | |
| })(); |
🤖 Prompt for AI Agents
In healthcheck.js around lines 4 to 13, the current fetch call uses a
non-standard timeout option and top-level await which may cause compatibility
issues. Replace the timeout option by implementing an AbortController to handle
the timeout manually. Wrap the fetch call inside an async function and invoke it
to avoid top-level await usage. This ensures proper timeout handling and
compatibility across different runtimes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
40-41: Comment says “Ensure the binary is executable” but no chmod is performedIf
bun build --compileever changes its default permissions, the image will
quietly break. Either delete the misleading comment or add an explicit
RUN chmod +x /app/serverbefore switching to the non-root user.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Dockerfile(2 hunks)
🔇 Additional comments (2)
Dockerfile (2)
6-9: Lockfile name may be wrong for BunThe canonical Bun lockfile is
bun.lockb, notbun.lock. If your repository
uses the binary format but the copy statement misses it, the build layer cache
will be invalidated unnecessarily andbun install --frozen-lockfilecould
exit with a non-zero status.-COPY package.json bun.lock ./ +COPY package.json bun.lockb ./Please verify which lockfile is committed.
45-47: Good move to switch the healthcheck tocurland align the timeoutOnce the curl binary compatibility issue above is fixed, the HEALTHCHECK
parameters look sane and the 5 s timeout matches the scriptless check.
| FROM alpine AS curl-downloader | ||
| RUN apk add --no-cache curl | ||
|
|
||
| FROM gcr.io/distroless/base | ||
|
|
||
| WORKDIR /app | ||
|
|
||
| # Copy the compiled binaries | ||
| COPY --from=build /app/server server | ||
| COPY --from=curl-downloader /usr/bin/curl /usr/bin/curl | ||
|
|
There was a problem hiding this comment.
Alpine-built curl is very likely to seg-fault in the Debian-based distroless image
apk add --no-cache curl produces a musl-linked binary /usr/bin/curl.
The final stage (gcr.io/distroless/base) is glibc-based and does not ship the musl
loader (/lib/ld-musl-x86_64.so.1) or any musl libraries, so the copied binary
will fail at runtime and the HEALTHCHECK will be reported as unhealthy.
Diff-style fix (one option):
-FROM alpine AS curl-downloader
-RUN apk add --no-cache curl
+FROM debian:stable-slim AS curl-downloader
+RUN apt-get update \
+ && apt-get install -y --no-install-recommends curl ca-certificates \
+ && rm -rf /var/lib/apt/lists/*or simply switch the runtime image to oven/bun:alpine and drop the extra
stage altogether.
📝 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 alpine AS curl-downloader | |
| RUN apk add --no-cache curl | |
| FROM gcr.io/distroless/base | |
| WORKDIR /app | |
| # Copy the compiled binaries | |
| COPY --from=build /app/server server | |
| COPY --from=curl-downloader /usr/bin/curl /usr/bin/curl | |
| FROM debian:stable-slim AS curl-downloader | |
| RUN apt-get update \ | |
| && apt-get install -y --no-install-recommends curl ca-certificates \ | |
| && rm -rf /var/lib/apt/lists/* | |
| FROM gcr.io/distroless/base | |
| WORKDIR /app | |
| # Copy the compiled binaries | |
| COPY --from=build /app/server server | |
| COPY --from=curl-downloader /usr/bin/curl /usr/bin/curl |
🤖 Prompt for AI Agents
In Dockerfile lines 29 to 39, the curl binary built with Alpine's musl libc is
copied into a glibc-based distroless image, causing runtime segfaults due to
missing musl libraries. To fix this, either build curl in a Debian-based image
compatible with the distroless base or switch the final runtime image to an
Alpine-based image like oven/bun:alpine to ensure libc compatibility and remove
the need for the extra curl-downloader stage.
1 participant
…cript
Summary by Sourcery
Update Dockerfile to use Alpine-based Bun image, enforce reproducible installs, add and configure a healthcheck script, switch to non-root user, and expose port 3000.
New Features:
Enhancements:
Summary by CodeRabbit