diff --git a/.devcontainer/.env.example b/.devcontainer/.env.example index 1437c2ed67..912cd19fd9 100644 --- a/.devcontainer/.env.example +++ b/.devcontainer/.env.example @@ -31,3 +31,17 @@ SKIP_SUPERUSER=false # .devcontainer/plugin-config.py.example β .devcontainer/plugin-config.py # Advanced NetBox configuration (optional): # .devcontainer/extra-configuration.py.example β .devcontainer/extra-configuration.py + +# Proxy Configuration (optional, for corporate networks with MITM proxies) +# Uncomment and set these if you're behind a proxy +# HTTP_PROXY=http://proxy.example.com:8080 +# HTTPS_PROXY=http://proxy.example.com:8080 +# NO_PROXY=localhost,127.0.0.1,postgres,redis +# REQUESTS_CA_BUNDLE=/path/to/ca-bundle.crt +# SSL_CERT_FILE=/path/to/ca-bundle.crt +# CURL_CA_BUNDLE=/path/to/ca-bundle.crt + +# Git SSL verification override (default: false) +# Only set to true if behind a MITM proxy and you cannot provide a CA bundle. +# Prefer placing a ca-bundle.crt in the workspace root instead. +# ALLOW_GIT_SSL_DISABLE=false diff --git a/.devcontainer/README.md b/.devcontainer/README.md index 98c4213cef..6f26c67492 100644 --- a/.devcontainer/README.md +++ b/.devcontainer/README.md @@ -65,6 +65,20 @@ If you need to test with a LibreNMS instance on a private network (local lab, co - **GitHub CLI**: Automatically configured for easy PR submission - **Logs**: Use `netbox-logs` to debug issues in real-time + +### π‘ LibreNMS Server Configuration + +You need a LibreNMS instance to use this plugin. Configure your LibreNMS server(s) in `plugin-config.py`: + +1. Copy the example config: + + ```bash + cp .devcontainer/config/plugin-config.py.example .devcontainer/config/plugin-config.py + ``` + +2. Edit it with your LibreNMS server URL(s) and API token(s) +3. Restart NetBox: `netbox-restart` + ## Out-of-the-box defaults Below are the dev container defaults. The field name to change these defaults is listed below each line. @@ -84,6 +98,8 @@ Below are the dev container defaults. The field name to change these defaults is - Plugin loader: enabled; reads `.devcontainer/config/plugin-config.py` if present - If `plugin-config.py` is missing: plugin is enabled with empty config (features wonβt work until configured) + + ## π§ Configuration ### NetBox Version and Environment (use .devcontainer/.env) @@ -139,6 +155,84 @@ You might experience issues with database schemas and migrations when changing N - Database: `DB_HOST`, `DB_NAME`, `DB_USER`, `DB_PASSWORD` - Redis: `REDIS_HOST`, `REDIS_PASSWORD` - Superuser: `SUPERUSER_NAME`, `SUPERUSER_EMAIL`, `SUPERUSER_PASSWORD`, `SKIP_SUPERUSER` +- Proxy: `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY`, `REQUESTS_CA_BUNDLE`, `SSL_CERT_FILE`, `CURL_CA_BUNDLE` + +### π Proxy Configuration (MITM Proxies) + +If you're behind a corporate proxy or MITM proxy (like Zscaler, BlueCoat, etc.), you need to configure the proxy at two levels: the Docker client (for building) and the container runtime (for package installation inside the container). + +**Step 1: Configure Docker client proxy** (`~/.docker/config.json`) + +This is **required** so that `apt-get`, `curl`, etc. work during the container image build (e.g., when installing devcontainer features like `git` and `github-cli`). + +Create or edit `~/.docker/config.json`: + +```json +{ + "proxies": { + "default": { + "httpProxy": "http://proxy.example.com:8080", + "httpsProxy": "http://proxy.example.com:8080", + "noProxy": "localhost,127.0.0.1,postgres,redis" + } + } +} +``` + +Docker automatically injects these as environment variables into every `RUN` instruction during `docker build`. No VS Code restart is needed β this takes effect immediately. + +> **Docker Desktop users:** You can configure the same settings via Docker Desktop Settings β Resources β Proxies, which writes this file for you. + +**Step 2: Create `.devcontainer/.env`** (for container runtime) + +```bash +cp .devcontainer/.env.example .devcontainer/.env +``` + +Add your proxy settings to `.devcontainer/.env`: + +```bash +# Proxy Configuration +HTTP_PROXY=http://proxy.example.com:8080 +HTTPS_PROXY=http://proxy.example.com:8080 +NO_PROXY=localhost,127.0.0.1,postgres,redis + +# CA Certificate Bundle (if your proxy uses a custom CA) +# Place your CA cert in the workspace, e.g., /workspaces/netbox-librenms-plugin/ca-bundle.crt +# REQUESTS_CA_BUNDLE=/workspaces/netbox-librenms-plugin/ca-bundle.crt +# SSL_CERT_FILE=/workspaces/netbox-librenms-plugin/ca-bundle.crt +# CURL_CA_BUNDLE=/workspaces/netbox-librenms-plugin/ca-bundle.crt +``` + +**Step 3: Add your CA certificate** (optional, only if your proxy intercepts TLS): + - Export your proxy's CA certificate (usually available from your IT department or browser) + - Save it as `ca-bundle.crt` in the root of your workspace + - Uncomment and update the `*_CA_BUNDLE` / `SSL_CERT_FILE` lines in `.env` + +**Step 4: Rebuild the container**: + - VS Code: Ctrl+Shift+P β "Dev Containers: Rebuild Container" + +**What gets configured:** +- `~/.docker/config.json` β proxy for Docker build steps (devcontainer features, apt in Dockerfile) +- `.devcontainer/.env` β proxy for running containers (apt, pip, curl at runtime) +- `setup.sh` auto-configures apt proxy and git SSL settings inside the container + +**Important Notes:** +- The `.env` file is ignored by git, so your proxy credentials stay private +- `~/.docker/config.json` is a per-user file outside the repo +- Add internal service names to `NO_PROXY` to avoid routing internal Docker traffic through the proxy +- **Proxy authentication:** Embedding credentials directly in the proxy URL (e.g., `http://username:password@proxy.example.com:8080`) is insecure β credentials can be visible in process listings, environment dumps, `docker inspect` output, and logs. Prefer safer alternatives such as Docker's `config.json` with `credsStore` or a secret manager for storing proxy credentials securely. + +**Common Issues:** + +*"Could not connect to archive.ubuntu.com" during build* +- β `~/.docker/config.json` is missing or has wrong proxy URL + +*"SSL certificate errors" during build* +- β Your proxy uses a MITM certificate. Export it and add it to the system trust store, or set `SSL_CERT_FILE` in `.env` + +*Container builds but apt/pip fails inside* +- β .env file is missing or has wrong proxy settings. Check .env matches Docker Desktop settings After any `.env` change, rebuild the dev container to apply environment updates. diff --git a/.devcontainer/config/plugin-config.py.example b/.devcontainer/config/plugin-config.py.example index 71da51d4d9..5e48833a48 100644 --- a/.devcontainer/config/plugin-config.py.example +++ b/.devcontainer/config/plugin-config.py.example @@ -5,7 +5,7 @@ Default plugin configuration for the NetBox LibreNMS Plugin in the dev container - Copy this file to .devcontainer/plugin-config.py - Edit values as needed. -- Add config for all other plugins here if any. +- Add config for all other plugins here if any. """ # Ensure our plugin is enabled in dev (the loader sets this as a default too) @@ -13,13 +13,13 @@ PLUGINS = [ "netbox_librenms_plugin", ] -# Sample configuration with three example servers +# Sample configuration with example servers PLUGINS_CONFIG = { "netbox_librenms_plugin": { "servers": { "production": { "display_name": "Production LibreNMS", - "librenms_url": "https://librenms-prod.exampel.com", + "librenms_url": "https://librenms-prod.example.com", "api_token": "your-prod-token", "cache_timeout": 300, "verify_ssl": True, diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 61548824e2..63a1a9b471 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -34,12 +34,18 @@ "SUPERUSER_NAME": "${localEnv:SUPERUSER_NAME:admin}", "SUPERUSER_EMAIL": "${localEnv:SUPERUSER_EMAIL:admin@example.com}", "SUPERUSER_PASSWORD": "${localEnv:SUPERUSER_PASSWORD:admin}", - "SKIP_SUPERUSER": "${localEnv:SKIP_SUPERUSER:false}" - }, - "features": { - "ghcr.io/devcontainers/features/git:1": {}, - "ghcr.io/devcontainers/features/github-cli:1": {} + "SKIP_SUPERUSER": "${localEnv:SKIP_SUPERUSER:false}", + "HTTP_PROXY": "${localEnv:HTTP_PROXY}", + "HTTPS_PROXY": "${localEnv:HTTPS_PROXY}", + "http_proxy": "${localEnv:HTTP_PROXY}", + "https_proxy": "${localEnv:HTTPS_PROXY}", + "NO_PROXY": "${localEnv:NO_PROXY}", + "no_proxy": "${localEnv:NO_PROXY}", + "REQUESTS_CA_BUNDLE": "${localEnv:REQUESTS_CA_BUNDLE}", + "SSL_CERT_FILE": "${localEnv:SSL_CERT_FILE}", + "CURL_CA_BUNDLE": "${localEnv:CURL_CA_BUNDLE}" }, + "features": {}, "customizations": { "vscode": { "extensions": [ @@ -72,4 +78,4 @@ "postCreateCommand": "bash .devcontainer/scripts/setup.sh", "postAttachCommand": "bash .devcontainer/scripts/welcome.sh", "remoteUser": "root" -} \ No newline at end of file +} diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml index d1b7ed03f4..50e1d278bb 100644 --- a/.devcontainer/docker-compose.yml +++ b/.devcontainer/docker-compose.yml @@ -22,6 +22,16 @@ services: SUPERUSER_EMAIL: ${SUPERUSER_EMAIL:-admin@example.com} SUPERUSER_PASSWORD: ${SUPERUSER_PASSWORD:-admin} SKIP_SUPERUSER: ${SKIP_SUPERUSER:-false} + # Proxy settings (optional) + HTTP_PROXY: ${HTTP_PROXY:-} + HTTPS_PROXY: ${HTTPS_PROXY:-} + http_proxy: ${HTTP_PROXY:-} + https_proxy: ${HTTPS_PROXY:-} + NO_PROXY: ${NO_PROXY:-} + no_proxy: ${NO_PROXY:-} + REQUESTS_CA_BUNDLE: ${REQUESTS_CA_BUNDLE:-} + SSL_CERT_FILE: ${SSL_CERT_FILE:-} + CURL_CA_BUNDLE: ${CURL_CA_BUNDLE:-} depends_on: postgres: condition: service_healthy diff --git a/.devcontainer/scripts/load-aliases.sh b/.devcontainer/scripts/load-aliases.sh index 8628966f55..fc8a979d0c 100755 --- a/.devcontainer/scripts/load-aliases.sh +++ b/.devcontainer/scripts/load-aliases.sh @@ -61,4 +61,7 @@ alias rq-jobs="cd /opt/netbox/netbox && source /opt/netbox/venv/bin/activate && alias rq-failed="cd /opt/netbox/netbox && source /opt/netbox/venv/bin/activate && python manage.py shell -c \"from django_rq import get_failed_queue; q = get_failed_queue(); print(f'Failed jobs: {len(q)}'); [print(f' {job.id[:8]}: {job.func_name}') for job in q.jobs[:10]]\"" alias rq-recent="cd /opt/netbox/netbox && source /opt/netbox/venv/bin/activate && python manage.py shell -c \"from core.models import Job; jobs = Job.objects.all().order_by('-created')[:10]; [print(f'{j.id}: {j.name[:50]} - {getattr(j.status, \\\"value\\\", j.status)} ({j.user})') for j in jobs]\"" -echo "β Aliases loaded! Try: rq-status, rq-stats, rq-recent" +# Help +alias dev-help='echo "π― NetBox LibreNMS Plugin Development Commands:"; echo ""; echo "π NetBox Server Management:"; echo " netbox-run-bg : Start NetBox in background"; echo " netbox-run : Start NetBox in foreground (for debugging)"; echo " netbox-stop : Stop NetBox and RQ worker"; echo " netbox-restart : Restart NetBox and RQ worker"; echo " netbox-reload : Reinstall plugin and restart NetBox"; echo " netbox-status : Check if NetBox and RQ worker are running"; echo " netbox-logs : View NetBox server logs"; echo ""; echo "βοΈ Background Jobs (RQ Worker):"; echo " rq-status : Check if RQ worker is running"; echo " rq-logs : View RQ worker logs"; echo " rq-stats : Show RQ queue statistics"; echo " rq-jobs : List jobs in default queue"; echo " rq-failed : List failed jobs"; echo " rq-recent : Show recent NetBox jobs"; echo ""; echo "π οΈ Development Tools:"; echo " netbox-shell : Open NetBox Django shell"; echo " netbox-test : Run plugin tests"; echo " netbox-manage : Run Django management commands"; echo " plugin-install : Reinstall plugin in development mode"; echo ""; echo "π§Ή Code Quality:"; echo " ruff-check : Check code with Ruff"; echo " ruff-format : Format code with Ruff"; echo " ruff-fix : Auto-fix code issues with Ruff"; echo ""; echo "π Diagnostics:"; echo " diagnose : Run startup diagnostics"; echo " dev-help : Show this help message"; echo ""; echo "π NetBox available at: http://localhost:8000 (admin/admin)"; echo ""' + +echo "β Aliases loaded! Try: rq-status, rq-stats, rq-recent, dev-help" diff --git a/.devcontainer/scripts/setup.sh b/.devcontainer/scripts/setup.sh old mode 100644 new mode 100755 index 18da8d24da..aad786f36d --- a/.devcontainer/scripts/setup.sh +++ b/.devcontainer/scripts/setup.sh @@ -7,6 +7,88 @@ echo "π€ Current user: $(whoami)" NETBOX_VERSION=${NETBOX_VERSION:-"latest"} echo "π¦ Using NetBox Docker image: netboxcommunity/netbox:${NETBOX_VERSION}" +# --------------------------------------------------------------------------- +# Detect plugin workspace directory (must contain pyproject.toml). +# Prints the resolved path to stdout on success, or an empty string on +# failure. Always exits 0 β callers must check for an empty result. +# --------------------------------------------------------------------------- +detect_plugin_workspace() { + if [ -f "$PWD/pyproject.toml" ]; then + echo "$PWD" + elif [ -d "/workspaces/netbox-librenms-plugin" ] && [ -f "/workspaces/netbox-librenms-plugin/pyproject.toml" ]; then + echo "/workspaces/netbox-librenms-plugin" + else + local candidate + candidate=$(find /workspaces -maxdepth 2 -type f -name pyproject.toml 2>/dev/null | head -n1 | xargs -r dirname || true) + if [ -n "$candidate" ] && [ -f "$candidate/pyproject.toml" ]; then + echo "$candidate" + else + echo "" + fi + fi +} + +# Configure proxy for apt and pip if proxy environment variables are set +if [ -n "$HTTP_PROXY" ] || [ -n "$HTTPS_PROXY" ]; then + echo "π Configuring proxy settings..." + + # Configure apt proxy + if [ -n "$HTTP_PROXY" ]; then + echo "Acquire::http::Proxy \"$HTTP_PROXY\";" > /etc/apt/apt.conf.d/80proxy + echo " β apt HTTP proxy: $HTTP_PROXY" + fi + if [ -n "$HTTPS_PROXY" ]; then + echo "Acquire::https::Proxy \"$HTTPS_PROXY\";" >> /etc/apt/apt.conf.d/80proxy + echo " β apt HTTPS proxy: $HTTPS_PROXY" + fi + + # Configure pip proxy via environment (already set, but ensure it's exported) + export HTTP_PROXY HTTPS_PROXY http_proxy https_proxy NO_PROXY no_proxy + + # Install custom CA certificate into the system trust store (for MITM proxies) + PLUGIN_WS_DIR_EARLY="$(detect_plugin_workspace)" + [ -z "$PLUGIN_WS_DIR_EARLY" ] && PLUGIN_WS_DIR_EARLY="/workspaces/netbox-librenms-plugin" + CA_BUNDLE_SRC="$PLUGIN_WS_DIR_EARLY/ca-bundle.crt" + if [ -f "$CA_BUNDLE_SRC" ]; then + echo "π Installing custom CA certificate into system trust store..." + mkdir -p /usr/local/share/ca-certificates/proxy + # Remove stale split fragments so they don't accumulate across rebuilds + find /usr/local/share/ca-certificates/proxy -maxdepth 1 -name 'cert-*' -delete 2>/dev/null || true + # Split the bundle into individual certs β update-ca-certificates needs one + # cert per file and skips non-CA leaf certs, so extract each PEM block as + # a separate .crt file. + csplit -z -f /usr/local/share/ca-certificates/proxy/cert- \ + "$CA_BUNDLE_SRC" '/-----BEGIN CERTIFICATE-----/' '{*}' \ + >/dev/null 2>&1 + # Rename split fragments to .crt + for f in /usr/local/share/ca-certificates/proxy/cert-*; do + mv "$f" "${f}.crt" 2>/dev/null || true + done + update-ca-certificates 2>/dev/null + echo " β CA certificate installed into system trust store" + # Point environment variables to the system bundle + export REQUESTS_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt + export SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt + export CURL_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt + export GIT_SSL_CAINFO=/etc/ssl/certs/ca-certificates.crt + # Configure pip globally so isolated virtualenvs (e.g. pre-commit) also + # use the system CA bundle instead of their bundled certifi. + pip config set global.cert /etc/ssl/certs/ca-certificates.crt 2>/dev/null || true + else + echo " βΉοΈ No ca-bundle.crt found at $CA_BUNDLE_SRC, skipping CA install" + # Only disable git SSL verification if explicitly opted-in via ALLOW_GIT_SSL_DISABLE. + # Silently disabling SSL is a security risk; prefer providing a CA bundle instead. + if [ "${ALLOW_GIT_SSL_DISABLE:-false}" = "true" ]; then + git config --global http.sslVerify false + echo " β οΈ git SSL verification disabled globally (ALLOW_GIT_SSL_DISABLE=true)" + else + echo " β οΈ No CA bundle found and git SSL verification was NOT disabled." + echo " If you need to disable it, set ALLOW_GIT_SSL_DISABLE=true in .devcontainer/.env" + echo " Preferred: provide a ca-bundle.crt in the workspace root instead." + fi + fi +fi + # Verify NetBox virtual environment exists if [ ! -f "/opt/netbox/venv/bin/activate" ]; then echo "β NetBox virtual environment not found at /opt/netbox/venv/" @@ -27,23 +109,37 @@ fi # Install dev tools echo "π§ Installing development dependencies..." apt-get update -qq -apt-get install -y -qq net-tools +apt-get install -y -qq net-tools git $PIP_CMD install pytest pytest-django ruff pre-commit -# Detect plugin workspace directory (must contain pyproject.toml) -if [ -f "$PWD/pyproject.toml" ]; then - PLUGIN_WS_DIR="$PWD" -elif [ -d "/workspaces/netbox-librenms-plugin" ] && [ -f "/workspaces/netbox-librenms-plugin/pyproject.toml" ]; then - PLUGIN_WS_DIR="/workspaces/netbox-librenms-plugin" -else - CANDIDATE_DIR=$(find /workspaces -maxdepth 2 -type f -name pyproject.toml 2>/dev/null | head -n1 | xargs dirname || true) - if [ -n "$CANDIDATE_DIR" ] && [ -f "$CANDIDATE_DIR/pyproject.toml" ]; then - PLUGIN_WS_DIR="$CANDIDATE_DIR" - else - echo "β Could not locate plugin workspace directory (pyproject.toml not found)." - echo " Checked: $PWD and /workspaces/*" - exit 1 - fi +# Install GitHub CLI (gh) +# NOTE: The chained && commands below mean a partial failure (e.g. wget succeeds +# but apt-get install gh fails) may leave artifacts (keyring, sources list, temp +# file). This is acceptable here because it only runs during container build β +# a rebuild will retry from scratch. If this block is ever moved to a runtime +# script, consider adding a trap or explicit cleanup on error. +if ! command -v gh >/dev/null 2>&1; then + echo "π§ Installing GitHub CLI..." + (type -p wget >/dev/null || apt-get install -y -qq wget) \ + && install -d -m 755 /etc/apt/keyrings \ + && out=$(mktemp) \ + && wget -qO "$out" https://cli.github.com/packages/githubcli-archive-keyring.gpg \ + && cat "$out" | tee /etc/apt/keyrings/githubcli-archive-keyring.gpg > /dev/null \ + && chmod go+r /etc/apt/keyrings/githubcli-archive-keyring.gpg \ + && echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | tee /etc/apt/sources.list.d/github-cli.list > /dev/null \ + && apt-get update -qq \ + && apt-get install -y -qq gh \ + && rm -f "$out" \ + && echo " β GitHub CLI installed: $(gh --version | head -1)" \ + || echo "β οΈ GitHub CLI installation failed (non-fatal)" +fi + +# Detect plugin workspace directory using the shared helper +PLUGIN_WS_DIR="$(detect_plugin_workspace)" +if [ -z "$PLUGIN_WS_DIR" ]; then + echo "β Could not locate plugin workspace directory (pyproject.toml not found)." + echo " Checked: $PWD and /workspaces/*" + exit 1 fi echo "π Plugin workspace: $PLUGIN_WS_DIR" @@ -160,42 +256,27 @@ python manage.py collectstatic --noinput >/dev/null 2>&1 || true # Set up pre-commit hooks echo "πͺ Installing pre-commit hooks..." cd "$PLUGIN_WS_DIR" +git config --global --add safe.directory "$PLUGIN_WS_DIR" pre-commit install --install-hooks 2>/dev/null || echo "β οΈ Pre-commit hook installation failed (may already be installed)" # Ensure scripts are executable chmod +x "$PLUGIN_WS_DIR/.devcontainer/scripts/start-netbox.sh" || true chmod +x "$PLUGIN_WS_DIR/.devcontainer/scripts/diagnose.sh" || true +chmod +x "$PLUGIN_WS_DIR/.devcontainer/scripts/load-aliases.sh" || true -# Aliases for convenience -cat >> ~/.bashrc << EOF -# NetBox LibreNMS Plugin Development Aliases -export PATH="/opt/netbox/venv/bin:\$PATH" -export DEBUG="\${DEBUG:-True}" -PLUGIN_DIR="$PLUGIN_WS_DIR" -alias netbox-run-bg="\$PLUGIN_DIR/.devcontainer/scripts/start-netbox.sh --background" -alias netbox-run="\$PLUGIN_DIR/.devcontainer/scripts/start-netbox.sh" -alias netbox-restart="netbox-stop && sleep 1 && netbox-run-bg" -alias netbox-reload="cd \$PLUGIN_DIR && (command -v uv >/dev/null 2>&1 && uv pip install -e . || pip install -e .) && netbox-restart" -alias netbox-stop="([ -f /tmp/netbox.pid ] && kill \\\$(cat /tmp/netbox.pid) && rm /tmp/netbox.pid && echo 'NetBox stopped' || echo 'NetBox not running'); ([ -f /tmp/rqworker.pid ] && kill \\\$(cat /tmp/rqworker.pid) && rm /tmp/rqworker.pid && echo 'RQ worker stopped' || echo 'RQ worker not running')" -alias netbox-logs="tail -f /tmp/netbox.log" -alias netbox-status="[ -f /tmp/netbox.pid ] && kill -0 \\\$(cat /tmp/netbox.pid) 2>/dev/null && echo 'NetBox is running (PID: '\\\$(cat /tmp/netbox.pid)')' || echo 'NetBox is not running'; [ -f /tmp/rqworker.pid ] && kill -0 \\\$(cat /tmp/rqworker.pid) 2>/dev/null && echo 'RQ worker is running (PID: '\\\$(cat /tmp/rqworker.pid)')' || echo 'RQ worker is not running'" -alias rq-logs="tail -f /tmp/rqworker.log" -alias rq-status="[ -f /tmp/rqworker.pid ] && kill -0 \\\$(cat /tmp/rqworker.pid) 2>/dev/null && echo 'RQ worker is running (PID: '\\\$(cat /tmp/rqworker.pid)')' || echo 'RQ worker is not running'" -alias netbox-shell="cd /opt/netbox/netbox && source /opt/netbox/venv/bin/activate && python manage.py shell" -alias netbox-test="cd \$PLUGIN_DIR && source /opt/netbox/venv/bin/activate && python -m pytest" -alias netbox-manage="cd /opt/netbox/netbox && source /opt/netbox/venv/bin/activate && python manage.py" -alias plugin-install="cd \$PLUGIN_DIR && (command -v uv >/dev/null 2>&1 && uv pip install -e . || pip install -e .)" -alias ruff-check="cd \$PLUGIN_DIR && ruff check ." -alias ruff-format="cd \$PLUGIN_DIR && ruff format ." -alias ruff-fix="cd \$PLUGIN_DIR && ruff check --fix ." -alias diagnose="\$PLUGIN_DIR/.devcontainer/scripts/diagnose.sh" -alias plugins-install='if [ -f "$PLUGIN_DIR/.devcontainer/extra-requirements.txt" ]; then source /opt/netbox/venv/bin/activate && pip install -r "$PLUGIN_DIR/.devcontainer/extra-requirements.txt"; else echo "No .devcontainer/extra-requirements.txt found"; fi' -# Help -alias dev-help='echo "π― NetBox LibreNMS Plugin Development Commands:"; echo ""; echo "π NetBox Server Management:"; echo " netbox-run-bg : Start NetBox in background"; echo " netbox-run : Start NetBox in foreground (for debugging)"; echo " netbox-stop : Stop NetBox and RQ worker"; echo " netbox-restart : Restart NetBox and RQ worker"; echo " netbox-reload : Reinstall plugin and restart NetBox"; echo " netbox-status : Check if NetBox and RQ worker are running"; echo " netbox-logs : View NetBox server logs"; echo ""; echo "βοΈ Background Jobs (RQ Worker):"; echo " rq-status : Check if RQ worker is running"; echo " rq-logs : View RQ worker logs"; echo ""; echo "π οΈ Development Tools:"; echo " netbox-shell : Open NetBox Django shell"; echo " netbox-test : Run plugin tests"; echo " netbox-manage : Run Django management commands"; echo " plugin-install : Reinstall plugin in development mode"; echo ""; echo "π§Ή Code Quality:"; echo " ruff-check : Check code with Ruff"; echo " ruff-format : Format code with Ruff"; echo " ruff-fix : Auto-fix code issues with Ruff"; echo ""; echo "π Diagnostics:"; echo " diagnose : Run startup diagnostics"; echo " dev-help : Show this help message"; echo ""; echo "π NetBox available at: http://localhost:8000 (admin/admin)"; echo ""' +# Load aliases and welcome message from the canonical source (load-aliases.sh). +# Appended to .bashrc so every interactive shell gets them automatically. +# Guard with a sentinel so rerunning setup.sh doesn't create duplicate entries. +BASHRC_SENTINEL="# NetBox LibreNMS Plugin β source aliases from the single canonical file" +if ! grep -qF "$BASHRC_SENTINEL" ~/.bashrc 2>/dev/null; then + cat >> ~/.bashrc << EOF +$BASHRC_SENTINEL +source "$PLUGIN_WS_DIR/.devcontainer/scripts/load-aliases.sh" # Show welcome message for new terminals -bash $PLUGIN_DIR/.devcontainer/scripts/welcome.sh +bash "$PLUGIN_WS_DIR/.devcontainer/scripts/welcome.sh" EOF +fi # Fix Git remote URLs for dev container compatibility echo "π§ Checking Git remote configuration..." @@ -221,4 +302,5 @@ else echo "β οΈ Warning: Plugin may not be properly installed" fi +echo "" echo "π NetBox LibreNMS Plugin Dev Environment Ready!" diff --git a/.devcontainer/scripts/start-netbox.sh b/.devcontainer/scripts/start-netbox.sh index 2a02a336ed..0ab8acaf46 100755 --- a/.devcontainer/scripts/start-netbox.sh +++ b/.devcontainer/scripts/start-netbox.sh @@ -95,4 +95,4 @@ else echo "π‘ If clicking the URL opens 0.0.0.0:8000, manually type: localhost:8000" echo "" python manage.py runserver 0.0.0.0:8000 -fi \ No newline at end of file +fi diff --git a/.devcontainer/scripts/welcome.sh b/.devcontainer/scripts/welcome.sh index c2776a398d..3d972d5753 100755 --- a/.devcontainer/scripts/welcome.sh +++ b/.devcontainer/scripts/welcome.sh @@ -1,5 +1,8 @@ #!/bin/bash +# Ensure aliases are available in the postAttach terminal session +source "$(dirname "$0")/load-aliases.sh" 2>/dev/null + echo "" echo "π― NetBox LibreNMS Plugin Development Environment" @@ -46,6 +49,7 @@ fi echo "" echo "π Quick start:" echo " β’ Type 'netbox-run' to start the development server" +echo " β’ Type 'netbox-restart' to restart NetBox (after config changes)" echo " β’ Type 'dev-help' to see all available commands" echo " β’ Edit code in the workspace - auto-reload is enabled" -echo "" \ No newline at end of file +echo "" diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f1d53cbd84..096b996682 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -26,7 +26,16 @@ - API serializers (`api/serializers.py`) mirror models for external consumption. Update serializers and `api/views.py` together to avoid contract drift. - Navigation and menu items are registered in `navigation.py`; extend there for new sections so NetBox renders links correctly. +## Permission System +- Uses two-tier permissions via `LibreNMSSettings` model: `view_librenmssettings` (read) and `change_librenmssettings` (write). See `docs/development/permissions.md`. +- All views inherit `LibreNMSPermissionMixin` from `views/mixins.py`. Permission constants live in `constants.py`. +- **Sync POST handlers** must call `require_write_permission()` at the start and return early if it returns a response. +- `require_write_permission()` handles HTMX requests with `HX-Redirect` header; regular requests get standard redirect. +- API endpoints use `LibreNMSPluginPermission` class in `api/views.py` (GET=view, others=change). +- Navigation menu permissions are set in `navigation.py` using permission constants. +- **Background job polling requires superuser** (NetBox core restriction on `/api/core/background-tasks/`). Non-superusers automatically fall back to synchronous modeβsee `should_use_background_job()` methods. + ## When in Doubt - Check docs in `docs/development/` for structure, view inheritance, mixins, and template conventions before introducing new patterns. - Review the existing sync views (e.g., `views/sync/interfaces.py`) as reference implementations for data flow and caching patterns. -- Coordinate any schema changes through Django migrations in `migrations/` and update `models.py` + admin/pydantic representations accordingly. \ No newline at end of file +- Coordinate any schema changes through Django migrations in `migrations/` and update `models.py` + admin/pydantic representations accordingly. diff --git a/.github/instructions/background-jobs.instructions.md b/.github/instructions/background-jobs.instructions.md index 2fcb4be068..cb824637f3 100644 --- a/.github/instructions/background-jobs.instructions.md +++ b/.github/instructions/background-jobs.instructions.md @@ -27,5 +27,11 @@ description: Background job architecture and task management patterns - Handle all RQ status values explicitly to avoid infinite polling - Use `cancelInProgress` flag to prevent polling interference during cancellation +## Superuser Requirement for Background Jobs +- NetBox's `/api/core/background-tasks/` endpoint requires **superuser** (`IsSuperuser` in `BaseRQViewSet`). +- Non-superuser users cannot poll job status; they get 403 Forbidden. +- The plugin automatically falls back to synchronous mode for non-superusersβsee `should_use_background_job()` in `list.py` and `actions.py`. +- This is a NetBox core design decision, not a plugin limitation. No amount of permissions (including `core.view_job`) bypasses it. + ## Custom Sync Endpoint `api/views.py::sync_job_status()` syncs database Job status with RQ job status, needed because NetBox worker doesn't always update DB when jobs stop before processing starts. diff --git a/.github/instructions/frontend.instructions.md b/.github/instructions/frontend.instructions.md index 7fd943e871..e127d7867a 100644 --- a/.github/instructions/frontend.instructions.md +++ b/.github/instructions/frontend.instructions.md @@ -14,6 +14,12 @@ description: Frontend patterns for templates, HTMX, and static assets - Modals use Tabler (Bootstrap-like) but **without** `bootstrap.Modal` helpers. - Buttons target the `htmx-modal-content` element and JavaScript in `librenms_import.html` toggles the wrapper. - Do not reintroduce `data-bs-toggle` or duplicate modal IDs. +- The import page uses `ModalManager` class and `filterModalManager` instanceβalways use this reference in fetch callbacks, not undefined `modalInstance` variables. + +## JavaScript Fetch Patterns +- Always check `response.ok` before processing fetch responses to catch HTTP errors. +- In catch blocks, show `error.message` for debugging rather than generic messages. +- The import filter form uses fetch with `Accept: application/json, text/html`βJSON for background jobs, HTML for synchronous mode. ## Form Controls - Device import dropdowns rely on TomSelect decorators set up elsewhere. diff --git a/.github/workflows/lint-format.yaml b/.github/workflows/lint-format.yaml index 5829a14ce2..afd4dd86e8 100644 --- a/.github/workflows/lint-format.yaml +++ b/.github/workflows/lint-format.yaml @@ -2,25 +2,19 @@ name: Lint and Format on: push: - branches: - - master - - develop pull_request: - branches: - - master - - develop jobs: format-and-lint: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: - python-version: '3.9' + python-version: '3.12' - name: Install dependencies run: | @@ -29,13 +23,6 @@ jobs: - name: Run Ruff linting run: ruff check . - continue-on-error: true - name: Run Ruff formatting check run: ruff format --check . - continue-on-error: true - - - name: Report formatting issues - if: always() - run: | - echo "If there are any formatting issues, run 'ruff check --fix .' and 'ruff format .' locally and push the changes." diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index aa82b8bb26..99a97b491d 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -1,4 +1,4 @@ -name: Tests +name: Test with all supported NetBox versions on: push: @@ -11,11 +11,12 @@ on: - develop jobs: - test: + test-netbox: runs-on: ubuntu-latest + strategy: matrix: - python-version: ['3.12', '3.13', '3.14'] + python-version: ["3.12", "3.13", "3.14"] services: redis: @@ -37,7 +38,7 @@ jobs: - 5432:5432 steps: - - name: Checkout plugin code + - name: Checkout code uses: actions/checkout@v4 with: path: netbox-librenms-plugin @@ -50,28 +51,28 @@ jobs: - name: Checkout NetBox uses: actions/checkout@v4 with: - repository: netbox-community/netbox + repository: "netbox-community/netbox" path: netbox ref: main - - name: Install NetBox dependencies - working-directory: netbox - run: | - python -m pip install --upgrade pip - pip install -r requirements.txt - - - name: Install plugin + - name: Install NetBox LibreNMS Plugin working-directory: netbox-librenms-plugin run: | - pip install . - pip install -r requirements_dev.txt + pip install -e . + pip install pytest pytest-django - - name: Configure NetBox - working-directory: netbox/netbox + - name: Set up configuration + working-directory: netbox run: | - cp netbox/configuration_testing.py netbox/configuration.py + ln -s $(pwd)/../netbox-librenms-plugin/media/configuration.testing.py netbox/netbox/configuration.py + + python -m pip install --upgrade pip + python -m pip install tblib + pip install -r requirements.txt -U - name: Run tests - working-directory: netbox + working-directory: netbox/netbox + env: + NETBOX_CONFIGURATION: netbox.configuration run: | - python -m pytest ../netbox-librenms-plugin/netbox_librenms_plugin/tests/ -v + python -m pytest ../../netbox-librenms-plugin/netbox_librenms_plugin/tests/ -v diff --git a/.gitignore b/.gitignore index 525045b674..538eb6f7e9 100644 --- a/.gitignore +++ b/.gitignore @@ -284,4 +284,8 @@ cython_debug/ .devcontainer/extra-requirements.txt .devcontainer/config/plugin-config.py .devcontainer/config/extra-configuration.py -.devcontainer/config/extra-plugins.py \ No newline at end of file +.devcontainer/config/extra-plugins.py +# Proxy CA certificates (keep local) +ca-bundle.crt +*.pem +.github/hooks/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8ea62321cc..7d553d0af5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -15,4 +15,4 @@ repos: - id: end-of-file-fixer - id: check-yaml - id: check-added-large-files - - id: check-merge-conflict \ No newline at end of file + - id: check-merge-conflict diff --git a/LICENSE b/LICENSE index f49a4e16e6..261eeb9e9f 100644 --- a/LICENSE +++ b/LICENSE @@ -198,4 +198,4 @@ distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and - limitations under the License. \ No newline at end of file + limitations under the License. diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index b4e7cb41b5..2ea7de296d 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -6,6 +6,7 @@ * [Initial Setup](usage_tips/README.md) * [Custom Field Setup](usage_tips/custom_field.md) * [Multi-Server Setup](usage_tips/multi_server_configuration.md) + * [Permissions & Access](usage_tips/permissions.md) * [Suggested Workflow](usage_tips/suggested_workflow.md) * [Import Devices](librenms_import/overview.md) * [Overview](librenms_import/overview.md) diff --git a/docs/development/README.md b/docs/development/README.md index d4faac47df..de81c7d02b 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -8,7 +8,3 @@ This guide is intended for developers and contributors working on the NetBox Lib - [Views & Inheritance](./views.md): How views are organized, inheritance patterns, and extension tips. - [Mixins](./mixins.md): Reusable logic for views, including API access and caching. - [Templates](./templates.md): Template structure, conventions, and customization tips. - - - - diff --git a/docs/development/mixins.md b/docs/development/mixins.md index 7762a5fd4c..c6e6cb3337 100644 --- a/docs/development/mixins.md +++ b/docs/development/mixins.md @@ -30,4 +30,3 @@ class MyCustomView(LibreNMSAPIMixin, CacheMixin, SomeBaseView): ``` Mixins can be combined as needed. Place mixins before the main base view to ensure their methods and properties are available. - diff --git a/docs/development/structure.md b/docs/development/structure.md index f8538b1502..daa1b0bb50 100644 --- a/docs/development/structure.md +++ b/docs/development/structure.md @@ -23,5 +23,3 @@ This document provides an overview of the NetBox LibreNMS Plugin's codebase orga - `js/` β JavaScript files - `tests/` β Test suite - `docs/` β Documentation - - diff --git a/docs/feature_list.md b/docs/feature_list.md index fd741c4afe..4ddc982ebc 100644 --- a/docs/feature_list.md +++ b/docs/feature_list.md @@ -63,4 +63,3 @@ * Customizable LibreNMS to NetBox interface type mappings * Interface Speed-based mapping rules * Bulk import support - diff --git a/docs/librenms_import/import_process.md b/docs/librenms_import/import_process.md index e0dc4d92a2..e30e7acbbb 100644 --- a/docs/librenms_import/import_process.md +++ b/docs/librenms_import/import_process.md @@ -93,4 +93,3 @@ After importing devices, typical next steps include: 5. **Sync IP addresses** - Pull IP address assignments from LibreNMS to NetBox These sync operations use the `librenms_id` custom field that was automatically set during import. - diff --git a/docs/librenms_import/import_settings.md b/docs/librenms_import/import_settings.md index a70684081c..5082b0a268 100644 --- a/docs/librenms_import/import_settings.md +++ b/docs/librenms_import/import_settings.md @@ -56,4 +56,3 @@ When using bulk import, you can override the default settings in the confirmatio - Test different naming conventions before changing global defaults The override only affects the current import operation and doesn't change your saved defaults. - diff --git a/docs/usage_tips/custom_field.md b/docs/usage_tips/custom_field.md index e0d8e72d72..80a50b2a7e 100644 --- a/docs/usage_tips/custom_field.md +++ b/docs/usage_tips/custom_field.md @@ -4,6 +4,9 @@ To enhance device identification and synchronization between NetBox and LibreNMS, this plugin supports using a custom field `librenms_id` on Device, Virtual Machine and Interface objects. While the plugin works without it, using this custom field is recommended for LibreNMS API lookups, and to assist with matching the remote device and remote interfaces for cable creation in Netbox. It can also be entered manually if no primary IP or FQDN is available. +!!! info "Automatic Creation" + As of version 0.4.2, the plugin **automatically creates** the `librenms_id` custom field when migrations are run. You no longer need to create it manually. The field is created for Device, Virtual Machine, Interface, and VM Interface objects. + For the Device and Virtual Machine objects the plugin will automatically populate the LibreNMS ID custom field when opening the LibreNMS Sync page if the device has been found in LibreNMS. For the Interface object, the plugin will automatically populate the LibreNMS ID custom field when the interface data is synced from LibreNMS. @@ -15,7 +18,10 @@ For the Interface object, the plugin will automatically populate the LibreNMS ID - **Efficient Synchronization:** Enhances the reliability of API lookups. - **Cable creation:** Allows better device identification for the creation of cables between NetBox devices. -## Suggested Custom Field Setup +## Manual Custom Field Setup (Legacy) + +!!! note + This section is only needed if you are running an older version of the plugin that does not auto-create the field, or if you need to recreate it after deletion. Follow these steps to create the `librenms_id` custom field in NetBox: @@ -30,7 +36,7 @@ Follow these steps to create the `librenms_id` custom field in NetBox: 3. **Configure the Custom Field:** - - **Object Types:** + - **Object Types:** - Check **dcim > device** - Check **virtualization > virtual machine** - Check **dcim > interface** diff --git a/docs/usage_tips/permissions.md b/docs/usage_tips/permissions.md new file mode 100644 index 0000000000..bdc7f0cf80 --- /dev/null +++ b/docs/usage_tips/permissions.md @@ -0,0 +1,152 @@ +# Permissions & Access Control + +## Overview + +The plugin uses a two-tier permission system that works with NetBox's built-in permissions. Both tiers must be satisfied for users to perform actions: + +1. **Plugin permissions** control access to plugin pages and features +2. **NetBox object permissions** control what objects users can create or modify + +This design ensures the plugin respects your existing NetBox permission structure. A user might have full plugin access but still be restricted to creating certain objects based on their NetBox permissions. + + +> Superusers have full access to all plugin features and NetBox objects by default. So the following applies only to regular users who can be granted specific permissions as needed. + + +## Two-Tier Permission Model + +### How Do the Tiers Work Together? + +A user needs both tiers of permissions to complete an action. For example, to view the Librenms Import page AND import a device: + +1. **Tier 1: Plugin permission**: User needs View AND Change permission on **LibreNMS Settings** + + - View: allows access to the plugin pages and pulling data from LibreNMS. + - Change: allows performing actions that modify Netbox or Librenms data + +The Plugin also enforces Netbox object permissions so the following permission would also be required: + +1. **Tier 2: Object permission**: User needs `dcim.add_device` (to create the device in NetBox) + +If either permission is missing, the operation fails with an appropriate error message. + +## Creating Permissions + +All permissions are created using Netbox's standard Object permissions UI. + +For details on how NetBox permissions work, see the [NetBox Permissions documentation](https://netboxlabs.com/docs/netbox/administration/permissions/). + +### Plugin Permissions + +To grant a user or group access to the plugin: + +1. Go to **Admin β Permissions** +2. Click **Add** +3. For **Object types**, select "NetBox Librenms Plugin | LibreNMS Settings" +4. Under **Actions**, check the permissions to grant: + - β **Can view** β for read-only access + - β **Can change** β for write access (requires View as well) +5. Assign to specific **Users** or **Groups** +6. Click **Save** + +### NetBox Object Permissions + +NetBox object permissions are created similarly but for different object types (DCIM, IPAM, VIRTUALIZATION, etc.). + +### Interface Type Mapping Permissions + +The Interface Type Mapping feature uses its own object permissions in addition to the plugin permissions. To manage interface mappings, users need: + +- **Plugin permission**: View permission on LibreNMS Settings (to access the page) +- **Object permissions**: `netbox_librenms_plugin.add_interfacetypemapping`, `netbox_librenms_plugin.change_interfacetypemapping`, or `netbox_librenms_plugin.delete_interfacetypemapping` as needed + +These permissions are enforced automatically by NetBox's generic views. + + +## Example Scenarios + +### Read-Only Access + +- **Plugin permissions**: Librenms Setting View only (Can view LibreNMS Plugin pages) +- **NetBox permissions**: View permissions for devices, interfaces, etc. + +Users can access all plugin pages, refresh data from LibreNMS, and review comparison tables, but cannot import devices or sync data. + +### Full Plugin Access + +- **Plugin permissions**: View + Change (Can view LibreNMS Plugin Pages and Import and sync devices data) +- **NetBox permissions**: Add/change permissions for devices, interfaces, cables, IP addresses + +Users have full access to all plugin features and can import devices, sync interfaces, and create cables. + + +## Further Details + +### Tier 1: Plugin Permissions + +Plugins permissions use the **LibreNMS Settings** model permissions: + +| Permission | NetBox UI Selection | Grants | +|------------|---------------------|--------| +| `view_librenmssettings` | LibreNMS Settings β β Can view | Access all plugin pages, view LibreNMS data | +| `change_librenmssettings` | LibreNMS Settings β β Can change | Import devices, sync data, save settings | + +Users without View permission won't see the LibreNMS menu or the LibreNMS Sync tab. Users with **View** but not **Change** can browse all plugin pages but cannot perform import or sync actions that modify Netbox data and Librenms data like Locations and Adding devices. + +### Tier 2: NetBox Object Permissions + +When the plugin creates or modifies NetBox objects (devices, interfaces, cables, IP addresses), NetBox enforces its standard object permissions. The plugin checks these permissions and will block operations if the user lacks the required access. + +| Plugin Action | Required Object Permissions | +|---------------|----------------------------| +| Import device | `dcim.add_device`, `dcim.add_interface` | +| Import device with VC | Above + `dcim.add_virtualchassis` | +| Import VM | `virtualization.add_virtualmachine` | +| Sync interfaces | `dcim.add_interface`, `dcim.change_interface` | +| Delete interfaces | `dcim.delete_interface` | +| Sync VM interfaces | `virtualization.add_vminterface`, `virtualization.change_vminterface` | +| Delete VM interfaces | `virtualization.delete_vminterface` | +| Sync cables | `dcim.add_cable`, `dcim.change_cable` | +| Sync IP addresses | `ipam.add_ipaddress`, `ipam.change_ipaddress` | +| Sync device fields | `dcim.change_device` | +| Create platform | `dcim.add_platform` | + + +### Why LibreNMS Settings Permissions? + +NetBox's permission system is object-basedβpermissions are tied to specific models like Device, Interface, or Cable. However, the plugin's Import and Sync pages are feature pages that don't have their own dedicated models. They work with LibreNMS data and create or modify existing NetBox objects. + +To control access to these pages, the plugin uses the **LibreNMS Settings** model permissions as a gate for all plugin features: + +- **No dedicated models for pages** β The Import and Sync pages aren't objects, so we need an existing model to attach permissions to +- **No custom migrations required** β Uses Django's built-in model permissions that NetBox already understands +- **Standard NetBox workflow** β Administrators assign permissions the same way they do for any other NetBox object +- **Single permission per access level** β One "View" permission for read access, one "Change" permission for write access + +While using a settings model for access control may seem unconventional, it provides a simple and maintainable way to gate plugin access without introducing custom permission infrastructure. + + +## Special note: Background Jobs and Superuser Access + +The device import page can use background jobs to help support large device sets, and virtual chassis detection. However, NetBox restricts access to background job status APIs to superusers. There is no permission in NetBox for this. This is a core design decision, not a plugin limitation. + +| User Type | Background Jobs | +|-----------|-----------------| +| Superuser | Full access to background jobs with real-time status updates | +| Non-superuser | Automatic fallback to synchronous processing | + +The plugin automatically detects whether the current user is a superuser and adjusts behavior accordingly. Non-superuser users don't need to change any settingsβthe plugin simply processes requests synchronously instead of as background jobs. All import and filter operations work correctly regardless of superuser status. + +## Troubleshooting + +**User can't see the LibreNMS menu** +: The user doesn't have View permission for the plugin. Add an Object Permission for "LibreNMS Settings" with "Can view" checked. + +**User sees pages but can't import or sync** +: The user has View permission but not Change permission. Edit their Object Permission to also include "Can change". + +**User gets "permission denied" when importing devices** +: The user has plugin permissions but may be missing NetBox object permissions. Check that they have `dcim.add_device` and related permissions. + +**Background jobs show 403 errors in console** +: In normal usage, the UI only enables background jobs for users allowed to use them and falls back to synchronous processing otherwise, so 403s from background job APIs should not appear. If you see these errors, it usually means a direct API call or custom integration is hitting background-task endpoints without superuser access; update that integration to use synchronous flows or run it with appropriate permissions. diff --git a/media/configuration.testing.py b/media/configuration.testing.py new file mode 100644 index 0000000000..aa58b59b1e --- /dev/null +++ b/media/configuration.testing.py @@ -0,0 +1,52 @@ +################################################################### +# This file serves as a base configuration for testing purposes # +# only. It is not intended for production use. # +################################################################### + +ALLOWED_HOSTS = ["*"] + +DATABASE = { + "NAME": "netbox", + "USER": "netbox", + "PASSWORD": "netbox", + "HOST": "localhost", + "PORT": "", + "CONN_MAX_AGE": 300, +} + +PLUGINS = [ + "netbox_librenms_plugin", +] + +PLUGINS_CONFIG = { + "netbox_librenms_plugin": { + "servers": { + "default": { + "librenms_url": "https://librenms.example.com", + "api_token": "test-token-for-testing", + } + } + } +} + +REDIS = { + "tasks": { + "HOST": "localhost", + "PORT": 6379, + "PASSWORD": "", + "DATABASE": 0, + "SSL": False, + }, + "caching": { + "HOST": "localhost", + "PORT": 6379, + "PASSWORD": "", + "DATABASE": 1, + "SSL": False, + }, +} + +SECRET_KEY = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" +API_TOKEN_PEPPERS = { + 1: "TEST-VALUE-DO-NOT-USE-TEST-VALUE-DO-NOT-USE-TEST-VALUE-DO-NOT-USE", +} diff --git a/mkdocs.yml b/mkdocs.yml index 89c66fc055..cc9b035d8b 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -10,6 +10,7 @@ nav: - Initial Setup: usage_tips/README.md - Custom Field Setup: usage_tips/custom_field.md - Multi-Server Setup: usage_tips/multi_server_configuration.md + - Permissions & Access: usage_tips/permissions.md - Suggested Workflow: usage_tips/suggested_workflow.md - Import Devices: - Overview: librenms_import/overview.md diff --git a/netbox_librenms_plugin/__init__.py b/netbox_librenms_plugin/__init__.py index f1720c85d6..5a2c2532c2 100644 --- a/netbox_librenms_plugin/__init__.py +++ b/netbox_librenms_plugin/__init__.py @@ -28,6 +28,7 @@ def ready(self): super().ready() from django.conf import settings + from django.db.models.signals import post_migrate plugin_config = getattr(settings, "PLUGINS_CONFIG", {}).get(self.name, {}) @@ -37,6 +38,12 @@ def ready(self): else: self._validate_legacy_config(plugin_config) + # Auto-create the librenms_id custom field after migrations complete + post_migrate.connect( + _ensure_librenms_id_custom_field, + dispatch_uid="netbox_librenms_plugin_ensure_cf", + ) + def _validate_multi_server_config(self, servers_config): """Validate multi-server configuration.""" if not servers_config or not isinstance(servers_config, dict): @@ -61,4 +68,63 @@ def _validate_legacy_config(self, plugin_config): ) +def _ensure_librenms_id_custom_field(sender, **kwargs): + """ + Auto-create the 'librenms_id' custom field if it doesn't exist. + Runs after migrations via post_migrate signal to ensure tables exist. + Uses dispatch_uid to avoid duplicate connections. + """ + # Only run once per migrate invocation (post_migrate fires per-app). + # The _executed flag is intentionally never reset: migrations are expected to + # run in short-lived CLI processes (manage.py migrate) where the flag is + # naturally cleared on exit. Long-running processes (e.g. gunicorn workers) + # should not rely on this handler re-executing after startup. + if getattr(_ensure_librenms_id_custom_field, "_executed", False): + return + _ensure_librenms_id_custom_field._executed = True # not reset; see comment above + + try: + from django.contrib.contenttypes.models import ContentType + + from extras.models import CustomField + + cf, created = CustomField.objects.get_or_create( + name="librenms_id", + defaults={ + "type": "integer", + "label": "LibreNMS ID", + "description": "LibreNMS Device ID for synchronization (auto-created by plugin)", + "required": False, + "ui_visible": "if-set", + "ui_editable": "yes", + "is_cloneable": False, + }, + ) + + # Ensure the field is assigned to the required object types + from dcim.models import Device, Interface + from virtualization.models import VirtualMachine, VMInterface + + required_models = [Device, VirtualMachine, Interface, VMInterface] + current_types = set(cf.object_types.values_list("pk", flat=True)) + + for model in required_models: + ct = ContentType.objects.get_for_model(model) + if ct.pk not in current_types: + cf.object_types.add(ct) + + if created: + import logging + + logging.getLogger("netbox_librenms_plugin").info( + "Auto-created 'librenms_id' custom field for Device, VirtualMachine, Interface, VMInterface" + ) + except Exception as e: + # Don't break startup if custom field creation fails (e.g., during initial migration), + # but log the error so it's not silently swallowed. + import logging + + logging.getLogger("netbox_librenms_plugin").exception("Failed to auto-create 'librenms_id' custom field: %s", e) + + config = LibreNMSSyncConfig diff --git a/netbox_librenms_plugin/api/serializers.py b/netbox_librenms_plugin/api/serializers.py index c06afd6ff9..6bcd0aef20 100644 --- a/netbox_librenms_plugin/api/serializers.py +++ b/netbox_librenms_plugin/api/serializers.py @@ -4,6 +4,10 @@ class InterfaceTypeMappingSerializer(NetBoxModelSerializer): + """Serialize InterfaceTypeMapping model for REST API.""" + class Meta: + """Meta options for InterfaceTypeMappingSerializer.""" + model = InterfaceTypeMapping fields = ["id", "librenms_type", "librenms_speed", "netbox_type", "description"] diff --git a/netbox_librenms_plugin/api/views.py b/netbox_librenms_plugin/api/views.py index 3523e2b5d3..7086145be0 100644 --- a/netbox_librenms_plugin/api/views.py +++ b/netbox_librenms_plugin/api/views.py @@ -4,11 +4,13 @@ from core.models import Job from django.http import JsonResponse from django.utils import timezone -from django.views.decorators.http import require_http_methods from django_rq import get_queue from netbox.api.viewsets import NetBoxModelViewSet +from rest_framework.decorators import api_view, permission_classes +from rest_framework.permissions import BasePermission, SAFE_METHODS from rq.job import Job as RQJob +from netbox_librenms_plugin.constants import PERM_CHANGE_PLUGIN, PERM_VIEW_PLUGIN from netbox_librenms_plugin.models import InterfaceTypeMapping from .serializers import InterfaceTypeMappingSerializer @@ -16,12 +18,31 @@ logger = logging.getLogger(__name__) +class LibreNMSPluginPermission(BasePermission): + """ + Permission class for LibreNMS plugin API endpoints. + + - Safe requests (GET, HEAD, OPTIONS) require netbox_librenms_plugin.view_librenmssettings + - All other requests require netbox_librenms_plugin.change_librenmssettings + """ + + def has_permission(self, request, view): + if request.method in SAFE_METHODS: + return request.user.has_perm(PERM_VIEW_PLUGIN) + return request.user.has_perm(PERM_CHANGE_PLUGIN) + + class InterfaceTypeMappingViewSet(NetBoxModelViewSet): + """API viewset for InterfaceTypeMapping CRUD operations.""" + + permission_classes = [LibreNMSPluginPermission] + queryset = InterfaceTypeMapping.objects.all() serializer_class = InterfaceTypeMappingSerializer -@require_http_methods(["POST"]) +@api_view(["POST"]) +@permission_classes([LibreNMSPluginPermission]) def sync_job_status(request, job_pk): """ Sync database Job status with RQ job status. diff --git a/netbox_librenms_plugin/constants.py b/netbox_librenms_plugin/constants.py new file mode 100644 index 0000000000..765d98d06f --- /dev/null +++ b/netbox_librenms_plugin/constants.py @@ -0,0 +1,3 @@ +# Plugin permissions (from LibreNMSSettings model) +PERM_VIEW_PLUGIN = "netbox_librenms_plugin.view_librenmssettings" +PERM_CHANGE_PLUGIN = "netbox_librenms_plugin.change_librenmssettings" diff --git a/netbox_librenms_plugin/filters.py b/netbox_librenms_plugin/filters.py index 324b1d62f4..9ec162a64c 100644 --- a/netbox_librenms_plugin/filters.py +++ b/netbox_librenms_plugin/filters.py @@ -4,6 +4,10 @@ class InterfaceTypeMappingFilterSet(django_filters.FilterSet): + """Filter set for InterfaceTypeMapping model.""" + class Meta: + """Meta options for InterfaceTypeMappingFilterSet.""" + model = InterfaceTypeMapping fields = ["librenms_type", "librenms_speed", "netbox_type", "description"] diff --git a/netbox_librenms_plugin/filtersets.py b/netbox_librenms_plugin/filtersets.py index 2b453514bb..482e709ad3 100644 --- a/netbox_librenms_plugin/filtersets.py +++ b/netbox_librenms_plugin/filtersets.py @@ -12,6 +12,7 @@ class SiteLocationFilterSet: """ def __init__(self, data, queryset): + """Initialize with form data and queryset.""" self.form_data = data self.queryset = queryset @@ -40,6 +41,8 @@ def _matches_search_criteria(self, item, search_term): @property def form(self): + """Return a bound filter form instance.""" + class FilterForm(forms.Form): """ Form to filter sites and locations by search term. @@ -77,6 +80,8 @@ class DeviceStatusFilterSet(NetBoxModelFilterSet): ) class Meta: + """Meta options for DeviceStatusFilterSet.""" + model = Device fields = ["site", "location", "device_type", "rack", "role"] search_fields = ["device", "site", "device_type", "rack", "role"] @@ -117,6 +122,8 @@ class VMStatusFilterSet(NetBoxModelFilterSet): ) class Meta: + """Meta options for VMStatusFilterSet.""" + model = VirtualMachine fields = ["site", "cluster", "platform"] search_fields = ["virtualmachine", "site", "cluster", "platform"] diff --git a/netbox_librenms_plugin/forms.py b/netbox_librenms_plugin/forms.py index 3f1bb8aef6..a5dc9a0e30 100644 --- a/netbox_librenms_plugin/forms.py +++ b/netbox_librenms_plugin/forms.py @@ -47,6 +47,47 @@ def _get_librenms_server_choices(): return choices +def _get_librenms_poller_group_choices(): + """ + Helper function to get poller group choices from LibreNMS API. + Shared between AddToLIbreSNMPV1V2 and AddToLIbreSNMPV3 forms. + Results are cached to avoid repeated API calls on every form instantiation. + """ + from django.core.cache import cache + + from .librenms_api import LibreNMSAPI + + choices = [("0", "Default (0)")] + + cache_key = "librenms_poller_group_choices" + cached_choices = cache.get(cache_key) + if cached_choices: + return cached_choices + + try: + api = LibreNMSAPI() + success, poller_groups = api.get_poller_groups() + + if success and poller_groups: + for group in poller_groups: + group_id = str(group.get("id", "")) + group_name = group.get("group_name", "") + group_descr = group.get("descr", "") + + if group_id: + if group_descr and group_descr != group_name: + label = f"{group_name} - {group_descr} ({group_id})" + else: + label = f"{group_name} ({group_id})" + choices.append((group_id, label)) + + cache.set(cache_key, choices, timeout=api.cache_timeout) + except Exception: + logger.exception("Failed to load LibreNMS poller groups") + + return choices + + class ServerConfigForm(NetBoxModelForm): """ Form for selecting the active LibreNMS server from configured servers. @@ -59,12 +100,14 @@ class ServerConfigForm(NetBoxModelForm): ) class Meta: + """Meta options for ServerConfigForm.""" + model = LibreNMSSettings fields = ["selected_server"] def __init__(self, *args, **kwargs): + """Initialize form and populate server choices.""" super().__init__(*args, **kwargs) - # Get available servers from configuration self.fields["selected_server"].choices = _get_librenms_server_choices() @@ -101,6 +144,8 @@ class ImportSettingsForm(NetBoxModelForm): ) class Meta: + """Meta options for ImportSettingsForm.""" + model = LibreNMSSettings fields = [ "vc_member_name_pattern", @@ -183,6 +228,8 @@ class InterfaceTypeMappingForm(NetBoxModelForm): """ class Meta: + """Meta options for InterfaceTypeMappingForm.""" + model = InterfaceTypeMapping fields = ["librenms_type", "librenms_speed", "netbox_type", "description"] @@ -200,6 +247,8 @@ class InterfaceTypeMappingImportForm(NetBoxModelImportForm): ) class Meta: + """Meta options for InterfaceTypeMappingImportForm.""" + model = InterfaceTypeMapping fields = ["librenms_type", "librenms_speed", "netbox_type", "description"] @@ -230,10 +279,11 @@ class InterfaceTypeMappingFilterForm(NetBoxModelFilterSetForm): model = InterfaceTypeMapping -class AddToLIbreSNMPV2(forms.Form): +class AddToLIbreSNMPV1V2(forms.Form): """ - Form for adding devices to LibreNMS using SNMPv2 authentication. + Form for adding devices to LibreNMS using SNMPv1 or SNMPv2c authentication. Collects hostname/IP and SNMP community string information. + The SNMP version (v1 or v2c) is selected via a toggle button in the template. """ hostname = forms.CharField( @@ -241,7 +291,6 @@ class AddToLIbreSNMPV2(forms.Form): max_length=255, required=True, ) - snmp_version = forms.CharField(widget=forms.HiddenInput(), initial="v2c") community = forms.CharField(label="SNMP Community", max_length=255, required=True) port = forms.IntegerField( label="SNMP Port", @@ -285,38 +334,9 @@ class AddToLIbreSNMPV2(forms.Form): ) def __init__(self, *args, **kwargs): + """Initialize form and populate poller group choices.""" super().__init__(*args, **kwargs) - # Populate poller groups from LibreNMS API - self.fields["poller_group"].choices = self._get_poller_group_choices() - - def _get_poller_group_choices(self): - """Get poller group choices from LibreNMS API.""" - from .librenms_api import LibreNMSAPI - - choices = [("0", "Default (0)")] - - try: - api = LibreNMSAPI() - success, poller_groups = api.get_poller_groups() - - if success and poller_groups: - for group in poller_groups: - group_id = str(group.get("id", "")) - group_name = group.get("group_name", "") - group_descr = group.get("descr", "") - - if group_id: - # Format: "Group Name (ID)" or "Group Name - Description (ID)" - if group_descr and group_descr != group_name: - label = f"{group_name} - {group_descr} ({group_id})" - else: - label = f"{group_name} ({group_id})" - choices.append((group_id, label)) - except Exception: - # If API call fails, just use default option - pass - - return choices + self.fields["poller_group"].choices = _get_librenms_poller_group_choices() class AddToLIbreSNMPV3(forms.Form): @@ -412,38 +432,9 @@ class AddToLIbreSNMPV3(forms.Form): ) def __init__(self, *args, **kwargs): + """Initialize form and populate poller group choices.""" super().__init__(*args, **kwargs) - # Populate poller groups from LibreNMS API - self.fields["poller_group"].choices = self._get_poller_group_choices() - - def _get_poller_group_choices(self): - """Get poller group choices from LibreNMS API.""" - from .librenms_api import LibreNMSAPI - - choices = [("0", "Default (0)")] - - try: - api = LibreNMSAPI() - success, poller_groups = api.get_poller_groups() - - if success and poller_groups: - for group in poller_groups: - group_id = str(group.get("id", "")) - group_name = group.get("group_name", "") - group_descr = group.get("descr", "") - - if group_id: - # Format: "Group Name (ID)" or "Group Name - Description (ID)" - if group_descr and group_descr != group_name: - label = f"{group_name} - {group_descr} ({group_id})" - else: - label = f"{group_name} ({group_id})" - choices.append((group_id, label)) - except Exception: - # If API call fails, just use default option - pass - - return choices + self.fields["poller_group"].choices = _get_librenms_poller_group_choices() class DeviceStatusFilterForm(NetBoxModelFilterSetForm): @@ -452,6 +443,7 @@ class DeviceStatusFilterForm(NetBoxModelFilterSetForm): """ def __init__(self, *args, **kwargs): + """Initialize form and remove saved filter field.""" super().__init__(*args, **kwargs) # Remove the saved filter field if it exists if "filter_id" in self.fields: diff --git a/netbox_librenms_plugin/import_utils.py b/netbox_librenms_plugin/import_utils.py index 1c22dacdc3..feac6e3e99 100644 --- a/netbox_librenms_plugin/import_utils.py +++ b/netbox_librenms_plugin/import_utils.py @@ -6,6 +6,7 @@ - Retrieving filtered LibreNMS devices - Importing single and multiple devices - Smart matching of NetBox objects +- Permission checking for import operations """ import logging @@ -14,6 +15,7 @@ from core.choices import JobStatusChoices from dcim.models import Device, DeviceRole, DeviceType, Rack, Site, VirtualChassis from django.core.cache import cache +from django.core.exceptions import PermissionDenied from django.db import transaction from django.utils import timezone from virtualization.models import Cluster @@ -28,6 +30,52 @@ logger = logging.getLogger(__name__) +# ============================================================================= +# Permission Check Helpers +# ============================================================================= + + +def check_user_permissions(user, permissions): + """ + Check if user has all required permissions. + + Args: + user: The user object to check permissions for + permissions: List of permission strings (e.g., ['dcim.add_device', 'dcim.add_interface']) + + Returns: + tuple: (has_all_permissions: bool, missing_permissions: list[str]) + + Raises: + PermissionDenied: If user is None (no user context available) + """ + if user is None: + raise PermissionDenied("No user context available for permission check") + + missing = [perm for perm in permissions if not user.has_perm(perm)] + return (len(missing) == 0, missing) + + +def require_permissions(user, permissions, action_description="perform this action"): + """ + Require user has all permissions, raising PermissionDenied if not. + + Args: + user: The user object to check permissions for + permissions: List of permission strings + action_description: Human-readable description for error message + + Raises: + PermissionDenied: If user lacks any required permission + """ + has_perms, missing = check_user_permissions(user, permissions) + if not has_perms: + missing_str = ", ".join(missing) + raise PermissionDenied( + f"You do not have permission to {action_description}. Missing permissions: {missing_str}" + ) + + def get_cache_metadata_key(server_key: str, filters: dict, vc_enabled: bool) -> str: """ Generate a consistent cache metadata key from filter parameters. @@ -658,6 +706,13 @@ def validate_device_for_import( "import_as_vm": import_as_vm, "existing_device": None, "existing_match_type": None, # Track how existing device was matched + "serial_action": None, # None, "link", "conflict", "update_serial", "hostname_differs" + "serial_confirmed": False, # True when librenms_id match and serial matches + "serial_duplicate": False, # True when incoming serial is already on a different device + "name_matches": False, # True when existing device name matches LibreNMS sysName + "name_sync_available": False, # True when existing device name differs from sysName + "suggested_name": None, # sysName to suggest when name_sync_available is True + "device_type_mismatch": False, # True when existing device's type differs from LibreNMS "issues": [], "warnings": [], "virtual_chassis": empty_virtual_chassis_data(), @@ -717,78 +772,166 @@ def validate_device_for_import( result["existing_device"] = existing_vm result["existing_match_type"] = "librenms_id" result["import_as_vm"] = True # Force VM mode since VM exists - result["warnings"].append(f"VM already imported to NetBox as '{existing_vm.name}'") result["can_import"] = False - return result + + # Check if name matches sysName + # Note: name_sync_available/suggested_name are intentionally not set for VMs + # because UpdateDeviceNameView only supports Device objects; VM name-sync + # would require a separate implementation. + sys_name = libre_device.get("sysName") or "" + if sys_name and existing_vm.name == sys_name: + result["name_matches"] = True # Check for existing Device (by librenms_id custom field) # Always query with int to match custom field type - try: - existing_device = Device.objects.filter(custom_field_data__librenms_id=int(librenms_id)).first() - except (ValueError, TypeError): - # librenms_id is not convertible to int; no match will be found - existing_device = None - - if existing_device: - logger.info(f"Found existing device: {existing_device.name} (matched by librenms_id={librenms_id})") - result["existing_device"] = existing_device - result["existing_match_type"] = "librenms_id" - result["warnings"].append(f"Device already imported to NetBox as '{existing_device.name}'") - result["can_import"] = False - return result + if not result["existing_device"]: + try: + existing_device = Device.objects.filter(custom_field_data__librenms_id=int(librenms_id)).first() + except (ValueError, TypeError): + # librenms_id is not convertible to int; no match will be found + existing_device = None + + if existing_device: + logger.info(f"Found existing device: {existing_device.name} (matched by librenms_id={librenms_id})") + result["existing_device"] = existing_device + result["existing_match_type"] = "librenms_id" + result["can_import"] = False + + # Check if name matches sysName + sys_name = libre_device.get("sysName") or "" + if sys_name and existing_device.name == sys_name: + result["name_matches"] = True + elif sys_name and existing_device.name != sys_name: + result["name_sync_available"] = True + result["suggested_name"] = sys_name + + # Check for serial drift on the linked device + incoming_serial = libre_device.get("serial") or "" + if incoming_serial and incoming_serial != "-": + if existing_device.serial and existing_device.serial == incoming_serial: + result["serial_confirmed"] = True + elif existing_device.serial and existing_device.serial != incoming_serial: + serial_conflict = ( + Device.objects.filter(serial=incoming_serial).exclude(pk=existing_device.pk).first() + ) + if serial_conflict: + result["serial_action"] = "conflict" + result["serial_duplicate"] = True + result["warnings"].append( + f"Serial conflict: incoming serial '{incoming_serial}' is already assigned to " + f"device '{serial_conflict.name}' (ID: {serial_conflict.pk}) in NetBox. " + f"Investigate which device should own this serial before updating." + ) + else: + result["serial_action"] = "update_serial" + result["warnings"].append( + f"Serial number differs (NetBox: '{existing_device.serial}', " + f"LibreNMS: '{incoming_serial}'). Hardware may have been replaced." + ) - # Check by hostname/name - Check both VMs and Devices for conflicts - existing_vm = VirtualMachine.objects.filter(name__iexact=hostname).first() - existing_device = Device.objects.filter(name__iexact=hostname).first() + # Only check hostname/serial/IP if not already matched by librenms_id + if not result["existing_device"]: + # Check by hostname/name - Check both VMs and Devices for conflicts + existing_vm = VirtualMachine.objects.filter(name__iexact=hostname).first() + existing_device = Device.objects.filter(name__iexact=hostname).first() - # If BOTH exist with same hostname, it's ambiguous - don't match either - if existing_vm and existing_device: - logger.warning( - f"Hostname conflict: Both VM '{existing_vm.name}' and Device " - f"'{existing_device.name}' exist with hostname '{hostname}'" - ) - result["warnings"].append( - f"Both a VM and Device exist with hostname '{hostname}' in NetBox. " - f"Cannot determine which to match. Please set the librenms_id custom field on the correct object." - ) - # Don't set existing_device, don't block import - let user proceed as new - # This allows them to import and then resolve the conflict manually - elif existing_vm: - logger.info(f"Found existing VM by hostname: {existing_vm.name}") - result["existing_device"] = existing_vm - result["existing_match_type"] = "hostname" - result["import_as_vm"] = True # Force VM mode since VM exists - result["warnings"].append( - f"VM with same hostname exists in NetBox as '{existing_vm.name}' (not linked to LibreNMS)" - ) - result["can_import"] = False - return result - elif existing_device: - logger.info(f"Found existing device by hostname: {existing_device.name}") - result["existing_device"] = existing_device - result["existing_match_type"] = "hostname" - result["warnings"].append( - f"Device with same hostname exists in NetBox as '{existing_device.name}' (not linked to LibreNMS)" - ) - result["can_import"] = False - return result - - # Check by primary IP (weaker match, IP could be reassigned) - only for devices - primary_ip = libre_device.get("ip") - if primary_ip and not import_as_vm: - from ipam.models import IPAddress - - existing_ip = IPAddress.objects.filter(address__startswith=primary_ip).first() - if existing_ip and existing_ip.assigned_object: - device = existing_ip.assigned_object.device if hasattr(existing_ip.assigned_object, "device") else None - if device: - result["existing_device"] = device - result["existing_match_type"] = "primary_ip" + # If BOTH exist with same hostname, it's ambiguous - don't match either + if existing_vm and existing_device: + logger.warning( + f"Hostname conflict: Both VM '{existing_vm.name}' and Device " + f"'{existing_device.name}' exist with hostname '{hostname}'" + ) + result["warnings"].append( + f"Both a VM and Device exist with hostname '{hostname}' in NetBox. " + f"Cannot determine which to match. Please set the librenms_id custom field on the correct object." + ) + # Don't set existing_device, don't block import - let user proceed as new + # This allows them to import and then resolve the conflict manually + elif existing_vm: + logger.info(f"Found existing VM by hostname: {existing_vm.name}") + result["existing_device"] = existing_vm + result["existing_match_type"] = "hostname" + result["import_as_vm"] = True # Force VM mode since VM exists + result["warnings"].append( + f"VM with same hostname exists in NetBox as '{existing_vm.name}' (not linked to LibreNMS)" + ) + result["can_import"] = False + elif existing_device: + logger.info(f"Found existing device by hostname: {existing_device.name}") + result["existing_device"] = existing_device + result["existing_match_type"] = "hostname" + + # Check for serial conflict on hostname-matched device + incoming_serial = libre_device.get("serial") or "" + if incoming_serial and incoming_serial != "-" and existing_device.serial != incoming_serial: + serial_conflict = ( + Device.objects.filter(serial=incoming_serial).exclude(pk=existing_device.pk).first() + ) + if serial_conflict: + result["serial_action"] = "conflict" + result["serial_duplicate"] = True + result["warnings"].append( + f"Serial conflict: incoming serial '{incoming_serial}' is already assigned to " + f"device '{serial_conflict.name}' (ID: {serial_conflict.pk}) in NetBox. " + f"Investigate which device should own this serial before importing." + ) + else: + result["serial_action"] = "update_serial" + result["warnings"].append( + f"Hostname matches but serial differs (NetBox: '{existing_device.serial}', " + f"LibreNMS: '{incoming_serial}'). Hardware may have been replaced." + ) + else: result["warnings"].append( - f"IP address {primary_ip} already assigned to device '{device.name}' (not linked to LibreNMS)" + f"Device with same hostname exists in NetBox as '{existing_device.name}' (not linked to LibreNMS)" ) - result["can_import"] = False - return result + + result["can_import"] = False + + # Check by serial number (strong physical match - hardware identity) + if not result["existing_device"]: + serial = libre_device.get("serial") or "" + if serial and serial != "-" and not import_as_vm: + existing_by_serial = Device.objects.filter(serial=serial).first() + if existing_by_serial: + logger.info(f"Found existing device by serial: {existing_by_serial.name} (serial={serial})") + result["existing_device"] = existing_by_serial + result["existing_match_type"] = "serial" + result["can_import"] = False + + if existing_by_serial.name and existing_by_serial.name.lower() == hostname.lower(): + result["warnings"].append( + f"Device with same serial and hostname exists as '{existing_by_serial.name}' " + f"(not linked to LibreNMS)" + ) + result["serial_action"] = "link" + else: + result["warnings"].append( + f"Device with same serial ({serial}) exists as '{existing_by_serial.name}' " + f"but hostname differs (LibreNMS: '{hostname}'). Device may have been reinstalled." + ) + result["serial_action"] = "hostname_differs" + + # Check by primary IP (weaker match, IP could be reassigned) - only for devices + if not result["existing_device"]: + primary_ip = libre_device.get("ip") + if primary_ip and not import_as_vm: + from ipam.models import IPAddress + + existing_ip = IPAddress.objects.filter(address__net_host=primary_ip).first() + if existing_ip and existing_ip.assigned_object: + device = ( + existing_ip.assigned_object.device + if hasattr(existing_ip.assigned_object, "device") + else None + ) + if device: + result["existing_device"] = device + result["existing_match_type"] = "primary_ip" + result["warnings"].append( + f"IP address {primary_ip} already assigned to device '{device.name}' (not linked to LibreNMS)" + ) + result["can_import"] = False # Validate based on import type (Device or VM) if import_as_vm: @@ -905,13 +1048,6 @@ def validate_device_for_import( if not hostname: result["issues"].append("Device has no hostname") - # Serial number check - serial = libre_device.get("serial", "") - if serial and serial != "-": - existing_serial = Device.objects.filter(serial=serial).first() - if existing_serial: - result["warnings"].append(f"Serial number {serial} already exists on device: {existing_serial.name}") - # 7. Virtual chassis detection (only for devices, not VMs) if include_vc_detection and not import_as_vm and api is not None: device_id = libre_device.get("device_id") @@ -938,19 +1074,39 @@ def validate_device_for_import( logger.debug(f"No device_id found for {hostname}") # 8. Determine if device/VM is ready to import - result["can_import"] = len(result["issues"]) == 0 - - if import_as_vm: - # For VMs: only cluster is required - result["is_ready"] = result["can_import"] and result["cluster"]["found"] + if result["existing_device"]: + # Already matched - can_import was already set to False + result["is_ready"] = False + # Populate role from existing device so the modal shows it + existing = result["existing_device"] + if hasattr(existing, "role") and existing.role: + result["device_role"]["found"] = True + result["device_role"]["role"] = existing.role + + # Check for device type mismatch between existing device and LibreNMS + if hasattr(existing, "device_type") and existing.device_type: + librenms_dt = result["device_type"].get("device_type") + if librenms_dt and existing.device_type.pk != librenms_dt.pk: + result["device_type_mismatch"] = True + result["warnings"].append( + f"Device type mismatch: NetBox has '{existing.device_type}' " + f"but LibreNMS reports '{librenms_dt}'. " + f"This may indicate the wrong device was matched." + ) else: - # For Devices: site, device_type, and device_role are required - result["is_ready"] = ( - result["can_import"] - and result["site"]["found"] - and result["device_type"]["found"] - and result["device_role"]["found"] - ) + result["can_import"] = len(result["issues"]) == 0 + + if import_as_vm: + # For VMs: only cluster is required + result["is_ready"] = result["can_import"] and result["cluster"]["found"] + else: + # For Devices: site, device_type, and device_role are required + result["is_ready"] = ( + result["can_import"] + and result["site"]["found"] + and result["device_type"]["found"] + and result["device_role"]["found"] + ) logger.debug( f"Validation for {libre_device.get('hostname')} ({'VM' if import_as_vm else 'Device'}): " @@ -1188,6 +1344,7 @@ def bulk_import_devices_shared( manual_mappings_per_device: dict = None, libre_devices_cache: dict = None, job=None, + user=None, ) -> dict: """ Shared function for importing multiple LibreNMS devices to NetBox. @@ -1204,6 +1361,8 @@ def bulk_import_devices_shared( libre_devices_cache: Optional dict mapping device_id to pre-fetched device data to avoid redundant API calls. Example: {123: {...device_data...}} job: Optional JobRunner instance for progress logging and cancellation checks + user: User performing the import (for permission checks). If job is provided, + user is extracted from job.job.user if not explicitly passed. Returns: dict: Bulk import result with structure: @@ -1215,12 +1374,27 @@ def bulk_import_devices_shared( 'virtual_chassis_created': int # Number of VCs created } + Raises: + PermissionDenied: If user lacks required permissions + Example: >>> # Synchronous usage - >>> result = bulk_import_devices_shared([1, 2, 3, 4, 5]) + >>> result = bulk_import_devices_shared([1, 2, 3, 4, 5], user=request.user) >>> # Background job usage >>> result = bulk_import_devices_shared([1, 2, 3], job=self) """ + # Extract user from job if not explicitly provided + if user is None and job is not None: + user = getattr(job.job, "user", None) + + # Check permissions at start of bulk operation + required_perms = [ + "dcim.add_device", + "dcim.add_interface", + "dcim.add_virtualchassis", + ] + require_permissions(user, required_perms, "import devices") + total = len(device_ids) success_list = [] failed_list = [] @@ -1360,6 +1534,7 @@ def bulk_import_devices( sync_options: dict = None, manual_mappings_per_device: dict = None, libre_devices_cache: dict = None, + user=None, ) -> dict: """ Import multiple LibreNMS devices to NetBox (synchronous). @@ -1375,6 +1550,7 @@ def bulk_import_devices( Example: {1179: {'device_role_id': 5}, 1180: {'device_role_id': 3}} libre_devices_cache: Optional dict mapping device_id to pre-fetched device data to avoid redundant API calls. Example: {123: {...device_data...}} + user: User performing the import (for permission checks) Returns: dict: Bulk import result with structure: @@ -1385,6 +1561,9 @@ def bulk_import_devices( 'skipped': List[dict], # Skipped devices (already exist, etc.) 'virtual_chassis_created': int # Number of VCs created } + + Raises: + PermissionDenied: If user lacks required permissions """ return bulk_import_devices_shared( device_ids=device_ids, @@ -1393,6 +1572,7 @@ def bulk_import_devices( manual_mappings_per_device=manual_mappings_per_device, libre_devices_cache=libre_devices_cache, job=None, # No job context for synchronous imports + user=user, ) @@ -1532,6 +1712,7 @@ def bulk_import_vms( sync_options: dict = None, libre_devices_cache: dict = None, job=None, + user=None, ) -> dict: """ Import multiple LibreNMS devices as VMs in NetBox. @@ -1549,6 +1730,8 @@ def bulk_import_vms( sync_options: Optional dict with use_sysname, strip_domain settings libre_devices_cache: Optional pre-fetched device data cache job: Optional JobRunner instance for background job logging/cancellation + user: User performing the import (for permission checks). If job is provided, + user is extracted from job.job.user if not explicitly passed. Returns: Dict with keys: @@ -1556,10 +1739,13 @@ def bulk_import_vms( - failed: List of {"device_id": int, "error": str} - skipped: List of {"device_id": int, "reason": str} + Raises: + PermissionDenied: If user lacks required permissions + Example: >>> # Synchronous import from view >>> vm_imports = {123: {"cluster_id": 5, "device_role_id": 2}} - >>> result = bulk_import_vms(vm_imports, api, sync_options) + >>> result = bulk_import_vms(vm_imports, api, sync_options, user=request.user) >>> print(f"Created {len(result['success'])} VMs") >>> >>> # Background job import @@ -1570,6 +1756,13 @@ def bulk_import_vms( apply_role_to_validation, ) + # Extract user from job if not explicitly provided + if user is None and job is not None: + user = getattr(job.job, "user", None) + + # Check permissions at start of bulk operation + require_permissions(user, ["virtualization.add_virtualmachine"], "import VMs") + result = {"success": [], "failed": [], "skipped": []} vm_ids = list(vm_imports.keys()) @@ -1987,6 +2180,44 @@ def create_virtual_chassis_with_members(master_device: Device, members_info: lis raise +def _refresh_existing_device(validation: dict) -> None: + """Refresh existing_device from DB to pick up changes made in NetBox since caching.""" + existing = validation.get("existing_device") + if not existing or not hasattr(existing, "pk"): + return + try: + from dcim.models import Device + from virtualization.models import VirtualMachine + + if validation.get("import_as_vm"): + refreshed = VirtualMachine.objects.filter(pk=existing.pk).first() + else: + refreshed = Device.objects.filter(pk=existing.pk).first() + + if refreshed: + validation["existing_device"] = refreshed + if hasattr(refreshed, "role") and refreshed.role: + validation["device_role"] = {"found": True, "role": refreshed.role} + else: + # Device was deleted since caching β recompute readiness + validation["existing_device"] = None + validation["existing_match_type"] = None + validation["can_import"] = True + if validation.get("import_as_vm"): + validation["is_ready"] = bool( + validation.get("site", {}).get("found") and validation.get("device_role", {}).get("found") + ) + else: + validation["is_ready"] = bool( + validation.get("site", {}).get("found") + and validation.get("device_type", {}).get("found") + and validation.get("device_role", {}).get("found") + ) + except Exception as e: + existing_id = getattr(existing, "pk", "unknown") if existing else "none" + logger.error(f"Failed to refresh existing device (pk={existing_id}): {e}") + + def process_device_filters( api: LibreNMSAPI, filters: dict, @@ -2147,6 +2378,10 @@ def process_device_filters( # Use cached validation device["_validation"] = cached_device["_validation"] + # Refresh existing_device from DB to avoid stale data + # (user may have changed role, name, etc. in NetBox) + _refresh_existing_device(device["_validation"]) + # Apply exclude_existing filter if enabled if exclude_existing: validation = device["_validation"] diff --git a/netbox_librenms_plugin/jobs.py b/netbox_librenms_plugin/jobs.py index a5cdfd615b..bb3d00e37d 100644 --- a/netbox_librenms_plugin/jobs.py +++ b/netbox_librenms_plugin/jobs.py @@ -34,6 +34,8 @@ class FilterDevicesJob(JobRunner): """ class Meta: + """Meta options for FilterDevicesJob.""" + name = "LibreNMS Device Filter" def run( @@ -146,6 +148,8 @@ class ImportDevicesJob(JobRunner): """ class Meta: + """Meta options for ImportDevicesJob.""" + name = "LibreNMS Device Import" def run( @@ -201,6 +205,7 @@ def run( manual_mappings_per_device=manual_mappings_per_device, libre_devices_cache=libre_devices_cache, job=self, # Pass job context for logging and cancellation + user=self.job.user, # Pass user for permission checks ) # Import VMs @@ -209,7 +214,9 @@ def run( self.logger.info(f"Importing {len(vm_imports)} VMs...") from netbox_librenms_plugin.import_utils import bulk_import_vms - vm_result = bulk_import_vms(vm_imports, api, sync_options, libre_devices_cache, job=self) + vm_result = bulk_import_vms( + vm_imports, api, sync_options, libre_devices_cache, job=self, user=self.job.user + ) # Combine results imported_device_pks = [item["device"].pk for item in device_result.get("success", []) if item.get("device")] diff --git a/netbox_librenms_plugin/librenms_api.py b/netbox_librenms_plugin/librenms_api.py index de4192f065..9a5fc3488b 100644 --- a/netbox_librenms_plugin/librenms_api.py +++ b/netbox_librenms_plugin/librenms_api.py @@ -37,11 +37,31 @@ def __init__(self, server_key=None): # Default to 'default' if still no server_key server_key = server_key or "default" - self.server_key = server_key # Get server configuration servers_config = get_plugin_config("netbox_librenms_plugin", "servers") + # If the requested server_key doesn't exist but there are configured servers, + # only fall back to the first available server when using the auto-default key. + # If a specific (non-default) server_key was requested but not found, raise + # immediately to avoid silently using the wrong LibreNMS instance. + if servers_config and isinstance(servers_config, dict) and server_key not in servers_config: + if server_key != "default": + available = list(servers_config.keys()) + raise KeyError( + f"Server '{server_key}' not found in LibreNMS plugin configuration. Available servers: {available}" + ) + first_key = next(iter(servers_config), None) + if first_key: + logger.info( + "Server '%s' not found in config, falling back to '%s'", + server_key, + first_key, + ) + server_key = first_key + + self.server_key = server_key + if servers_config and isinstance(servers_config, dict) and server_key in servers_config: # Multi-server configuration config = servers_config[server_key] @@ -347,13 +367,13 @@ def add_device(self, data): Args: Dictionary containing device data including: - hostname: Device hostname or IP - - snmp_version: SNMP version (v2c or v3) + - snmp_version: SNMP version (v1, v2c, or v3) - force_add: Skip checks for duplicate device and SNMP reachability (optional, default False) - port: SNMP port (optional, defaults to config value) - transport: SNMP transport protocol (optional: udp, tcp, udp6, tcp6) - port_association_mode: Port identification method (optional: ifIndex, ifName, ifDescr, ifAlias) - poller_group: Poller group ID (optional, defaults to 0) - - community: SNMP community string (for v2c) + - community: SNMP community string (for v1 or v2c) - authlevel, authname, authpass, authalgo, cryptopass, cryptoalgo: SNMP v3 parameters Returns: @@ -375,7 +395,7 @@ def add_device(self, data): if data.get("poller_group") is not None: payload["poller_group"] = data["poller_group"] - if data["snmp_version"] == "v2c": + if data["snmp_version"] in ("v1", "v2c"): payload["community"] = data["community"] elif data["snmp_version"] == "v3": payload.update( diff --git a/netbox_librenms_plugin/models.py b/netbox_librenms_plugin/models.py index 64c44a4b59..cd79f47550 100644 --- a/netbox_librenms_plugin/models.py +++ b/netbox_librenms_plugin/models.py @@ -35,10 +35,13 @@ class LibreNMSSettings(models.Model): ) class Meta: + """Meta options for LibreNMSSettings.""" + verbose_name = "LibreNMS Settings" verbose_name_plural = "LibreNMS Settings" def get_absolute_url(self): + """Return the URL for the settings page.""" return reverse("plugins:netbox_librenms_plugin:settings") def __str__(self): @@ -46,6 +49,8 @@ def __str__(self): class InterfaceTypeMapping(NetBoxModel): + """Map LibreNMS interface types and speeds to NetBox interface types.""" + librenms_type = models.CharField(max_length=100) netbox_type = models.CharField( max_length=50, @@ -59,9 +64,12 @@ class InterfaceTypeMapping(NetBoxModel): ) def get_absolute_url(self): + """Return the URL for this mapping's detail page.""" return reverse("plugins:netbox_librenms_plugin:interfacetypemapping_detail", args=[self.pk]) class Meta: + """Meta options for InterfaceTypeMapping.""" + unique_together = ["librenms_type", "librenms_speed"] def __str__(self): diff --git a/netbox_librenms_plugin/navigation.py b/netbox_librenms_plugin/navigation.py index 81703f58b0..a08e62740f 100644 --- a/netbox_librenms_plugin/navigation.py +++ b/netbox_librenms_plugin/navigation.py @@ -1,7 +1,9 @@ from netbox.plugins import PluginMenu, PluginMenuButton, PluginMenuItem +from netbox_librenms_plugin.constants import PERM_VIEW_PLUGIN + menu = PluginMenu( - label="LibreNMS Plugin", # This will be your main menu heading + label="LibreNMS", icon_class="mdi mdi-network", groups=( ( @@ -10,12 +12,12 @@ PluginMenuItem( link="plugins:netbox_librenms_plugin:settings", link_text="Plugin Settings", - permissions=["netbox_librenms_plugin.view_librenmssettings"], + permissions=[PERM_VIEW_PLUGIN], ), PluginMenuItem( link="plugins:netbox_librenms_plugin:interfacetypemapping_list", link_text="Interface Mappings", - permissions=["netbox_librenms_plugin.view_interfacetypemapping"], + permissions=[PERM_VIEW_PLUGIN], buttons=( PluginMenuButton( link="plugins:netbox_librenms_plugin:interfacetypemapping_add", @@ -37,7 +39,7 @@ PluginMenuItem( link="plugins:netbox_librenms_plugin:librenms_import", link_text="LibreNMS Import", - permissions=["dcim.view_device"], + permissions=[PERM_VIEW_PLUGIN], ), ), ), @@ -47,17 +49,17 @@ PluginMenuItem( link="plugins:netbox_librenms_plugin:site_location_sync", link_text="Site & Location Sync", - permissions=["dcim.view_site"], + permissions=[PERM_VIEW_PLUGIN], ), PluginMenuItem( link="plugins:netbox_librenms_plugin:device_status_list", link_text="Device Status", - permissions=["dcim.view_device"], + permissions=[PERM_VIEW_PLUGIN], ), PluginMenuItem( link="plugins:netbox_librenms_plugin:vm_status_list", link_text="VM Status", - permissions=["virtualization.view_virtualmachine"], + permissions=[PERM_VIEW_PLUGIN], ), ), ), diff --git a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js index bc8c84bd1b..ab1e8a756d 100644 --- a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js +++ b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js @@ -252,7 +252,7 @@ const manager = new ModalManager(modalElement); manager.show(); - + // Store backdrop reference for legacy compatibility if (fallbackBackdropRef && manager.backdropElement) { fallbackBackdropRef.element = manager.backdropElement; @@ -265,6 +265,8 @@ * * @param {HTMLElement} modalElement - The modal element to hide * @param {Object} fallbackBackdropRef - Reference object containing fallback backdrop (deprecated) + * WONTFIX: fallbackBackdropRef is unused β _hideManual uses querySelector which is + * correct for this plugin since only one modal is ever open at a time (Tabler, no Bootstrap). */ function hideModal(modalElement, fallbackBackdropRef) { if (!modalElement) { @@ -272,6 +274,14 @@ } const manager = new ModalManager(modalElement); + + // Try to recover an existing Bootstrap instance before falling back to manual + if (typeof bootstrap !== 'undefined' && bootstrap.Modal) { + manager.instance = bootstrap.Modal.getInstance(modalElement); + } else if (typeof window.bootstrap !== 'undefined' && window.bootstrap.Modal) { + manager.instance = window.bootstrap.Modal.getInstance(modalElement); + } + manager.hide(); } @@ -293,6 +303,7 @@ function pollJobStatus(jobId, jobPk, pollUrl, baseUrl, originalFilters, deviceCount) { const messageEl = document.getElementById('filter-progress-message'); const cancelBtn = document.getElementById('cancel-filter-btn'); + const filterModal = document.getElementById('filter-processing-modal'); // Get CSRF token from cookie or form (needed for cancel and status sync) let csrfToken = getCookie('csrftoken'); @@ -357,11 +368,8 @@ messageEl.textContent = 'Job already completed, loading results...'; } cancelBtn.textContent = 'Completed'; - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } setTimeout(() => { @@ -403,11 +411,8 @@ messageEl.textContent = 'Job cancelled successfully.'; } cancelBtn.textContent = 'Cancelled'; - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } setTimeout(() => { @@ -424,11 +429,8 @@ messageEl.textContent = 'Job stopped (status sync failed).'; } cancelBtn.textContent = 'Stopped'; - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } setTimeout(() => { @@ -441,11 +443,8 @@ messageEl.textContent = 'Job completed, loading results...'; } cancelBtn.textContent = 'Completed'; - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } setTimeout(() => { @@ -460,11 +459,8 @@ } cancelBtn.textContent = 'Close'; cancelBtn.disabled = false; - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } setTimeout(() => window.location.href = baseUrl, 1000); @@ -545,11 +541,8 @@ if (statusValue === 'completed' || statusValue === 'finished') { pollingStopped = true; // Stop future polls - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } // Small delay to let modal close before redirect @@ -559,21 +552,15 @@ return; // Stop polling } else if (statusValue === 'stopped') { pollingStopped = true; - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } setTimeout(() => window.location.href = baseUrl, 100); } else if (statusValue === 'failed') { pollingStopped = true; - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } const errorMsg = data.data?.error; @@ -583,11 +570,8 @@ setTimeout(() => window.location.href = baseUrl, 100); } else if (statusValue === 'errored') { pollingStopped = true; - - const modal = document.getElementById('filter-processing-modal'); - if (modal) { - const manager = new ModalManager(modal); - manager.hide(); + if (filterModal) { + hideModal(filterModal); } const errorMsg = data.data?.error || 'Job encountered an error. Please try again.'; @@ -667,7 +651,7 @@ }); if (deviceCount) deviceCount.style.display = 'none'; - + if (cancelBtn) { cancelBtn.innerHTML = ' Close'; cancelBtn.onclick = function () { @@ -765,6 +749,10 @@ } }) .then(response => { + // Check for HTTP errors first + if (!response.ok) { + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + } // Check if response is JSON (background job) or HTML (synchronous) const contentType = response.headers.get('content-type'); if (contentType && contentType.includes('application/json')) { @@ -789,15 +777,11 @@ } else { // Unexpected JSON response alert('Unexpected response from server. Please try again.'); - if (modalInstance) { - modalInstance.hide(); - } + filterModalManager.hide(); } } else if (result.type === 'html') { // Synchronous response - navigate to the URL to reload with results - if (modalInstance) { - modalInstance.hide(); - } + filterModalManager.hide(); // Navigate to the results URL, allowing proper browser history window.location.href = finalUrl; } @@ -813,7 +797,9 @@ // Request was cancelled by user - silent } else { console.error('Error fetching filtered results:', error); - alert('Error loading filtered results. Please try again.'); + // Show more specific error if available + const errorMsg = error.message || 'Error loading filtered results. Please try again.'; + alert(errorMsg); } // Hide modal on error @@ -984,11 +970,8 @@ if (failedCount && failedCount.dataset.failedCount === '0') { setTimeout(() => { const resultsModal = document.getElementById('import-results-modal'); - if (resultsModal && typeof bootstrap !== 'undefined' && bootstrap.Modal) { - const modalInstance = bootstrap.Modal.getInstance(resultsModal); - if (modalInstance) { - modalInstance.hide(); - } + if (resultsModal) { + hideModal(resultsModal); } window.location.reload(); }, MODAL_AUTO_CLOSE_MS); diff --git a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_sync.js b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_sync.js index 392e84ead4..49de315dee 100644 --- a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_sync.js +++ b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_sync.js @@ -18,6 +18,22 @@ const TOMSELECT_INIT_DELAY_MS = 100; const COUNTDOWN_UPDATE_INTERVAL_MS = 1000; +// Helper to read CSRF token from cookies +function getCookie(name) { + let cookieValue = null; + if (document.cookie && document.cookie !== '') { + const cookies = document.cookie.split(';'); + for (let i = 0; i < cookies.length; i++) { + const cookie = cookies[i].trim(); + if (cookie.substring(0, name.length + 1) === (name + '=')) { + cookieValue = decodeURIComponent(cookie.substring(name.length + 1)); + break; + } + } + } + return cookieValue; +} + /** * Extract device/VM ID and type from current URL pathname. * Supports multiple URL patterns: @@ -640,21 +656,23 @@ function initializeTabs() { /** * Toggle SNMP form visibility based on selected version. - * Shows either SNMPv2c or SNMPv3 configuration form. + * Shows either SNMPv1/v2c or SNMPv3 configuration form. */ function toggleSNMPForms() { - const snmpSelect = document.querySelector('#add-device-modal select.form-select'); + const snmpSelect = document.getElementById('snmp-version-select'); if (!snmpSelect) return; const version = snmpSelect.value; - const v2Form = document.getElementById('snmpv2-form'); + const v1v2Form = document.getElementById('snmpv1v2-form'); const v3Form = document.getElementById('snmpv3-form'); - if (version === 'v2c') { - v2Form.style.display = 'block'; + if (!v1v2Form || !v3Form) return; + + if (version === 'v1v2c') { + v1v2Form.style.display = 'block'; v3Form.style.display = 'none'; - } else { - v2Form.style.display = 'none'; + } else if (version === 'v3') { + v1v2Form.style.display = 'none'; v3Form.style.display = 'block'; } } @@ -664,7 +682,7 @@ function toggleSNMPForms() { * Sets up version toggle and displays correct form. */ function initializeSNMPModalScripts() { - const snmpSelect = document.querySelector('#add-device-modal select.form-select'); + const snmpSelect = document.getElementById('snmp-version-select'); if (snmpSelect) { snmpSelect.addEventListener('change', toggleSNMPForms); // Initial call to set the correct form visibility @@ -709,11 +727,26 @@ function updateInterfaceNameField() { window.history.pushState({}, '', url); // Set HTMX headers for subsequent requests - htmx.config.defaultHeaders['X-Interface-Name-Field'] = this.value; + if (typeof htmx !== 'undefined') { + htmx.config.defaultHeaders['X-Interface-Name-Field'] = this.value; + } + + // Persist to user preferences via API + const savePrefUrl = this.closest('[data-save-pref-url]')?.dataset.savePrefUrl; + if (savePrefUrl) { + const csrfToken = document.querySelector('[name=csrfmiddlewaretoken]')?.value || getCookie('csrftoken'); + if (csrfToken) { + fetch(savePrefUrl, { + method: 'POST', + headers: {'Content-Type': 'application/json', 'X-CSRFToken': csrfToken}, + body: JSON.stringify({key: 'interface_name_field', value: this.value}) + }).catch(err => console.debug('Failed to save interface_name_field pref:', err)); + } + } // Refresh current tab content const activeTab = document.querySelector('.tab-pane.active'); - if (activeTab) { + if (activeTab && typeof htmx !== 'undefined') { htmx.trigger(activeTab, 'htmx:refresh'); } }); diff --git a/netbox_librenms_plugin/tables/VM_status.py b/netbox_librenms_plugin/tables/VM_status.py index dd86d0e428..5227ca9bbd 100644 --- a/netbox_librenms_plugin/tables/VM_status.py +++ b/netbox_librenms_plugin/tables/VM_status.py @@ -34,6 +34,8 @@ def render_librenms_status(self, value, record): return mark_safe(f'{status}') class Meta(VirtualMachineTable.Meta): + """Meta options for VMStatusTable.""" + model = VirtualMachine fields = ( "pk", diff --git a/netbox_librenms_plugin/tables/cables.py b/netbox_librenms_plugin/tables/cables.py index e08918b307..8f3a93fd08 100644 --- a/netbox_librenms_plugin/tables/cables.py +++ b/netbox_librenms_plugin/tables/cables.py @@ -46,6 +46,7 @@ class LibreNMSCableTable(tables.Table): ) def __init__(self, *args, device=None, **kwargs): + """Initialize table with optional device context.""" self.device = device super().__init__(*args, **kwargs) self.tab = "cables" @@ -53,26 +54,31 @@ def __init__(self, *args, device=None, **kwargs): self.prefix = "cables_" def render_remote_device(self, value, record): + """Render remote device name as a link if URL is available.""" if url := record.get("remote_device_url"): return format_html('{}', url, value) return value def render_local_port(self, value, record): + """Render local port name as a link if URL is available.""" if url := record.get("local_port_url"): return format_html('{}', url, value) return value def render_remote_port(self, value, record): + """Render remote port name as a link if URL is available.""" if url := record.get("remote_port_url"): return format_html('{}', url, value) return value def render_cable_status(self, value, record): + """Render cable status as a link if cable URL is available.""" if url := record.get("cable_url"): return format_html('{}', url, value) return value def configure(self, request): + """Configure pagination for the table using the current request.""" paginate = { "paginator_class": EnhancedPaginator, "per_page": get_table_paginate_count(request, self.prefix), @@ -80,6 +86,8 @@ def configure(self, request): tables.RequestConfig(request, paginate).configure(self) class Meta: + """Define column sequence, row attributes, and table styling.""" + sequence = [ "selection", "local_port", @@ -108,10 +116,12 @@ class VCCableTable(LibreNMSCableTable): ) def __init__(self, *args, device=None, **kwargs): + """Initialize the VC cable table with device context.""" super().__init__(*args, device=device, **kwargs) def render_device_selection(self, value, record): - members = self.device.virtual_chassis.members.all() + """Render a dropdown to select the virtual chassis member for a port.""" + members = self.device.virtual_chassis.members.order_by("vc_position", "name") chassis_member = get_virtual_chassis_member(self.device, record["local_port"]) selected_member_id = chassis_member.id if chassis_member else self.device.id @@ -127,6 +137,8 @@ def render_device_selection(self, value, record): ) class Meta(LibreNMSCableTable.Meta): + """Define column sequence and attributes for the VC cable table.""" + sequence = [ "selection", "device_selection", diff --git a/netbox_librenms_plugin/tables/device_status.py b/netbox_librenms_plugin/tables/device_status.py index c5dedaf1c8..c7f60d07b8 100644 --- a/netbox_librenms_plugin/tables/device_status.py +++ b/netbox_librenms_plugin/tables/device_status.py @@ -25,6 +25,7 @@ class DeviceStatusTable(DeviceTable): ) def render_librenms_status(self, value, record): + """Render LibreNMS sync status with link to sync page.""" sync_url = reverse( "plugins:netbox_librenms_plugin:device_librenms_sync", kwargs={"pk": record.pk}, @@ -52,6 +53,8 @@ def render_librenms_status(self, value, record): return mark_safe(f'{status}') class Meta(DeviceTable.Meta): + """Meta options for DeviceStatusTable.""" + model = Device fields = ( "pk", @@ -89,7 +92,9 @@ class DeviceImportTable(tables.Table): name = "DeviceImportTable" # Required by NetBox table utilities def __init__(self, *args, **kwargs): + """Initialize table with cached querysets and apply sorting.""" super().__init__(*args, **kwargs) + # Cache querysets to avoid N queries per render from dcim.models import DeviceRole from virtualization.models import Cluster @@ -127,6 +132,7 @@ def _sort_data(self): # Sort the data list in place # Handle None values by treating them as empty strings for sorting def sort_key(item): + """Return lowercase sort value for a data field.""" value = item.get(data_key, "") return (value or "").lower() if isinstance(value, str) else str(value or "") @@ -425,6 +431,7 @@ def render_actions(self, value, record): """ Render action buttons for import using HTMX. Shows Import button if can import, otherwise shows Preview/Configure. + Permission checks are handled by backend require_write_permission() which shows toast. """ validation = record.get("_validation", {}) device_id = record.get("device_id") @@ -437,7 +444,7 @@ def render_actions(self, value, record): buttons = [] if existing: - # Link to existing device/VM in NetBox + # Link to existing device/VM in NetBox + details button for conflict resolution if isinstance(existing, VirtualMachine): url_name = "virtualization:virtualmachine" title = "View VM in NetBox" @@ -450,6 +457,44 @@ def render_actions(self, value, record): f'' ) + + # Add details/conflict button for conflict resolution actions + details_url = self._build_validation_details_url(device_id, validation) + match_type = validation.get("existing_match_type", "") + serial_action = validation.get("serial_action") + has_mismatch = validation.get("device_type_mismatch", False) + has_actions = match_type in ("hostname", "serial") and serial_action is not None + has_name_sync = validation.get("name_sync_available", False) + has_sync_needed = match_type == "librenms_id" and serial_action in ("update_serial", "conflict") + + if has_mismatch: + btn_class = "btn-outline-danger" + btn_icon = "mdi-alert-circle" + btn_label = " Conflict" + elif has_actions: + btn_class = "btn-outline-warning" + btn_icon = "mdi-alert" + btn_label = " Conflict" + elif has_name_sync or has_sync_needed: + btn_class = "btn-outline-warning" + btn_icon = "mdi-information-outline" + btn_label = " Details" + else: + btn_class = "btn-outline-info" + btn_icon = "mdi-information-outline" + btn_label = " Details" + + btn_title = "Resolve conflict" if (has_actions or has_mismatch) else "View details" + buttons.append( + f'' + ) elif is_ready: # Ready to import - show Import and Details buttons details_url = self._build_validation_details_url(device_id, validation) @@ -625,6 +670,8 @@ def _build_vc_attributes(validation: dict, record: dict) -> str: ) class Meta: + """Meta options for DeviceImportTable.""" + # No model - we're working with LibreNMS API dictionaries, not Django model instances # This prevents NetBoxTable from auto-adding custom fields from Device model diff --git a/netbox_librenms_plugin/tables/interfaces.py b/netbox_librenms_plugin/tables/interfaces.py index 2e56edc7ad..e5aa6d06c1 100644 --- a/netbox_librenms_plugin/tables/interfaces.py +++ b/netbox_librenms_plugin/tables/interfaces.py @@ -21,6 +21,8 @@ class LibreNMSInterfaceTable(tables.Table): """ class Meta: + """Meta options for LibreNMSInterfaceTable.""" + sequence = [ "selection", "name", @@ -38,6 +40,7 @@ class Meta: } def __init__(self, *args, device=None, interface_name_field=None, **kwargs): + """Initialize table with device context and interface name field.""" self.device = device self.interface_name_field = interface_name_field or get_interface_name_field() @@ -49,9 +52,9 @@ def __init__(self, *args, device=None, interface_name_field=None, **kwargs): self._meta.row_attrs = { "data-interface": lambda record: record.get(self.interface_name_field), "data-name": lambda record: record.get(self.interface_name_field), - "data-enabled": lambda record: record.get("ifAdminStatus", "").lower() - if record.get("ifAdminStatus") - else "", + "data-enabled": lambda record: ( + str(record.get("ifAdminStatus", "")).lower() if record.get("ifAdminStatus") else "" + ), } super().__init__(*args, **kwargs) @@ -317,6 +320,7 @@ class VCInterfaceTable(LibreNMSInterfaceTable): ) def __init__(self, *args, device=None, interface_name_field=None, **kwargs): + """Initialize VC interface table with device and name field.""" super().__init__(*args, device=device, interface_name_field=interface_name_field, **kwargs) # Ensure device_selection column is visible if hasattr(self.device, "virtual_chassis") and self.device.virtual_chassis: @@ -356,11 +360,14 @@ def render_device_selection(self, value, record): ) def format_interface_data(self, port_data, device): + """Format interface data including VC device selection column.""" formatted_data = super().format_interface_data(port_data, device) formatted_data["device_selection"] = self.render_device_selection(None, port_data) return formatted_data class Meta: + """Meta options for VCInterfaceTable.""" + sequence = [ "selection", "device_selection", @@ -384,6 +391,8 @@ class LibreNMSVMInterfaceTable(LibreNMSInterfaceTable): """ class Meta(LibreNMSInterfaceTable.Meta): + """Meta options for LibreNMSVMInterfaceTable.""" + sequence = [ "selection", "name", diff --git a/netbox_librenms_plugin/tables/ipaddresses.py b/netbox_librenms_plugin/tables/ipaddresses.py index 65ef58b4b8..c076d54ba8 100644 --- a/netbox_librenms_plugin/tables/ipaddresses.py +++ b/netbox_librenms_plugin/tables/ipaddresses.py @@ -12,9 +12,12 @@ class IPAddressTable(tables.Table): """ def __init__(self, *args, **kwargs): + """Initialize IP address table.""" super().__init__(*args, **kwargs) class Meta: + """Meta options for IPAddressTable.""" + sequence = [ "selection", "address", diff --git a/netbox_librenms_plugin/tables/locations.py b/netbox_librenms_plugin/tables/locations.py index 8cd017d9de..b37a8731f0 100644 --- a/netbox_librenms_plugin/tables/locations.py +++ b/netbox_librenms_plugin/tables/locations.py @@ -11,17 +11,19 @@ class SiteLocationSyncTable(tables.Table): """ netbox_site = tables.Column(linkify=True) - latitude = tables.Column(accessor="netbox_site.latitude") - longitude = tables.Column(accessor="netbox_site.longitude") - librenms_location = tables.Column(accessor="librenms_location.location", verbose_name="LibreNMS Location") - librenms_latitude = tables.Column(accessor="librenms_location.lat", verbose_name="LibreNMS Latitude") - librenms_longitude = tables.Column(accessor="librenms_location.lng", verbose_name="LibreNMS Longitude") + latitude = tables.Column(accessor="netbox_site__latitude") + longitude = tables.Column(accessor="netbox_site__longitude") + librenms_location = tables.Column(accessor="librenms_location__location", verbose_name="LibreNMS Location") + librenms_latitude = tables.Column(accessor="librenms_location__lat", verbose_name="LibreNMS Latitude") + librenms_longitude = tables.Column(accessor="librenms_location__lng", verbose_name="LibreNMS Longitude") actions = tables.Column(empty_values=()) def render_latitude(self, value, record): + """Render latitude with sync-status styling.""" return self.render_coordinate(value, record.is_synced) def render_longitude(self, value, record): + """Render longitude with sync-status styling.""" return self.render_coordinate(value, record.is_synced) def render_coordinate(self, value, is_synced): @@ -68,6 +70,8 @@ def configure(self, request): tables.RequestConfig(request, paginate).configure(self) class Meta: + """Meta options for SiteLocationSyncTable.""" + fields = ( "netbox_site", "latitude", diff --git a/netbox_librenms_plugin/tables/mappings.py b/netbox_librenms_plugin/tables/mappings.py index beb4e77ca4..73949fd2c8 100644 --- a/netbox_librenms_plugin/tables/mappings.py +++ b/netbox_librenms_plugin/tables/mappings.py @@ -16,6 +16,8 @@ class InterfaceTypeMappingTable(NetBoxTable): actions = columns.ActionsColumn(actions=("edit", "delete")) class Meta: + """Meta options for InterfaceTypeMappingTable.""" + model = InterfaceTypeMapping fields = ( "id", diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_cable_sync_content.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_cable_sync_content.html index f12904ff40..c42534a73d 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_cable_sync_content.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_cable_sync_content.html @@ -70,6 +70,13 @@ +{% else %} +
No cable data loaded. Click Refresh Cables to fetch data from LibreNMS.
+No interface data loaded. Click Refresh Interfaces to fetch data from LibreNMS.
+No IP address data loaded. Click Refresh IP Addresses to fetch data from LibreNMS.
+| Status | ++ {% if libre_device.status == 1 %} + Up + {% elif libre_device.status == 0 %} + Down + {% else %} + Unknown + {% endif %} + | +
|---|---|
| Hostname | +{{ libre_device.hostname }} | +
| ID | +{{ libre_device.device_id }} | +
| IP | +{{ libre_device.ip|default:"β" }} | +
| Location | +{{ libre_device.location|default:"β" }} | +
| Field | +NetBox Value | +LibreNMS Value | +
|---|---|---|
| Name | ++ {% if validation.existing_device %} + + {{ validation.existing_device.name }} + + {% if validation.name_sync_available %} + + {% endif %} + {% else %} + New device + {% endif %} + | +{{ libre_device.sysName|default:libre_device.hostname }} | +
| Site | ++ {% if validation.existing_device and validation.existing_device.site %} + {{ validation.existing_device.site }} + {% if validation.site.site and validation.existing_device.site.pk == validation.site.site.pk %} + + {% endif %} + {% elif validation.site.site %} + {{ validation.site.site.name }} + {% else %} + No matching site + {% endif %} + | +{{ libre_device.location|default:"β" }} | +
| Device Type | ++ {% if validation.device_type_mismatch %} + + {{ validation.existing_device.device_type }} + + + + {% elif validation.existing_device and validation.existing_device.device_type %} + {{ validation.existing_device.device_type }} + {% if sync_info and not sync_info.device_type_synced and sync_info.librenms_device_type %} + + {% elif validation.device_type.device_type %} + + {% endif %} + {% elif validation.device_type.device_type %} + {{ validation.device_type.device_type }} + {% else %} + No matching type + {% endif %} + | +{{ libre_device.hardware|default:"β" }} | +
| Serial | ++ {% if validation.existing_device %} + {% if validation.existing_device.serial %} + {{ validation.existing_device.serial }} + {% else %} + Not set + {% endif %} + {% if sync_info and not sync_info.serial_synced %} + {% if validation.serial_action == 'conflict' %} + + + + {% else %} + + {% endif %} + {% elif sync_info and sync_info.serial_synced and sync_info.librenms_serial != '-' %} + + {% endif %} + {% else %} + β + {% endif %} + | ++ {{ libre_device.serial|default:"β" }} + {% if validation.serial_duplicate %} + Conflict + {% endif %} + | +
| + {% if validation.import_as_vm %}Role{% else %}Device Role{% endif %} + | ++ {% if validation.existing_device and validation.existing_device.role %} + {{ validation.existing_device.role }} + + {% elif validation.device_role.role %} + {{ validation.device_role.role.name }} + {% else %} + No role assigned + {% endif %} + | +β | +
| Platform | ++ {% if validation.existing_device and validation.existing_device.platform %} + {{ validation.existing_device.platform }} + {% if sync_info and not sync_info.platform_synced %} + {% if sync_info.platform_info.platform_exists %} + + {% else %} + + + + {% endif %} + {% elif sync_info and sync_info.platform_synced %} + + {% endif %} + {% elif validation.platform.platform %} + {{ validation.platform.platform.name }} + {% elif sync_info and sync_info.platform_info.platform_exists %} + Not set + {% if validation.existing_device %} + + {% endif %} + {% else %} + Optional + {% endif %} + | +{{ libre_device.os|default:"β" }} | +
| Cluster | ++ {% if validation.cluster.cluster %} + {{ validation.cluster.cluster.name }} + {% else %} + No cluster assigned + {% endif %} + | +β | +
| Rack | ++ {% if validation.rack.rack %} + + {% if validation.rack.rack.location %} + {{ validation.rack.rack.location.name }} β {{ validation.rack.rack.name }} + {% else %} + {{ validation.rack.rack.name }} + {% endif %} + + {% else %} + Optional + {% endif %} + | +β | +
| Primary IP | ++ {% if validation.existing_device and validation.existing_device.primary_ip %} + {{ validation.existing_device.primary_ip }} + + {% elif libre_device.ip %} + {{ libre_device.ip }} + + {% else %} + No primary IP + {% endif %} + | +{{ libre_device.ip|default:"β" }} | +
- - The following NetBox objects will be used when importing this - {% if validation.import_as_vm %}Virtual Machine{% else %}Device{% endif %}: - -
-| Import Type: | -- - Virtual Machine - | -
| Cluster: | -- {% if validation.cluster.cluster %} - - {{ validation.cluster.cluster.name }} - {% else %} - - No cluster assigned (required for VMs) - {% endif %} - | -
| Role: | -- {% if validation.device_role.role %} - - {{ validation.device_role.role.name }} - {% else %} - - Optional - not assigned - {% endif %} - | -
| Device Role: | -- {% if validation.device_role.role %} - - {{ validation.device_role.role.name }} - {% else %} - - No device role assigned - {% endif %} - | -
| Platform: | -- {% if validation.platform.platform %} - - {{ validation.platform.platform.name }} - {% else %} - - Optional - will be auto-mapped if available - {% endif %} - | -
| Rack: | -- {% if validation.rack.rack %} - - - {% if validation.rack.rack.location %} - {{ validation.rack.rack.location.name }} - {{ validation.rack.rack.name }} - {% else %} - {{ validation.rack.rack.name }} - {% endif %} - - {% else %} - - Optional - not assigned - {% endif %} - | -
| Primary IP: | -- {% if libre_device.ip %} - - {{ libre_device.ip }} - {% else %} - - No primary IP - will import without - {% endif %} - | -
These warnings won't prevent import but should be reviewed:
-This section allows you to map LibreNMS interface types to NetBox interface types. - When synchronizing interfaces from LibreNMS, these mappings will be used to ensure +
This section allows you to map LibreNMS interface types to NetBox interface types. + When synchronizing interfaces from LibreNMS, these mappings will be used to ensure correct interface type assignment in NetBox.
Example: Map LibreNMS type "ethernetCsmacd" to NetBox type "1000base-t"