-
-
Notifications
You must be signed in to change notification settings - Fork 80
ci: improve docker build - POC #1183
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: develop
Are you sure you want to change the base?
Changes from all commits
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,23 @@ | ||
| # Do NOT ignore .git - needed in the image for version metadata | ||
| # .git | ||
| node_modules | ||
| **/node_modules | ||
| npm-debug.log | ||
| yarn-error.log | ||
| dist | ||
| .DS_Store | ||
| .gitignore | ||
| .vscode | ||
| .env | ||
| .env.local | ||
| logs | ||
| *.log | ||
| coverage | ||
| tests | ||
| docs | ||
| .idea | ||
| .cache | ||
| # Ignore local config backups | ||
| server/src/configs/koji_backups/** | ||
| # Keep package and source files but ignore nested node_modules | ||
| packages/*/node_modules | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,9 @@ jobs: | |
| release: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4.1.1 | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 1 | ||
|
Comment on lines
+13
to
+14
|
||
| - name: Set .gitsha | ||
| run: 'echo ${{ github.sha }} > .gitsha' | ||
| - name: Set .gitref | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,16 +5,63 @@ | |||||||||
| # - mount areas.json to /home/node/server/src/configs/areas.json | ||||||||||
| # - Also mount every other configuration file necessary into the according directory. | ||||||||||
|
|
||||||||||
| FROM node:22-alpine | ||||||||||
| FROM node:22-alpine AS builder | ||||||||||
|
|
||||||||||
| ENV NPM_CONFIG_PREFIX=/home/node/.npm-global | ||||||||||
| ENV PATH=$PATH:/home/node/.npm-global/bin | ||||||||||
|
|
||||||||||
| WORKDIR /home/node | ||||||||||
| COPY package.json . | ||||||||||
| COPY yarn.lock . | ||||||||||
| RUN apk add git | ||||||||||
| RUN npm install -g yarn | ||||||||||
|
|
||||||||||
| # Install minimal build deps | ||||||||||
| RUN apk add --no-cache git python3 make g++ | ||||||||||
|
|
||||||||||
| # Install yarn (node:22 includes corepack but ensure yarn v1 available) | ||||||||||
| RUN npm install -g yarn@1.22.19 | ||||||||||
|
|
||||||||||
| # Copy package manifests first for better layer caching | ||||||||||
| COPY package.json yarn.lock ./ | ||||||||||
| COPY packages ./packages | ||||||||||
| COPY server ./server | ||||||||||
| COPY public ./public | ||||||||||
| COPY ReactMap.js ./ReactMap.js | ||||||||||
|
Comment on lines
+23
to
+26
|
||||||||||
| COPY packages ./packages | |
| COPY server ./server | |
| COPY public ./public | |
| COPY ReactMap.js ./ReactMap.js |
Copilot
AI
Dec 31, 2025
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.
The .dockerignore file excludes 'dist' on line 7, but the runtime stage attempts to copy it from the builder stage. If the dist directory is built during the build process, this works correctly. However, if there's a pre-existing dist directory in the source, it would be excluded from the builder stage, which is the intended behavior. The comment on line 28 of the Dockerfile should clarify that dist is intentionally excluded and will be generated by 'yarn build'.
| # Copy remaining source needed for build (excluding node_modules via .dockerignore) | |
| # Copy remaining source needed for build; .dockerignore intentionally excludes node_modules and any pre-existing dist, which will be regenerated by 'yarn build' |
Copilot
AI
Dec 31, 2025
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.
Installing yarn globally in the runtime stage may be unnecessary if the application doesn't need to run any yarn commands at runtime. Since the production dependencies are already installed in the builder stage and copied over, consider removing this installation to further reduce image size and attack surface. Only keep it if runtime yarn commands are actually required.
| # Install yarn in runtime if you need it (keeps compatibility). | |
| RUN npm install -g yarn@1.22.19 |
Copilot
AI
Dec 31, 2025
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.
The yarn.lock file should be copied to the runtime stage along with package.json to maintain dependency version integrity. While it's not strictly necessary for runtime, it ensures that any operations involving yarn in the container use the exact same dependency versions as the builder stage.
| COPY --from=builder /home/node/package.json ./package.json | |
| COPY --from=builder /home/node/package.json ./package.json | |
| COPY --from=builder /home/node/yarn.lock ./yarn.lock |
Fabio1988 marked this conversation as resolved.
Show resolved
Hide resolved
Fabio1988 marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 31, 2025
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.
The .gitsha and .gitref files created by the GitHub workflow (lines 15-18 in docker.yml) are not explicitly copied to the runtime stage. These files are needed by server/src/services/checkForUpdates.js to determine the current version when running in Docker. Add these files to the runtime stage copy operations.
| COPY --from=builder /home/node/config ./config | |
| COPY --from=builder /home/node/config ./config | |
| COPY --from=builder /home/node/.gitsha ./.gitsha | |
| COPY --from=builder /home/node/.gitref ./.gitref |
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.
The comment states that .git is needed in the image for version metadata, but the actual version metadata comes from the .gitsha and .gitref files created by the GitHub workflow, not from the .git directory itself. The comment is misleading and should be updated to clarify that .git can be ignored because the workflow explicitly creates .gitsha and .gitref files.