feat: Add ZABBIX_URL, prioritize env over config. (#277)

Author: pederhan

Diff for: CHANGELOG

@@ -7,7 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- released start -->
 
-<!-- ## [Unreleased] -->
+## [Unreleased]
+
+### Added
+
+- Environment variable `ZABBIX_URL` to specify the URL for the Zabbix API.
+
+### Changed
+
+- Authentication info from environment variables now take priority over the configuration file.
 
 ## [3.4.2](https://github.com/unioslo/zabbix-cli/releases/tag/3.4.2) - 2024-12-16
 

Diff for: docs/guide/authentication.md

@@ -2,12 +2,12 @@
 
 Zabbix-cli provides several ways to authenticate. They are tried in the following order:
 
-1. [Token - Config file](#config-file)
 1. [Token - Environment variables](#environment-variables)
+1. [Token - Config file](#config-file)
 1. [Token - Auth token file](#auth-token-file)
+1. [Password - Environment variables](#environment-variables_1)
 1. [Password - Config file](#config-file_1)
 1. [Password - Auth file](#auth-file)
-1. [Password - Environment variables](#environment-variables_1)
 1. [Password - Prompt](#prompt)
 
 ## Token
@@ -17,6 +17,14 @@ The application supports authenticating with an API or session token. API tokens
 !!! info "Session vs API token"
     Semantically, a session token and API token are the same thing from an API authentication perspective. They are both sent as the `auth` parameter in the Zabbix API requests.
 
+### Environment variables
+
+The API token can be set as an environment variable:
+
+```bash
+export ZABBIX_API_TOKEN="API TOKEN"
+```
+
 ### Config file
 
 The token can be set directly in the config file:
@@ -26,14 +34,6 @@ The token can be set directly in the config file:
 auth_token = "API_TOKEN"
 ```
 
-### Environment variables
-
-The API token can be set as an environment variable:
-
-```bash
-export ZABBIX_API_TOKEN="API TOKEN"
-```
-
 ### Auth token file
 
 The application can store and reuse session tokens between runs. This feature is enabled by default and configurable via the following options:
@@ -62,6 +62,15 @@ When `allow_insecure_auth_file` is set to `false`, the application will attempt
 
 The application supports authenticating with a username and password. The password can be set in the config file, an auth file, as environment variables, or prompted for when starting the application.
 
+### Environment variables
+
+The username and password can be set as environment variables:
+
+```bash
+export ZABBIX_USERNAME="Admin"
+export ZABBIX_PASSWORD="zabbix"
+```
+
 ### Config file
 
 The password can be set directly in the config file:
@@ -87,20 +96,45 @@ The location of the auth file file can be changed in the config file:
 auth_file = "~/.zabbix-cli_auth"
 ```
 
-### Environment variables
+### Prompt
 
-The username and password can be set as environment variables:
+When all other authentication methods fail, the application will prompt for a username and password. The default username in the prompt can be configured:
 
-```bash
-export ZABBIX_USERNAME="Admin"
-export ZABBIX_PASSWORD="zabbix"
+```toml
+[api]
+username = "Admin"
 ```
 
-### Prompt
+## URL
 
-When all other authentication methods fail, the application will prompt for a username and password. The default username in the prompt can be configured:
+The URL of the Zabbix API can be set in the config file, as an environment variable, or prompted for when starting the application.
+
+They are processed in the following order:
+
+1. [Environment variables](#environment-variables_2)
+1. [Config file](#config-file_2)
+1. [Prompt](#prompt_1)
+
+The URL should not include `/api_jsonrpc.php`.
+
+### Config file
+
+The URL of the Zabbix API can be set in the config file:
 
 ```toml
+
 [api]
-username = "Admin"
+url = "http://zabbix.example.com"
+```
+
+### Environment variables
+
+The URL can also be set as an environment variable:
+
+```bash
+export ZABBIX_URL="http://zabbix.example.com"
 ```
+
+### Prompt
+
+When all other methods fail, the application will prompt for the URL of the Zabbix API.

Diff for: pyproject.toml

@@ -61,6 +61,7 @@ test = [
   "pytest-cov>=5.0.0",
   "inline-snapshot>=0.13.0",
   "freezegun>=1.5.1",
+  "pytest-httpserver>=1.1.0",
 ]
 docs = [
   "mkdocs>=1.5.3",

Diff for: tests/conftest.py

@@ -8,6 +8,7 @@
 import pytest
 import typer
 from packaging.version import Version
+from pytest_httpserver import HTTPServer
 from typer.testing import CliRunner
 from zabbix_cli.app import StatefulApp
 from zabbix_cli.config.model import Config
@@ -98,14 +99,30 @@ def config(tmp_path: Path) -> Iterator[Config]:
 
 
 @pytest.fixture(name="zabbix_client")
-def zabbix_client(monkeypatch: pytest.MonkeyPatch) -> Iterator[ZabbixAPI]:
+def zabbix_client() -> Iterator[ZabbixAPI]:
     config = Config.sample_config()
     client = ZabbixAPI.from_config(config)
+    yield client
 
-    # Patch the version check
-    monkeypatch.setattr(client, "api_version", lambda: Version("7.0.0"))
 
-    yield client
+@pytest.fixture(name="zabbix_client_mock_version")
+def zabbix_client_mock_version(
+    zabbix_client: ZabbixAPI, monkeypatch: pytest.MonkeyPatch
+) -> Iterator[ZabbixAPI]:
+    monkeypatch.setattr(zabbix_client, "api_version", lambda: Version("7.0.0"))
+    yield zabbix_client
+
+
+def add_httpserver_version_endpoint(
+    httpserver: HTTPServer, version: Version, id: int = 0
+) -> None:
+    """Add an endpoint emulating the Zabbix apiiinfo.version method."""
+    httpserver.expect_oneshot_request(
+        "/api_jsonrpc.php",
+        json={"jsonrpc": "2.0", "method": "apiinfo.version", "params": {}, "id": id},
+        method="POST",
+        headers={"Content-Type": "application/json-rpc"},
+    ).respond_with_json({"jsonrpc": "2.0", "result": str(version), "id": id})
 
 
 @pytest.fixture(name="force_color")

Diff for: tests/pyzabbix/test_client.py

@@ -1,15 +1,20 @@
 from __future__ import annotations
 
 from typing import Any
+from typing import Literal
 
 import pytest
 from inline_snapshot import snapshot
+from packaging.version import Version
+from pytest_httpserver import HTTPServer
 from zabbix_cli.exceptions import ZabbixAPILoginError
 from zabbix_cli.exceptions import ZabbixAPILogoutError
 from zabbix_cli.pyzabbix.client import ZabbixAPI
 from zabbix_cli.pyzabbix.client import add_param
 from zabbix_cli.pyzabbix.client import append_param
 
+from tests.conftest import add_httpserver_version_endpoint
+
 
 @pytest.mark.parametrize(
     "inp, key, value, expect",
@@ -75,14 +80,13 @@ def test_add_param(inp: Any, subkey: str, value: Any, expect: dict[str, Any]) ->
     # Check in-place modification
 
 
-def test_login_fails(zabbix_client: ZabbixAPI) -> None:
-    zabbix_client.set_url("http://some-url-that-will-fail.gg")
-    assert zabbix_client.url == snapshot(
-        "http://some-url-that-will-fail.gg/api_jsonrpc.php"
-    )
+def test_login_fails(zabbix_client_mock_version: ZabbixAPI) -> None:
+    client = zabbix_client_mock_version
+    client.set_url("http://some-url-that-will-fail.gg")
+    assert client.url == snapshot("http://some-url-that-will-fail.gg/api_jsonrpc.php")
 
     with pytest.raises(ZabbixAPILoginError) as exc_info:
-        zabbix_client.login(user="username", password="password")
+        client.login(user="username", password="password")
 
     assert exc_info.exconly() == snapshot(
         "zabbix_cli.exceptions.ZabbixAPILoginError: Failed to log in to Zabbix: Failed to send request to http://some-url-that-will-fail.gg/api_jsonrpc.php (user.login) with params {'username': 'username', 'password': 'password'}"
@@ -92,19 +96,121 @@ def test_login_fails(zabbix_client: ZabbixAPI) -> None:
     )
 
 
-def test_logout_fails(zabbix_client: ZabbixAPI) -> None:
+def test_logout_fails(zabbix_client_mock_version: ZabbixAPI) -> None:
     """Test that we get the correct exception type when login fails
     due to a connection error."""
-    zabbix_client.set_url("http://some-url-that-will-fail.gg")
-    assert zabbix_client.url == snapshot(
-        "http://some-url-that-will-fail.gg/api_jsonrpc.php"
-    )
+    client = zabbix_client_mock_version
+    client.set_url("http://some-url-that-will-fail.gg")
+    assert client.url == snapshot("http://some-url-that-will-fail.gg/api_jsonrpc.php")
 
-    zabbix_client.auth = "authtoken123456789"
+    client.auth = "authtoken123456789"
 
     with pytest.raises(ZabbixAPILogoutError) as exc_info:
-        zabbix_client.logout()
+        client.logout()
 
     assert exc_info.exconly() == snapshot(
         "zabbix_cli.exceptions.ZabbixAPILogoutError: Failed to log out of Zabbix: Failed to send request to http://some-url-that-will-fail.gg/api_jsonrpc.php (user.logout) with params {}"
     )
+
+
+@pytest.mark.parametrize(
+    "inp, expect",
+    [
+        pytest.param(
+            "http://localhost",
+            "http://localhost/api_jsonrpc.php",
+            id="localhost-no-slash",
+        ),
+        pytest.param(
+            "http://localhost/",
+            "http://localhost/api_jsonrpc.php",
+            id="localhost-with-slash",
+        ),
+        pytest.param(
+            "http://localhost/api_jsonrpc.php",
+            "http://localhost/api_jsonrpc.php",
+            id="localhost-full-url",
+        ),
+        pytest.param(
+            "http://localhost/api_jsonrpc.php/",
+            "http://localhost/api_jsonrpc.php",
+            id="localhost-full-url-with-slash",
+        ),
+        pytest.param(
+            "http://example.com",
+            "http://example.com/api_jsonrpc.php",
+            id="tld-no-slash",
+        ),
+        pytest.param(
+            "http://example.com/",
+            "http://example.com/api_jsonrpc.php",
+            id="tld-with-slash",
+        ),
+        pytest.param(
+            "http://example.com/api_jsonrpc.php",
+            "http://example.com/api_jsonrpc.php",
+            id="tld-full-url",
+        ),
+        pytest.param(
+            "http://example.com/api_jsonrpc.php/",
+            "http://example.com/api_jsonrpc.php",
+            id="tld-full-url-with-slash",
+        ),
+    ],
+)
+def test_client_server_url(inp: str, expect: str) -> None:
+    zabbix_client = ZabbixAPI(server=inp)
+    assert zabbix_client.url == expect
+
+
+AuthMethod = Literal["header", "body"]
+
+
+@pytest.mark.parametrize(
+    "version,expect_method",
+    [
+        pytest.param(Version("5.0.0"), "body", id="5.0.0"),
+        pytest.param(Version("5.2.0"), "body", id="5.2.0"),
+        pytest.param(Version("6.0.0"), "body", id="6.0.0"),
+        pytest.param(Version("6.2.0"), "body", id="6.2.0"),
+        pytest.param(Version("6.4.0"), "header", id="6.4.0"),
+        pytest.param(Version("7.0.0"), "header", id="7.0.0"),
+        pytest.param(Version("7.2.0"), "header", id="7.2.0"),
+    ],
+)
+def test_client_auth_method(
+    zabbix_client: ZabbixAPI,
+    httpserver: HTTPServer,
+    version: Version,
+    expect_method: AuthMethod,
+) -> None:
+    # Add endpoint for version check
+    zabbix_client.set_url(httpserver.url_for("/api_jsonrpc.php"))
+
+    # Set a mock token we can use for testing
+    zabbix_client.auth = "token123"
+
+    # Add endpoint that returns the parametrized version
+    add_httpserver_version_endpoint(httpserver, version, id=0)
+
+    assert zabbix_client.version == version
+
+    data = {"jsonrpc": "2.0", "method": "fake.method", "params": {}, "id": 1}
+    headers = {}
+
+    # We expect auth token to be in header on >= 6.4.0
+    if expect_method == "header":
+        headers["Authorization"] = f"Bearer {zabbix_client.auth}"
+    else:
+        data["auth"] = zabbix_client.auth
+
+    httpserver.expect_oneshot_request(
+        "/api_jsonrpc.php",
+        json=data,
+        headers=headers,
+        method="POST",
+    ).respond_with_json({"jsonrpc": "2.0", "result": "authtoken123456789", "id": 1})
+
+    # Will fail if the auth method is not set correctly
+    resp = zabbix_client.do_request("fake.method")
+    assert resp.result == "authtoken123456789"