From 93982227ac759b28b53c1e3406d299d8ff34a438 Mon Sep 17 00:00:00 2001 From: houseme Date: Sun, 30 Nov 2025 02:43:59 +0800 Subject: [PATCH] Improve health check handlers for endpoint and console (GET/HEAD, safer error handling) (#942) * Improve health check handlers for endpoint and console - Add unified GET/HEAD handling for `/health` and `/rustfs/console/health` - Implement proper method filtering and 405 with `Allow: GET, HEAD` - Avoid panics by removing `unwrap()` in health check logic - Add safe fallbacks for JSON serialization and uptime calculation - Ensure HEAD requests return only status and headers (empty body) - Keep response format backward compatible for monitoring systems * fix --- rustfs/src/admin/console.rs | 128 ++++++++++++++++++++++++++--------- rustfs/src/admin/handlers.rs | 33 +++++++-- rustfs/src/admin/mod.rs | 1 + rustfs/src/admin/router.rs | 18 ++++- 4 files changed, 140 insertions(+), 40 deletions(-) diff --git a/rustfs/src/admin/console.rs b/rustfs/src/admin/console.rs index 748ad54d..dc3523c3 100644 --- a/rustfs/src/admin/console.rs +++ b/rustfs/src/admin/console.rs @@ -15,7 +15,7 @@ use crate::config::build; use crate::license::get_license; use axum::{ - Json, Router, + Router, body::Body, extract::Request, middleware, @@ -405,7 +405,7 @@ fn setup_console_middleware_stack( .route("/favicon.ico", get(static_handler)) .route(&format!("{CONSOLE_PREFIX}/license"), get(license_handler)) .route(&format!("{CONSOLE_PREFIX}/config.json"), get(config_handler)) - .route(&format!("{CONSOLE_PREFIX}/health"), get(health_check)) + .route(&format!("{CONSOLE_PREFIX}/health"), get(health_check).head(health_check)) .nest(CONSOLE_PREFIX, Router::new().fallback_service(get(static_handler))) .fallback_service(get(static_handler)); @@ -437,42 +437,104 @@ fn setup_console_middleware_stack( } /// Console health check handler with comprehensive health information -async fn health_check() -> Json { - use rustfs_ecstore::new_object_layer_fn; +async fn health_check(method: Method) -> Response { + let builder = Response::builder() + .status(StatusCode::OK) + .header("content-type", "application/json"); + match method { + // GET: Returns complete JSON + Method::GET => { + let mut health_status = "ok"; + let mut details = json!({}); - let mut health_status = "ok"; - let mut details = json!({}); + // Check storage backend health + if let Some(_store) = rustfs_ecstore::new_object_layer_fn() { + details["storage"] = json!({"status": "connected"}); + } else { + health_status = "degraded"; + details["storage"] = json!({"status": "disconnected"}); + } - // Check storage backend health - if let Some(_store) = new_object_layer_fn() { - details["storage"] = json!({"status": "connected"}); - } else { - health_status = "degraded"; - details["storage"] = json!({"status": "disconnected"}); - } + // Check IAM system health + match rustfs_iam::get() { + Ok(_) => { + details["iam"] = json!({"status": "connected"}); + } + Err(_) => { + health_status = "degraded"; + details["iam"] = json!({"status": "disconnected"}); + } + } - // Check IAM system health - match rustfs_iam::get() { - Ok(_) => { - details["iam"] = json!({"status": "connected"}); + let body_json = json!({ + "status": health_status, + "service": "rustfs-console", + "timestamp": chrono::Utc::now().to_rfc3339(), + "version": env!("CARGO_PKG_VERSION"), + "details": details, + "uptime": std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs() + }); + + // Return a minimal JSON when serialization fails to avoid panic + let body_str = serde_json::to_string(&body_json).unwrap_or_else(|e| { + error!( + target: "rustfs::console::health", + "failed to serialize health check body: {}", + e + ); + // Simplified back-up JSON + "{\"status\":\"error\",\"service\":\"rustfs-console\"}".to_string() + }); + builder.body(Body::from(body_str)).unwrap_or_else(|e| { + error!( + target: "rustfs::console::health", + "failed to build GET health response: {}", + e + ); + Response::builder() + .status(StatusCode::INTERNAL_SERVER_ERROR) + .body(Body::from("failed to build response")) + .unwrap_or_else(|_| Response::new(Body::from(""))) + }) } - Err(_) => { - health_status = "degraded"; - details["iam"] = json!({"status": "disconnected"}); - } - } - Json(json!({ - "status": health_status, - "service": "rustfs-console", - "timestamp": chrono::Utc::now().to_rfc3339(), - "version": env!("CARGO_PKG_VERSION"), - "details": details, - "uptime": std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_secs() - })) + // HEAD: Only status + headers are returned, body is empty + Method::HEAD => builder.body(Body::empty()).unwrap_or_else(|e| { + error!( + target: "rustfs::console::health", + "failed to build HEAD health response: {}", + e + ); + Response::builder() + .status(StatusCode::INTERNAL_SERVER_ERROR) + .body(Body::from("failed to build response")) + .unwrap_or_else(|e| { + error!( + target: "rustfs::console::health", + "failed to build HEAD health empty response, reason: {}", + e + ); + Response::new(Body::from("")) + }) + }), + + // Other methods: 405 + _ => Response::builder() + .status(StatusCode::METHOD_NOT_ALLOWED) + .header("allow", "GET, HEAD") + .body(Body::from("Method Not Allowed")) + .unwrap_or_else(|e| { + error!( + target: "rustfs::console::health", + "failed to build 405 response: {}", + e + ); + Response::new(Body::from("Method Not Allowed")) + }), + } } /// Parse CORS allowed origins from configuration diff --git a/rustfs/src/admin/handlers.rs b/rustfs/src/admin/handlers.rs index 941cb7ea..6c35dcdf 100644 --- a/rustfs/src/admin/handlers.rs +++ b/rustfs/src/admin/handlers.rs @@ -20,7 +20,7 @@ use crate::auth::get_session_token; use crate::error::ApiError; use bytes::Bytes; use futures::{Stream, StreamExt}; -use http::{HeaderMap, Uri}; +use http::{HeaderMap, HeaderValue, Uri}; use hyper::StatusCode; use matchit::Params; use rustfs_common::heal_channel::HealOpts; @@ -103,9 +103,23 @@ pub struct HealthCheckHandler {} #[async_trait::async_trait] impl Operation for HealthCheckHandler { - async fn call(&self, _req: S3Request, _params: Params<'_, '_>) -> S3Result> { + async fn call(&self, req: S3Request, _params: Params<'_, '_>) -> S3Result> { use serde_json::json; + // Extract the original HTTP Method (encapsulated by s3s into S3Request) + let method = req.method; + + // Only GET and HEAD are allowed + if method != http::Method::GET && method != http::Method::HEAD { + // 405 Method Not Allowed + let mut headers = HeaderMap::new(); + headers.insert(http::header::ALLOW, HeaderValue::from_static("GET, HEAD")); + return Ok(S3Response::with_headers( + (StatusCode::METHOD_NOT_ALLOWED, Body::from("Method Not Allowed".to_string())), + headers, + )); + } + let health_info = json!({ "status": "ok", "service": "rustfs-endpoint", @@ -113,10 +127,19 @@ impl Operation for HealthCheckHandler { "version": env!("CARGO_PKG_VERSION") }); - let body = serde_json::to_string(&health_info).unwrap_or_else(|_| "{}".to_string()); - let response_body = Body::from(body); + let mut headers = HeaderMap::new(); + headers.insert(CONTENT_TYPE, HeaderValue::from_static("application/json")); - Ok(S3Response::new((StatusCode::OK, response_body))) + if method == http::Method::HEAD { + // HEAD: only returns the header and status code, not the body + return Ok(S3Response::with_headers((StatusCode::OK, Body::empty()), headers)); + } + + // GET: Return JSON body normally + let body_str = serde_json::to_string(&health_info).unwrap_or_else(|_| "{}".to_string()); + let body = Body::from(body_str); + + Ok(S3Response::with_headers((StatusCode::OK, body), headers)) } } diff --git a/rustfs/src/admin/mod.rs b/rustfs/src/admin/mod.rs index b6324dff..f6ce6b04 100644 --- a/rustfs/src/admin/mod.rs +++ b/rustfs/src/admin/mod.rs @@ -45,6 +45,7 @@ pub fn make_admin_route(console_enabled: bool) -> std::io::Result // Health check endpoint for monitoring and orchestration r.insert(Method::GET, "/health", AdminOperation(&HealthCheckHandler {}))?; + r.insert(Method::HEAD, "/health", AdminOperation(&HealthCheckHandler {}))?; r.insert(Method::GET, "/profile/cpu", AdminOperation(&TriggerProfileCPU {}))?; r.insert(Method::GET, "/profile/memory", AdminOperation(&TriggerProfileMemory {}))?; diff --git a/rustfs/src/admin/router.rs b/rustfs/src/admin/router.rs index 8bb77596..c3a63b42 100644 --- a/rustfs/src/admin/router.rs +++ b/rustfs/src/admin/router.rs @@ -85,7 +85,13 @@ where { fn is_match(&self, method: &Method, uri: &Uri, headers: &HeaderMap, _: &mut Extensions) -> bool { let path = uri.path(); - if method == Method::GET && (path == "/health" || path == "/profile/cpu" || path == "/profile/memory") { + // Profiling endpoints + if method == Method::GET && (path == "/profile/cpu" || path == "/profile/memory") { + return true; + } + + // Health check + if (method == Method::HEAD || method == Method::GET) && path == "/health" { return true; } @@ -105,9 +111,17 @@ where async fn check_access(&self, req: &mut S3Request) -> S3Result<()> { // Allow unauthenticated access to health check let path = req.uri.path(); - if req.method == Method::GET && (path == "/health" || path == "/profile/cpu" || path == "/profile/memory") { + + // Profiling endpoints + if req.method == Method::GET && (path == "/profile/cpu" || path == "/profile/memory") { return Ok(()); } + + // Health check + if (req.method == Method::HEAD || req.method == Method::GET) && path == "/health" { + return Ok(()); + } + // Allow unauthenticated access to console static files if console is enabled if self.console_enabled && is_console_path(path) { return Ok(());