Bare metal Installer rework #1173
Conversation
split installer structure into systems, updated non-functional Debian12 installer with some minor fixes to Ubuntu24 installer. Updated docs.
WalkthroughDocs and installers were reorganized into OS-specific installers (Debian 12, Ubuntu 24). New INSTALLER_DIR usage centralizes paths; nginx config name standardized to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant I as installer (install.<os>.sh)
participant S as start script (start.<os>.sh)
participant N as NGINX
participant A as App (Python)
Note over U,I: Run OS-specific installer (Debian12 / Ubuntu24)
U->>I: run installer
I->>I: ensure INSTALL_DIR, set INSTALLER_DIR
I->>S: chmod +x ${INSTALLER_DIR}/start.<os>.sh
I->>S: execute start.<os>.sh
S->>N: stop nginx (if running)
S->>A: kill existing app server (if running)
S->>S: remove old UI & old nginx config (unprivileged)
S->>N: ln -s ${INSTALLER_DIR}/netalertx.conf -> /etc/nginx/...
S->>N: nginx -t
alt config OK
S->>N: start/reload nginx
S->>A: start app server
else config invalid
S-->>U: print error and abort
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
install/debian12/install_dependencies.debian12.sh (2)
15-24: Conflicting NGINX/PHP packages installed twice (light vs core).You install nginx-light and then also nginx/nginx-core/php8.2-fpm unconditionally. These conflict and can lead to unpredictable states. Pick one set or gate the “alternate dependencies” behind a flag.
Apply one of the following minimal fixes:
-# alternate dependencies -sudo apt-get install nginx nginx-core mtr php-fpm php8.2-fpm php-cli php8.2 php8.2-sqlite3 -y -sudo phpenmod -v 8.2 sqlite3 +# (optional) Alternate stack (nginx-core/php8.2). Enable only if desired. +# apt-get install -y nginx nginx-core mtr php-fpm php8.2-fpm php-cli php8.2 php8.2-sqlite3 +# phpenmod -v 8.2 sqlite3Or, if the core stack is the target, remove nginx-light from the primary list instead.
15-19: Missing apt-get update before installs.If this script is run standalone (and in start.debian12.sh it is), package installs may fail without an update.
# Install dependencies +apt-get update -y apt-get install -y \install/debian12/install.debian12.sh (1)
26-31: Unsafe directory wipe without confirmation.
rm -R $INSTALL_DIR/will nuke /app with no guardrails. The Ubuntu installer has safer prompts; mirror that here at minimum check path safety and require explicit intent.-# Clean the directory -rm -R $INSTALL_DIR/ +# Clean the directory (guarded) +if [ -d "$INSTALL_DIR" ]; then + echo "About to DELETE ALL contents in ${INSTALL_DIR}. Type 'install' to proceed or anything else to abort." + read -r choice + if [ "$choice" != "install" ]; then echo "Aborting."; exit 1; fi + [ -n "$INSTALL_DIR" ] && [ "$INSTALL_DIR" != "/" ] && rm -rf -- "$INSTALL_DIR/"* "$INSTALL_DIR"/.[!.]* "$INSTALL_DIR"/..?* +fiinstall/debian12/start.debian12.sh (1)
27-30: sed -i backup suffix bug (creates stray “e” backup files).
sed -iesets a backup suffix of “e”. Use-i -einstead.- sed -ie "s|Europe/Berlin|${TZ}|g" $INSTALL_PATH/config/$CONF_FILE + sed -i -e "s|Europe/Berlin|${TZ}|g" "$INSTALL_PATH/config/$CONF_FILE" - sed -ie "s|Europe/Berlin|${TZ}|g" $INSTALL_PATH/back/$CONF_FILE.bak + sed -i -e "s|Europe/Berlin|${TZ}|g" "$INSTALL_PATH/back/$CONF_FILE.bak" - sed -ie 's/listen /listen '"${LISTEN_ADDR}":'/g' /etc/nginx/conf.d/$NGINX_CONF_FILE + sed -i -e 's/listen /listen '"${LISTEN_ADDR}":'/g' "/etc/nginx/conf.d/$NGINX_CONF_FILE"Also applies to: 92-93
🧹 Nitpick comments (6)
install/debian12/install_dependencies.debian12.sh (1)
21-23: Drop sudo; script already enforces root.Inconsistent sudo usage inside a root-only script is noise and can mask privilege issues.
-sudo apt-get install nginx nginx-core mtr php-fpm php8.2-fpm php-cli php8.2 php8.2-sqlite3 -y -sudo phpenmod -v 8.2 sqlite3 +# apt-get install -y nginx nginx-core mtr php-fpm php8.2-fpm php-cli php8.2 php8.2-sqlite3 +# phpenmod -v 8.2 sqlite3install/ubuntu24/start.ubuntu24.sh (1)
51-59: Consider adding apt-get update for standalone runs.This script is often invoked via install.ubuntu24.sh (which updates first), but for direct use, a quick update reduces failure modes.
# Install dependencies +apt-get update -y apt-get install -y \install/ubuntu24/install.ubuntu24.sh (1)
56-72: Safer cleanup pattern — nice.Using mountpoint checks and dotfile patterns reduces foot-guns. One nit: consider
set -euo pipefailat top to fail fast.#!/usr/bin/env bash +set -euo pipefaildocs/HW_INSTALL.md (2)
8-11: List formatting inside blockquote.Remove extra spaces after
>to satisfy MD007/MD027.-> - [slammingprogramming](https://github.com/slammingprogramming) -> - [ingoratsdorf](https://github.com/ingoratsdorf) +> - [slammingprogramming](https://github.com/slammingprogramming) +> - [ingoratsdorf](https://github.com/ingoratsdorf)
27-34: Top-level list indentation.Outdent bullets to level 0 per MD007.
- - dependencies will be installed from the respective system repos - - required python modules will be installed - - `/app` directory will be deleted and newly created - - `/app` will contain the whole repository (downloaded by the install script) - - The default NGINX site `/etc/nginx/sites-enabled/default` will be disabled (sym-link deleted or backed up to `sites-available`) - - `/var/www/html/netalertx` directory will be deleted and newly created - - `/etc/nginx/conf.d/netalertx.conf` will be sym-linked to the appropriate installer location (depending on your system installer script) - - Some files (IEEE device vendors info, ...) will be created in the directory where the installation script is executed +- dependencies will be installed from the respective system repos +- required python modules will be installed +- `/app` directory will be deleted and newly created +- `/app` will contain the whole repository (downloaded by the install script) +- The default NGINX site `/etc/nginx/sites-enabled/default` will be disabled (sym-link deleted or backed up to `sites-available`) +- `/var/www/html/netalertx` directory will be deleted and newly created +- `/etc/nginx/conf.d/netalertx.conf` will be sym-linked to the appropriate installer location (depending on your system installer script) +- Some files (IEEE device vendors info, …) will be created in the directory where the installation script is executedinstall/debian12/start.debian12.sh (1)
56-66: Inconsistent sudo usage in a root-only script.You enforce root at start; drop sudo for consistency, or remove the root check if you intend mixed privilege.
-sudo rm /etc/nginx/sites-enabled/default -sudo mv /etc/nginx/sites-enabled/default /etc/nginx/sites-available/default.bkp_netalertx +rm /etc/nginx/sites-enabled/default +mv /etc/nginx/sites-enabled/default /etc/nginx/sites-available/default.bkp_netalertx @@ -sudo ln -s "${INSTALL_PATH}/install/debian12/netalertx.conf" /etc/nginx/conf.d/$NGINX_CONF_FILE +ln -s "${INSTALL_PATH}/install/debian12/netalertx.conf" "/etc/nginx/conf.d/$NGINX_CONF_FILE" @@ - sudo sed -i 's/listen 20211/listen '"$PORT"'/g' /etc/nginx/conf.d/$NGINX_CONF_FILE + sed -i 's/listen 20211/listen '"$PORT"'/g' "/etc/nginx/conf.d/$NGINX_CONF_FILE" @@ -sudo chgrp -R www-data $INSTALL_PATH +chgrp -R www-data "$INSTALL_PATH"Also applies to: 82-87, 157-157
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/HW_INSTALL.md(1 hunks)install/debian12/install.debian12.sh(2 hunks)install/debian12/install_dependencies.debian12.sh(2 hunks)install/debian12/netalertx.conf(1 hunks)install/debian12/start.debian12.sh(4 hunks)install/ubuntu24/install.ubuntu24.sh(5 hunks)install/ubuntu24/start.ubuntu24.sh(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/HW_INSTALL.md
10-10: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
10-10: Multiple spaces after blockquote symbol
(MD027, no-multiple-space-blockquote)
14-14: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🪛 Shellcheck (0.10.0)
install/debian12/start.debian12.sh
[warning] 12-12: INSTALLER_DIR appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker_dev
🔇 Additional comments (6)
install/debian12/netalertx.conf (1)
17-19: LGTM — formatting-only change.Config remains functionally identical; socket path matches Debian 12 PHP 8.2.
install/debian12/install.debian12.sh (1)
38-39: LGTM — switch to Debian12 start script.Executable bit + hand-off to the new path looks correct.
install/ubuntu24/start.ubuntu24.sh (2)
13-17: Path normalization via INSTALLER_DIR and config symlink looks solid.cd to INSTALLER_DIR, standardized NGINX config name, and symlink target are coherent with the new layout.
Also applies to: 62-63, 106-107
60-61: System-wide python alternative change is risky.
update-alternatives --install /usr/bin/python …can affect other tools. Prefer relying onpython3and your venv.Would you be okay removing this line and standardizing on
python3/venv everywhere?install/ubuntu24/install.ubuntu24.sh (1)
15-18: Good safety UX and centralization via INSTALLER_DIR.Clear prompts for install/update/start and normalized pathing to the ubuntu24 starter script.
Also applies to: 40-46, 102-106
install/debian12/start.debian12.sh (1)
41-48: Dependency install lacks error handling and preflight update.If the dependency script fails, continue-on-error may leave a half-configured system. Add
set -euo pipefailand preface with an update inside the dependency script (see earlier comment).Would you like me to submit a follow-up patch adding strict mode and consistent updates?
| > [!WARNING] | ||
| > A warning to the installation method below: Piping to bash is [controversial](https://pi-hole.net/2016/07/25/curling-and-piping-to-bash) and may | ||
| be dangerous, as you cannot see the code that's about to be executed on your system. | ||
|
|
||
| Alternatively you can download the installation script `install/install.debian.sh` from the repository and check the code yourself (beware other scripts are | ||
| downloaded too - only from this repo). | ||
| If you trust this repo, you can download the install script via one of the methods (curl/wget) below and it will fo its best to install NetAlertX on your system. | ||
|
|
||
| Alternatively you can download the installation script from the repository and check the code yourself. | ||
|
|
There was a problem hiding this comment.
Fix wording and markdownlint issues in the WARNING block.
Correct typo and remove blank line/multiple spaces after blockquote symbols to satisfy MD027/MD028.
-> [!WARNING]
-> A warning to the installation method below: Piping to bash is [controversial](https://pi-hole.net/2016/07/25/curling-and-piping-to-bash) and may
-be dangerous, as you cannot see the code that's about to be executed on your system.
+> [!WARNING]
+> A warning about the method below: piping to bash is [controversial](https://pi-hole.net/2016/07/25/curling-and-piping-to-bash) and may be dangerous,
+> as you cannot see the code that's about to be executed on your system.
-If you trust this repo, you can download the install script via one of the methods (curl/wget) below and it will fo its best to install NetAlertX on your system.
+If you trust this repo, you can download the install script via one of the methods (curl/wget) below and it will do its best to install NetAlertX on your system.
-Alternatively you can download the installation script from the repository and check the code yourself.
+Alternatively, you can download the installation script from the repository and review the code yourself.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > [!WARNING] | |
| > A warning to the installation method below: Piping to bash is [controversial](https://pi-hole.net/2016/07/25/curling-and-piping-to-bash) and may | |
| be dangerous, as you cannot see the code that's about to be executed on your system. | |
| Alternatively you can download the installation script `install/install.debian.sh` from the repository and check the code yourself (beware other scripts are | |
| downloaded too - only from this repo). | |
| If you trust this repo, you can download the install script via one of the methods (curl/wget) below and it will fo its best to install NetAlertX on your system. | |
| Alternatively you can download the installation script from the repository and check the code yourself. | |
| > [!WARNING] | |
| > A warning about the method below: piping to bash is [controversial](https://pi-hole.net/2016/07/25/curling-and-piping-to-bash) and may be dangerous, | |
| > as you cannot see the code that's about to be executed on your system. | |
| If you trust this repo, you can download the install script via one of the methods (curl/wget) below and it will do its best to install NetAlertX on your system. | |
| Alternatively, you can download the installation script from the repository and review the code yourself. |
🤖 Prompt for AI Agents
In docs/HW_INSTALL.md around lines 15 to 22, the WARNING block contains a typo
("fo" → "do") and has extra blank lines/multiple spaces after the blockquote
markers causing MD027/MD028 violations; update the sentence to "it will do its
best to install NetAlertX on your system.", remove the stray blank line and any
extra spaces after the ">" markers so the blockquote lines are contiguous and
properly formatted, and ensure the warning text reads clearly without
leading/trailing extra whitespace.
| # setup virtual python environment so we can use pip3 to install packages | ||
| apt-get install python3.11-venv -y | ||
| apt-get install python3-venv -y | ||
| python3 -m venv myenv | ||
| source myenv/bin/activate | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Virtualenv created in an indeterminate directory.
Because this script can be invoked from anywhere, creating “myenv” in CWD is brittle. Create and activate the venv relative to the script’s own directory.
-# setup virtual python environment so we can use pip3 to install packages
-apt-get install python3-venv -y
-python3 -m venv myenv
-source myenv/bin/activate
+# setup virtual python environment so we can use pip3 to install packages
+apt-get install -y python3-venv
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
+python3 -m venv "${SCRIPT_DIR}/.venv"
+source "${SCRIPT_DIR}/.venv/bin/activate"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In install/debian12/install_dependencies.debian12.sh around lines 25-29, the
script creates the virtualenv in the current working directory ("myenv"), which
is brittle; change it to create and activate the venv relative to the script
file (not CWD). Resolve this by computing the script directory (the directory of
"${BASH_SOURCE[0]}"), create the venv inside that directory (e.g., a subpath
like "$SCRIPT_DIR/.venv" or similar), and source the activate script from that
absolute path so the venv location is deterministic regardless of where the
script is invoked.
install/debian12/start.debian12.sh
Outdated
| INSTALL_DIR=/app # Specify the installation directory here | ||
|
|
||
| # DO NOT CHANGE ANYTHING BELOW THIS LINE! | ||
| INSTALLER_DIR=$INSTALL_DIR/install/ubuntu24 |
There was a problem hiding this comment.
Wrong INSTALLER_DIR and it’s unused.
Value points to ubuntu24 and the variable isn’t used; venv activation later will likely fail because no cwd is set.
-INSTALLER_DIR=$INSTALL_DIR/install/ubuntu24
+INSTALLER_DIR=$INSTALL_DIR/install/debian12Also use INSTALLER_DIR to anchor venv and file operations (see follow-up diffs).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| INSTALLER_DIR=$INSTALL_DIR/install/ubuntu24 | |
| INSTALLER_DIR=$INSTALL_DIR/install/debian12 |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 12-12: INSTALLER_DIR appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
In install/debian12/start.debian12.sh around line 12, INSTALLER_DIR is
incorrectly set to the ubuntu24 path and then never used; change it to point to
the debian12 installer directory (e.g., $INSTALL_DIR/install/debian12), export
or declare the variable, and use INSTALLER_DIR as the anchor for all subsequent
venv and file operations (create/activate the virtualenv using
$INSTALLER_DIR/venv or the intended venv path, and use cd $INSTALLER_DIR before
running activation or relative file commands) so activation won’t fail due to
wrong cwd and paths.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
|
||
| "${INSTALL_PATH}/install/debian12/install_dependencies.debian12.sh" # if modifying this file transfer the changes into the root Dockerfile.debian as well! | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Venv location/activation is inconsistent; will fail in Debian flow.
Dependencies script creates “myenv” in an unknown CWD; here you source myenv/bin/activate without changing directories. Anchor venv under INSTALLER_DIR and activate it consistently.
-"${INSTALL_PATH}/install/debian12/install_dependencies.debian12.sh" # if modifying this file transfer the changes into the root Dockerfile.debian as well!
+export INSTALLER_DIR="$INSTALL_DIR/install/debian12"
+"${INSTALL_PATH}/install/debian12/install_dependencies.debian12.sh"
@@
-# Activate the virtual python environment
-source myenv/bin/activate
+# Activate the virtual python environment
+source "${INSTALLER_DIR}/.venv/bin/activate"
@@
-python $INSTALL_PATH/server/
+# Run in background like Ubuntu to avoid blocking the installer
+python "$INSTALL_PATH/server/" 2>/dev/null 1>/dev/null &Note: pair this with the dependency-script change that creates .venv in the script directory.
Also applies to: 176-184
🤖 Prompt for AI Agents
In install/debian12/start.debian12.sh around lines 46-48 (and similarly lines
176-184), the script sources a virtualenv created with a relative name (“myenv”)
but never changes to the dependency script directory so activation will fail;
update the flow so the dependency script creates the venv as .venv in its own
script directory (INSTALL_PATH/install/debian12/.venv) and then source it with
an absolute path using INSTALLER_DIR/…: specifically, ensure INSTALLER_DIR is
set to the installer script directory, run the dependency installer to create
INSTALLER_DIR/.venv, and replace any occurrences of sourcing myenv/bin/activate
with sourcing "${INSTALLER_DIR}/.venv/bin/activate" (and mirror the same change
at lines 176-184).
|
Thanks a lot @ingoratsdorf 🙏 looks great. Left a comment to check, otherwise looks good to me |
There was a problem hiding this comment.
since the file is renamed we need to update the docs as well. I think tehre are 2 impacted files:
- HW_INSTALL.md
- REVERSE_PROXY.md
EDIT:
Only REVERSE_PROXY.md needs an update as you seem have removed the line from HW_INSTALL.m mentioning this file
There was a problem hiding this comment.
Ah... ok, I'll check that.
I left the docker template in its original place. But need to change the referencing to the Debian one.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/REVERSE_PROXY.md (4)
20-27: NGINX examples mix Apache directives; fix invalid directives and TLS syntax.
- proxy_preserve_host and proxy_pass_reverse are not NGINX directives. Use proxy_set_header and proxy_redirect instead.
- In TLS servers, SSLEngine/SSLCertificate* are Apache-only. Use ssl_certificate/ssl_certificate_key and add ssl on the listen line.
Apply the following minimal fixes per block.
HTTP (Direct path) server blocks:
- proxy_preserve_host on; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/;HTTP (Sub path) server blocks (inside location /netalertx/):
- proxy_preserve_host on; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/;HTTPS (Direct path) server blocks:
- server { - listen 443; + server { + listen 443 ssl; server_name netalertx; - SSLEngine On; - SSLCertificateFile /etc/ssl/certs/netalertx.pem; - SSLCertificateKeyFile /etc/ssl/private/netalertx.key; - proxy_preserve_host on; + ssl_certificate /etc/ssl/certs/netalertx.pem; + ssl_certificate_key /etc/ssl/private/netalertx.key; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/;HTTPS (Sub path) server blocks (inside location /netalertx/):
- SSLEngine On; - SSLCertificateFile /etc/ssl/certs/netalertx.pem; - SSLCertificateKeyFile /etc/ssl/private/netalertx.key; + listen 443 ssl; + ssl_certificate /etc/ssl/certs/netalertx.pem; + ssl_certificate_key /etc/ssl/private/netalertx.key; location ^~ /netalertx/ { - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_pass http://localhost:20211/;Note: If you need Location header rewriting, keep proxy_redirect or add:
proxy_redirect off; # if your app generates absolute URLs correctlyAlso applies to: 46-56, 76-95, 114-124, 143-156, 175-196
29-36: Step order: run nginx -t before reload.Users should validate config first; also prefer reload over restart.
-3. Activate the new website by running the following command: - - `nginx -s reload` or `systemctl restart nginx` - -4. Check your config with `nginx -t`. If there are any issues, it will tell you. +3. Check your config with `nginx -t`. If there are any issues, it will tell you. + +4. Activate the new website by running the following command: + + `nginx -s reload` or `systemctl reload nginx` -5. Once NGINX restarts, you should be able to access the proxy website at http://netalertx/ +5. Once NGINX reloads, you should be able to access the proxy website at http://netalertx/
215-221: Apache subpath examples use NGINX syntax; fix to valid Apache mappings.Replace location ^~ (NGINX) with and map /netalertx/ correctly.
HTTP (Sub path):
- <VirtualHost *:80> - ServerName netalertx - location ^~ /netalertx/ { - ProxyPreserveHost On - ProxyPass / http://localhost:20211/ - ProxyPassReverse / http://localhost:20211/ - } - </VirtualHost> + <VirtualHost *:80> + ServerName netalertx + ProxyPreserveHost On + ProxyPass "/netalertx/" "http://localhost:20211/" + ProxyPassReverse "/netalertx/" "http://localhost:20211/" + </VirtualHost>HTTPS (Sub path):
- <VirtualHost *:443> - ServerName netalertx - SSLEngine On - SSLCertificateFile /etc/ssl/certs/netalertx.pem - SSLCertificateKeyFile /etc/ssl/private/netalertx.key - location ^~ /netalertx/ { - ProxyPreserveHost On - ProxyPass / http://localhost:20211/ - ProxyPassReverse / http://localhost:20211/ - } - </VirtualHost> + <VirtualHost *:443> + ServerName netalertx + SSLEngine On + SSLCertificateFile /etc/ssl/certs/netalertx.pem + SSLCertificateKeyFile /etc/ssl/private/netalertx.key + ProxyPreserveHost On + ProxyPass "/netalertx/" "http://localhost:20211/" + ProxyPassReverse "/netalertx/" "http://localhost:20211/" + </VirtualHost>Optional: mention enabling required modules:
- a2enmod proxy proxy_http ssl headers
Also applies to: 240-248, 267-276, 295-306
431-435: Broken YAML: restart field split across lines.This will fail to parse.
- restart: u - nless-stopped + restart: unless-stopped
🧹 Nitpick comments (3)
docs/REVERSE_PROXY.md (3)
223-223: Apache syntax check command: add Debian/Ubuntu variant.httpd -t is not present on Debian/Ubuntu. Suggest documenting apache2ctl -t as an alternative.
-4. Check your config with `httpd -t`. If there are any issues, it will tell you. +4. Check your config with `httpd -t` (or `apache2ctl -t` on Debian/Ubuntu). If there are any issues, it will tell you.Also applies to: 250-250, 278-278, 308-308
384-386: Fix typos for clarity.“Asuming” ➜ “Assuming”; “domainname” ➜ “domain name”.
-Asuming the user already has a working Traefik setup, this is what's needed to make NetAlertX work at a URL like www.domain.com/netalertx/. -Note: Everything in these configs assumes '**www.domain.com**' as your domainname and '**section31**' as an arbitrary name for your certificate setup. You will have to substitute these with your own. +Assuming the user already has a working Traefik setup, this is what's needed to make NetAlertX work at a URL like www.domain.com/netalertx/. +Note: Everything in these configs assumes '**www.domain.com**' as your domain name and '**section31**' as an arbitrary name for your certificate setup. You will have to substitute these with your own.
33-33: Address markdownlint MD034 (bare URLs).Wrap bare URLs in angle brackets or convert to Markdown links.
Example:
-... at http://netalertx/ +... at <http://netalertx/>Also applies to: 65-65, 103-103, 132-132, 164-164, 204-204, 229-229, 256-256, 284-284, 314-314
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/REVERSE_PROXY.md(12 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/REVERSE_PROXY.md
35-35: Bare URL used
(MD034, no-bare-urls)
65-65: Bare URL used
(MD034, no-bare-urls)
103-103: Bare URL used
(MD034, no-bare-urls)
132-132: Bare URL used
(MD034, no-bare-urls)
164-164: Bare URL used
(MD034, no-bare-urls)
204-204: Bare URL used
(MD034, no-bare-urls)
229-229: Bare URL used
(MD034, no-bare-urls)
256-256: Bare URL used
(MD034, no-bare-urls)
284-284: Bare URL used
(MD034, no-bare-urls)
314-314: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker_dev
🔇 Additional comments (2)
docs/REVERSE_PROXY.md (2)
6-10: Good context about config file locations.Clear distinction between bare-metal and Docker templates. LGTM.
496-509: SWAG mapping path likely not canonical; verify container’s default site path.LinuxServer SWAG typically uses /config/nginx/site-confs rather than /etc/nginx/sites-available. Please confirm and adjust the bind mount.
Would you like me to look up SWAG’s current default NGINX site path and propose the exact docker run snippet?
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
docs/REVERSE_PROXY.md (8)
19-27: NGINX: invalid context and directives in “Direct Path” example.
- proxy_pass/proxy_* must be inside a location block, not at server level.
- proxy_preserve_host/proxy_pass_reverse are not valid NGINX directives.
Fix the block as below.
- server { - listen 80; - server_name netalertx; - proxy_preserve_host on; - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; - } + server { + listen 80; + server_name netalertx; + location / { + proxy_set_header Host $host; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_pass http://localhost:20211/; + } + }
46-57: NGINX: remove invalid directive proxy_pass_reverse; replace proxy_preserve_host.
- proxy_pass_reverse is not an NGINX directive; use proxy_redirect if needed (you already set it for sub-path).
- Use proxy_set_header Host $host instead of proxy_preserve_host.
- proxy_preserve_host on; + # preserve Host header + proxy_set_header Host $host; ... - proxy_pass_reverse http://localhost:20211/; + # no proxy_pass_reverse in nginx ... - proxy_pass_reverse http://localhost:20211/; + # no proxy_pass_reverse in nginx ... - proxy_pass_reverse http://localhost:20211/; + # no proxy_pass_reverse in nginx ... - proxy_pass_reverse http://localhost:20211/; + # no proxy_pass_reverse in nginxAlso applies to: 81-85, 121-123, 150-154, 182-186
114-124: NGINX HTTPS: Apache SSL directives used; fix to valid NGINX syntax.Use
listen 443 ssl;andssl_certificate/ssl_certificate_key, and move proxy_* into a location.- server { - listen 443; - server_name netalertx; - SSLEngine On; - SSLCertificateFile /etc/ssl/certs/netalertx.pem; - SSLCertificateKeyFile /etc/ssl/private/netalertx.key; - proxy_preserve_host on; - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; - } + server { + listen 443 ssl; + server_name netalertx; + ssl_certificate /etc/ssl/certs/netalertx.pem; + ssl_certificate_key /etc/ssl/private/netalertx.key; + location / { + proxy_set_header Host $host; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_pass http://localhost:20211/; + } + }
143-156: NGINX HTTPS (Sub Path): fix SSL directives and remove proxy_pass_reverse.- server { - listen 443; + server { + listen 443 ssl; server_name netalertx; - SSLEngine On; - SSLCertificateFile /etc/ssl/certs/netalertx.pem; - SSLCertificateKeyFile /etc/ssl/private/netalertx.key; + ssl_certificate /etc/ssl/certs/netalertx.pem; + ssl_certificate_key /etc/ssl/private/netalertx.key; location ^~ /netalertx/ { - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; + proxy_set_header Host $host; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_pass http://localhost:20211/; proxy_redirect ~^/(.*)$ /netalertx/$1; rewrite ^/netalertx/?(.*)$ /$1 break; } - } + }
175-196: NGINX HTTPS (Sub Path + sub_filter): fix SSL directives and remove proxy_pass_reverse.- server { - listen 443; + server { + listen 443 ssl; server_name netalertx; - SSLEngine On; - SSLCertificateFile /etc/ssl/certs/netalertx.pem; - SSLCertificateKeyFile /etc/ssl/private/netalertx.key; + ssl_certificate /etc/ssl/certs/netalertx.pem; + ssl_certificate_key /etc/ssl/private/netalertx.key; location ^~ /netalertx/ { - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; + proxy_set_header Host $host; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_pass http://localhost:20211/; proxy_redirect ~^/(.*)$ /netalertx/$1; rewrite ^/netalertx/?(.*)$ /$1 break;
240-248: Apache (Sub Path): using NGINX “location” in Apache vhost.Replace with Apache’s and correct ProxyPass/ProxyPassReverse for a prefix.
- <VirtualHost *:80> - ServerName netalertx - location ^~ /netalertx/ { - ProxyPreserveHost On - ProxyPass / http://localhost:20211/ - ProxyPassReverse / http://localhost:20211/ - } - </VirtualHost> + <VirtualHost *:80> + ServerName netalertx + ProxyPreserveHost On + ProxyPass /netalertx/ http://localhost:20211/ + ProxyPassReverse /netalertx/ http://localhost:20211/ + <Location "/netalertx/"> + RequestHeader set X-Forwarded-Prefix "/netalertx" + </Location> + </VirtualHost>
295-306: Apache HTTPS (Sub Path): same “location” issue; fix like HTTP variant.- <VirtualHost *:443> - ServerName netalertx - SSLEngine On - SSLCertificateFile /etc/ssl/certs/netalertx.pem - SSLCertificateKeyFile /etc/ssl/private/netalertx.key - location ^~ /netalertx/ { - ProxyPreserveHost On - ProxyPass / http://localhost:20211/ - ProxyPassReverse / http://localhost:20211/ - } - </VirtualHost> + <VirtualHost *:443> + ServerName netalertx + SSLEngine On + SSLCertificateFile /etc/ssl/certs/netalertx.pem + SSLCertificateKeyFile /etc/ssl/private/netalertx.key + ProxyPreserveHost On + ProxyPass /netalertx/ http://localhost:20211/ + ProxyPassReverse /netalertx/ http://localhost:20211/ + <Location "/netalertx/"> + RequestHeader set X-Forwarded-Prefix "/netalertx" + </Location> + </VirtualHost>
433-435: Traefik compose snippet: broken “restart” value split across lines.Join it into a single line.
- restart: u - nless-stopped + restart: unless-stopped
🧹 Nitpick comments (4)
docs/REVERSE_PROXY.md (4)
6-11: Good clarification; add one concrete example path.Consider adding a concrete example like
/app/install/debian12/netalertx.confto reduce ambiguity for newcomers.-> The first one you can find in the respective bare metal installer folder `/app/install/<system>/netalertx.conf`. +> The first one you can find in the respective bare metal installer folder `/app/install/<system>/netalertx.conf` (for example: `/app/install/debian12/netalertx.conf`).
33-36: Order validation before reload.Run
nginx -tbefore reloading.-3. Activate the new website by running the following command: - - `nginx -s reload` or `systemctl restart nginx` - -4. Check your config with `nginx -t`. If there are any issues, it will tell you. +3. Check your config with `nginx -t`. If there are any issues, it will tell you. +4. Activate the new website by running the following command: + + `nginx -s reload` or `systemctl restart nginx`
35-35: Wrap bare URLs to satisfy markdownlint MD034.Angle-bracket autolinks keep them readable and lint-clean.
-5. Once NGINX restarts, you should be able to access the proxy website at http://netalertx/ +5. Once NGINX restarts, you should be able to access the proxy website at <http://netalertx/>(Apply similarly to other bare URLs in this file.)
Also applies to: 65-65, 103-103, 132-132, 164-164, 204-204, 229-229, 256-256, 284-284, 314-314, 384-384
384-389: Minor wording tweaks.
- domainname → domain name
- dockerimage → Docker image
-Note: Everything in these configs assumes '**www.domain.com**' as your domainname +Note: Everything in these configs assumes '**www.domain.com**' as your domain name ... -By mapping the file instead of changing the file in-place, the changes persist if an updated dockerimage is pulled. +By mapping the file instead of changing the file in-place, the changes persist if an updated Docker image is pulled.Also applies to: 470-471
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/REVERSE_PROXY.md(12 hunks)install/debian12/start.debian12.sh(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- install/debian12/start.debian12.sh
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/REVERSE_PROXY.md
35-35: Bare URL used
(MD034, no-bare-urls)
65-65: Bare URL used
(MD034, no-bare-urls)
103-103: Bare URL used
(MD034, no-bare-urls)
132-132: Bare URL used
(MD034, no-bare-urls)
164-164: Bare URL used
(MD034, no-bare-urls)
204-204: Bare URL used
(MD034, no-bare-urls)
229-229: Bare URL used
(MD034, no-bare-urls)
256-256: Bare URL used
(MD034, no-bare-urls)
284-284: Bare URL used
(MD034, no-bare-urls)
314-314: Bare URL used
(MD034, no-bare-urls)
384-384: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker_dev
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
docs/REVERSE_PROXY.md (8)
20-27: Nginx snippet contains invalid directives (will fail to load).
- Nginx does not support
proxy_pass_reverse(Apache-only).- Nginx does not support
proxy_preserve_host(useproxy_set_header Host $host;etc.).Replace with a minimal, valid example:
- server { - listen 80; - server_name netalertx; - proxy_preserve_host on; - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; - } + server { + listen 80; + server_name netalertx; + location / { + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_pass http://localhost:20211/; + } + }
29-36: Swap validation and reload order for Direct Path (HTTP).Run
nginx -tbefore reloading to avoid activating a broken config.-3. Activate the new website by running the following command: - - `nginx -s reload` or `systemctl restart nginx` - -4. Check your config with `nginx -t`. If there are any issues, it will tell you. +3. Check your config with `nginx -t`. If there are any issues, it will tell you. + +4. Activate the new website by running the following command: + + `nginx -s reload` or `systemctl restart nginx`
46-57: Fix Nginx Sub Path example (invalid directives).Remove
proxy_pass_reverseand replaceproxy_preserve_hostwith proper headers.- server { + server { listen 80; server_name netalertx; - proxy_preserve_host on; location ^~ /netalertx/ { - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_pass http://localhost:20211/; proxy_redirect ~^/(.*)$ /netalertx/$1; rewrite ^/netalertx/?(.*)$ /$1 break; } }Also applies to: 59-66
76-95: Fix Nginx Sub Path + sub_filter example (invalid directives).Same invalid directives; also add headers to preserve scheme/host.
- server { + server { listen 80; server_name netalertx; - proxy_preserve_host on; location ^~ /netalertx/ { - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_pass http://localhost:20211/; proxy_redirect ~^/(.*)$ /netalertx/$1; rewrite ^/netalertx/?(.*)$ /$1 break; sub_filter_once off; sub_filter_types *; sub_filter 'href="/' 'href="/netalertx/'; sub_filter '(?>$host)/css' '/netalertx/css'; sub_filter '(?>$host)/js' '/netalertx/js'; sub_filter '/img' '/netalertx/img'; sub_filter '/lib' '/netalertx/lib'; sub_filter '/php' '/netalertx/php'; } }Also applies to: 97-105
114-125: Nginx HTTPS sections mix Apache directives (invalid in Nginx).
SSLEngine On,SSLCertificateFile,SSLCertificateKeyFileare Apache; Nginx useslisten 443 ssl;,ssl_certificate,ssl_certificate_key.- Remove
proxy_preserve_hostandproxy_pass_reverse.Provide valid HTTPS examples:
- server { - listen 443; - server_name netalertx; - SSLEngine On; - SSLCertificateFile /etc/ssl/certs/netalertx.pem; - SSLCertificateKeyFile /etc/ssl/private/netalertx.key; - proxy_preserve_host on; - proxy_pass http://localhost:20211/; - proxy_pass_reverse http://localhost:20211/; - } + server { + listen 443 ssl; + server_name netalertx; + ssl_certificate /etc/ssl/certs/netalertx.pem; + ssl_certificate_key /etc/ssl/private/netalertx.key; + location / { + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_pass http://localhost:20211/; + } + }For Sub Path/HTTPS, combine with the earlier Sub Path location, adding the
listen 443 ssl;andssl_*lines.Also applies to: 126-133, 136-156, 158-165, 175-196, 198-205
240-248: Apache Sub Path example uses Nginx syntax and wrong ProxyPass form.
- Apache uses
<Location>blocks, notlocation ^~ ....- Inside
<Location>,ProxyPasstakes only the backend URL.Replace with:
- <VirtualHost *:80> - ServerName netalertx - location ^~ /netalertx/ { - ProxyPreserveHost On - ProxyPass / http://localhost:20211/ - ProxyPassReverse / http://localhost:20211/ - } - </VirtualHost> + <VirtualHost *:80> + ServerName netalertx + ProxyPreserveHost On + <Location /netalertx/> + ProxyPass http://localhost:20211/ + ProxyPassReverse http://localhost:20211/ + </Location> + </VirtualHost>Also applies to: 250-257
295-306: Apache HTTPS Sub Path example uses Nginx syntax; fix as for HTTP Sub Path.Use
<Location /netalertx/>and the correctProxyPass/ProxyPassReverseform:- <VirtualHost *:443> - ServerName netalertx - SSLEngine On - SSLCertificateFile /etc/ssl/certs/netalertx.pem - SSLCertificateKeyFile /etc/ssl/private/netalertx.key - location ^~ /netalertx/ { - ProxyPreserveHost On - ProxyPass / http://localhost:20211/ - ProxyPassReverse / http://localhost:20211/ - } - </VirtualHost> + <VirtualHost *:443> + ServerName netalertx + SSLEngine On + SSLCertificateFile /etc/ssl/certs/netalertx.pem + SSLCertificateKeyFile /etc/ssl/private/netalertx.key + ProxyPreserveHost On + <Location /netalertx/> + ProxyPass http://localhost:20211/ + ProxyPassReverse http://localhost:20211/ + </Location> + </VirtualHost>Also applies to: 308-315
384-435: Fix broken YAML in Traefik compose and a tiny wording nit.
- The
restart:line is split across two lines; fix torestart: unless-stopped.- Optional: s“domainname” → “domain name”.
- restart: u - nless-stopped + restart: unless-stopped
🧹 Nitpick comments (3)
docs/REVERSE_PROXY.md (3)
215-222: Apache HTTP (Direct Path) looks fine; consider enabling required modules.Add a tip to ensure
proxy proxy_http headersare enabled (a2enmod ...) before enabling the site.Also applies to: 223-231
267-276: Apache HTTPS (Direct Path) looks OK; add module note.Consider a note to ensure
sslmodule is enabled (a2enmod ssl) in addition to proxy modules.Also applies to: 278-286
33-33: Satisfy markdownlint MD034 (bare URLs).Wrap bare URLs in angle brackets to appease linters without changing rendering, e.g.,
<http://netalertx/>.Also applies to: 65-65, 103-103, 132-132, 164-164, 204-204, 229-229, 256-256, 284-284, 314-314, 384-384
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/REVERSE_PROXY.md(12 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/REVERSE_PROXY.md
35-35: Bare URL used
(MD034, no-bare-urls)
65-65: Bare URL used
(MD034, no-bare-urls)
103-103: Bare URL used
(MD034, no-bare-urls)
132-132: Bare URL used
(MD034, no-bare-urls)
164-164: Bare URL used
(MD034, no-bare-urls)
204-204: Bare URL used
(MD034, no-bare-urls)
229-229: Bare URL used
(MD034, no-bare-urls)
256-256: Bare URL used
(MD034, no-bare-urls)
284-284: Bare URL used
(MD034, no-bare-urls)
314-314: Bare URL used
(MD034, no-bare-urls)
384-384: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker_dev
🔇 Additional comments (3)
docs/REVERSE_PROXY.md (3)
6-11: Good clarification on config file locations and installer paths.The note about OS-specific netalertx.conf vs Docker’s template is helpful and matches the installer rework.
324-376: SWAG example LGTM.Good use of sub_filter and redirect to normalize trailing slash; headers are already handled via included proxy.conf.
Also applies to: 378-379
471-509: Good persistence advice for default file mapping.Mounting the modified default file is a practical workaround until upstream images include the change.
|
Think we are good :-) |
|
Thanks a lot @ingoratsdorf! |
2 participants
Installer-rework
split installer structure into systems, updated non-functional Debian12 installer with some minor fixes to Ubuntu24 installer.
Updated docs.
Summary by CodeRabbit
New Features
Refactor
Documentation