diff --git a/src-tauri/src/commands/proxmox.rs b/src-tauri/src/commands/proxmox.rs index 1efea8a3..1607422d 100644 --- a/src-tauri/src/commands/proxmox.rs +++ b/src-tauri/src/commands/proxmox.rs @@ -300,12 +300,106 @@ async fn get_proxmox_client_for_cluster( let client_arc = Arc::new(Mutex::new(client)); { let mut clusters = state.proxmox_clusters.lock().await; + // Re-check under write lock: a concurrent task may have already created a client + // for this cluster between our read-check and here, so prefer the existing one. + if let Some(existing) = clusters.get(cluster_id) { + return Ok(existing.clone()); + } clusters.insert(cluster_id.to_string(), client_arc.clone()); } Ok(client_arc) } +/// Ping a Proxmox cluster — authenticates and calls the version endpoint to verify +/// that the API is reachable and credentials are valid. +#[tauri::command] +pub async fn ping_proxmox_cluster( + cluster_id: String, + state: State<'_, AppState>, +) -> Result { + let client = get_proxmox_client_for_cluster(&cluster_id, &state).await?; + let client_guard = client.lock().await; + + client_guard + .get::("version", client_guard.ticket.as_deref()) + .await + .map_err(|e| format!("Connection test failed: {}", e)) +} + +/// Update an existing Proxmox cluster's metadata and credentials atomically. +/// Unlike the remove-then-add pattern this is a single SQL UPDATE so there is +/// no window where the record is missing. +#[tauri::command] +pub async fn update_proxmox_cluster( + id: String, + name: String, + cluster_type: ClusterType, + connection: ClusterConnection, + username: String, + password: &str, + state: State<'_, AppState>, +) -> Result { + let credentials = serde_json::json!({ "password": password, "username": username }); + let encrypted_credentials = crate::integrations::auth::encrypt_token( + &serde_json::to_string(&credentials).map_err(|e| e.to_string())?, + ) + .map_err(|e| format!("Failed to encrypt credentials: {}", e))?; + + let updated_at = Utc::now().format("%Y-%m-%d %H:%M:%S").to_string(); + + { + let db = state + .db + .lock() + .map_err(|e| format!("Failed to lock database: {}", e))?; + + let rows = db + .execute( + "UPDATE proxmox_clusters \ + SET name=?1, cluster_type=?2, url=?3, port=?4, username=?5, \ + encrypted_credentials=?6, updated_at=?7 \ + WHERE id=?8", + rusqlite::params![ + name, + match cluster_type { + ClusterType::VE => "ve", + ClusterType::PBS => "pbs", + }, + connection.url, + connection.port, + username, + encrypted_credentials, + updated_at, + id, + ], + ) + .map_err(|e| format!("Failed to update cluster: {}", e))?; + + if rows == 0 { + return Err(format!("Cluster {} not found", id)); + } + } + + // Evict the stale authenticated client — it will re-authenticate with new credentials + // on the next API call. + { + let mut clusters = state.proxmox_clusters.lock().await; + clusters.remove(&id); + } + + Ok(ClusterInfo { + id, + name, + cluster_type, + url: connection.url, + port: connection.port, + username, + created_at: String::new(), + updated_at, + }) +} + /// List all Proxmox VMs #[tauri::command] pub async fn list_proxmox_vms( @@ -2110,4 +2204,41 @@ mod tests { assert!(result.is_ok()); assert_eq!(result.unwrap().len(), 2); } + + #[test] + fn test_update_proxmox_cluster_not_found_error() { + let err = format!("Cluster {} not found", "missing-id"); + assert_eq!(err, "Cluster missing-id not found"); + } + + #[test] + fn test_update_proxmox_cluster_rows_zero_means_not_found() { + let rows: usize = 0; + let result: Result<(), String> = if rows == 0 { + Err(format!("Cluster {} not found", "ghost-id")) + } else { + Ok(()) + }; + assert!(result.is_err()); + assert!(result.unwrap_err().contains("ghost-id")); + } + + #[test] + fn test_update_proxmox_cluster_rows_nonzero_succeeds() { + let rows: usize = 1; + let result: Result<(), String> = if rows == 0 { + Err(format!("Cluster {} not found", "real-id")) + } else { + Ok(()) + }; + assert!(result.is_ok()); + } + + #[test] + fn test_ping_proxmox_cluster_error_message_format() { + let raw = anyhow::anyhow!("connection refused"); + let msg = format!("Connection test failed: {}", raw); + assert!(msg.starts_with("Connection test failed:")); + assert!(msg.contains("connection refused")); + } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 93a95a18..d2601bbf 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -225,6 +225,8 @@ pub fn run() { // Proxmox - Existing commands::proxmox::add_proxmox_cluster, commands::proxmox::remove_proxmox_cluster, + commands::proxmox::update_proxmox_cluster, + commands::proxmox::ping_proxmox_cluster, commands::proxmox::list_proxmox_clusters, commands::proxmox::get_proxmox_cluster, commands::proxmox::list_proxmox_vms, diff --git a/src/components/Proxmox/RemotesList.tsx b/src/components/Proxmox/RemotesList.tsx index 2056f7e2..a23952e8 100644 --- a/src/components/Proxmox/RemotesList.tsx +++ b/src/components/Proxmox/RemotesList.tsx @@ -42,7 +42,6 @@ function ActionsMenu({ const menuRef = useRef(null); useEffect(() => { - if (!open) return; const handleClickOutside = (e: MouseEvent) => { if (menuRef.current && !menuRef.current.contains(e.target as Node)) { setOpen(false); @@ -50,7 +49,7 @@ function ActionsMenu({ }; document.addEventListener('mousedown', handleClickOutside); return () => document.removeEventListener('mousedown', handleClickOutside); - }, [open]); + }, []); return (
diff --git a/src/lib/proxmoxClient.ts b/src/lib/proxmoxClient.ts index 2c92e177..38077bb7 100644 --- a/src/lib/proxmoxClient.ts +++ b/src/lib/proxmoxClient.ts @@ -40,6 +40,36 @@ export async function removeProxmoxCluster(id: string): Promise { await invoke("remove_proxmox_cluster", { id }); } +/** + * Update an existing Proxmox cluster's metadata and credentials atomically. + * Uses a single SQL UPDATE so there is no window where the record is missing. + */ +export async function updateProxmoxCluster( + id: string, + name: string, + clusterType: ClusterType, + connection: { url: string; port: number }, + username: string, + password: string +): Promise { + return await invoke("update_proxmox_cluster", { + id, + name, + clusterType, + connection, + username, + password, + }); +} + +/** + * Ping a Proxmox cluster — authenticates and calls the version endpoint to verify + * the API is reachable and credentials are valid. + */ +export async function pingProxmoxCluster(clusterId: string): Promise { + return await invoke("ping_proxmox_cluster", { clusterId }); +} + /** * List all Proxmox clusters */ diff --git a/src/pages/Proxmox/RemotesPage.tsx b/src/pages/Proxmox/RemotesPage.tsx index a50bffb2..2cbfe988 100644 --- a/src/pages/Proxmox/RemotesPage.tsx +++ b/src/pages/Proxmox/RemotesPage.tsx @@ -6,7 +6,7 @@ import { AddRemoteForm } from '@/components/Proxmox'; import { EditRemoteForm } from '@/components/Proxmox'; import { RemoveRemoteDialog } from '@/components/Proxmox'; import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/index'; -import { listProxmoxClusters, addProxmoxCluster, removeProxmoxCluster, getProxmoxCluster } from '@/lib/proxmoxClient'; +import { listProxmoxClusters, addProxmoxCluster, removeProxmoxCluster, updateProxmoxCluster, pingProxmoxCluster } from '@/lib/proxmoxClient'; import { ClusterType } from '@/lib/domain'; import { toast } from 'sonner'; @@ -102,11 +102,7 @@ export function ProxmoxRemotesPage() { const clusterType = config.type === 'pve' ? 've' : 'pbs'; const { hostname, port } = parseRemoteUrl(config.url, config.type); - // Edit operation requires remove-then-add since backend doesn't support update. - // If add fails after remove, the remote will be lost - this is a known limitation - // until backend supports atomic update operations. - await removeProxmoxCluster(config.id); - await addProxmoxCluster( + await updateProxmoxCluster( config.id, config.name, clusterType as ClusterType, @@ -139,15 +135,11 @@ export function ProxmoxRemotesPage() { const handleConnectRemote = async (remote: RemoteInfo) => { try { toast.info(`Testing connection to ${remote.name}...`); - const result = await getProxmoxCluster(remote.id); - if (result !== null) { - toast.success(`Connected to ${remote.name}`); - setRemotes((prev) => - prev.map((r) => (r.id === remote.id ? { ...r, status: 'connected' } : r)) - ); - } else { - toast.error(`Cluster ${remote.name} not found`); - } + await pingProxmoxCluster(remote.id); + toast.success(`Connected to ${remote.name}`); + setRemotes((prev) => + prev.map((r) => (r.id === remote.id ? { ...r, status: 'connected' } : r)) + ); } catch (err) { console.error('Failed to connect remote:', err); toast.error('Connection failed: ' + String(err));