-
-
Notifications
You must be signed in to change notification settings - Fork 365
Fix Hadolint Linting Issues Across Dockerfiles #1304
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
Conversation
WalkthroughThis PR hardens Docker configurations across multiple Dockerfiles by improving shell quoting, adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 1
🧹 Nitpick comments (1)
.devcontainer/resources/devcontainer-Dockerfile (1)
29-31: Consider verifying hadolint binary integrity.The hadolint binary is downloaded without checksum or signature verification. While this is a development container, adding verification would improve supply chain security.
Apply this diff to add checksum verification:
# Install hadolint (Dockerfile linter) -RUN curl -L https://github.com/hadolint/hadolint/releases/latest/download/hadolint-Linux-x86_64 -o /usr/local/bin/hadolint && \ +RUN curl -L https://github.com/hadolint/hadolint/releases/latest/download/hadolint-Linux-x86_64 -o /usr/local/bin/hadolint && \ + curl -L https://github.com/hadolint/hadolint/releases/latest/download/hadolint-Linux-x86_64.sha256 -o /tmp/hadolint.sha256 && \ + echo "$(cat /tmp/hadolint.sha256) /usr/local/bin/hadolint" | sha256sum -c - && \ + rm /tmp/hadolint.sha256 && \ chmod +x /usr/local/bin/hadolint
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.devcontainer/Dockerfile(6 hunks).devcontainer/devcontainer.json(1 hunks).devcontainer/resources/devcontainer-Dockerfile(2 hunks).github/workflows/code_checks.yml(1 hunks).hadolint.yaml(1 hunks)Dockerfile(4 hunks)Dockerfile.debian(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:18-19
Timestamp: 2025-09-20T03:01:19.912Z
Learning: In the NetAlertX repository, .devcontainer/Dockerfile is auto-generated and should not be reviewed directly. Review comments about dependencies and build steps should be directed at the root Dockerfile where the actual source commands are located.
📚 Learning: 2025-09-20T03:01:19.912Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:18-19
Timestamp: 2025-09-20T03:01:19.912Z
Learning: In the NetAlertX repository, .devcontainer/Dockerfile is auto-generated and should not be reviewed directly. Review comments about dependencies and build steps should be directed at the root Dockerfile where the actual source commands are located.
Applied to files:
.devcontainer/resources/devcontainer-DockerfileDockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T02:56:24.501Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/devcontainer.json:5-8
Timestamp: 2025-09-20T02:56:24.501Z
Learning: In the NetAlertX devcontainer setup, the final .devcontainer/Dockerfile is generated by combining the root Dockerfile with .devcontainer/resources/devcontainer-Dockerfile using the generate-dockerfile.sh script. The devcontainer.json should reference the generated file, not the root Dockerfile.
Applied to files:
.devcontainer/resources/devcontainer-DockerfileDockerfile.devcontainer/Dockerfile
📚 Learning: 2025-10-26T17:09:18.621Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: .devcontainer/scripts/setup.sh:146-148
Timestamp: 2025-10-26T17:09:18.621Z
Learning: In `.devcontainer/scripts/setup.sh` and other devcontainer setup scripts for NetAlertX, chmod 666 on /var/run/docker.sock is acceptable because devcontainer environments are single-user development contexts where convenience can take priority over strict permission hardening.
Applied to files:
.devcontainer/resources/devcontainer-DockerfileDockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:09:29.159Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/scripts/setup.sh:103-116
Timestamp: 2025-09-20T14:09:29.159Z
Learning: In NetAlertX devcontainer setup, the netalertx user has write permissions to /var/log/nginx/ directory as it's explicitly chowned to netalertx:www-data in the Dockerfile, so setup.sh can write to nginx log files without sudo.
Applied to files:
.devcontainer/resources/devcontainer-DockerfileDockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:13:33.635Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:11-16
Timestamp: 2025-09-20T14:13:33.635Z
Learning: In the NetAlertX repository with Alpine 3.22 base image, the `python -m venv` command works correctly in the devcontainer setup, likely due to symlink creation in the root Dockerfile that makes `python` available as an alias to `python3`.
Applied to files:
.devcontainer/resources/devcontainer-DockerfileDockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:13:33.635Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:11-16
Timestamp: 2025-09-20T14:13:33.635Z
Learning: In the NetAlertX devcontainer setup, the `python -m venv /opt/venv` command works successfully on Alpine 3.22 despite the typical Alpine behavior of not providing a /usr/bin/python symlink by default. The build completes successfully and pytest runs without issues.
Applied to files:
.devcontainer/resources/devcontainer-DockerfileDockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:08:48.256Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/scripts/setup.sh:90-96
Timestamp: 2025-09-20T14:08:48.256Z
Learning: In the NetAlertX devcontainer setup, the setup.sh script intentionally removes user_notifications.json from the API directory during development environment initialization to prevent notification clutter that accumulates during container launches and development work.
Applied to files:
.devcontainer/resources/devcontainer-Dockerfile.devcontainer/Dockerfile
📚 Learning: 2025-10-19T01:40:57.095Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1230
File: .devcontainer/resources/devcontainer-overlay/services/config/nginx/netalertx.conf.template:1-2
Timestamp: 2025-10-19T01:40:57.095Z
Learning: In the NetAlertX repository, .devcontainer/resources/devcontainer-overlay/services/config/nginx/netalertx.conf.template is an auto-generated file that is intentionally committed to source control. It cannot be regenerated automatically outside the devcontainer environment and is required for the devcontainer to start, creating a bootstrap dependency.
Applied to files:
.devcontainer/resources/devcontainer-DockerfileDockerfile.devcontainer/Dockerfile
📚 Learning: 2025-09-20T14:08:44.152Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/Dockerfile:82-92
Timestamp: 2025-09-20T14:08:44.152Z
Learning: In devcontainer builds, source repository files are not available during the Docker build phase. The source code gets mounted into the container after it's built and started, so COPY commands referencing source files will fail. Configuration files need to be handled at runtime (e.g., in setup scripts) rather than during the build stage.
Applied to files:
.devcontainer/Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker_dev
🔇 Additional comments (10)
.devcontainer/devcontainer.json (1)
79-79: LGTM!The hadolint extension addition properly enables Dockerfile linting within the devcontainer environment.
.github/workflows/code_checks.yml (1)
87-87: LGTM!Adding the config flag ensures consistent hadolint behavior between CI and local development environments.
.devcontainer/resources/devcontainer-Dockerfile (1)
10-10: LGTM!The DL3006 ignore is appropriate here since
runneris a multi-stage build reference, not an external image requiring explicit tagging.Dockerfile (4)
35-36: LGTM!Adding
--no-cache-dirreduces image size by preventing pip from caching downloaded packages.
150-169: LGTM!The quoting improvements enhance shell safety by properly handling paths and variables that might contain spaces or special characters. The
--no-cacheflag for libcap and quotedreadlinkoutput are appropriate hardening measures.
186-187: LGTM!Quoting the user and group names ensures proper handling even if they contain special characters.
193-214: LGTM!The hadolint SC2114 ignore is appropriate for the intentional system directory removal in the hardened image. Using
printfinstead ofechofor the sudo wrapper provides more consistent behavior across shells.Dockerfile.debian (2)
135-141: LGTM!The hadolint ignores are appropriate for Debian-based images where version pinning is less critical. Using
--no-install-recommendsand cleaning apt lists are standard best practices that reduce image size.
146-157: LGTM!The quiet wget flag,
--no-install-recommends, and apt list cleanup follow Dockerfile best practices. The symlink fromphp-fpm8.3tophp-fpm83maintains Alpine compatibility..devcontainer/Dockerfile (1)
1-1: Auto-generated file inherits reviewed changes correctly.Based on learnings, this file is auto-generated by combining the root Dockerfile with
.devcontainer/resources/devcontainer-Dockerfile. The changes correctly reflect the improvements made to the source files (quoting, caching flags, hadolint integration) and don't require separate review.
This PR addresses various hadolint warnings across the project's Dockerfiles to improve Docker best practices and maintainability.
Changes Made
Testing
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.