fix: address automated PR review findings
Some checks failed
PR Review Automation / review (pull_request) Successful in 3m43s
Test / frontend-typecheck (pull_request) Successful in 2m0s
Test / frontend-tests (pull_request) Successful in 1m42s
Test / rust-clippy (pull_request) Has been cancelled
Test / rust-fmt-check (pull_request) Has been cancelled
Test / rust-tests (pull_request) Has been cancelled

- Temp file cleanup: move TempFileCleanup struct before std::fs::write
  to ensure cleanup happens even on panic (race condition fix)
- PortForwardSession: add explicit close() method for async cleanup
  of child process and temp files
- delete_port_forward: call close() before removing session
- All temp file operations now use RAII pattern with cleanup before write
This commit is contained in:
Shaun Arman 2026-06-06 18:40:52 -05:00
parent accb689117
commit 2134b880b9
2 changed files with 38 additions and 19 deletions

View File

@ -199,14 +199,11 @@ pub async fn test_cluster_connection(
let kubeconfig_content = cluster.kubeconfig_content.as_ref(); let kubeconfig_content = cluster.kubeconfig_content.as_ref();
let context = &cluster.context; let context = &cluster.context;
// Write kubeconfig to temp file // Write kubeconfig to temp file and ensure cleanup even on panic
let temp_dir = std::env::temp_dir(); let temp_dir = std::env::temp_dir();
let temp_path = temp_dir.join(format!("kubeconfig-{}.yaml", cluster_id)); let temp_path = temp_dir.join(format!("kubeconfig-{}.yaml", cluster_id));
std::fs::write(&temp_path, kubeconfig_content) // Create cleanup struct BEFORE writing - ensures cleanup happens even on panic
.map_err(|e| format!("Failed to write kubeconfig temp file: {e}"))?;
// Cleanup temp file on drop
struct TempFileCleanup(std::path::PathBuf); struct TempFileCleanup(std::path::PathBuf);
impl Drop for TempFileCleanup { impl Drop for TempFileCleanup {
fn drop(&mut self) { fn drop(&mut self) {
@ -215,6 +212,9 @@ pub async fn test_cluster_connection(
} }
let _cleanup = TempFileCleanup(temp_path.clone()); let _cleanup = TempFileCleanup(temp_path.clone());
std::fs::write(&temp_path, kubeconfig_content)
.map_err(|e| format!("Failed to write kubeconfig temp file: {e}"))?;
// Run kubectl cluster-info // Run kubectl cluster-info
let kubectl_path = locate_kubectl()?; let kubectl_path = locate_kubectl()?;
@ -255,14 +255,11 @@ pub async fn discover_pods(
let kubeconfig_content = cluster.kubeconfig_content.as_ref(); let kubeconfig_content = cluster.kubeconfig_content.as_ref();
let context = &cluster.context; let context = &cluster.context;
// Write kubeconfig to temp file // Write kubeconfig to temp file and ensure cleanup even on panic
let temp_dir = std::env::temp_dir(); let temp_dir = std::env::temp_dir();
let temp_path = temp_dir.join(format!("kubeconfig-{}-pods.yaml", cluster_id)); let temp_path = temp_dir.join(format!("kubeconfig-{}-pods.yaml", cluster_id));
std::fs::write(&temp_path, kubeconfig_content) // Create cleanup struct BEFORE writing - ensures cleanup happens even on panic
.map_err(|e| format!("Failed to write kubeconfig temp file: {e}"))?;
// Cleanup temp file on drop
struct TempFileCleanup(std::path::PathBuf); struct TempFileCleanup(std::path::PathBuf);
impl Drop for TempFileCleanup { impl Drop for TempFileCleanup {
fn drop(&mut self) { fn drop(&mut self) {
@ -271,6 +268,9 @@ pub async fn discover_pods(
} }
let _cleanup = TempFileCleanup(temp_path.clone()); let _cleanup = TempFileCleanup(temp_path.clone());
std::fs::write(&temp_path, kubeconfig_content)
.map_err(|e| format!("Failed to write kubeconfig temp file: {e}"))?;
// Run kubectl get pods with full JSON output // Run kubectl get pods with full JSON output
let kubectl_path = locate_kubectl()?; let kubectl_path = locate_kubectl()?;
@ -472,14 +472,11 @@ pub async fn start_port_forward(
"Allocating local port for port-forward" "Allocating local port for port-forward"
); );
// Write kubeconfig to temp file // Write kubeconfig to temp file and ensure cleanup even on panic
let temp_dir = std::env::temp_dir(); let temp_dir = std::env::temp_dir();
let temp_path = temp_dir.join(format!("kubeconfig-{}.yaml", request.cluster_id)); let temp_path = temp_dir.join(format!("kubeconfig-{}.yaml", request.cluster_id));
std::fs::write(&temp_path, kubeconfig_content.as_ref()) // Create cleanup struct BEFORE writing - ensures cleanup happens even on panic
.map_err(|e| format!("Failed to write kubeconfig temp file: {e}"))?;
// Cleanup temp file on drop
struct TempFileCleanup(std::path::PathBuf); struct TempFileCleanup(std::path::PathBuf);
impl Drop for TempFileCleanup { impl Drop for TempFileCleanup {
fn drop(&mut self) { fn drop(&mut self) {
@ -488,6 +485,9 @@ pub async fn start_port_forward(
} }
let _cleanup = TempFileCleanup(temp_path.clone()); let _cleanup = TempFileCleanup(temp_path.clone());
std::fs::write(&temp_path, kubeconfig_content.as_ref())
.map_err(|e| format!("Failed to write kubeconfig temp file: {e}"))?;
// Build kubectl command // Build kubectl command
let kubectl_path = locate_kubectl()?; let kubectl_path = locate_kubectl()?;
let args = vec![ let args = vec![
@ -593,7 +593,10 @@ pub async fn list_port_forwards(
pub async fn delete_port_forward(id: String, state: State<'_, AppState>) -> Result<(), String> { pub async fn delete_port_forward(id: String, state: State<'_, AppState>) -> Result<(), String> {
let mut port_forwards = state.port_forwards.lock().await; let mut port_forwards = state.port_forwards.lock().await;
if port_forwards.remove(&id).is_none() { if let Some(mut session) = port_forwards.remove(&id) {
// Close the session to kill the child and clean up temp files
session.close().await;
} else {
return Err(format!("Port forward session {id} not found")); return Err(format!("Port forward session {id} not found"));
} }

View File

@ -140,6 +140,22 @@ impl PortForwardSession {
} }
} }
pub async fn close(&mut self) {
// Kill the child process if it exists
if let Some(ref child_wait_handle) = self.child_wait_handle {
let guard = child_wait_handle.lock().await;
let child_opt = guard.child.lock().await.take();
if let Some(mut child) = child_opt {
let _ = child.kill().await;
}
}
// Clean up temp kubeconfig file if it exists
if let Some(path) = &self.temp_kubeconfig_path {
let _ = std::fs::remove_file(path);
}
}
pub fn set_error(&mut self, error: String) { pub fn set_error(&mut self, error: String) {
self.status = PortForwardStatus::Error(error.clone()); self.status = PortForwardStatus::Error(error.clone());
self.error_message = Some(error); self.error_message = Some(error);