Skip to content

Conversation

@adamoutler
Copy link
Collaborator

@adamoutler adamoutler commented Nov 24, 2025

This PR addresses various hadolint warnings across the project's Dockerfiles to improve Docker best practices and maintainability.

Changes Made

  1. Devcontainer Setup
  • Added hadolint installation to the devcontainer Dockerfile for local linting
  • Created .hadolint.yaml configuration file with global ignores.
  1. CI/CD Updates
  • Updated code_checks.yml to use the hadolint config file in GitHub Actions
  • Ensures consistent linting between local development and CI

Testing

  • All Dockerfiles build successfully
  • Hadolint warnings reduced from ~15 to 0 in affected files
  • Devcontainer builds and functions normally

Summary by CodeRabbit

Release Notes

  • Chores
    • Optimized container build process with improved dependency caching and cleanup, resulting in smaller image sizes.
    • Enhanced build reliability through stricter shell scripting practices and path quoting consistency.
    • Integrated Dockerfile linting tooling for improved code quality validation in the development environment.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Walkthrough

This PR hardens Docker configurations across multiple Dockerfiles by improving shell quoting, adding --no-cache-dir and --no-cache flags for dependency installation, introducing Dockerfile linting via hadolint, and optimizing image layer caching in the Debian build.

Changes

Cohort / File(s) Summary
Hadolint Configuration & Setup
​.hadolint.yaml, ​.devcontainer/resources/devcontainer-Dockerfile, ​.devcontainer/devcontainer.json, ​.github/workflows/code_checks.yml
New hadolint configuration file with DL3018 rule ignored. Devcontainer Dockerfile installs hadolint via curl with DL3006 ignore directive. VSCode extension exiasr.hadolint added to devcontainer extensions. CI workflow now uses --config .hadolint.yaml flag for hadolint invocation.
Dockerfile Hardening & Optimization
​.devcontainer/Dockerfile, ​Dockerfile, ​Dockerfile.debian
Shell paths and variables now consistently quoted in conditionals and redirections. Pip installs use --no-cache-dir and libcap uses --no-cache. Sudo wrapper creation switched from echo -ne to printf. Group/user names quoted in addgroup/adduser calls. Debian build replaces apt update with apt-get update, adds --no-install-recommends, removes apt lists post-installation for reduced image size. PHP repository setup enhanced with quiet wget -q.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify quoting patterns do not inadvertently break shell variable expansions or path resolution (especially buildtimestamp.txt redirection and setcap binary path)
  • Confirm printf replacement for echo -ne produces identical output for the sudo wrapper
  • Validate hadolint configuration doesn't introduce false failures on existing Dockerfiles
  • Test Debian-specific changes: --no-install-recommends behavior and apt list cleanup don't affect PHP 8.3 installation reliability

Possibly related PRs

Poem

🐰 With quotes around our paths so clear,
And caches trimmed without a fear,
Hadolint hops to lint each line,
Our Dockerfiles now hardened, fine!
Hop, hop, build, deploy with cheer!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix Hadolint Linting Issues Across Dockerfiles' directly and clearly summarizes the main change: addressing hadolint warnings across the project's Dockerfiles through configuration and code fixes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between c753da9 and 30071c6.

📒 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-Dockerfile
  • Dockerfile
  • .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-Dockerfile
  • Dockerfile
  • .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-Dockerfile
  • Dockerfile
  • .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-Dockerfile
  • Dockerfile
  • .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-Dockerfile
  • Dockerfile
  • .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-Dockerfile
  • Dockerfile
  • .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-Dockerfile
  • Dockerfile
  • .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 runner is a multi-stage build reference, not an external image requiring explicit tagging.

Dockerfile (4)

35-36: LGTM!

Adding --no-cache-dir reduces 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-cache flag for libcap and quoted readlink output 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 printf instead of echo for 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-recommends and 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 from php-fpm8.3 to php-fpm83 maintains 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.

@jokob-sk jokob-sk merged commit fa9fc2c into netalertx:main Nov 24, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants