-
Notifications
You must be signed in to change notification settings - Fork 46
Test docker file changes #3270
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: master
Are you sure you want to change the base?
Test docker file changes #3270
Conversation
📝 WalkthroughWalkthroughUpdated the microplan Dockerfile with a downgraded Alpine base image version, added Python 2 and build tools (make, g++), created a python-to-python2 symlink for node-gyp compatibility, and enabled yarn debug mode. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
health/micro-ui/web/microplan/Dockerfile (1)
42-49: Consider adding HEALTHCHECK and non-root user (optional).While not blockers, the final nginx stage lacks:
- A HEALTHCHECK instruction to enable container health monitoring
- A non-root user for improved security posture
These are optional improvements for production hardening.
If you'd like to add these, I can help generate the appropriate configuration for nginx health checks and user setup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
health/micro-ui/web/microplan/package.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
health/micro-ui/web/microplan/Dockerfile(1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
health/micro-ui/web/microplan/Dockerfile
[low] 1-49: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-49: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 Hadolint (2.14.0)
health/micro-ui/web/microplan/Dockerfile
[warning] 3-3: Pin versions in apk add. Instead of apk add <package> use apk add <package>=<version>
(DL3018)
[info] 3-3: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[warning] 5-5: Pin versions in apk add. Instead of apk add <package> use apk add <package>=<version>
(DL3018)
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
health/micro-ui/web/microplan/Dockerfile (1)
11-11: Verify thatYARN_DEBUG=trueis intentional.Debug logging may increase output verbosity and should be explicitly intended. Confirm whether this flag is:
- Needed to debug the build (temporary, should be removed before merge)
- Required for production diagnostics (should be documented in comments)
- Added unintentionally
| @@ -1,6 +1,10 @@ | |||
| FROM node:14-alpine3.16 AS build | |||
| FROM node:14-alpine3.14 AS build | |||
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.
🧩 Analysis chain
Verify the rationale for downgrading the Alpine base image.
Downgrading from alpine3.16 to alpine3.14 is a regression that may expose the image to unpatched security vulnerabilities. This change requires strong justification (e.g., compatibility with node-gyp or a known regression in 3.16).
Please clarify why the base image was downgraded and confirm that alpine3.14 does not introduce security gaps or other regressions.
I'll help you verify this review comment. Let me gather the necessary information about the Dockerfile and the Alpine version concern.
---
🏁 Script executed:
```shell
#!/bin/bash
# Fetch the Dockerfile to see the full context and all changes
find . -name "Dockerfile" -path "*/health/micro-ui/web/microplan/*" -type f 2>/dev/null | head -5 | xargs -I {} cat {}
Length of output: 1274
🌐 Web query:
Alpine Linux 3.14 vs 3.16 security vulnerabilities CVEs
💡 Result:
Short answer: Alpine 3.14 and 3.16 both received numerous package CVE fixes while they were maintained, but 3.14 is EOL and no longer receives security fixes (EOL May 1, 2023) while 3.16 reached EOL on May 23, 2024. That means 3.14 is unpatched for any CVEs disclosed after its EOL; 3.16 is unpatched for CVEs disclosed after May 23, 2024. [1][2]
Notable examples:
- Alpine 3.14 releases included multiple BusyBox fixes (many CVE‑2021‑4237x) and an apk-tools fix (CVE‑2021‑36159). [3][4]
- Alpine 3.16 releases included fixes for BusyBox/openssl and other packages (e.g., CVE‑2022‑30065, CVE‑2022‑2097) and later package advisories (example: lighttpd / upstream malicious code CVE‑2024‑3094 affecting Alpine:3.16 packages). [5][6]
If you need a full per‑release CVE list:
- For 3.14, review the 3.14 release posts and package change logs (search alpinelinux.org/posts for 3.14.x) and the distribution’s package git history. [3][4]
- For 3.16, review the 3.16 release posts and package changelogs and common-vuln databases (Alpine posts + NVD/Snyk) for CVEs affecting packages in that release. [2][5][6]
Recommendation: upgrade to a currently supported Alpine release (see Alpine EOL schedule) to ensure you receive security fixes. [1]
Sources:
[1] Alpine Linux EOL summary (third‑party listing).
[2] Alpine 3.16 release notes (example).
[3] Alpine 3.14.3 release notes (BusyBox CVEs).
[4] Alpine 3.14.1 release notes (apk‑tools CVE).
[5] Alpine 3.16.1 release notes (BusyBox / OpenSSL fixes).
[6] Snyk advisory re: lighttpd / CVE‑2024‑3094 affecting Alpine:3.16.
(If you want, I can produce a side‑by‑side list of all CVEs touching packages in 3.14 vs 3.16 with links to each advisory.)
🌐 Web query:
Node 14 node-gyp Python 2 alpine compatibility requirements
💡 Result:
Short answer
- node-gyp itself (older v3–v8 series commonly used by native modules with Node 14) historically required Python 2.7; newer node-gyp releases (v8+) require Python 3 (3.6–3.9+ depending on version). Verify the node-gyp version used by your module. [1][2]
Alpine practical requirements (Node 14 images)
- Install a build toolchain and Python in the image before running npm install/rebuild. Typical apk packages to add: python2 (or python3 if your node-gyp supports it), make, g++, build-base (or gcc), linux-headers and libc6-compat. Example:
apk add --no-cache python2 make g++ build-base linux-headers libc6-compat. - If a native dependency requires Python 2 but your base has python3, install python2 (apk package name: python2) and point npm/node-gyp to it via npm config set python /usr/bin/python2 or --python when invoking node-gyp. [3][4]
Notes and tips
- Use a multi-stage build to remove the build deps after npm install to keep images small. Add python/make/g++ in the same RUN layer as npm install so removal works correctly. [4][5]
- Alpine-musl vs glibc: some prebuilt binaries expecting glibc will fail; either rebuild on Alpine or add libc6-compat / use a glibc-compatible image. [5]
References
- node-gyp README (Python compatibility/details). [UNPKG node-gyp README]. [1]
- Discussions about node-gyp/Python versions and Node 14 (Stack Overflow examples). [2]
- Alpine + node-gyp build instructions / apk packages (dev.to/blog posts). [3]
- Docker / node:alpine common fixes (Stack Overflow). [4]
Would you like a ready Dockerfile snippet for node:14-alpine that installs/removes build deps and forces python2 if needed?
Downgrade to Alpine 3.14 introduces unpatched security vulnerabilities—use a currently-supported Alpine version.
Alpine 3.14 reached end-of-life on May 1, 2023 and no longer receives security fixes, while Alpine 3.16 reached EOL on May 23, 2024. Downgrading from 3.16 to 3.14 exposes the image to unpatched CVEs that have accumulated over the past 2+ years.
Update to a currently-supported Alpine release (3.18 or 3.19) to ensure security patches are applied.
Additional observations:
- Lines 2–6: Consolidate into a single
RUNcommand with--no-cacheto reduce layers and improve cache efficiency. - Line 11: Verify whether
YARN_DEBUG=trueis intentional for production or should be removed/moved to debug-only builds. - Line 33: The second-stage
nginx:mainline-alpineis also unversioned and has maintenance/security risks—pin to a specific version. - Node 14 with native modules requires Python 2, so the Python 2 installation (lines 4–6) is justified despite being EOL.
🧰 Tools
🪛 Checkov (3.2.334)
[low] 1-49: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-49: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🤖 Prompt for AI Agents
In health/micro-ui/web/microplan/Dockerfile around line 1 (and referencing lines
2–6, 11, 33), the base image was downgraded to alpine3.14 which is EOL and
exposes unpatched CVEs; update the base to a currently-supported Alpine variant
(e.g., node:14-alpine3.18 or node:14-alpine3.19) and pin the nginx second-stage
to a specific maintained tag instead of unversioned nginx:mainline-alpine;
consolidate the multiple package-install RUN steps (lines 2–6) into a single RUN
with --no-cache to reduce layers and improve cache efficiency, remove or
conditionally set YARN_DEBUG=true for non-production builds (line 11) if it’s
not intended for production, and retain the Python2 installation only if
native-module build requirements demand it while documenting that justification
in a comment.
Choose the appropriate template for your PR:
Feature/Bugfix Request
JIRA ID
Module
Description
Summary by CodeRabbit