-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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?
Changes from all commits
51ca4b4
0aec8eb
755a362
a49a636
747e4dd
c3c52c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: Docker Image CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: [ "main" ] | ||
| pull_request: | ||
| branches: [ "main" ] | ||
|
|
||
| jobs: | ||
| build_and_push: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - 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 | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,21 @@ | ||||||||||||||||||||||||||||
| # ---- Base ---- | ||||||||||||||||||||||||||||
| FROM node:22-alpine AS base | ||||||||||||||||||||||||||||
| WORKDIR /app | ||||||||||||||||||||||||||||
| COPY package.json package-lock.json* ./ | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # ---- Dependencies ---- | ||||||||||||||||||||||||||||
| FROM base AS dependencies | ||||||||||||||||||||||||||||
| RUN npm config set registry https://registry.npmmirror.com/ && \ | ||||||||||||||||||||||||||||
| npm install --ignore-scripts && \ | ||||||||||||||||||||||||||||
| npm cache clean --force | ||||||||||||||||||||||||||||
|
Comment on lines
+8
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainPrefer reproducible installs and configurable registry; avoid hard-coding a third-party mirror.
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 --forcePlease 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 {} \; || trueLength 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:
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 --forceAction 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # ---- Build ---- | ||||||||||||||||||||||||||||
| FROM dependencies AS build | ||||||||||||||||||||||||||||
| COPY . . | ||||||||||||||||||||||||||||
| RUN npm run build:web | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # ---- Production ---- | ||||||||||||||||||||||||||||
| FROM nginx:alpine AS production | ||||||||||||||||||||||||||||
| COPY --from=build /app/release/app/dist/renderer /usr/share/nginx/html | ||||||||||||||||||||||||||||
| EXPOSE 80 | ||||||||||||||||||||||||||||
| CMD ["nginx", "-g", "daemon off;"] | ||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||||||||||||||||
| version: '3.8' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| services: | ||||||||||||||||||||||
| chatbox: | ||||||||||||||||||||||
| build: . | ||||||||||||||||||||||
| ports: | ||||||||||||||||||||||
| - "3000:3000" | ||||||||||||||||||||||
|
Comment on lines
+6
to
+7
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| volumes: | ||||||||||||||||||||||
| - ./data:/root/.config/xyz.chatboxapp.ce | ||||||||||||||||||||||
|
Comment on lines
+5
to
+9
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Align compose with published image and remove unused volume.
Apply: chatbox:
- build: .
+ image: highkay/chatbox-web:latest
ports:
- "3000:80"
- volumes:
- - ./data:/root/.config/xyz.chatboxapp.ce
+ restart: unless-stopped📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
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:
Gate Docker Hub login and pushing to main branch pushes only, and build PRs without pushing.
Apply:
📝 Committable suggestion
🧰 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