-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add Dockerfile and compose file for web platform build. #2558
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds containerization and CI/CD: a multi-stage Dockerfile to build a web renderer and serve via Nginx, a docker-compose.yml to run the app locally, and a GitHub Actions workflow to build and push the Docker image to Docker Hub on pushes/PRs to main. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev
participant GitHub
participant Actions as GitHub Actions
participant DockerHub
Dev->>GitHub: Push/PR to main
GitHub-->>Actions: Trigger workflow
Actions->>Actions: Checkout repository
Actions->>DockerHub: Login with secrets
Actions->>DockerHub: Build & push image (highkay/chatbox-web:latest)
sequenceDiagram
participant User
participant DockerCompose
participant Container
participant Nginx
User->>DockerCompose: docker-compose up
DockerCompose->>Container: Build image from Dockerfile
Container->>Nginx: Start nginx serving /usr/share/nginx/html
User->>Nginx: HTTP GET /
Nginx-->>User: Serve built static assets
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
本次提交对 Dockerfile 进行了重大重构,旨在优化前端应用的容器化部署。此前的镜像是基于 Node.js 运行后端服务并同时使用 `serve` 提供前端文件,而新版则转向使用 Nginx 专门托管静态前端资源。 主要变更如下: - **切换生产镜像**:最终镜像由 `node:20-slim` 更改为 `nginx:alpine`,大幅减小了镜像体积。 - **优化构建流程**:采用更清晰的多阶段构建,分离了依赖安装、代码构建和最终生产环境。 - **简化部署**:移除了在容器内运行 Node.js 后端的逻辑,容器现在只负责提供静态文件。 BREAKING CHANGE: Docker 容器不再包含或运行 Node.js 后端。它现在仅通过 Nginx 在端口 80 上提供前端静态资源,取代了之前在端口 3000 上运行的 Node 服务。
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 4
🧹 Nitpick comments (7)
.github/workflows/docker-publish.yml (3)
12-24
: Set up Buildx and enable build cache to speed up CI.Improve performance by enabling Buildx and GitHub Actions cache for layers.
Apply:
steps: - name: Checkout code uses: actions/checkout@v4 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + - name: Login to Docker Hub if: github.event_name == 'push' && github.ref == 'refs/heads/main' uses: docker/login-action@v3 with: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} - name: Build Docker image (no push on PRs) uses: docker/build-push-action@v5 with: context: . push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + cache-from: type=gha + cache-to: type=gha,mode=max tags: | highkay/chatbox-web:latest highkay/chatbox-web:sha-${{ github.sha }}
1-7
: Consider publishing on tags and adding labels/metadata.For cleaner release hygiene, trigger pushes on version tags (e.g., v*) and add OCI labels.
If desired, I can provide a follow-up patch to:
- Add on: push: tags: [ 'v*' ] and tag images with both latest and the version.
- Use docker/metadata-action to generate labels and tags.
Would you like me to prepare that?
27-27
: Add a trailing newline at EOF.YAML lint flagged missing newline. Add a newline at the end of the file.
- tags: highkay/chatbox-web:latest + tags: highkay/chatbox-web:latest +Dockerfile (3)
13-15
: Set NODE_ENV for optimized builds and minimize build context.
- Most bundlers honor NODE_ENV=production for smaller output.
- Ensure .dockerignore excludes node_modules, .git, caches, etc., for faster context upload.
Apply:
FROM dependencies AS build COPY . . -RUN npm run build:web +ENV NODE_ENV=production +RUN npm run build:webAdditionally, add a .dockerignore at repo root:
node_modules .git .gitignore dist build .release .cache npm-debug.log* Dockerfile docker-compose.yml .github
19-21
: Validate build output path; consider serving a fallback for SPA routing.
- Path
/app/release/app/dist/renderer
looks project-specific. Ifbuild:web
outputs elsewhere, this COPY will fail.- If this is an SPA, consider providing an nginx.conf with try_files to index.html.
I can:
- Adjust the COPY path once you confirm the actual output directory of
npm run build:web
.- Provide a minimal nginx.conf that supports SPA routing if needed. Want me to include it?
18-21
: Optional hardening: drop privileges or pin base image digests.
- Running nginx as root is default; consider a non-root config or pin image digests for supply-chain integrity.
I can provide a hardened variant (user, port >1024, readonly FS) if desired.
docker-compose.yml (1)
1-1
: Compose v2 ignores the version field; safe to remove.Not required and sometimes flagged by linters.
-version: '3.8' +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/docker-publish.yml
(1 hunks)Dockerfile
(1 hunks)docker-compose.yml
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/docker-publish.yml
[error] 27-27: no new line character at the end of file
(new-line-at-end-of-file)
- name: Login to Docker Hub | ||
uses: docker/login-action@v3 | ||
with: | ||
username: ${{ secrets.DOCKERHUB_USERNAME }} | ||
password: ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
||
- name: Build and push Docker image | ||
uses: docker/build-push-action@v5 | ||
with: | ||
context: . | ||
push: true | ||
tags: highkay/chatbox-web:latest |
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.
🛠️ Refactor suggestion
Do not push images or require secrets on PRs; gate login/push to main pushes only.
Right now:
- Secrets are referenced on PRs, which won’t be available for forks and will fail the job.
- The build step always pushes to Docker Hub, including for PRs.
Gate Docker Hub login and pushing to main branch pushes only, and build PRs without pushing.
Apply:
- name: Login to Docker Hub
+ if: github.event_name == 'push' && github.ref == 'refs/heads/main'
uses: docker/login-action@v3
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
- - name: Build and push Docker image
+ - name: Build Docker image (no push on PRs)
uses: docker/build-push-action@v5
with:
context: .
- push: true
- tags: highkay/chatbox-web:latest
+ push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
+ tags: |
+ highkay/chatbox-web:latest
+ highkay/chatbox-web:sha-${{ github.sha }}
📝 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.
- name: Login to Docker Hub | |
uses: docker/login-action@v3 | |
with: | |
username: ${{ secrets.DOCKERHUB_USERNAME }} | |
password: ${{ secrets.DOCKERHUB_TOKEN }} | |
- name: Build and push Docker image | |
uses: docker/build-push-action@v5 | |
with: | |
context: . | |
push: true | |
tags: highkay/chatbox-web:latest | |
- name: Login to Docker Hub | |
if: github.event_name == 'push' && github.ref == 'refs/heads/main' | |
uses: docker/login-action@v3 | |
with: | |
username: ${{ secrets.DOCKERHUB_USERNAME }} | |
password: ${{ secrets.DOCKERHUB_TOKEN }} | |
- name: Build Docker image (no push on PRs) | |
uses: docker/build-push-action@v5 | |
with: | |
context: . | |
push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} | |
tags: | | |
highkay/chatbox-web:latest | |
highkay/chatbox-web:sha-${{ github.sha }} | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 27-27: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In .github/workflows/docker-publish.yml around lines 16–27, the workflow logs
into Docker Hub and pushes images unconditionally which references secrets on
PRs and pushes from forks; change the workflow so the docker/login-action step
and the build-and-push step that pushes are gated to only run on main branch
pushes (e.g. add an if: condition like github.ref == 'refs/heads/main' or
github.event_name == 'push' && github.ref == 'refs/heads/main' to those steps),
and keep a separate build-only step that runs for PRs without using secrets or
pushing (set push: false or run docker build without login). Ensure no step
references DOCKERHUB_USERNAME/DOCKERHUB_TOKEN unless behind the main-branch-only
if condition so secrets are never required on PRs.
build: . | ||
ports: | ||
- "3000:3000" | ||
volumes: | ||
- ./data:/root/.config/xyz.chatboxapp.ce |
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.
🛠️ Refactor suggestion
Align compose with published image and remove unused volume.
- The nginx-based image won’t use
/root/.config/xyz.chatboxapp.ce
. This looks unrelated to a static web server. - Since CI pushes highkay/chatbox-web:latest, you can use that image directly in compose.
Apply:
chatbox:
- build: .
+ image: highkay/chatbox-web:latest
ports:
- "3000:80"
- volumes:
- - ./data:/root/.config/xyz.chatboxapp.ce
+ restart: unless-stopped
📝 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.
build: . | |
ports: | |
- "3000:3000" | |
volumes: | |
- ./data:/root/.config/xyz.chatboxapp.ce | |
chatbox: | |
image: highkay/chatbox-web:latest | |
ports: | |
- "3000:80" | |
restart: unless-stopped |
🤖 Prompt for AI Agents
In docker-compose.yml around lines 5 to 9, the service is building locally and
mounting a volume ./data to /root/.config/xyz.chatboxapp.ce which is unused by
the nginx-based image; replace the build directive with image:
highkay/chatbox-web:latest and remove the volumes entry so the compose file uses
the published image and no longer mounts the irrelevant ./data volume.
ports: | ||
- "3000:3000" |
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.
Port mismatch: container serves on 80, but compose maps to 3000:3000.
The Dockerfile exposes 80 (nginx). The current mapping publishes container port 3000 which won’t receive traffic.
Apply:
- ports:
- - "3000:3000"
+ ports:
+ - "3000:80"
📝 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.
ports: | |
- "3000:3000" | |
ports: | |
- "3000:80" |
🤖 Prompt for AI Agents
In docker-compose.yml around lines 6 to 7, the compose mapping uses "3000:3000"
but the container (nginx) listens on port 80; update the ports mapping so the
host port maps to container port 80 (for example change to "3000:80" if you want
host 3000 to reach container 80, or "80:80" if you prefer host 80), and ensure
the Dockerfile still exposes port 80.
RUN npm config set registry https://registry.npmmirror.com/ && \ | ||
npm install --ignore-scripts && \ | ||
npm cache clean --force |
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.
💡 Verification agent
🧩 Analysis chain
Prefer reproducible installs and configurable registry; avoid hard-coding a third-party mirror.
- Hard-coding npmmirror.com is a supply chain and availability risk.
- Use npm ci for reproducible builds when a lockfile exists.
- Keep the default registry unless overridden via build-arg.
Apply:
-FROM base AS dependencies
-RUN npm config set registry https://registry.npmmirror.com/ && \
- npm install --ignore-scripts && \
- npm cache clean --force
+FROM base AS dependencies
+# Allow overriding registry at build-time: docker build --build-arg NPM_REGISTRY=https://registry.npmmirror.com/ .
+ARG NPM_REGISTRY=https://registry.npmjs.org/
+RUN npm config set registry ${NPM_REGISTRY}
+# Use npm ci when lockfile exists; fallback to install otherwise
+RUN if [ -f package-lock.json ]; then \
+ npm ci --ignore-scripts; \
+ else \
+ npm install --ignore-scripts; \
+ fi && \
+ npm cache clean --force
Please confirm that build does not rely on postinstall scripts (ignore-scripts is enabled). If it does, we should drop the flag.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Locate Dockerfile(s) ==="
find . -type f -name Dockerfile -print -exec printf "\n--- %s ---\n\n" {} \; -exec sed -n '1,160p' {} \; || true
echo -e "\n=== Check for lockfiles at repo root ==="
for f in package-lock.json npm-shrinkwrap.json yarn.lock pnpm-lock.yaml; do
if [ -f "$f" ]; then
echo "FOUND: $f"
ls -l "$f"
fi
done
echo -e "\n=== Top-level package.json scripts (if exists) ==="
if [ -f package.json ]; then
if command -v jq >/dev/null 2>&1; then
jq '.scripts // {}' package.json || true
else
echo "(jq not available) showing package.json scripts via grep:"
sed -n '1,240p' package.json | sed -n '/"scripts"/,/"dependencies"/p' || true
fi
else
echo "No top-level package.json found."
fi
echo -e "\n=== Search repository for any 'postinstall' scripts/mentions ==="
rg -n --hidden --no-ignore-vcs 'postinstall' || true
echo -e "\n=== Search for other package.json files (workspaces/monorepo) and their scripts (limited listing) ==="
rg -n --hidden --no-ignore-vcs '"scripts"\s*:\s*\{' --glob '!**/node_modules/**' || true
find . -name package.json -not -path "./node_modules/*" -print -exec sh -c 'printf "\n-- %s --\n" "$1"; if command -v jq >/dev/null 2>&1; then jq ".name, .scripts // {}" "$1"; else sed -n "1,240p" "$1" | rg -n "\"scripts\"|\"postinstall\"|\"name\"" || true; fi' sh {} \;
Length of output: 7390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Root Dockerfile ==="
if [ -f Dockerfile ]; then
nl -ba Dockerfile | sed -n '1,240p'
else
echo "No root Dockerfile"
fi
echo -e "\n=== team-sharing/Dockerfile ==="
if [ -f team-sharing/Dockerfile ]; then
nl -ba team-sharing/Dockerfile | sed -n '1,240p'
else
echo "No team-sharing/Dockerfile"
fi
echo -e "\n=== Search for hard-coded registries / npmmirror mentions ==="
rg -n --hidden --no-ignore-vcs 'npmmirror|registry.npmmirror.com|npm config set registry' --glob '!**/node_modules/**' || true
echo -e "\n=== Search for npm install / npm ci usages and --ignore-scripts occurrences ==="
rg -n --hidden --no-ignore-vcs 'npm (install|ci).*--ignore-scripts|npm ci|npm install' --glob '!**/node_modules/**' || true
echo -e "\n=== Show package.json postinstall scripts (all package.json files) ==="
find . -name package.json -not -path "./node_modules/*" -print -exec sh -c 'echo; echo "-- $1 --"; sed -n "1,240p" "$1" | sed -n "/\"postinstall\"/,/},/p"' sh {} \; || true
Length of output: 229
Fix required: don't hard-code a third‑party npm mirror; use npm ci and do not ignore postinstall scripts
Verified: repo contains package-lock.json and package.json defines important postinstall steps (root package.json and release/app/package.json; webpack config also invokes npm run postinstall). The Dockerfile snippet in the review hard‑codes registry to registry.npmmirror.com and uses --ignore-scripts, which will skip required postinstall hooks and likely break the build.
Files needing attention:
- Dockerfile (repo root) — lines ~8–10 (replace hard-coded registry and remove --ignore-scripts)
- team-sharing/Dockerfile — check for same pattern
- package.json (root) and release/app/package.json — contain postinstall scripts (confirm behaviour)
Apply this patch (example):
-FROM base AS dependencies
-RUN npm config set registry https://registry.npmmirror.com/ && \
- npm install --ignore-scripts && \
- npm cache clean --force
+FROM base AS dependencies
+# Allow overriding registry at build-time: docker build --build-arg NPM_REGISTRY=...
+ARG NPM_REGISTRY=https://registry.npmjs.org/
+RUN npm config set registry ${NPM_REGISTRY}
+# Use npm ci when a lockfile exists for reproducible installs; DO NOT skip lifecycle scripts
+RUN if [ -f package-lock.json ]; then \
+ npm ci; \
+ else \
+ npm install; \
+ fi && \
+ npm cache clean --force
Action required: remove --ignore-scripts (build relies on postinstall). Also audit other Dockerfiles (team-sharing) and CI/build pipelines for hard-coded registries and update to use a build-arg or the default registry.
📝 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.
RUN npm config set registry https://registry.npmmirror.com/ && \ | |
npm install --ignore-scripts && \ | |
npm cache clean --force | |
# Allow overriding registry at build-time: docker build --build-arg NPM_REGISTRY=... | |
ARG NPM_REGISTRY=https://registry.npmjs.org/ | |
RUN npm config set registry ${NPM_REGISTRY} | |
# Use npm ci when a lockfile exists for reproducible installs; DO NOT skip lifecycle scripts | |
RUN if [ -f package-lock.json ]; then \ | |
npm ci; \ | |
else \ | |
npm install; \ | |
fi && \ | |
npm cache clean --force |
🤖 Prompt for AI Agents
In Dockerfile around lines 8 to 10, the build currently hard-codes
registry.npmmirror.com and uses "npm install --ignore-scripts", which skips
required postinstall hooks; replace this with "npm ci" (to use package-lock.json
and produce reproducible installs), remove the --ignore-scripts flag so
postinstall scripts run, and stop hard-coding the registry by accepting a build
ARG (defaulting to the public npm registry) or using the default registry; also
search team-sharing/Dockerfile and any CI/build pipeline steps for the same
pattern and apply the same changes there so postinstall hooks in root
package.json and release/app/package.json execute correctly.
Description
Add Dockerfile and compose file for web platform build.
Additional Notes
Screenshots
Contributor Agreement
By submitting this Pull Request, I confirm that I have read and agree to the following terms:
Please check the box below to confirm:
[ x ] I have read and agree with the above statement.
Summary by CodeRabbit
New Features
Chores