feat: Devcontainer #1184
feat: Devcontainer #1184
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds a full devcontainer setup with a generated multi-stage Dockerfile, VS Code configs, Xdebug settings, and scripts to build, start, and test services. Introduces nginx/PHP-FPM configuration, backend debug attach, test wiring via pytest, and documentation describing workflows. Minor repo updates include ignore rules and pytest discovery config. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer (VS Code)
participant DC as DevContainer
participant SU as setup.sh
participant NG as nginx
participant PF as php-fpm
participant BE as Python Backend (debugpy)
Dev->>DC: Open workspace (devcontainer.json)
DC->>DC: Build multi-stage Dockerfile (target=devcontainer)
DC->>SU: postStartCommand: setup.sh
SU->>SU: Provision /app (links, assets, perms)
SU->>PF: Start php-fpm
SU->>NG: Start nginx
SU->>BE: Launch backend with debugpy (0.0.0.0:5678)
Dev-->>BE: Attach debugpy (localhost:5678)
Dev-->>NG: Open forwarded port (frontend)
Note over NG,PF: PHP requests proxied to php-fpm
sequenceDiagram
autonumber
participant Gen as generate-dockerfile.sh
participant Root as Root Dockerfile
participant Frag as resources/devcontainer-Dockerfile
participant Out as .devcontainer/Dockerfile
Gen->>Root: Read & filter (remove COPY ${INSTALL_DIR})
Gen->>Out: Write autogenerated header + filtered content
Gen->>Out: Patch lines (aiofreepybox, cert COPY)
Gen->>Frag: Append devcontainer stage
Frag-->>Out: Final combined Dockerfile
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 10
🧹 Nitpick comments (29)
front/.gitignore (1)
1-1: Also add to Docker ignore to keep images lean.
.gitignore doesn’t affect Docker build context. If any Dockerfile doesCOPY .(or builds from front/), add the same rule to front/.dockerignore to avoid shipping this artifact into images.# front/.dockerignore +buildtimestamp.txtOptionally, if buildtimestamp.txt can appear elsewhere, centralize with a root .gitignore pattern like
**/buildtimestamp.txt..github/copilot-instructions.md (6)
5-11: Clarify GraphQL vs REST (avoid misleading scope).API map below only documents Flask/REST. If GraphQL isn’t actually shipped, drop it here; otherwise reference concrete GraphQL entry points/files.
Apply if REST-only:
- - Backend (Python): main loop + GraphQL/REST endpoints orchestrate scans, plugins, workflows, notifications, and JSON export. + - Backend (Python): main loop + REST endpoints orchestrate scans, plugins, workflows, notifications, and JSON export.
23-26: Add minimal auth security guardrails for API usage.Make expectations explicit (HTTPS, no query-string tokens, standard failures).
- Authorization: all routes expect header `Authorization: Bearer <API_TOKEN>` via `get_setting_value('API_TOKEN')`. + - Authorization: all routes expect header `Authorization: Bearer <API_TOKEN>` via `get_setting_value('API_TOKEN')`. + - Use HTTPS only; never send tokens via query params or in logs. + - Return 401 (missing/invalid) vs 403 (authenticated but not allowed) consistently. + - Consider basic rate‑limiting on write endpoints and verify `Origin/Referer` for browser calls.
28-32: Strengthen guidance on secrets/logging.Call out secret handling and log masking to prevent accidental leakage.
- Settings: add/modify via `ccd()` in `server/initialise.py` or per‑plugin manifest. Never hardcode ports or secrets; use `get_setting_value()`. + - Settings: add/modify via `ccd()` in `server/initialise.py` or per‑plugin manifest. Never hardcode ports or secrets; use `get_setting_value()`. Do not log secrets; mask values at source.
33-37: Align pytest/devcontainer wording with actual base image.Text mentions “Alpine packages”; devcontainer may be Debian/Ubuntu. Prefer base‑agnostic phrasing.
- - Testing: pytest available via Alpine packages. Tests live in `test/`; app code is under `server/`. PYTHONPATH is preconfigured to include workspace and `/opt/venv` site‑packages. + - Testing: pytest is preinstalled in the devcontainer. Tests live in `test/`; app code is under `server/`. PYTHONPATH is preconfigured to include the workspace and `/opt/venv` site‑packages.
43-46: Verify referenced docs exist; add devcontainer doc link.Ensure these files are present in this PR; if there’s a dedicated devcontainer doc/README, link it here.
- Docs: `docs/PLUGINS_DEV.md`, `docs/SETTINGS_SYSTEM.md`, `docs/API_*.md`, `docs/DEBUG_*.md` + - Docs: `docs/PLUGINS_DEV.md`, `docs/SETTINGS_SYSTEM.md`, `docs/API_*.md`, `docs/DEBUG_*.md`, `docs/DEVCONTAINER.md` (or `.devcontainer/README.md`)
45-45: Casing consistency (nit).Use consistent “Nginx/PHP‑FPM” casing as used earlier.
- - Logs: backend `/app/log/app.log`, plugin logs under `/app/log/plugins/`, nginx/php logs under `/var/log/*` + - Logs: backend `/app/log/app.log`, plugin logs under `/app/log/plugins/`, Nginx/PHP‑FPM logs under `/var/log/*`.devcontainer/README.md (1)
10-14: Call out required port forwards (5678, 9003, 20211) and Linux note for host.docker.internal.Please add a short note that the devcontainer exposes/forwards:
- 20211 (nginx), 5678 (debugpy), 9003 (Xdebug).
On Linux, host.docker.internal may not exist depending on Docker; clarify that xdebug.discover_client_host=true mitigates this..devcontainer/resources/netalertx-devcontainer.conf (1)
21-25: Use Xdebug 3 settings instead of deprecated remote_enable.xdebug.remote_enable is a v2 directive. Prefer Xdebug 3 keys to avoid confusion.
Apply this diff:
- fastcgi_param PHP_VALUE "xdebug.remote_enable=1"; + fastcgi_param PHP_VALUE "xdebug.mode=debug\nxdebug.start_with_request=yes";.devcontainer/scripts/setup.sh (4)
28-31: Avoid hard‑coding TZ; respect host/user configuration.Hard-setting TZ=Europe/Paris can surprise non‑EU contributors and tests. Prefer inheriting TZ or default to UTC if unset.
Apply this diff:
-export TZ=Europe/Paris +export TZ="${TZ:-UTC}"
161-164: Limit nginx fastcgi_pass rewrite to the intended file.Wildcard editing of /etc/nginx/http.d/*.conf is brittle. Use NGINX_CONFIG_FILE.
Apply this diff:
- sudo sed -i 's|fastcgi_pass .*|fastcgi_pass 127.0.0.1:9000;|' /etc/nginx/http.d/*.conf + sudo sed -i 's|fastcgi_pass .*|fastcgi_pass 127.0.0.1:9000;|' "${NGINX_CONFIG_FILE}"
141-149: Port wait uses ss; provide a fallback for minimal images.ss may be absent. Fallback to netstat or skip check.
Apply this diff:
- while ss -ltn | grep -q ":${PORT}[[:space:]]" && [ $tries -lt 10 ]; do + while { ss -ltn 2>/dev/null || netstat -ltn 2>/dev/null || true; } | grep -q ":${PORT}[[:space:]]" && [ $tries -lt 10 ]; do
21-23: Consistency: LOGS_LOCATION is unused and differs from /app/log.Either remove or set it to /app/log to avoid confusion.
Apply this diff:
-export LOGS_LOCATION=/app/logs +export LOGS_LOCATION=/app/log.vscode/settings.json (1)
11-13: JSON comments trip strict linters (Biome parse error).VS Code accepts comments but Biome flagged this file. Either remove comments or exclude .vscode from Biome.
Option A (remove comments):
- // Let the Python extension invoke pytest via the interpreter; avoid hardcoded paths - // Removed python.testing.pytestPath and legacy pytest.command overridesOption B: ignore via Biome config:
- Add an ignore rule for ".vscode/*.json".
.devcontainer/xdebug-trigger.ini (1)
1-11: Deduplicate Xdebug config to a single active .ini.This coexists with 99-xdebug.ini; duplicate/contradicting keys can confuse troubleshooting. Keep one source of truth and ensure setup.sh copies the intended file.
.devcontainer/scripts/run-tests.sh (1)
9-13: Make test runner a bit more robust (exec + sanity check).Minor hardening so failures propagate and we don’t run against a missing tests dir.
# Make sure PYTHONPATH includes server and workspace export PYTHONPATH="/workspaces/NetAlertX:/workspaces/NetAlertX/server:/app:/app/server:${PYTHONPATH:-}" # Default to running the full test suite under /workspaces/NetAlertX/test -pytest -q --maxfail=1 --disable-warnings test "$@" +test -d test || { echo "test directory not found at $(pwd)/test" >&2; exit 1; } +exec pytest -q --maxfail=1 --disable-warnings test "$@".devcontainer/scripts/restart-backend.sh (2)
6-12: Use strict mode and log output; remove unused vars.ShellCheck flagged unused LOG_DIR/PID. Also, without logs it’s hard to diagnose startup issues.
-set -e +set -eu - -LOG_DIR=/app/log +LOG_DIR=/app/log APP_DIR=/app/server PY=python3 PORT_DEBUG=5678 @@ -setsid nohup ${PY} -m debugpy --listen 0.0.0.0:${PORT_DEBUG} /app/server/__main__.py >/dev/null 2>&1 & -PID=$! +mkdir -p "$LOG_DIR" +setsid nohup "${PY}" -m debugpy --listen "0.0.0.0:${PORT_DEBUG}" /app/server/__main__.py \ + >>"$LOG_DIR/backend.out" 2>&1 & +echo $! > "$LOG_DIR/backend.pid" sleep 2Also applies to: 20-23
2-4: Update the task name in the comment.Script says it’s invoked by “Restart GraphQL” but the task is “Start Backend (Python)”.
.devcontainer/devcontainer.json (1)
23-33: Remove deprecated/duplicate PHP debug extension.
felixfbecker.php-debugis deprecated and can conflict withxdebug.php-debug."extensions": [ "ms-python.python", "ms-azuretools.vscode-docker", - "felixfbecker.php-debug", "bmewburn.vscode-intelephense-client", "xdebug.php-debug", "ms-python.vscode-pylance", "pamaron.pytest-runner", "coderabbit.coderabbit-vscode", "ms-python.black-formatter" ].devcontainer/scripts/generate-dockerfile.sh (3)
6-6: Enable strict mode.
[raise_ninor_issue]-# Generator for .devcontainer/Dockerfile +set -eu +# Generator for .devcontainer/Dockerfile
14-17: Silence ShellCheck SC1007 and make path resolution clearer.-SCRIPT_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)" +SCRIPT_DIR="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd)" DEVCONTAINER_DIR="${SCRIPT_DIR%/scripts}" ROOT_DIR="${DEVCONTAINER_DIR%/.devcontainer}"
24-31: Make sed transforms less brittle.Anchor the COPY deletion and relax the certificate path match for future Python updates.
-sed '/${INSTALL_DIR}/d' "${ROOT_DIR}/Dockerfile" >> "$OUT_FILE" +sed -E '/^[[:space:]]*COPY[[:space:]]+\.[[:space:]]+\$\{INSTALL_DIR\}\//d' "${ROOT_DIR}/Dockerfile" >> "$OUT_FILE" @@ -# don't cat the file, just copy it in because it doesn't exist at build time -sed -i 's|^ RUN cat ${INSTALL_DIR}/install/freebox_certificate.pem >> /opt/venv/lib/python3.12/site-packages/aiofreepybox/freebox_certificates.pem$| COPY install/freebox_certificate.pem /opt/venv/lib/python3.12/site-packages/aiofreepybox/freebox_certificates.pem |' "$OUT_FILE" +# don't cat the file, just copy it in because it doesn't exist at build time +sed -i -E 's|^[[:space:]]*RUN[[:space:]]+cat[[:space:]]+\$\{INSTALL_DIR\}/install/freebox_certificate.pem[[:space:]]*>>[[:space:]]*/opt/venv/lib/python.*/site-packages/aiofreepybox/freebox_certificates.pem$| COPY install/freebox_certificate.pem /opt/venv/lib/python3.12/site-packages/aiofreepybox/freebox_certificates.pem |' "$OUT_FILE".devcontainer/Dockerfile (2)
73-81: Avoid duplicate pytest installs and unnecessary sudo in build.You already installed pytest via apk; the additional pip install (and
sudoin a root build stage) is redundant.-RUN apk add --no-cache git nano vim jq php83-pecl-xdebug py3-pip nodejs sudo gpgconf pytest pytest-cov && \ +RUN apk add --no-cache git nano vim jq php83-pecl-xdebug py3-pip nodejs sudo gpgconf pytest pytest-cov && \ @@ -# setup pytest -RUN sudo /opt/venv/bin/python -m pip install -U pytest pytest-cov +# setup pytest (already installed via apk; keep pip upgrade only if needed) +# RUN /opt/venv/bin/python -m pip install -U pytest pytest-covAlso applies to: 106-108
112-112: Keep container alive reliably on Alpine.
sleep infinitycan be flaky with BusyBox. Use a portable pattern.-ENTRYPOINT ["/bin/sh","-c","sleep infinity"] +ENTRYPOINT ["/bin/sh","-c","tail -f /dev/null"].vscode/tasks.json (2)
61-66: Start frontend tasks: consider explicit daemonization.Starting both in background is fine; ensure nginx doesn’t fail silently. Optional: add
-g 'daemon off;'and run php-fpm in foreground within setsid/supervision if needed.
78-81: Stop task: safer/consistent process stop.
pkill -f 'php-fpm83|nginx|crond|python3'may treat|literally depending on pkill. Use killall for clarity.-"command": "pkill -f 'php-fpm83|nginx|crond|python3' || true", +"command": "killall php-fpm83 nginx crond python3 2>/dev/null || true",.devcontainer/resources/devcontainer-Dockerfile (3)
11-17: Tooling set: verify package names on Alpine and avoid duplication.
gpgconfmay be part ofgnupgon Alpine; also pytest is installed twice (apk and later pip).-RUN apk add --no-cache git nano vim jq php83-pecl-xdebug py3-pip nodejs sudo gpgconf pytest pytest-cov && \ +RUN apk add --no-cache git nano vim jq php83-pecl-xdebug py3-pip nodejs sudo gnupg pytest pytest-cov && \ @@ -# setup pytest -RUN sudo /opt/venv/bin/python -m pip install -U pytest pytest-cov +# setup pytest (already installed via apk; keep pip upgrade only if needed) +# RUN /opt/venv/bin/python -m pip install -U pytest pytest-cov
9-10: Avoid hard-coding Python minor version in PYTHONPATH.This will break on Python upgrades.
-ENV PYTHONPATH=/workspaces/NetAlertX/test:/workspaces/NetAlertX/server:/app:/app/server:/opt/venv/lib/python3.12/site-packages +ENV PYTHONPATH=/workspaces/NetAlertX/test:/workspaces/NetAlertX/server:/app:/app/serverYou already export PYTHONPATH dynamically in run-tests.sh.
51-51: Same keep-alive reliability note as above.-ENTRYPOINT ["/bin/sh","-c","sleep infinity"] +ENTRYPOINT ["/bin/sh","-c","tail -f /dev/null"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.devcontainer/Dockerfile(1 hunks).devcontainer/README.md(1 hunks).devcontainer/devcontainer.json(1 hunks).devcontainer/resources/99-xdebug.ini(1 hunks).devcontainer/resources/devcontainer-Dockerfile(1 hunks).devcontainer/resources/netalertx-devcontainer.conf(1 hunks).devcontainer/scripts/generate-dockerfile.sh(1 hunks).devcontainer/scripts/restart-backend.sh(1 hunks).devcontainer/scripts/run-tests.sh(1 hunks).devcontainer/scripts/setup.sh(1 hunks).devcontainer/scripts/stream-logs.sh(1 hunks).devcontainer/xdebug-trigger.ini(1 hunks).github/copilot-instructions.md(1 hunks).vscode/launch.json(1 hunks).vscode/settings.json(1 hunks).vscode/tasks.json(1 hunks)front/.gitignore(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
.devcontainer/scripts/generate-dockerfile.sh
[warning] 14-14: Remove space after = if trying to assign a value (for empty string, use var='' ... ).
(SC1007)
.devcontainer/scripts/setup.sh
[warning] 100-100: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
.devcontainer/scripts/restart-backend.sh
[warning] 8-8: LOG_DIR appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 22-22: PID appears unused. Verify use (or export if used externally).
(SC2034)
🪛 Biome (2.1.2)
.vscode/settings.json
[error] 12-13: Expected a property but instead found '}'.
Expected a property here.
(parse)
🔇 Additional comments (5)
front/.gitignore (1)
1-1: LGTM — ignore rule looks correct.
No functional risks; safe to merge..github/copilot-instructions.md (1)
47-49: LGTM overall — concise, actionable guidance.Clear paths, helpers, and validation expectations; will help keep contributions aligned.
pyproject.toml (1)
1-5: Pytest discovery config looks good.Patterns align with test/ layout and VS Code settings.
If you have async tests, consider adding:
asyncio_mode = "auto"under [tool.pytest.ini_options]..vscode/launch.json (2)
4-23: Debugpy attach mapping LGTM.Port and path mappings align with /app layout and restart-backend.sh usage.
Confirm that devcontainer forwards 5678 and that restart-backend.sh starts debugpy in listen mode.
24-32: PHP Xdebug listen config LGTM.Port and mapping are consistent with nginx/php-fpm and Xdebug settings.
|
Coderabit doesn't understand the difference between a deployment container and a devcontainer. Every comment is irrelevant, and there's basically 0-chance I'm going to Unit Test a devcontainer. Don't merge this yet BTW. I am missing some resources in the repository. For sure the app.conf and something else. |
|
Coderabit dockerfile sequence diagram is wrong sequenceDiagram
autonumber
participant Gen as "generate-dockerfile.sh"
participant Root as "Production Dockerfile"
participant Frag as "resources/devcontainer-Dockerfile"
participant Out as ".devcontainer/Dockerfile"
Gen->>Out: Create/overwrite with auto-gen header
Gen->>Root: Read and filter out lines with INSTALL_DIR
Root-->>Gen: Return filtered content
Gen->>Out: Append filtered content
Gen->>Out: Modify in-place patch aiofreepybox and cert lines
Gen->>Frag: Read content
Frag-->>Gen: Return content
Gen->>Out: Append fragment content
|
…prevent exception.
|
During initialization of the devcontainer this became a problem. Empty string works fine. |
adamoutler
left a comment
adamoutler
left a comment
There was a problem hiding this comment.
Review complete.
|
@adamoutler ready to merge? |
|
Almost. Still verifying some things. |
|
No rush, just Ping me when you think it's ready 👍 |
|
I'm sorry. Not ready yet. I decided to start working in the devcontainer on an ntopng plugin to ensure things were working properly. However, I found a problem with the devcontainer and stashed my changes, then I couldn't get the container working properly. Turns out the backend was crashing out because it found a folder, but no |
|
Take all the time you need - and especially, please don't apologise - I'm grateful for the time and effort you put into this. |
|
Good to go. |
|
I copied over your overview into the docs: https://github.com/jokob-sk/NetAlertX/blob/main/docs/DEV_DEVCONTAINER.md |
2 participants
This pull request introduces a Visual Studio Code Development Container to the NetAlertX project. This is a significant enhancement that solves the "it works on my machine" problem by providing every developer with a consistent, pre-configured, and reproducible development environment that runs inside Docker.
For anyone new to devcontainers, think of it as a development environment in a box. It contains all the tools, extensions, and configurations needed to work on NetAlertX, ensuring that every contributor has the exact same setup, regardless of their local operating system. This drastically simplifies onboarding and debugging.
An Owner's Manual for the NetAlertX Devcontainer
This devcontainer is designed to mirror the production container environment as closely as possible, while providing a rich set of tools for development.
How to Get Started
Prerequisites:
Launch the Devcontainer:
Key Workflows & Features
Once you're inside the container, everything is set up for you.
1. Services (Frontend & Backend)
The container's startup script (
.devcontainer/scripts/setup.sh) automatically starts the Nginx/PHP frontend and the Python backend. You can restart them at any time using the built-in tasks.2. Integrated Debugging (Just Press F5!)
Debugging for both the Python backend and PHP frontend is pre-configured and ready to go.
5678. Simply open a Python file (e.g.,server/__main__.py), set a breakpoint, and press F5 (or select "Python Backend Debug: Attach") to connect the debugger.9003. In VS Code, start listening for Xdebug connections and use a browser extension (like "Xdebug helper") to start a debugging session for the web UI.3. Common Tasks (F1 -> Run Task)
We've created several VS Code Tasks to simplify common operations. Access them by pressing
F1and typing "Tasks: Run Task".Generate Dockerfile: This is important. The actual.devcontainer/Dockerfileis auto-generated. If you need to change the container environment, edit.devcontainer/resources/devcontainer-Dockerfileand then run this task.Re-Run Startup Script: Manually re-runs the.devcontainer/scripts/setup.shscript to re-link files and restart services.Start Backend (Python)/Start Frontend (nginx and PHP-FPM): Manually restart the services if needed.4. Running Tests
The environment includes
pytest. You can run tests directly from the VS Code Test Explorer UI or by runningpytest -qin the integrated terminal. The necessaryPYTHONPATHis already configured so that tests can correctly import the server modules.How to Maintain This Devcontainer
The setup is designed to be easy to manage. Here are the core principles:
DockerfileDirectly: The main.devcontainer/Dockerfileis a combination of the project's rootDockerfileand a special dev-only stage. To add new tools or dependencies, edit.devcontainer/resources/devcontainer-Dockerfileand then run theGenerate Dockerfiletask.apk add), add them to the resource Dockerfile..devcontainer/scripts/setup.sh..github/copilot-instructions.mdfile is an excellent resource to help AI and humans understand the project's architecture, conventions, and how to use existing helper functions instead of hardcoding values.This setup provides a powerful and consistent foundation for all current and future contributors to NetAlertX.
Summary by CodeRabbit
New Features
Documentation
Chores
✔️ Tasks
/docs