Feat: make errors more helpful - #1263
Conversation
WalkthroughAdds an entrypoint checks directory, many new entrypoint.d health/permission/mount-check scripts (including a Python mount analyzer), updates entrypoint orchestration to run those checks and continue on failures, removes several legacy service scripts, updates Dockerfiles to include the checks path and adjust permissions, and adds extensive Docker troubleshooting docs and compose test fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Container Init
participant E as entrypoint.sh
participant EC as ${ENTRYPOINT_CHECKS}/*
participant M as 10-mounts.py
participant A as Application
C->>E: start
E->>E: render banner
loop each script in ${ENTRYPOINT_CHECKS}
E->>EC: exec script (executable)
EC-->>E: output + exit code
E->>E: log result (continue even on failure)
end
alt 10-mounts.py reports write errors and NETALERTX_DEBUG != "1"
EC->>E: prints table & warnings
E->>C: propagate non-zero exit (entrypoint behavior may stop)
else
E->>A: proceed to start application
end
sequenceDiagram
participant P as 10-mounts.py
participant V as Env vars
participant F as /proc/mounts
participant O as stderr/table
P->>V: read PATHS_TO_CHECK and flags
P->>F: parse mounted filesystems
loop per path
P->>V: resolve path value
P->>F: match mount point & fstype
P->>P: evaluate writeable/mounted/ramdisk & risk flags
end
P->>O: print diagnostic table
alt write errors and NETALERTX_DEBUG != "1"
P->>O: print warnings
P->>P: exit(1)
else
P->>P: exit(0)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Focus areas for review:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-09-20T03:01:19.912ZApplied to files:
📚 Learning: 2025-10-19T15:29:49.532ZApplied to files:
📚 Learning: 2025-09-20T02:56:24.501ZApplied to files:
📚 Learning: 2025-10-19T01:40:57.095ZApplied to files:
📚 Learning: 2025-09-20T14:08:44.152ZApplied to files:
📚 Learning: 2025-10-26T17:09:18.621ZApplied to files:
📚 Learning: 2025-09-20T14:09:29.159ZApplied to files:
📚 Learning: 2025-10-26T17:12:30.237ZApplied to files:
🔇 Additional comments (4)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (3)
test/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_mounted.yml (1)
44-52: Unused volume declarations reduce clarity.Lines 44-52 declare 8 named volumes, but only 3 are actually referenced in the service (netalertx_db, netalertx_config, test_system_services_active_config). The others (test_netalertx_db, test_netalertx_config, test_netalertx_api, test_netalertx_log, test_system_services_run) appear unused.
Consider removing unused declarations or adding comments explaining their purpose. If they're shared across multiple test configurations, consolidating them into a separate compose file or clarifying their intent would improve maintainability.
test/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_ramdisk.yml (1)
41-49: Duplicate unused volume declarations.This file repeats the same unused volume declarations from the previous mount-test configuration (8 declared, only 3 referenced in netalertx_db, netalertx_config, and test_system_services_run).
Consider consolidating these common volume declarations into a shared base or compose fragment to reduce duplication across test configurations.
test/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_no-mount.yml (1)
40-48: Unused volume declarations reduce clarity (consistent pattern).This file repeats the same 8 unused volume declarations from previous test configurations. Only 3 are actually referenced (netalertx_db, netalertx_config, test_netalertx_api).
Consolidate these common declarations across all mount-test configurations into a shared compose fragment or base file to improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/docker_tests/configurations/test_results.logis excluded by!**/*.log
📒 Files selected for processing (64)
.devcontainer/Dockerfile(3 hunks)Dockerfile(3 hunks)docs/docker-troubleshooting/excessive-capabilities.md(1 hunks)docs/docker-troubleshooting/file-permissions.md(1 hunks)docs/docker-troubleshooting/incorrect-user.md(1 hunks)docs/docker-troubleshooting/missing-capabilities.md(1 hunks)docs/docker-troubleshooting/mount-configuration-issues.md(1 hunks)docs/docker-troubleshooting/network-mode.md(1 hunks)docs/docker-troubleshooting/nginx-configuration-mount.md(1 hunks)docs/docker-troubleshooting/port-conflicts.md(1 hunks)docs/docker-troubleshooting/read-only-filesystem.md(1 hunks)docs/docker-troubleshooting/running-as-root.md(1 hunks)install/production-filesystem/entrypoint.d/0-storage-permission.sh(1 hunks)install/production-filesystem/entrypoint.d/10-mounts.py(1 hunks)install/production-filesystem/entrypoint.d/15-first-run-config.sh(1 hunks)install/production-filesystem/entrypoint.d/20-first-run-db.sh(2 hunks)install/production-filesystem/entrypoint.d/30-writable-config.sh(1 hunks)install/production-filesystem/entrypoint.d/35-nginx-config.sh(3 hunks)install/production-filesystem/entrypoint.d/60-user-netalertx.sh(1 hunks)install/production-filesystem/entrypoint.d/80-host-mode-network.sh(3 hunks)install/production-filesystem/entrypoint.d/85-layer-2-capabilities.sh(2 hunks)install/production-filesystem/entrypoint.d/90-excessive-capabilities.sh(1 hunks)install/production-filesystem/entrypoint.d/95-appliance-integrity.sh(1 hunks)install/production-filesystem/entrypoint.d/99-ports-available.sh(1 hunks)install/production-filesystem/entrypoint.sh(1 hunks)install/production-filesystem/services/scripts/check-app-permissions.sh(0 hunks)install/production-filesystem/services/scripts/check-nonpersistent-storage.sh(0 hunks)install/production-filesystem/services/scripts/check-persistent-storage.sh(0 hunks)install/production-filesystem/services/scripts/check-ramdisk.sh(0 hunks)install/production-filesystem/services/scripts/check-root.sh(0 hunks)test/docker_tests/configurations/README.md(1 hunks)test/docker_tests/configurations/docker-compose.missing-caps.yml(1 hunks)test/docker_tests/configurations/docker-compose.readonly.yml(1 hunks)test/docker_tests/configurations/docker-compose.writable.yml(1 hunks)test/docker_tests/configurations/mount-tests/README.md(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_mounted.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_no-mount.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_ramdisk.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_unwritable.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_mounted.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_no-mount.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_ramdisk.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_unwritable.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_mounted.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_no-mount.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_ramdisk.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_unwritable.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_mounted.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_no-mount.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_ramdisk.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_unwritable.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_mounted.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_no-mount.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_ramdisk.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_unwritable.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_mounted.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_no-mount.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_ramdisk.yml(1 hunks)test/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_unwritable.yml(1 hunks)test/docker_tests/configurations/test_all_docker_composes.sh(1 hunks)test/docker_tests/test_container_environment.py(10 hunks)test/docker_tests/test_docker_compose_scenarios.py(1 hunks)test/docker_tests/test_mount_diagnostics_pytest.py(1 hunks)test/docker_tests/test_ports_available.py(1 hunks)
💤 Files with no reviewable changes (5)
- install/production-filesystem/services/scripts/check-app-permissions.sh
- install/production-filesystem/services/scripts/check-ramdisk.sh
- install/production-filesystem/services/scripts/check-root.sh
- install/production-filesystem/services/scripts/check-persistent-storage.sh
- install/production-filesystem/services/scripts/check-nonpersistent-storage.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_container_environment.pyinstall/production-filesystem/entrypoint.d/10-mounts.pytest/docker_tests/test_mount_diagnostics_pytest.pytest/docker_tests/test_docker_compose_scenarios.pytest/docker_tests/test_ports_available.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_container_environment.pytest/docker_tests/test_mount_diagnostics_pytest.pytest/docker_tests/test_docker_compose_scenarios.pytest/docker_tests/test_ports_available.py
🧠 Learnings (18)
📓 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.
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.
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.
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.
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/scripts/stream-logs.sh:5-6
Timestamp: 2025-09-20T14:08:44.292Z
Learning: The .devcontainer/scripts/stream-logs.sh script in NetAlertX is designed as a diagnostic tool for troubleshooting devcontainer startup issues. When log files don't exist, this indicates that the executable/services didn't start properly, which is valuable diagnostic information. Pre-creating missing files would mask this diagnostic behavior.
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.
📚 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:
docs/docker-troubleshooting/running-as-root.mddocs/docker-troubleshooting/file-permissions.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_mounted.ymlinstall/production-filesystem/entrypoint.d/90-excessive-capabilities.shinstall/production-filesystem/entrypoint.d/30-writable-config.shdocs/docker-troubleshooting/incorrect-user.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_ramdisk.yml.devcontainer/Dockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_unwritable.ymlinstall/production-filesystem/entrypoint.d/35-nginx-config.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_mounted.ymldocs/docker-troubleshooting/mount-configuration-issues.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_unwritable.ymlinstall/production-filesystem/entrypoint.d/60-user-netalertx.shtest/docker_tests/configurations/docker-compose.writable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_mounted.ymlinstall/production-filesystem/entrypoint.d/0-storage-permission.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_mounted.ymlDockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_unwritable.yml
📚 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:
docs/docker-troubleshooting/running-as-root.mddocs/docker-troubleshooting/file-permissions.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_mounted.ymldocs/docker-troubleshooting/port-conflicts.mddocs/docker-troubleshooting/incorrect-user.md.devcontainer/Dockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_mounted.ymldocs/docker-troubleshooting/mount-configuration-issues.mdinstall/production-filesystem/entrypoint.d/60-user-netalertx.shtest/docker_tests/configurations/docker-compose.writable.ymltest/docker_tests/configurations/README.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_ramdisk.ymltest/docker_tests/configurations/docker-compose.missing-caps.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_no-mount.ymltest/docker_tests/configurations/docker-compose.readonly.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_mounted.ymlDockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_unwritable.yml
📚 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:
docs/docker-troubleshooting/running-as-root.mddocs/docker-troubleshooting/nginx-configuration-mount.mddocs/docker-troubleshooting/file-permissions.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_mounted.ymlinstall/production-filesystem/entrypoint.d/30-writable-config.shdocs/docker-troubleshooting/incorrect-user.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_ramdisk.yml.devcontainer/Dockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_unwritable.ymlinstall/production-filesystem/entrypoint.d/35-nginx-config.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_mounted.ymldocs/docker-troubleshooting/mount-configuration-issues.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_unwritable.ymlinstall/production-filesystem/entrypoint.d/60-user-netalertx.shtest/docker_tests/configurations/docker-compose.writable.ymltest/docker_tests/configurations/README.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_unwritable.ymltest/docker_tests/configurations/docker-compose.missing-caps.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_no-mount.ymltest/docker_tests/configurations/docker-compose.readonly.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_mounted.ymlinstall/production-filesystem/entrypoint.d/0-storage-permission.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_mounted.ymlDockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_unwritable.yml
📚 Learning: 2025-10-19T15:29:46.423Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1230
File: front/plugins/dhcp_servers/script.py:44-44
Timestamp: 2025-10-19T15:29:46.423Z
Learning: In the NetAlertX dhcp_servers plugin (front/plugins/dhcp_servers/script.py), the nmap command uses both 'sudo' and '--privileged' flag to maintain cross-platform compatibility. While the hardened Docker image stubs sudo and uses capabilities, hardware installations (Debian 12, Ubuntu 24) and the Debian Dockerfile require sudo for raw socket access. This approach ensures the plugin works across all deployment targets.
Applied to files:
docs/docker-troubleshooting/running-as-root.mddocs/docker-troubleshooting/missing-capabilities.mddocs/docker-troubleshooting/network-mode.mdinstall/production-filesystem/entrypoint.d/85-layer-2-capabilities.shdocs/docker-troubleshooting/excessive-capabilities.md
📚 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:
docs/docker-troubleshooting/nginx-configuration-mount.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_mounted.ymldocs/docker-troubleshooting/port-conflicts.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_ramdisk.yml.devcontainer/Dockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_no-mount.ymlinstall/production-filesystem/entrypoint.d/35-nginx-config.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_mounted.ymldocs/docker-troubleshooting/mount-configuration-issues.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_unwritable.ymlinstall/production-filesystem/entrypoint.d/60-user-netalertx.shtest/docker_tests/configurations/docker-compose.writable.ymltest/docker_tests/configurations/README.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_unwritable.ymltest/docker_tests/configurations/docker-compose.missing-caps.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_no-mount.ymltest/docker_tests/configurations/docker-compose.readonly.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_mounted.ymlDockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_unwritable.yml
📚 Learning: 2025-10-26T16:45:41.247Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: install/production-filesystem/services/scripts/check-ramdisk.sh:0-0
Timestamp: 2025-10-26T16:45:41.247Z
Learning: In NetAlertX check scripts (install/production-filesystem/services/scripts/check-*.sh), not all checks should exit with non-zero status. Some checks, like check-ramdisk.sh, are warning-only and exit 0 even when issues are detected, allowing the application to start despite suboptimal configuration.
Applied to files:
install/production-filesystem/entrypoint.d/95-appliance-integrity.shinstall/production-filesystem/entrypoint.d/30-writable-config.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_ramdisk.ymltest/docker_tests/test_container_environment.pyinstall/production-filesystem/entrypoint.d/35-nginx-config.shinstall/production-filesystem/entrypoint.shinstall/production-filesystem/entrypoint.d/60-user-netalertx.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_ramdisk.ymlinstall/production-filesystem/entrypoint.d/0-storage-permission.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_no-mount.yml
📚 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:
test/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_mounted.yml
📚 Learning: 2025-10-26T15:39:36.707Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: install/production-filesystem/services/scripts/check-first-run-config.sh:0-0
Timestamp: 2025-10-26T15:39:36.707Z
Learning: In NetAlertX startup scripts, critical initialization failures (e.g., unable to create config directory or copy default config files) should exit with non-zero status to fail fast and provide clear error messages, rather than continuing in a broken state.
Applied to files:
install/production-filesystem/entrypoint.d/30-writable-config.shtest/docker_tests/test_container_environment.pytest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_unwritable.ymlinstall/production-filesystem/entrypoint.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_unwritable.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_unwritable.yml
📚 Learning: 2025-09-20T14:08:44.292Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/scripts/stream-logs.sh:5-6
Timestamp: 2025-09-20T14:08:44.292Z
Learning: The .devcontainer/scripts/stream-logs.sh script in NetAlertX is designed as a diagnostic tool for troubleshooting devcontainer startup issues. When log files don't exist, this indicates that the executable/services didn't start properly, which is valuable diagnostic information. Pre-creating missing files would mask this diagnostic behavior.
Applied to files:
install/production-filesystem/entrypoint.d/30-writable-config.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_unwritable.ymlinstall/production-filesystem/entrypoint.d/60-user-netalertx.shtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.log_unwritable.yml
📚 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:
docs/docker-troubleshooting/incorrect-user.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_no-mount.ymlinstall/production-filesystem/entrypoint.d/60-user-netalertx.shtest/docker_tests/configurations/docker-compose.writable.ymltest/docker_tests/configurations/README.mdtest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_ramdisk.ymltest/docker_tests/configurations/docker-compose.readonly.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_mounted.yml
📚 Learning: 2025-10-19T15:29:49.532Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1230
File: install/production-filesystem/build/init-nginx.sh:3-3
Timestamp: 2025-10-19T15:29:49.532Z
Learning: In Docker build-time initialization scripts (like those in install/production-filesystem/build/), environment variables set in the Dockerfile do not need explicit validation within the scripts. The Docker build process will fail if required environment variables are undefined, providing implicit validation.
Applied to files:
.devcontainer/DockerfileDockerfile
📚 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/Dockerfiletest/docker_tests/configurations/mount-tests/docker-compose.mount-test.db_mounted.ymlinstall/production-filesystem/entrypoint.d/60-user-netalertx.shtest/docker_tests/configurations/docker-compose.writable.ymltest/docker_tests/configurations/README.mdtest/docker_tests/configurations/docker-compose.missing-caps.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.run_no-mount.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_no-mount.ymltest/docker_tests/configurations/docker-compose.readonly.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.config_mounted.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.active_config_ramdisk.ymltest/docker_tests/configurations/mount-tests/docker-compose.mount-test.api_mounted.yml
📚 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
📚 Learning: 2025-10-26T17:12:30.237Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: install/production-filesystem/services/start-php-fpm.sh:25-26
Timestamp: 2025-10-26T17:12:30.237Z
Learning: In NetAlertX's start-php-fpm.sh, PHP-FPM error logging is handled through the PHP-FPM configuration file (PHP_FPM_CONFIG_FILE) rather than through shell redirection. The current redirection pattern (stdout to LOG_APP_PHP_ERRORS, stderr to /dev/stderr) is intentional.
Applied to files:
.devcontainer/DockerfileDockerfile
📚 Learning: 2025-10-26T17:11:17.499Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: install/production-filesystem/services/scripts/update_vendors.sh:22-22
Timestamp: 2025-10-26T17:11:17.499Z
Learning: In install/production-filesystem/services/scripts/update_vendors.sh, suppressing wget stderr with 2>/dev/null is intentional; the script's own error handling (line 27: "ERROR: Failed to download or process OUI data") provides adequate feedback when downloads fail in offline environments.
Applied to files:
install/production-filesystem/entrypoint.d/80-host-mode-network.sh
📚 Learning: 2025-10-10T22:16:02.770Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1214
File: install/ubuntu24/uninstall.sh:129-141
Timestamp: 2025-10-10T22:16:02.770Z
Learning: NetAlertX uninstall procedures should only remove files from the repository (specifically /app files) and should not touch system packages like PHP, nginx, avahi, or other shared system components to avoid damaging user systems.
Applied to files:
install/production-filesystem/entrypoint.d/60-user-netalertx.sh
📚 Learning: 2025-10-10T22:16:02.770Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1214
File: install/ubuntu24/uninstall.sh:129-141
Timestamp: 2025-10-10T22:16:02.770Z
Learning: In the NetAlertX project, hardware installs are not officially supported. Hardware installation methods will be moved from source code to documentation.
Applied to files:
install/production-filesystem/entrypoint.d/60-user-netalertx.sh
🧬 Code graph analysis (2)
test/docker_tests/test_container_environment.py (1)
test/docker_tests/test_ports_available.py (4)
_assert_contains(153-161)_build_volume_args(80-85)_run_container(88-150)_setup_mount_tree(59-77)
test/docker_tests/test_ports_available.py (1)
test/docker_tests/test_container_environment.py (4)
_setup_mount_tree(94-149)_build_volume_args(166-176)_run_container(187-279)_assert_contains(283-293)
🪛 LanguageTool
docs/docker-troubleshooting/read-only-filesystem.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...nt filesystem modifications. ## How to Correct the Issue Enable read-only mode: - In...
(FIX_RESOLVE)
docs/docker-troubleshooting/running-as-root.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...ervice account for security. ## How to Correct the Issue Switch to the dedicated 'net...
(FIX_RESOLVE)
docs/docker-troubleshooting/nginx-configuration-mount.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...n running in read-only mode. ## How to Correct the Issue If you want to use a custom ...
(FIX_RESOLVE)
docs/docker-troubleshooting/file-permissions.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...nt, not root or other users. ## How to Correct the Issue Fix permissions on the host ...
(FIX_RESOLVE)
docs/docker-troubleshooting/port-conflicts.md
[style] ~19-~19: Consider using a different verb for a more formal wording.
Context: ...es are set to the same value ## How to Correct the Issue ### Check for Existing NetAl...
(FIX_RESOLVE)
docs/docker-troubleshooting/incorrect-user.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...r proper security isolation. ## How to Correct the Issue Restore the container to the...
(FIX_RESOLVE)
docs/docker-troubleshooting/missing-capabilities.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...elevated network privileges. ## How to Correct the Issue Add the required capabilitie...
(FIX_RESOLVE)
docs/docker-troubleshooting/network-mode.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...scovery and active scanning. ## How to Correct the Issue Enable host networking mode:...
(FIX_RESOLVE)
docs/docker-troubleshooting/mount-configuration-issues.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ... others should be temporary. ## How to Correct the Issue Review and correct your volu...
(FIX_RESOLVE)
docs/docker-troubleshooting/excessive-capabilities.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...ilities for proper function. ## How to Correct the Issue Limit capabilities to only t...
(FIX_RESOLVE)
🪛 Ruff (0.14.2)
test/docker_tests/test_container_environment.py
202-202: subprocess call: check for execution of untrusted input
(S603)
203-203: Starting a process with a partial executable path
(S607)
283-283: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
665-665: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
666-666: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
694-694: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
695-695: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
install/production-filesystem/entrypoint.d/10-mounts.py
173-173: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
175-175: Do not assign a lambda expression, use a def
Rewrite bool_to_check as a def
(E731)
test/docker_tests/test_mount_diagnostics_pytest.py
1-1: Shebang is present but file is not executable
(EXE001)
12-12: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
12-12: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
13-13: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
13-13: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
21-21: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
43-43: Comment contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
44-44: Comment contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
86-86: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
160-160: subprocess call: check for execution of untrusted input
(S603)
161-161: Starting a process with a partial executable path
(S607)
232-232: Unused function argument: netalertx_test_image
(ARG001)
247-247: subprocess call: check for execution of untrusted input
(S603)
254-254: subprocess call: check for execution of untrusted input
(S603)
268-268: subprocess call: check for execution of untrusted input
(S603)
269-269: Starting a process with a partial executable path
(S607)
275-275: subprocess call: check for execution of untrusted input
(S603)
276-276: Starting a process with a partial executable path
(S607)
287-287: subprocess call: check for execution of untrusted input
(S603)
288-288: Starting a process with a partial executable path
(S607)
374-374: subprocess call: check for execution of untrusted input
(S603)
393-393: subprocess call: check for execution of untrusted input
(S603)
401-401: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
401-401: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
test/docker_tests/test_docker_compose_scenarios.py
22-22: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
38-38: subprocess call: check for execution of untrusted input
(S603)
50-50: subprocess call: check for execution of untrusted input
(S603)
50-50: Starting a process with a partial executable path
(S607)
55-55: subprocess call: check for execution of untrusted input
(S603)
55-55: Starting a process with a partial executable path
(S607)
66-66: Redefinition of unused time from line 11
Remove definition: time
(F811)
81-81: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
103-103: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
126-126: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
168-168: Unused function argument: env_vars
(ARG001)
168-168: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
180-180: subprocess call: check for execution of untrusted input
(S603)
191-191: subprocess call: check for execution of untrusted input
(S603)
191-191: Starting a process with a partial executable path
(S607)
196-196: subprocess call: check for execution of untrusted input
(S603)
196-196: Starting a process with a partial executable path
(S607)
244-244: Redefinition of unused test_host_network_compose from line 218
(F811)
273-273: Redefinition of unused test_host_network_compose from line 244
(F811)
359-359: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
364-364: f-string without any placeholders
Remove extraneous f prefix
(F541)
365-365: f-string without any placeholders
Remove extraneous f prefix
(F541)
366-366: f-string without any placeholders
Remove extraneous f prefix
(F541)
367-367: f-string without any placeholders
Remove extraneous f prefix
(F541)
412-412: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
417-417: f-string without any placeholders
Remove extraneous f prefix
(F541)
418-418: f-string without any placeholders
Remove extraneous f prefix
(F541)
419-419: f-string without any placeholders
Remove extraneous f prefix
(F541)
420-420: f-string without any placeholders
Remove extraneous f prefix
(F541)
test/docker_tests/test_ports_available.py
43-43: subprocess call: check for execution of untrusted input
(S603)
44-44: Starting a process with a partial executable path
(S607)
56-56: subprocess call: check for execution of untrusted input
(S603)
56-56: Starting a process with a partial executable path
(S607)
70-70: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
107-107: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
131-131: subprocess call: check for execution of untrusted input
(S603)
153-153: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
157-161: Avoid specifying long messages outside the exception class
(TRY003)
164-164: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
168-172: Avoid specifying long messages outside the exception class
(TRY003)
220-220: Unused function argument: dummy_container
(ARG001)
🪛 Shellcheck (0.11.0)
test/docker_tests/configurations/test_all_docker_composes.sh
[warning] 39-39: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 40-40: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 47-47: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 51-51: To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify).
(SC2069)
install/production-filesystem/entrypoint.sh
[warning] 42-42: RESET appears unused. Verify use (or export if used externally).
(SC2034)
|
This is a big PR - let me know once ready and I'll merge. The table is amazing - easy to recognize what's wrong |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
install/production-filesystem/entrypoint.d/0-storage-permission.sh (1)
57-58: Fixfindcommand syntax errors.Lines 57–58 are missing the terminator for
-exec; each must end with+(or;if preferred). Currently, these commands are malformed and will fail to execute.- find ${READ_WRITE_PATHS} -type d -exec chmod u+rwx {} - find ${READ_WRITE_PATHS} -type f -exec chmod u+rw {} + find "${READ_WRITE_PATHS}" -type d -exec chmod u+rwx {} + 2>/dev/null || true + find "${READ_WRITE_PATHS}" -type f -exec chmod u+rw {} + 2>/dev/null || trueThe diff also quotes the variable to prevent word-splitting on paths with spaces, adds error suppression for consistency with line 54, and uses
+(parallel execution) for efficiency.test/docker_tests/configurations/test_all_docker_composes.sh (1)
49-55: Remove duplicatecdanddocker-compose upcommands.Lines 49 and 51 contain duplicate
cdcommands (the second lacks error handling). Lines 53 and 55 contain duplicatedocker-compose upinvocations with different redirect orders. Line 55's redirect order is incorrect (2>&1 must come after >>).Apply this diff:
cd "$dirname" || exit 1 - # Change to the directory containing the docker-compose file - cd "$dirname" + echo "Running docker-compose up..." >> "$LOG_FILE" timeout 10s docker-compose -f "$basename" up >> "$LOG_FILE" 2>&1 - echo "Running docker-compose up..." >> "$LOG_FILE" - timeout 10s docker-compose -f "$basename" up 2>&1 >> "$LOG_FILE"test/docker_tests/test_ports_available.py (1)
122-124: Resolve script path relative to repo root.The hard-coded relative path
"install/production-filesystem/entrypoint.d/99-ports-available.sh"assumes execution from the repo root. In CI or non-standard environments, thisopen()will raiseFileNotFoundError. Use dynamic repo root resolution similar to test_container_environment.py (lines 113-131 in relevant_code_snippets).Apply this diff:
- # Copy the script content and run it - script_path = "install/production-filesystem/entrypoint.d/99-ports-available.sh" - with open(script_path, 'r') as f: + # Determine repo root from env or by walking up from this file + repo_root_env = os.environ.get("NETALERTX_REPO_ROOT") + if repo_root_env: + repo_root = pathlib.Path(repo_root_env) + else: + repo_root = None + cur = pathlib.Path(__file__).resolve() + for parent in cur.parents: + if (parent / "pyproject.toml").exists() or (parent / ".git").exists(): + repo_root = parent + break + if repo_root is None: + repo_root = cur.parents[2] + + script_path = repo_root / "install" / "production-filesystem" / "entrypoint.d" / "99-ports-available.sh" + with script_path.open('r', encoding='utf-8') as f: script_content = f.read()
🧹 Nitpick comments (2)
install/production-filesystem/entrypoint.d/0-storage-permission.sh (1)
54-54: Quote variable expansion inchownfor robustness.Unquoted variable expansion can cause word-splitting if any paths contain spaces or special characters. This is a lower-risk issue than lines 57–58 (syntax errors), but quoting is a defensive best practice:
- chown -R netalertx ${READ_WRITE_PATHS} 2>/dev/null || true + chown -R netalertx "${READ_WRITE_PATHS}" 2>/dev/null || truetest/docker_tests/test_docker_compose_scenarios.py (1)
273-276: Remove extraneous f-string prefixes.Lines 273-276 (and 326-329) use f-string prefixes without any placeholders. Remove the
fprefix for clarity.Apply this diff:
"volumes": [ - f"./test_data/app_log:/app/log", - f"./test_data/app_api:/app/api", - f"./test_data/nginx_conf:/services/config/nginx/conf.active", - f"./test_data/services_run:/services/run" + "./test_data/app_log:/app/log", + "./test_data/app_api:/app/api", + "./test_data/nginx_conf:/services/config/nginx/conf.active", + "./test_data/services_run:/services/run" ],Apply the same fix to lines 326-329.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/docker-troubleshooting/nginx-configuration-mount.md(1 hunks)install/production-filesystem/entrypoint.d/0-storage-permission.sh(1 hunks)install/production-filesystem/entrypoint.d/80-host-mode-network.sh(2 hunks)install/production-filesystem/entrypoint.d/90-excessive-capabilities.sh(1 hunks)test/docker_tests/configurations/test_all_docker_composes.sh(1 hunks)test/docker_tests/test_docker_compose_scenarios.py(1 hunks)test/docker_tests/test_mount_diagnostics_pytest.py(1 hunks)test/docker_tests/test_ports_available.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- install/production-filesystem/entrypoint.d/90-excessive-capabilities.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_mount_diagnostics_pytest.pytest/docker_tests/test_ports_available.pytest/docker_tests/test_docker_compose_scenarios.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_mount_diagnostics_pytest.pytest/docker_tests/test_ports_available.pytest/docker_tests/test_docker_compose_scenarios.py
🧠 Learnings (8)
📚 Learning: 2025-10-26T16:45:41.247Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1235
File: install/production-filesystem/services/scripts/check-ramdisk.sh:0-0
Timestamp: 2025-10-26T16:45:41.247Z
Learning: In NetAlertX check scripts (install/production-filesystem/services/scripts/check-*.sh), not all checks should exit with non-zero status. Some checks, like check-ramdisk.sh, are warning-only and exit 0 even when issues are detected, allowing the application to start despite suboptimal configuration.
Applied to files:
install/production-filesystem/entrypoint.d/80-host-mode-network.shinstall/production-filesystem/entrypoint.d/0-storage-permission.sh
📚 Learning: 2025-09-20T14:08:44.292Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/scripts/stream-logs.sh:5-6
Timestamp: 2025-09-20T14:08:44.292Z
Learning: The .devcontainer/scripts/stream-logs.sh script in NetAlertX is designed as a diagnostic tool for troubleshooting devcontainer startup issues. When log files don't exist, this indicates that the executable/services didn't start properly, which is valuable diagnostic information. Pre-creating missing files would mask this diagnostic behavior.
Applied to files:
install/production-filesystem/entrypoint.d/80-host-mode-network.shtest/docker_tests/test_ports_available.py
📚 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:
test/docker_tests/test_ports_available.pytest/docker_tests/test_docker_compose_scenarios.pydocs/docker-troubleshooting/nginx-configuration-mount.md
📚 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:
test/docker_tests/test_ports_available.pyinstall/production-filesystem/entrypoint.d/0-storage-permission.sh
📚 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:
test/docker_tests/test_ports_available.py
📚 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:
test/docker_tests/test_ports_available.py
📚 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:
docs/docker-troubleshooting/nginx-configuration-mount.md
📚 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:
docs/docker-troubleshooting/nginx-configuration-mount.mdinstall/production-filesystem/entrypoint.d/0-storage-permission.sh
🧬 Code graph analysis (1)
test/docker_tests/test_ports_available.py (1)
test/docker_tests/test_container_environment.py (4)
_setup_mount_tree(94-149)_build_volume_args(166-176)_run_container(187-279)_assert_contains(283-293)
🪛 LanguageTool
docs/docker-troubleshooting/nginx-configuration-mount.md
[style] ~15-~15: Consider using a different verb for a more formal wording.
Context: ...n running in read-only mode. ## How to Correct the Issue If you want to use a custom ...
(FIX_RESOLVE)
🪛 Ruff (0.14.2)
test/docker_tests/test_mount_diagnostics_pytest.py
1-1: Shebang is present but file is not executable
(EXE001)
12-12: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
12-12: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
13-13: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
13-13: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
21-21: Docstring contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
43-43: Comment contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
44-44: Comment contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
86-86: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
160-160: subprocess call: check for execution of untrusted input
(S603)
161-161: Starting a process with a partial executable path
(S607)
309-309: Unused function argument: netalertx_test_image
(ARG001)
324-324: subprocess call: check for execution of untrusted input
(S603)
331-331: subprocess call: check for execution of untrusted input
(S603)
345-345: subprocess call: check for execution of untrusted input
(S603)
346-346: Starting a process with a partial executable path
(S607)
352-352: subprocess call: check for execution of untrusted input
(S603)
353-353: Starting a process with a partial executable path
(S607)
364-364: subprocess call: check for execution of untrusted input
(S603)
365-365: Starting a process with a partial executable path
(S607)
381-381: subprocess call: check for execution of untrusted input
(S603)
415-415: subprocess call: check for execution of untrusted input
(S603)
423-423: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
423-423: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
test/docker_tests/test_ports_available.py
43-43: subprocess call: check for execution of untrusted input
(S603)
44-44: Starting a process with a partial executable path
(S607)
56-56: subprocess call: check for execution of untrusted input
(S603)
56-56: Starting a process with a partial executable path
(S607)
70-70: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
107-107: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
131-131: subprocess call: check for execution of untrusted input
(S603)
153-153: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
157-161: Avoid specifying long messages outside the exception class
(TRY003)
164-164: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
168-172: Avoid specifying long messages outside the exception class
(TRY003)
220-220: Unused function argument: dummy_container
(ARG001)
test/docker_tests/test_docker_compose_scenarios.py
30-30: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
52-52: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
75-75: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
117-117: Unused function argument: env_vars
(ARG001)
117-117: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
129-129: subprocess call: check for execution of untrusted input
(S603)
140-140: subprocess call: check for execution of untrusted input
(S603)
140-140: Starting a process with a partial executable path
(S607)
145-145: subprocess call: check for execution of untrusted input
(S603)
145-145: Starting a process with a partial executable path
(S607)
268-268: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
273-273: f-string without any placeholders
Remove extraneous f prefix
(F541)
274-274: f-string without any placeholders
Remove extraneous f prefix
(F541)
275-275: f-string without any placeholders
Remove extraneous f prefix
(F541)
276-276: f-string without any placeholders
Remove extraneous f prefix
(F541)
321-321: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
326-326: f-string without any placeholders
Remove extraneous f prefix
(F541)
327-327: f-string without any placeholders
Remove extraneous f prefix
(F541)
328-328: f-string without any placeholders
Remove extraneous f prefix
(F541)
329-329: f-string without any placeholders
Remove extraneous f prefix
(F541)
🪛 Shellcheck (0.11.0)
test/docker_tests/configurations/test_all_docker_composes.sh
[warning] 51-51: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 55-55: To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify).
(SC2069)
[error] 70-70: Parsing stopped here. Is this keyword correctly matched up?
(SC1089)
install/production-filesystem/entrypoint.d/0-storage-permission.sh
[error] 57-57: Missing ';' or + terminating -exec. You can't use |/||/&&, and ';' has to be a separate, quoted argument.
(SC2067)
[error] 58-58: Missing ';' or + terminating -exec. You can't use |/||/&&, and ';' has to be a separate, quoted argument.
(SC2067)
🔇 Additional comments (9)
docs/docker-troubleshooting/nginx-configuration-mount.md (1)
1-36: Comprehensive troubleshooting guide with clear remediation steps.The documentation clearly explains the issue, security implications, root cause, and provides actionable remediation with specific commands and configuration snippets. Content aligns well with the PR's goal of making errors more helpful with direct remediation guidance.
The remediation steps reference correct mount paths (
/app/system/services/active/config), user ID (20211), and permissions, which align with the mount-configuration status system introduced in this PR.Note: The past review flagged a duplicate "Additional Resources" section at lines 37–38, but that's not visible in the current code, suggesting it's already been addressed. If the duplicate remains beyond the visible range, remove the redundant block to avoid repetition.
install/production-filesystem/entrypoint.d/80-host-mode-network.sh (3)
47-66: Stdout/stderr redirection correctly restored.The diagnostic warning messages on lines 49, 50, and 65 now correctly use
>&2redirection to send output to stderr. This aligns with Unix conventions and is consistent with other diagnostic scripts in the entrypoint.d directory, addressing the issue flagged in the previous review.
2-2: Documentation link aligns with PR objectives.The updated comment on Line 2 and the new documentation link on Line 62 are well-positioned to help users troubleshoot network configuration issues. This supports the PR's goal of providing actionable remediation steps via direct documentation links.
Also applies to: 62-62
43-66: Warning-only behavior is appropriate.The script correctly exits with status 0, allowing the container to start despite non-optimal networking configuration. This follows the pattern of diagnostic-only checks (per the learnings from check-ramdisk.sh) and gives users the option to reconfigure while still running the application.
install/production-filesystem/entrypoint.d/0-storage-permission.sh (1)
1-51: Security alert and initialization look good. The comprehensive warning with direct documentation link and clear remediation guidance aligns well with the PR's goal of making error messages more helpful. The POSIX shell constructs and environment variable setup are sound.test/docker_tests/test_ports_available.py (1)
220-240: Fixture parameterdummy_containeris required for test execution.While Ruff flags
dummy_containeras unused (ARG001), this fixture parameter is essential—it ensures the dummy container is started before the test runs. Removing it would break the test's port-conflict verification. The static analysis hint is a false positive.test/docker_tests/test_mount_diagnostics_pytest.py (3)
293-303: LGTM: active_config_ scenarios now execute correctly.The
active_config_block is properly structured as a peerelifalongside other path handlers (db_, config_, api_, log_, run_), ensuring these test scenarios execute as intended.
376-407: LGTM: Live-container diagnostics path is now reachable.The diagnostic execution and assertions for running containers are correctly unindented and will execute when containers remain up, properly validating the mount diagnostic table output.
86-86: Unicode characters are intentional for table parsing.The ➖ (HEAVY MINUS SIGN) characters flagged by Ruff (RUF001-003) are intentional—they match the emoji output format of the mount diagnostic table from 10-mounts.py. These should not be changed to ASCII hyphens.
| local dirname=$(dirname "$file") | ||
| local basename=$(basename "$file") |
| find "$SCRIPT_DIR" -name "docker-compose*.yml" -type f | sort | while read -r file; do | ||
| extract_comments "$file" | ||
| run_test "$file" | ||
| done |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
test/docker_tests/configurations/test_all_docker_composes.sh (1)
48-48: Fix stderr/stdout redirection order.The redirect
2>&1 >> "$LOG_FILE"sends stderr to the original stdout before redirecting stdout to the file, causing stderr to be lost. Move2>&1to come after the file redirect.- timeout 10s docker-compose -f "$basename" up 2>&1 >> "$LOG_FILE" + timeout 10s docker-compose -f "$basename" up >> "$LOG_FILE" 2>&1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/docker_tests/configurations/test_all_docker_composes.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
test/docker_tests/configurations/test_all_docker_composes.sh
[error] 37-37: Couldn't parse this brace group. Fix to allow more checks.
(SC1073)
[error] 61-61: Expected a '}'. If you have one, try a ; or \n in front of it.
(SC1056)
[error] 61-61: Missing '}'. Fix any mentioned problems and try again.
(SC1072)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/docker_tests/test_docker_compose_scenarios.py (2)
279-282: Remove unnecessary f-string prefixes.These strings contain no placeholders; the
fprefix is unnecessary.Apply this diff:
"volumes": [ - f"./test_data/app_log:/app/log", - f"./test_data/app_api:/app/api", - f"./test_data/nginx_conf:/services/config/nginx/conf.active", - f"./test_data/services_run:/services/run" + "./test_data/app_log:/app/log", + "./test_data/app_api:/app/api", + "./test_data/nginx_conf:/services/config/nginx/conf.active", + "./test_data/services_run:/services/run" ],
332-335: Remove unnecessary f-string prefixes.These strings contain no placeholders; the
fprefix is unnecessary.Apply this diff:
"volumes": [ - f"./test_data/app_log:/app/log", - f"./test_data/app_api:/app/api", - f"./test_data/nginx_conf:/services/config/nginx/conf.active", - f"./test_data/services_run:/services/run" + "./test_data/app_log:/app/log", + "./test_data/app_api:/app/api", + "./test_data/nginx_conf:/services/config/nginx/conf.active", + "./test_data/services_run:/services/run" ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/docker_tests/test_docker_compose_scenarios.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_docker_compose_scenarios.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_docker_compose_scenarios.py
🧠 Learnings (1)
📚 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:
test/docker_tests/test_docker_compose_scenarios.py
🪛 Ruff (0.14.2)
test/docker_tests/test_docker_compose_scenarios.py
30-30: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
52-52: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
75-75: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
134-134: subprocess call: check for execution of untrusted input
(S603)
146-146: subprocess call: check for execution of untrusted input
(S603)
146-146: Starting a process with a partial executable path
(S607)
151-151: subprocess call: check for execution of untrusted input
(S603)
151-151: Starting a process with a partial executable path
(S607)
274-274: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
279-279: f-string without any placeholders
Remove extraneous f prefix
(F541)
280-280: f-string without any placeholders
Remove extraneous f prefix
(F541)
281-281: f-string without any placeholders
Remove extraneous f prefix
(F541)
282-282: f-string without any placeholders
Remove extraneous f prefix
(F541)
327-327: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
332-332: f-string without any placeholders
Remove extraneous f prefix
(F541)
333-333: f-string without any placeholders
Remove extraneous f prefix
(F541)
334-334: f-string without any placeholders
Remove extraneous f prefix
(F541)
335-335: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (1)
test/docker_tests/test_docker_compose_scenarios.py (1)
117-156: LGTM! Environment variable handling is now correct.The
env_varsparameter is properly merged with the current environment and passed to allsubprocess.runcalls. This addresses the previous review concern.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
test/docker_tests/test_ports_available.py (4)
42-42: Remove redundant import.
subprocessis already imported at line 8. This duplicate import is unnecessary.Apply this diff:
- import subprocess
70-75: Rename unused loop variable.The
targetvariable fromVOLUME_MAP.items()is not used within the loop body. Rename it to_targetto signal it's intentionally unused.Apply this diff:
paths = {} - for key, target in VOLUME_MAP.items(): + for key, _target in VOLUME_MAP.items(): folder_name = f"{label}_{key.upper()}_INTENTIONAL_NETALERTX_TEST"
121-128: Consider safer script injection method.The current shell escaping pattern using
chr(39)+chr(92)+chr(39)+chr(39)is fragile and difficult to maintain. For injecting multi-line shell scripts, consider using base64 encoding or mounting the script as a volume instead.Here's a safer alternative using base64:
# Copy the script content and run it script_path = pathlib.Path("install/production-filesystem/entrypoint.d/99-ports-available.sh") with script_path.open('r', encoding='utf-8') as f: script_content = f.read() - # Use printf to avoid shell interpretation issues - script = f"printf '%s\\n' '{script_content.replace(chr(39), chr(39)+chr(92)+chr(39)+chr(39))}' > /tmp/ports-check.sh && chmod +x /tmp/ports-check.sh && sh /tmp/ports-check.sh" + # Use base64 to safely inject the script + import base64 + encoded = base64.b64encode(script_content.encode('utf-8')).decode('ascii') + script = f"echo '{encoded}' | base64 -d > /tmp/ports-check.sh && chmod +x /tmp/ports-check.sh && sh /tmp/ports-check.sh" cmd.extend(["--entrypoint", "/bin/sh", IMAGE, "-c", script])
153-153: Fix implicit Optional type hints.PEP 484 prohibits implicit
Optional. Use explicit union syntax for optional parameters.Apply this diff:
-def _assert_contains(result, snippet: str, cmd: list[str] = None) -> None: +def _assert_contains(result, snippet: str, cmd: list[str] | None = None) -> None: """Assert that the result output contains the given snippet.""" if snippet not in result.output: cmd_str = " ".join(cmd) if cmd else "" raise AssertionError( f"Expected to find '{snippet}' in container output.\n" f"Got:\n{result.output}\n" f"Container command:\n{cmd_str}" ) -def _assert_not_contains(result, snippet: str, cmd: list[str] = None) -> None: +def _assert_not_contains(result, snippet: str, cmd: list[str] | None = None) -> None: """Assert that the result output does not contain the given snippet."""Also applies to: 164-164
test/docker_tests/test_docker_compose_scenarios.py (2)
152-152: Consider using list unpacking for concatenation.For consistency with modern Python style, consider using unpacking syntax instead of
+concatenation.Apply this diff:
- up_cmd = cmd + ["up"] + up_cmd = [*cmd, "up"] if detached: up_cmd.append("-d") else: up_cmd.extend([ "--abort-on-container-exit", "--timeout", str(timeout) ]) # ... later in the function ... - logs_cmd = cmd + ["logs"] + logs_cmd = [*cmd, "logs"] logs_result = subprocess.run( # ... and at the end ... if detached: - logs_cmd_display = cmd + ["logs"] + logs_cmd_display = [*cmd, "logs"] print("[compose logs command]", " ".join(logs_cmd_display))Also applies to: 179-179, 230-230
371-374: Remove extraneous f-string prefixes.These strings don't contain any interpolation placeholders, so the
fprefix is unnecessary.Apply this diff:
"volumes": [ - f"./test_data/app_log:/app/log", - f"./test_data/app_api:/app/api", - f"./test_data/nginx_conf:/services/config/nginx/conf.active", - f"./test_data/services_run:/services/run" + "./test_data/app_log:/app/log", + "./test_data/app_api:/app/api", + "./test_data/nginx_conf:/services/config/nginx/conf.active", + "./test_data/services_run:/services/run" ],And similarly at lines 424-427:
"volumes": [ - f"./test_data/app_log:/app/log", - f"./test_data/app_api:/app/api", - f"./test_data/nginx_conf:/services/config/nginx/conf.active", - f"./test_data/services_run:/services/run" + "./test_data/app_log:/app/log", + "./test_data/app_api:/app/api", + "./test_data/nginx_conf:/services/config/nginx/conf.active", + "./test_data/services_run:/services/run" ],Also applies to: 424-427
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/docker_tests/test_docker_compose_scenarios.py(1 hunks)test/docker_tests/test_ports_available.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 'logger.mylog' for logging at levels: none, minimal, verbose, debug, or trace.
Files:
test/docker_tests/test_docker_compose_scenarios.pytest/docker_tests/test_ports_available.py
test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests must reside under 'test/' and use pytest.
Files:
test/docker_tests/test_docker_compose_scenarios.pytest/docker_tests/test_ports_available.py
🧠 Learnings (7)
📚 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:
test/docker_tests/test_docker_compose_scenarios.pytest/docker_tests/test_ports_available.py
📚 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:
test/docker_tests/test_docker_compose_scenarios.pytest/docker_tests/test_ports_available.py
📚 Learning: 2025-11-01T19:02:10.609Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1263
File: install/production-filesystem/entrypoint.sh:60-86
Timestamp: 2025-11-01T19:02:10.609Z
Learning: In the NetAlertX project (install/production-filesystem/entrypoint.sh), when fail-fast behavior is adopted for entrypoint checks in the future, be lenient during review because tests will be removed or changed to accommodate that behavior. The current continue-on-failure approach exists to support the existing test suite, but this is expected to change along with corresponding test adjustments.
Applied to files:
test/docker_tests/test_docker_compose_scenarios.pytest/docker_tests/test_ports_available.py
📚 Learning: 2025-09-20T14:08:44.292Z
Learnt from: adamoutler
Repo: jokob-sk/NetAlertX PR: 1184
File: .devcontainer/scripts/stream-logs.sh:5-6
Timestamp: 2025-09-20T14:08:44.292Z
Learning: The .devcontainer/scripts/stream-logs.sh script in NetAlertX is designed as a diagnostic tool for troubleshooting devcontainer startup issues. When log files don't exist, this indicates that the executable/services didn't start properly, which is valuable diagnostic information. Pre-creating missing files would mask this diagnostic behavior.
Applied to files:
test/docker_tests/test_docker_compose_scenarios.pytest/docker_tests/test_ports_available.py
📚 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:
test/docker_tests/test_ports_available.py
📚 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:
test/docker_tests/test_ports_available.py
📚 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:
test/docker_tests/test_ports_available.py
🧬 Code graph analysis (1)
test/docker_tests/test_ports_available.py (1)
test/docker_tests/test_container_environment.py (4)
_setup_mount_tree(94-149)_build_volume_args(166-176)_run_container(187-279)_assert_contains(283-293)
🪛 Ruff (0.14.2)
test/docker_tests/test_docker_compose_scenarios.py
33-33: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
55-55: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
85-85: Probable insecure usage of temporary file or directory: "/tmp:uid=20211,gid=20211,mode=1700,rw,noexec,nosuid,nodev,async,noatime,nodiratime"
(S108)
152-152: Consider [*cmd, "up"] instead of concatenation
Replace with [*cmd, "up"]
(RUF005)
168-168: subprocess call: check for execution of untrusted input
(S603)
179-179: Consider [*cmd, "logs"] instead of concatenation
Replace with [*cmd, "logs"]
(RUF005)
180-180: subprocess call: check for execution of untrusted input
(S603)
198-198: subprocess call: check for execution of untrusted input
(S603)
210-210: subprocess call: check for execution of untrusted input
(S603)
210-210: Starting a process with a partial executable path
(S607)
215-215: subprocess call: check for execution of untrusted input
(S603)
215-215: Starting a process with a partial executable path
(S607)
230-230: Consider [*cmd, "logs"] instead of concatenation
Replace with [*cmd, "logs"]
(RUF005)
366-366: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
371-371: f-string without any placeholders
Remove extraneous f prefix
(F541)
372-372: f-string without any placeholders
Remove extraneous f prefix
(F541)
373-373: f-string without any placeholders
Remove extraneous f prefix
(F541)
374-374: f-string without any placeholders
Remove extraneous f prefix
(F541)
419-419: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
424-424: f-string without any placeholders
Remove extraneous f prefix
(F541)
425-425: f-string without any placeholders
Remove extraneous f prefix
(F541)
426-426: f-string without any placeholders
Remove extraneous f prefix
(F541)
427-427: f-string without any placeholders
Remove extraneous f prefix
(F541)
test/docker_tests/test_ports_available.py
43-43: subprocess call: check for execution of untrusted input
(S603)
44-44: Starting a process with a partial executable path
(S607)
56-56: subprocess call: check for execution of untrusted input
(S603)
56-56: Starting a process with a partial executable path
(S607)
70-70: Loop control variable target not used within loop body
Rename unused target to _target
(B007)
107-107: Probable insecure usage of temporary file or directory: "/tmp:mode=777"
(S108)
131-131: subprocess call: check for execution of untrusted input
(S603)
153-153: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
157-161: Avoid specifying long messages outside the exception class
(TRY003)
164-164: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
168-172: Avoid specifying long messages outside the exception class
(TRY003)
220-220: Unused function argument: dummy_container
(ARG001)
🔇 Additional comments (2)
test/docker_tests/test_ports_available.py (1)
220-240: LGTM: Fixture usage is correct.The
dummy_containerparameter appears unused in the function body, but this is the correct pytest pattern—the fixture's side effect (occupying ports 20211 and 20212) is what the test relies on. The static analysis warning can be safely ignored.test/docker_tests/test_docker_compose_scenarios.py (1)
291-342: LGTM: Test assertions are now consistent.The test correctly verifies that a properly configured startup shows success indicators in the mounts table without critical errors or permission issues. The assertions align with the expected behavior.
|
I think CodeRabbit is done @jokob-sk . I'm fine with this if you are. It does not include fail-fast. Errors will continue until they cause the system to attempt to start, fail to start, and then reboot instead of failing immediately and rebooting. I will rework the tests for a fail fast soon. That would be a separate reworking required. |
|
@jokob-sk I realized there were changes to the /Dockerfile already in this so I addressed that request from earlier with making the scripts executable instead of needing to run |
2 participants
🎯 Pull Request: Enhanced Error Visibility and Comprehensive Testing
Overview
This PR significantly improves NetAlertX's error reporting and diagnostic capabilities by introducing a visual status grid, comprehensive documentation links, and extensive unit testing. The primary feature is an intuitive ✅/❌/➖ status table that makes configuration issues immediately visible to users.
Key Features
🏗️ Visual Status Grid
📚 Comprehensive Documentation
🧪 Extensive Test Suite
Technical Changes
Script Reorganization
/entrypoint.d/directoryEnhanced Diagnostics
10-mounts.py)Testing Infrastructure
Example Output
Impact
Testing
This PR transforms NetAlertX's error handling from cryptic messages to actionable, visually clear diagnostics that guide users to correct configurations quickly and confidently.
I will check back tomorrow on CodeRabbit analysis.
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes