feat: manual Proxmox re-sync button + make connection config env-only
Add a 'Re-sync now' button to the Proxmox auto-sync settings section that triggers an immediate inventory import (POST /proxmox/sync-now) using the server env config — the manual counterpart to scheduled auto-sync. Also fix a dual-source-of-truth bug: Proxmox connection config (host, port, token, verify_tls) is now env-only and never persisted to scan_config.json. Previously save_overrides() dumped host/port/verify alongside scan settings, so saving an unrelated setting wrote an empty proxmox_host that load_overrides then clobbered PROXMOX_HOST with on every boot. Only the auto-sync activation (sync_enabled + sync_interval) stays user-editable and persisted. ha-relevant: no
This commit is contained in:
@@ -29,6 +29,7 @@ from app.schemas.proxmox import (
|
||||
ProxmoxImportPendingResponse,
|
||||
ProxmoxImportResponse,
|
||||
ProxmoxNodeOut,
|
||||
ProxmoxSyncConfig,
|
||||
ProxmoxTestConnectionResponse,
|
||||
)
|
||||
from app.schemas.scan import ScanRunResponse
|
||||
@@ -141,6 +142,43 @@ async def import_proxmox_to_pending(
|
||||
return run
|
||||
|
||||
|
||||
@router.post("/sync-now", response_model=ScanRunResponse)
|
||||
async def sync_proxmox_now(
|
||||
background_tasks: BackgroundTasks,
|
||||
db: AsyncSession = Depends(get_db),
|
||||
_: str = Depends(get_current_user),
|
||||
) -> ScanRun:
|
||||
"""Trigger an immediate Proxmox inventory sync using the server env config.
|
||||
|
||||
Same background flow as ``/import-pending`` but sources host + token from
|
||||
``settings`` (env) rather than the request body — the manual counterpart to
|
||||
the scheduled auto-sync job. Requires the env token to be configured.
|
||||
"""
|
||||
if not (settings.proxmox_host and settings.proxmox_token_id and settings.proxmox_token_secret):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Cannot sync: no Proxmox host/token configured on the server.",
|
||||
)
|
||||
run = ScanRun(
|
||||
status="running",
|
||||
kind="proxmox",
|
||||
ranges=[f"{settings.proxmox_host}:{settings.proxmox_port}"],
|
||||
)
|
||||
db.add(run)
|
||||
await db.commit()
|
||||
await db.refresh(run)
|
||||
background_tasks.add_task(
|
||||
_background_proxmox_import,
|
||||
run.id,
|
||||
settings.proxmox_host,
|
||||
settings.proxmox_port,
|
||||
settings.proxmox_token_id,
|
||||
settings.proxmox_token_secret,
|
||||
settings.proxmox_verify_tls,
|
||||
)
|
||||
return run
|
||||
|
||||
|
||||
async def _background_proxmox_import(
|
||||
run_id: str,
|
||||
host: str,
|
||||
@@ -442,22 +480,24 @@ async def get_proxmox_config(_: str = Depends(get_current_user)) -> ProxmoxConfi
|
||||
|
||||
@router.post("/config", response_model=ProxmoxConfig)
|
||||
async def save_proxmox_config(
|
||||
payload: ProxmoxConfig,
|
||||
payload: ProxmoxSyncConfig,
|
||||
_: str = Depends(get_current_user),
|
||||
) -> ProxmoxConfig:
|
||||
"""Persist non-secret Proxmox config and apply the auto-sync schedule live.
|
||||
"""Persist the auto-sync activation (enabled + interval) and apply it live.
|
||||
|
||||
The token is NOT accepted here — it is env-only by design.
|
||||
This is the ONLY Proxmox config the app writes. Connection settings
|
||||
(host, port, token, verify_tls) are env-only and are never accepted or
|
||||
persisted here — enabling auto-sync requires host + token already set in the
|
||||
server env, since the scheduled job reads them from there.
|
||||
"""
|
||||
if payload.sync_enabled and not (settings.proxmox_token_id and settings.proxmox_token_secret):
|
||||
if payload.sync_enabled and not (
|
||||
settings.proxmox_host and settings.proxmox_token_id and settings.proxmox_token_secret
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Cannot enable auto-sync: no Proxmox API token configured on the server.",
|
||||
detail="Cannot enable auto-sync: no Proxmox host/token configured in the server env.",
|
||||
)
|
||||
try:
|
||||
settings.proxmox_host = payload.host
|
||||
settings.proxmox_port = payload.port
|
||||
settings.proxmox_verify_tls = payload.verify_tls
|
||||
settings.proxmox_sync_enabled = payload.sync_enabled
|
||||
settings.proxmox_sync_interval = payload.sync_interval
|
||||
settings.save_overrides()
|
||||
|
||||
@@ -121,13 +121,10 @@ class Settings(BaseSettings):
|
||||
self.scanner_http_probe_enabled = bool(data["scanner_http_probe_enabled"])
|
||||
if "scanner_http_verify_tls" in data:
|
||||
self.scanner_http_verify_tls = bool(data["scanner_http_verify_tls"])
|
||||
# Proxmox non-secret config (token stays env-only, never here).
|
||||
if "proxmox_host" in data:
|
||||
self.proxmox_host = str(data["proxmox_host"])
|
||||
if "proxmox_port" in data:
|
||||
self.proxmox_port = int(data["proxmox_port"])
|
||||
if "proxmox_verify_tls" in data:
|
||||
self.proxmox_verify_tls = bool(data["proxmox_verify_tls"])
|
||||
# Proxmox auto-sync activation only. Connection config (host, port,
|
||||
# token, verify_tls) is env-only by design — never read from or
|
||||
# written to this file. Persisting host here previously created a
|
||||
# dual source of truth that silently clobbered PROXMOX_HOST.
|
||||
if "proxmox_sync_enabled" in data:
|
||||
self.proxmox_sync_enabled = bool(data["proxmox_sync_enabled"])
|
||||
if "proxmox_sync_interval" in data:
|
||||
@@ -146,11 +143,9 @@ class Settings(BaseSettings):
|
||||
"scanner_http_ranges": self.scanner_http_ranges,
|
||||
"scanner_http_probe_enabled": self.scanner_http_probe_enabled,
|
||||
"scanner_http_verify_tls": self.scanner_http_verify_tls,
|
||||
# Proxmox: only non-secret config. Token fields are intentionally
|
||||
# excluded — they must never be written to disk by the app.
|
||||
"proxmox_host": self.proxmox_host,
|
||||
"proxmox_port": self.proxmox_port,
|
||||
"proxmox_verify_tls": self.proxmox_verify_tls,
|
||||
# Proxmox: only the auto-sync activation is persisted. Connection
|
||||
# config (host, port, token, verify_tls) is env-only and must never
|
||||
# be written to disk — that is the single source of truth.
|
||||
"proxmox_sync_enabled": self.proxmox_sync_enabled,
|
||||
"proxmox_sync_interval": self.proxmox_sync_interval,
|
||||
}))
|
||||
|
||||
@@ -64,8 +64,11 @@ class ProxmoxImportPendingResponse(BaseModel):
|
||||
|
||||
|
||||
class ProxmoxConfig(BaseModel):
|
||||
"""Non-secret Proxmox connection + auto-sync config. Never carries a token —
|
||||
``token_configured`` reflects whether a server-side token is present."""
|
||||
"""Non-secret Proxmox connection + auto-sync config (GET response).
|
||||
|
||||
Connection fields (host/port/verify_tls) are env-only and read-only here —
|
||||
surfaced for display. ``token_configured`` reflects whether a server-side
|
||||
token is present. Never carries the token itself."""
|
||||
|
||||
host: str = ""
|
||||
port: int = Field(8006, ge=1, le=65535)
|
||||
@@ -73,3 +76,12 @@ class ProxmoxConfig(BaseModel):
|
||||
sync_enabled: bool = False
|
||||
sync_interval: int = Field(3600, ge=300)
|
||||
token_configured: bool = False
|
||||
|
||||
|
||||
class ProxmoxSyncConfig(BaseModel):
|
||||
"""User-editable auto-sync config (POST body). The ONLY persisted Proxmox
|
||||
settings. Connection fields (host/port/token/verify_tls) are env-only and
|
||||
are deliberately not accepted here."""
|
||||
|
||||
sync_enabled: bool = False
|
||||
sync_interval: int = Field(3600, ge=300)
|
||||
|
||||
@@ -95,6 +95,33 @@ async def test_import_pending_creates_scan_run(client: AsyncClient, headers: dic
|
||||
assert data["status"] == "running"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_now_creates_scan_run(client: AsyncClient, headers: dict) -> None:
|
||||
settings.proxmox_host = "pve"
|
||||
settings.proxmox_token_id = "u@pam!t"
|
||||
settings.proxmox_token_secret = "s"
|
||||
with patch("app.api.routes.proxmox._background_proxmox_import", new_callable=AsyncMock):
|
||||
res = await client.post("/api/v1/proxmox/sync-now", headers=headers)
|
||||
assert res.status_code == 200
|
||||
data = res.json()
|
||||
assert data["kind"] == "proxmox"
|
||||
assert data["status"] == "running"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_now_rejected_without_token(client: AsyncClient, headers: dict) -> None:
|
||||
settings.proxmox_host = "pve"
|
||||
# _clear_env_token leaves token empty
|
||||
res = await client.post("/api/v1/proxmox/sync-now", headers=headers)
|
||||
assert res.status_code == 400
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_now_requires_auth(client: AsyncClient) -> None:
|
||||
res = await client.post("/api/v1/proxmox/sync-now")
|
||||
assert res.status_code == 401
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_requires_auth(client: AsyncClient) -> None:
|
||||
res = await client.post("/api/v1/proxmox/import-pending", json={"host": "pve"})
|
||||
@@ -114,14 +141,38 @@ async def test_config_omits_token(client: AsyncClient, headers: dict) -> None:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_enable_sync_without_token_rejected(client: AsyncClient, headers: dict) -> None:
|
||||
# _clear_env_token leaves token empty → enabling auto-sync is rejected.
|
||||
res = await client.post(
|
||||
"/api/v1/proxmox/config",
|
||||
json={"host": "pve", "port": 8006, "verify_tls": True, "sync_enabled": True, "sync_interval": 600},
|
||||
json={"sync_enabled": True, "sync_interval": 600},
|
||||
headers=headers,
|
||||
)
|
||||
assert res.status_code == 400
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_save_config_persists_only_sync_fields(client: AsyncClient, headers: dict) -> None:
|
||||
"""Connection config is env-only: even if a client sends host/port/verify,
|
||||
the endpoint ignores them and persists only the sync activation."""
|
||||
settings.proxmox_host = "pve"
|
||||
settings.proxmox_token_id = "u@pam!t"
|
||||
settings.proxmox_token_secret = "s"
|
||||
saved: dict = {}
|
||||
with patch.object(type(settings), "save_overrides", lambda self: saved.update(
|
||||
host=self.proxmox_host, enabled=self.proxmox_sync_enabled, interval=self.proxmox_sync_interval
|
||||
)), patch("app.api.routes.proxmox.set_proxmox_sync_enabled"), \
|
||||
patch("app.api.routes.proxmox.reschedule_proxmox_sync"):
|
||||
res = await client.post(
|
||||
"/api/v1/proxmox/config",
|
||||
json={"host": "attacker", "port": 1, "verify_tls": False, "sync_enabled": True, "sync_interval": 900},
|
||||
headers=headers,
|
||||
)
|
||||
assert res.status_code == 200
|
||||
# host untouched by the request body; sync activation applied.
|
||||
assert settings.proxmox_host == "pve"
|
||||
assert saved == {"host": "pve", "enabled": True, "interval": 900}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_background_import_broadcasts_refresh() -> None:
|
||||
"""After persisting, the import emits a scan update so an open inventory
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
"""Tests for GET/POST /api/v1/settings."""
|
||||
import json
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from httpx import AsyncClient
|
||||
|
||||
from app.core.config import Settings
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def headers(client: AsyncClient):
|
||||
@@ -84,3 +87,46 @@ async def test_update_settings_rejects_too_short_service_interval(client: AsyncC
|
||||
headers=headers,
|
||||
)
|
||||
assert res.status_code == 422
|
||||
|
||||
|
||||
def test_proxmox_connection_config_is_env_only_never_from_overrides(tmp_path):
|
||||
"""Connection config (host/port/verify_tls) is env-only: a stale value in
|
||||
scan_config.json must be ignored so it can never clobber the env. Only the
|
||||
auto-sync activation is read back."""
|
||||
s = Settings(secret_key="x", sqlite_path=str(tmp_path / "homelab.db"))
|
||||
s.proxmox_host = "pve.local" # as if set from env
|
||||
s.proxmox_port = 8006
|
||||
s.proxmox_verify_tls = True
|
||||
(tmp_path / "scan_config.json").write_text(json.dumps({
|
||||
"proxmox_host": "stale-host",
|
||||
"proxmox_port": 9999,
|
||||
"proxmox_verify_tls": False,
|
||||
"proxmox_sync_enabled": True,
|
||||
"proxmox_sync_interval": 600,
|
||||
}))
|
||||
s.load_overrides()
|
||||
# Connection config untouched by the file (env values survive).
|
||||
assert s.proxmox_host == "pve.local"
|
||||
assert s.proxmox_port == 8006
|
||||
assert s.proxmox_verify_tls is True
|
||||
# Auto-sync activation is the only thing loaded.
|
||||
assert s.proxmox_sync_enabled is True
|
||||
assert s.proxmox_sync_interval == 600
|
||||
|
||||
|
||||
def test_save_overrides_omits_proxmox_connection_config(tmp_path):
|
||||
"""save_overrides must never write host/port/verify_tls (nor the token) —
|
||||
only the sync activation. This is what prevents the dual source of truth."""
|
||||
s = Settings(secret_key="x", sqlite_path=str(tmp_path / "homelab.db"))
|
||||
s.proxmox_host = "pve.local"
|
||||
s.proxmox_sync_enabled = True
|
||||
s.proxmox_sync_interval = 900
|
||||
s.save_overrides()
|
||||
written = json.loads((tmp_path / "scan_config.json").read_text())
|
||||
assert "proxmox_host" not in written
|
||||
assert "proxmox_port" not in written
|
||||
assert "proxmox_verify_tls" not in written
|
||||
assert "proxmox_token_id" not in written
|
||||
assert "proxmox_token_secret" not in written
|
||||
assert written["proxmox_sync_enabled"] is True
|
||||
assert written["proxmox_sync_interval"] == 900
|
||||
|
||||
Reference in New Issue
Block a user