From ededd39d5b0f8a9e07be728202aa679b7ad492df Mon Sep 17 00:00:00 2001 From: Adam Outler Date: Sun, 26 Oct 2025 17:53:46 +0000 Subject: [PATCH] Coderabbit fixes --- .devcontainer/devcontainer.json | 3 +- .vscode/tasks.json | 2 +- install/production-filesystem/entrypoint.sh | 2 +- server/api_server/nettools_endpoint.py | 45 +++++++++++++++++-- .../test_container_environment.py | 8 ++-- 5 files changed, 50 insertions(+), 10 deletions(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 9a54132f..335a3b3c 100755 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -47,7 +47,8 @@ "Install Pip Requriements": "/opt/venv/bin/pip3 install pytest docker debugpy" }, "postStartCommand": { - "Start Environment":"${containerWorkspaceFolder}/.devcontainer/scripts/setup.sh" + "Start Environment":"${containerWorkspaceFolder}/.devcontainer/scripts/setup.sh", + "Build test-container":"nohup screen docker buildx build --platform linux/amd64 --tag netalertx-test . & disown" }, "customizations": { "vscode": { diff --git a/.vscode/tasks.json b/.vscode/tasks.json index c4107b98..8fc25743 100755 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -164,7 +164,7 @@ { "label": "[Any] Build Unit Test Docker image", "type": "shell", - "command": "docker buildx build -t netalertx-test .; echo '🧪 Unit Test Docker image built: netalertx-test'", + "command": "docker buildx build -t netalertx-test . && echo '🧪 Unit Test Docker image built: netalertx-test'", "presentation": { "echo": true, "reveal": "always", diff --git a/install/production-filesystem/entrypoint.sh b/install/production-filesystem/entrypoint.sh index 84298403..25e67573 100755 --- a/install/production-filesystem/entrypoint.sh +++ b/install/production-filesystem/entrypoint.sh @@ -71,7 +71,7 @@ for script in ${SYSTEM_SERVICES_SCRIPTS}/check-*.sh; do done -if [ ${FAILED_STATUS} ]; then +if [ -n "${FAILED_STATUS}" ]; then echo "Container startup checks failed with exit code ${FAILED_STATUS}." exit ${FAILED_STATUS} fi diff --git a/server/api_server/nettools_endpoint.py b/server/api_server/nettools_endpoint.py index 10b2864e..3d9209be 100755 --- a/server/api_server/nettools_endpoint.py +++ b/server/api_server/nettools_endpoint.py @@ -2,13 +2,37 @@ import subprocess import re import sys import ipaddress -import speedtest as speedtest_cli +import shutil +import os from flask import jsonify # Register NetAlertX directories INSTALL_PATH = "/app" sys.path.extend([f"{INSTALL_PATH}/front/plugins", f"{INSTALL_PATH}/server"]) +# Resolve speedtest-cli path once at module load and validate it. +# We do this once to avoid repeated PATH lookups and to fail fast when +# the binary isn't available or executable. +SPEEDTEST_CLI_PATH = None + +def _get_speedtest_cli_path(): + """Resolve and validate the speedtest-cli executable path.""" + path = shutil.which("speedtest-cli") + if path is None: + raise RuntimeError( + "speedtest-cli not found in PATH. Please install it: pip install speedtest-cli" + ) + if not os.access(path, os.X_OK): + raise RuntimeError(f"speedtest-cli found at {path} but is not executable") + return path + +try: + SPEEDTEST_CLI_PATH = _get_speedtest_cli_path() +except Exception as e: + # Warn but don't crash import — the endpoint will return 503 when called. + print(f"Warning: {e}", file=sys.stderr) + SPEEDTEST_CLI_PATH = None + def wakeonlan(mac): # Validate MAC @@ -78,10 +102,18 @@ def speedtest(): API endpoint to run a speedtest using speedtest-cli. Returns JSON with the test output or error. """ + # If the CLI wasn't found at module load, return a 503 so the caller + # knows the service is unavailable rather than failing unpredictably. + if SPEEDTEST_CLI_PATH is None: + return jsonify({ + "success": False, + "error": "speedtest-cli is not installed or not found in PATH" + }), 503 + try: - # Run speedtest-cli command + # Run speedtest-cli command using the resolved absolute path result = subprocess.run( - ["speedtest-cli", "--secure", "--simple"], + [SPEEDTEST_CLI_PATH, "--secure", "--simple"], capture_output=True, text=True, check=True @@ -98,6 +130,13 @@ def speedtest(): "details": e.stderr.strip() }), 500 + except Exception as e: + return jsonify({ + "success": False, + "error": "Failed to run speedtest", + "details": str(e) + }), 500 + def nslookup(ip): """ diff --git a/test/docker_tests/test_container_environment.py b/test/docker_tests/test_container_environment.py index 5a39487d..b0043957 100644 --- a/test/docker_tests/test_container_environment.py +++ b/test/docker_tests/test_container_environment.py @@ -723,7 +723,7 @@ def test_missing_mount_app_log(tmp_path: pathlib.Path) -> None: volumes = _build_volume_args(paths, skip={"app_log"}) result = _run_container("missing-mount-app-log", volumes, user="20211:20211") _assert_contains(result, "Write permission denied", result.args) - _assert_contains(result, "/app/api", result.args) + _assert_contains(result, "/app/log", result.args) def test_missing_mount_app_api(tmp_path: pathlib.Path) -> None: @@ -737,7 +737,7 @@ def test_missing_mount_app_api(tmp_path: pathlib.Path) -> None: volumes = _build_volume_args(paths, skip={"app_api"}) result = _run_container("missing-mount-app-api", volumes, user="20211:20211") _assert_contains(result, "Write permission denied", result.args) - _assert_contains(result, "/app/config", result.args) + _assert_contains(result, "/app/api", result.args) def test_missing_mount_nginx_conf(tmp_path: pathlib.Path) -> None: @@ -751,7 +751,7 @@ def test_missing_mount_nginx_conf(tmp_path: pathlib.Path) -> None: volumes = _build_volume_args(paths, skip={"nginx_conf"}) result = _run_container("missing-mount-nginx-conf", volumes, user="20211:20211") _assert_contains(result, "Write permission denied", result.args) - _assert_contains(result, "/app/api", result.args) + _assert_contains(result, "/services/config/nginx/conf.active", result.args) assert result.returncode != 0 @@ -766,7 +766,7 @@ def test_missing_mount_services_run(tmp_path: pathlib.Path) -> None: volumes = _build_volume_args(paths, skip={"services_run"}) result = _run_container("missing-mount-services-run", volumes, user="20211:20211") _assert_contains(result, "Write permission denied", result.args) - _assert_contains(result, "/app/api", result.args) + _assert_contains(result, "/services/run", result.args) _assert_contains(result, "Container startup checks failed with exit code", result.args)