Add an uninstall script for Ubuntu and fix the documentation #1214
Conversation
Makes the script a little less invasive
WalkthroughAdds a new Ubuntu 24 uninstall script for NetAlertX that stops/disables the service, cleans systemd/defaults, unmounts tmpfs, removes nginx artifacts, deletes app files/venv, and optionally purges data and packages. It supports flags (--purge-data, --purge-packages, --help), reads defaults, and performs guarded, conditional cleanup steps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant SH as uninstall.sh
participant SD as systemd
participant NG as nginx
participant FS as Filesystem
participant APT as apt-get
U->>SH: Run uninstall.sh [--purge-data] [--purge-packages]
SH->>SH: Validate root, load defaults/env, parse flags
SH->>SD: systemctl stop/disable netalertx.service
SH->>SD: Remove unit/defaults, daemon-reload
alt tmpfs mounts exist
SH->>FS: umount install_dir/log, install_dir/api
end
alt nginx config found
SH->>NG: Remove site config / restore default
SH->>NG: Test config (nginx -t)
alt config valid
SH->>NG: Reload
else config invalid
SH->>NG: Skip reload
end
end
SH->>FS: Remove Python venv
alt --purge-data or confirmed
SH->>FS: Remove install directory (guarded)
end
alt --purge-packages or confirmed
SH->>APT: purge related packages
end
SH-->>U: Completion message/status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
install/ubuntu24/uninstall.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
install/ubuntu24/uninstall.sh (1)
server/api_server/nettools_endpoint.py (1)
traceroute(28-72)
🪛 Shellcheck (0.11.0)
install/ubuntu24/uninstall.sh
[warning] 221-221: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
[warning] 222-222: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
[warning] 226-226: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
| VENV_DIR="${VIRTUAL_ENV:-$DEFAULT_VENV_DIR}" | ||
| PHPVERSION="${PHPVERSION:-$DEFAULT_PHPVERSION}" |
There was a problem hiding this comment.
Respect custom VENV_DIR from /etc/default
After sourcing /etc/default/netalertx, we expect any configured VENV_DIR to be honored, but the reassignment uses VIRTUAL_ENV instead, overriding the sourced value with the default path. On installs that changed the virtualenv location, the uninstall will miss the real venv and try to delete /opt/netalertx-python instead. Please preserve a pre-set VENV_DIR while still falling back to VIRTUAL_ENV and the default.
Apply this diff:
- VENV_DIR="${VIRTUAL_ENV:-$DEFAULT_VENV_DIR}"
+ VENV_DIR="${VENV_DIR:-${VIRTUAL_ENV:-$DEFAULT_VENV_DIR}}"📝 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.
| VENV_DIR="${VIRTUAL_ENV:-$DEFAULT_VENV_DIR}" | |
| PHPVERSION="${PHPVERSION:-$DEFAULT_PHPVERSION}" | |
| VENV_DIR="${VENV_DIR:-${VIRTUAL_ENV:-$DEFAULT_VENV_DIR}}" | |
| PHPVERSION="${PHPVERSION:-$DEFAULT_PHPVERSION}" |
🤖 Prompt for AI Agents
In install/ubuntu24/uninstall.sh around lines 35 to 36, the script currently
overwrites any VENV_DIR set by /etc/default/netalertx by using VIRTUAL_ENV
first; change the assignment so it preserves an already-set VENV_DIR, falling
back to VIRTUAL_ENV then to DEFAULT_VENV_DIR. Update the logic to: if VENV_DIR
is non-empty use it, else use VIRTUAL_ENV if set, otherwise use
DEFAULT_VENV_DIR; implement this via a single shell assignment that prefers an
existing VENV_DIR, then VIRTUAL_ENV, then the default.
There was a problem hiding this comment.
thanks a lot for the PR @gonzague - can you check these 2 comments (nginx restore and VENV_DIR) to see if valid concerns?
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!
| if [ -f /etc/nginx/sites-available/default.bkp_netalertx ]; then | ||
| if [ -e /etc/nginx/sites-enabled/default ]; then | ||
| rm -f /etc/nginx/sites-enabled/default | ||
| fi | ||
| mv /etc/nginx/sites-available/default.bkp_netalertx /etc/nginx/sites-enabled/default | ||
| NEED_NGINX_RELOAD=true | ||
| echo "[UNINSTALL] Restored nginx default site" | ||
| elif [ ! -e /etc/nginx/sites-enabled/default ] && [ -f /etc/nginx/sites-available/default ]; | ||
| then | ||
| ln -s ../sites-available/default /etc/nginx/sites-enabled/default | ||
| NEED_NGINX_RELOAD=true | ||
| echo "[UNINSTALL] Re-enabled nginx default site" | ||
| fi |
There was a problem hiding this comment.
Restore nginx default site to sites-available, not sites-enabled
Moving default.bkp_netalertx directly into /etc/nginx/sites-enabled/default leaves /etc/nginx/sites-available/default missing and replaces the usual symlink with a plain file. That breaks the standard Debian/Ubuntu nginx layout and complicates future site management. Instead, move the backup back into sites-available and (re)create the symlink in sites-enabled.
Apply this diff:
- mv /etc/nginx/sites-available/default.bkp_netalertx /etc/nginx/sites-enabled/default
- NEED_NGINX_RELOAD=true
- echo "[UNINSTALL] Restored nginx default site"
+ mv /etc/nginx/sites-available/default.bkp_netalertx /etc/nginx/sites-available/default
+ if [ ! -e /etc/nginx/sites-enabled/default ]; then
+ ln -s ../sites-available/default /etc/nginx/sites-enabled/default
+ fi
+ NEED_NGINX_RELOAD=true
+ echo "[UNINSTALL] Restored nginx default site"🤖 Prompt for AI Agents
In install/ubuntu24/uninstall.sh around lines 129-141, the script currently
moves default.bkp_netalertx into /etc/nginx/sites-enabled/default which replaces
the normal symlink with a regular file; instead move the backup into
/etc/nginx/sites-available/default (restoring the file there), then ensure
/etc/nginx/sites-enabled/default is a symlink to ../sites-available/default
(remove any existing regular file or stale symlink first, then create the
symlink), set NEED_NGINX_RELOAD=true and echo the restored message; do the
analogous fix in the elif branch by creating the symlink to
../sites-available/default rather than linking incorrectly.
There was a problem hiding this comment.
you should not include packages or remove anything other than the files from this repository. you could wreck someone's php, nginx, avahi, or other. the only files you should touch are the /app files.
Additionaly, there's no official support for hardware installs. The HW installs make copies of the GitHub instead of installing directly from source, making them outside the realm of anything which should be included. HW installs will be removed from source and moved to documentation shortly.
I recommend converting this to a sectional uninstall doc.
There was a problem hiding this comment.
@adamoutler, understood. You're absolutely right that the uninstall script in its current form is too invasive and could potentially damage user systems by touching nginx, PHP, systemd, and other system packages that may be shared with other applications.
The key concerns I'm noting:
- Safety: Removing or modifying system packages (nginx, PHP, avahi) could break other applications or services on the user's system
- Scope: The script should only handle files from this repository (specifically
/appdirectory) - Policy: Hardware installs aren't officially supported and will be moved to documentation
Converting this to a sectional uninstall documentation makes much more sense. The documentation could:
- Provide step-by-step instructions for users who want to manually uninstall
- Clearly scope what files belong to NetAlertX (primarily
/appcontents) - Let users decide what system-level changes to reverse (nginx config, systemd units, etc.)
- Include warnings about not removing shared system packages
This approach gives users control while avoiding the risk of automated removal damaging their systems.
✏️ Learnings added
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#1214
File: install/ubuntu24/uninstall.sh:129-141
Timestamp: 2025-10-10T22:16:02.739Z
Learning: In the NetAlertX project, hardware installs are not officially supported. Hardware installation methods will be moved from source code to documentation.
Learnt from: adamoutler
PR: jokob-sk/NetAlertX#1214
File: install/ubuntu24/uninstall.sh:129-141
Timestamp: 2025-10-10T22:16:02.740Z
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.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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!
3 participants
Add a script that uninstalls NetAlertX and update the documentation :
`mini@mini:~/netalertx$ sudo ./uninstall.sh
[UNINSTALL] Starting NetAlertX uninstallation for Ubuntu
[UNINSTALL] Target install dir : /app
[UNINSTALL] Web UI location : /var/www/html/netalertx
[UNINSTALL] Nginx config file : /etc/nginx/conf.d/netalertx.conf
[UNINSTALL] Python venv : /opt/netalertx-python
[UNINSTALL] Stopping services
[UNINSTALL] Removing systemd unit and defaults
[UNINSTALL] Systemd cleanup complete
[UNINSTALL] Unmounting tmpfs mounts
[UNINSTALL] Unmounted /app/log
[UNINSTALL] Unmounted /app/api
[UNINSTALL] Cleaning nginx and web artifacts
[UNINSTALL] Removed web UI link /var/www/html/netalertx
[UNINSTALL] Removed nginx config /etc/nginx/conf.d/netalertx.conf
[UNINSTALL] Re-enabled nginx default site
[UNINSTALL] Removing application files
[UNINSTALL] Removed virtual environment /opt/netalertx-python
[UNINSTALL] /app contains NetAlertX binaries, config, and database.
[UNINSTALL] Type 'purge' to delete it now, or press Enter to keep it:
purge
[UNINSTALL] Removed /app
[UNINSTALL] Reloading nginx
nginx: the configuration file /etc/nginx/nginx.conf syntax is ok
nginx: configuration file /etc/nginx/nginx.conf test is successful
[UNINSTALL] Package cleanup
[UNINSTALL] The installer may have added these packages:
[UNINSTALL] Removing them can affect other applications that rely on them.
[UNINSTALL] Type 'purge' to remove these packages now, or press Enter to keep them: purge
[UNINSTALL] Removing installer packages via apt-get purge.
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:
The following packages have unmet dependencies:
shim-signed : Depends: grub-efi-amd64-signed (>= 1.191~) but it is not going to be installed or
grub-efi-arm64-signed (>= 1.191~) but it is not installable or
base-files (< 12.3) but 13ubuntu10.3 is to be installed
Depends: grub-efi-amd64-signed (>= 1.187.2~) but it is not going to be installed or
grub-efi-arm64-signed (>= 1.187.2~) but it is not installable
Depends: grub2-common (>= 2.04-1ubuntu24) but it is not going to be installed
E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages.
[UNINSTALL] Failed to purge some packages; please review the output above.
[UNINSTALL] NetAlertX removal complete
[UNINSTALL] Thank you for using NetAlertX.
`
Summary by CodeRabbit