refactor(proxmox): unify ProxmoxClient construction via from_server
Collapse four duplicated host+auth client-building blocks into a single ProxmoxClient.from_server() classmethod (the canonical source of truth): - _helpers._get_proxmox_client now delegates to from_server (107 callers untouched) - ipam._build_proxmox_client removed; its one call site uses from_server - task_queue_service keeps its per-server cache pool, builds via from_server - monitoring_worker keeps its ValueError-on-missing-auth contract, builds via from_server Behaviour preserved (token-first, then password; non-default port appended). Verified: containers rebuild healthy, smoke test of both auth paths passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -740,16 +740,6 @@ async def cleanup_orphan_allocations(
|
||||
}
|
||||
|
||||
|
||||
def _build_proxmox_client(server: ProxmoxServer) -> ProxmoxClient:
|
||||
"""Build ProxmoxClient from ProxmoxServer model."""
|
||||
host = server.ip_address or server.hostname
|
||||
if server.port and server.port != 8006:
|
||||
host = f"{host}:{server.port}"
|
||||
if server.use_password and server.password:
|
||||
return ProxmoxClient(host=host, user=server.api_user, password=server.password, verify_ssl=server.verify_ssl)
|
||||
return ProxmoxClient(host=host, user=server.api_user, token_name=server.api_token_name, token_value=server.api_token_value, verify_ssl=server.verify_ssl)
|
||||
|
||||
|
||||
def _extract_primary_ipv4(interfaces: list, parsed_networks: list):
|
||||
"""Extract first IPv4 that belongs to any IPAM network from interface list."""
|
||||
for iface in interfaces:
|
||||
@@ -900,7 +890,7 @@ def link_allocations_to_vms(
|
||||
continue
|
||||
|
||||
try:
|
||||
client = _build_proxmox_client(server)
|
||||
client = ProxmoxClient.from_server(server)
|
||||
if not client.is_connected():
|
||||
logger.warning(f"[IPAM link] Server {server.name} not reachable, skipping")
|
||||
for vm in server_vms:
|
||||
|
||||
@@ -72,24 +72,7 @@ def require_vm_access(db: Session, current_user: User, server_id: int, vmid: int
|
||||
|
||||
def _get_proxmox_client(server: ProxmoxServer) -> ProxmoxClient:
|
||||
"""Build a ProxmoxClient from a ProxmoxServer model (password or token auth)."""
|
||||
host = server.ip_address or getattr(server, 'hostname', '')
|
||||
if hasattr(server, 'port') and server.port and server.port != 8006:
|
||||
host = f"{host}:{server.port}"
|
||||
|
||||
if getattr(server, 'use_password', False) and getattr(server, 'password', ''):
|
||||
return ProxmoxClient(
|
||||
host=host,
|
||||
user=server.api_user,
|
||||
password=server.password,
|
||||
verify_ssl=server.verify_ssl
|
||||
)
|
||||
return ProxmoxClient(
|
||||
host=host,
|
||||
user=server.api_user,
|
||||
token_name=server.api_token_name,
|
||||
token_value=server.api_token_value,
|
||||
verify_ssl=server.verify_ssl
|
||||
)
|
||||
return ProxmoxClient.from_server(server)
|
||||
|
||||
|
||||
def get_next_vmid(db: Session, server_id: int) -> int:
|
||||
|
||||
@@ -83,6 +83,34 @@ def get_cached_client(host: str, user: str, password_hash: str = None,
|
||||
)
|
||||
|
||||
class ProxmoxClient(VmMixin, LxcMixin, ClusterMixin, StorageMixin, NetworkMixin, SnapshotMixin):
|
||||
@classmethod
|
||||
def from_server(cls, server) -> "ProxmoxClient":
|
||||
"""Single source of truth for building a client from a ProxmoxServer model.
|
||||
|
||||
Picks token auth when available, otherwise password auth, and appends a
|
||||
non-default port to the host. Callers across the API and services layers
|
||||
(see api/proxmox/_helpers.py and services/task_queue_service.py) should
|
||||
go through here instead of duplicating the host/auth branching.
|
||||
"""
|
||||
host = server.ip_address or getattr(server, "hostname", "") or ""
|
||||
if getattr(server, "port", None) and server.port != 8006:
|
||||
host = f"{host}:{server.port}"
|
||||
|
||||
if getattr(server, "use_password", False) and getattr(server, "password", ""):
|
||||
return cls(
|
||||
host=host,
|
||||
user=server.api_user,
|
||||
password=server.password,
|
||||
verify_ssl=server.verify_ssl,
|
||||
)
|
||||
return cls(
|
||||
host=host,
|
||||
user=server.api_user,
|
||||
token_name=server.api_token_name,
|
||||
token_value=server.api_token_value,
|
||||
verify_ssl=server.verify_ssl,
|
||||
)
|
||||
|
||||
def __init__(self, host: str, user: str = "root@pam", password: str = None,
|
||||
token_name: str = None, token_value: str = None, verify_ssl: bool = True,
|
||||
timeout: int = 30):
|
||||
|
||||
@@ -147,30 +147,16 @@ class TaskQueueProcessor:
|
||||
server = db.query(ProxmoxServer).filter(ProxmoxServer.id == server_id).first()
|
||||
if not server:
|
||||
return None
|
||||
|
||||
|
||||
# Require valid credentials before building (from_server would otherwise
|
||||
# construct a client with no usable auth).
|
||||
has_password = server.use_password and server.password
|
||||
has_token = server.api_token_name and server.api_token_value
|
||||
if not (has_password or has_token):
|
||||
return None
|
||||
|
||||
try:
|
||||
host = server.ip_address or server.hostname
|
||||
if server.port and server.port != 8006:
|
||||
host = f"{host}:{server.port}"
|
||||
|
||||
if server.use_password and server.password:
|
||||
client = ProxmoxClient(
|
||||
host=host,
|
||||
user=server.api_user,
|
||||
password=server.password,
|
||||
verify_ssl=server.verify_ssl
|
||||
)
|
||||
elif server.api_token_name and server.api_token_value:
|
||||
client = ProxmoxClient(
|
||||
host=host,
|
||||
user=server.api_user,
|
||||
token_name=server.api_token_name,
|
||||
token_value=server.api_token_value,
|
||||
verify_ssl=server.verify_ssl
|
||||
)
|
||||
else:
|
||||
return None
|
||||
|
||||
client = ProxmoxClient.from_server(server)
|
||||
self._proxmox_clients[server_id] = client
|
||||
return client
|
||||
except Exception as e:
|
||||
|
||||
@@ -86,32 +86,12 @@ class MonitoringWorker:
|
||||
self.last_server_alerts: Dict[int, float] = {} # server_id -> last_alert_time
|
||||
|
||||
def _create_proxmox_client(self, server: ProxmoxServer) -> ProxmoxClient:
|
||||
"""Create ProxmoxClient from server model"""
|
||||
# ALWAYS prefer ip_address over hostname (hostname may not be DNS-resolvable)
|
||||
host = server.ip_address or server.hostname
|
||||
if server.port and server.port != 8006:
|
||||
host = f"{host}:{server.port}"
|
||||
|
||||
# Determine authentication method
|
||||
if server.use_password and server.password:
|
||||
# Password authentication
|
||||
return ProxmoxClient(
|
||||
host=host,
|
||||
user=server.api_user,
|
||||
password=server.password,
|
||||
verify_ssl=server.verify_ssl
|
||||
)
|
||||
elif server.api_token_name and server.api_token_value:
|
||||
# API Token authentication
|
||||
return ProxmoxClient(
|
||||
host=host,
|
||||
user=server.api_user,
|
||||
token_name=server.api_token_name,
|
||||
token_value=server.api_token_value,
|
||||
verify_ssl=server.verify_ssl
|
||||
)
|
||||
else:
|
||||
"""Create ProxmoxClient from server model, raising if auth is missing."""
|
||||
has_password = server.use_password and server.password
|
||||
has_token = server.api_token_name and server.api_token_value
|
||||
if not (has_password or has_token):
|
||||
raise ValueError(f"Server {server.name} has no valid authentication configured")
|
||||
return ProxmoxClient.from_server(server)
|
||||
|
||||
def _notify_server_offline(self, db, server: ProxmoxServer, error: str, users: List[User]):
|
||||
"""Send notification that server went offline"""
|
||||
|
||||
Reference in New Issue
Block a user