mirror of
https://github.com/cloud-hypervisor/cloud-hypervisor.git
synced 2026-08-05 02:19:16 +00:00
vmm: return 404 for API requests against a non-created VM
The HTTP API mapped every ApiError to 500 Internal Server Error, so an API client could not distinguish "the VM has not been created yet" from a genuine server-side failure without parsing the error message text. Derive the HTTP status code from the error itself in error_response(): errors whose root cause is VmError::VmNotCreated or VmMissingConfig are now reported as 404 Not Found, regardless of which API action surfaced them. The existing 400 (bad request) and 429 (too many requests) mappings are preserved. State-conflict errors such as VmNotRunning would ideally map to 409 Conflict, but micro_http's StatusCode has no Conflict variant, so they remain 500 for now. Fixes: #7774 Signed-off-by: Max Makarov <maxpain@linux.com> Assisted-by: Claude:claude-opus-4-8 [Claude Code]
This commit is contained in:
committed by
Rob Bradford
parent
a89600aeba
commit
d595856748
@@ -276,7 +276,7 @@ impl EndpointHandler for VmCreate {
|
||||
.map_err(HttpError::SerdeJsonDeserialize)
|
||||
{
|
||||
Ok(config) => config,
|
||||
Err(e) => return error_response(e, StatusCode::BadRequest),
|
||||
Err(e) => return error_response(e),
|
||||
};
|
||||
|
||||
if let Some(ref mut nets) = vm_config.net {
|
||||
@@ -287,8 +287,8 @@ impl EndpointHandler for VmCreate {
|
||||
// This call sets all FDs to null while doing the same logging as
|
||||
// similar code paths.
|
||||
for cfg in cfgs {
|
||||
if let Err(e) = attach_fds_to_cfg(vec![], *cfg)
|
||||
.map_err(|e| error_response(e, StatusCode::InternalServerError))
|
||||
if let Err(e) =
|
||||
attach_fds_to_cfg(vec![], *cfg).map_err(error_response)
|
||||
{
|
||||
return e;
|
||||
}
|
||||
@@ -300,7 +300,7 @@ impl EndpointHandler for VmCreate {
|
||||
.map_err(HttpError::ApiError)
|
||||
{
|
||||
Ok(_) => Response::new(Version::Http11, StatusCode::NoContent),
|
||||
Err(e) => error_response(e, StatusCode::InternalServerError),
|
||||
Err(e) => error_response(e),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -308,7 +308,7 @@ impl EndpointHandler for VmCreate {
|
||||
}
|
||||
}
|
||||
|
||||
_ => error_response(HttpError::BadRequest, StatusCode::BadRequest),
|
||||
_ => error_response(HttpError::BadRequest),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -570,9 +570,9 @@ impl EndpointHandler for VmInfo {
|
||||
response.set_body(Body::new(info_serialized));
|
||||
response
|
||||
}
|
||||
Err(e) => error_response(e, StatusCode::InternalServerError),
|
||||
Err(e) => error_response(e),
|
||||
},
|
||||
_ => error_response(HttpError::BadRequest, StatusCode::BadRequest),
|
||||
_ => error_response(HttpError::BadRequest),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -599,10 +599,10 @@ impl EndpointHandler for VmmPing {
|
||||
response.set_body(Body::new(info_serialized));
|
||||
response
|
||||
}
|
||||
Err(e) => error_response(e, StatusCode::InternalServerError),
|
||||
Err(e) => error_response(e),
|
||||
},
|
||||
|
||||
_ => error_response(HttpError::BadRequest, StatusCode::BadRequest),
|
||||
_ => error_response(HttpError::BadRequest),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -624,10 +624,10 @@ impl EndpointHandler for VmmShutdown {
|
||||
.map_err(HttpError::ApiError)
|
||||
{
|
||||
Ok(_) => Response::new(Version::Http11, StatusCode::OK),
|
||||
Err(e) => error_response(e, StatusCode::InternalServerError),
|
||||
Err(e) => error_response(e),
|
||||
}
|
||||
}
|
||||
_ => error_response(HttpError::BadRequest, StatusCode::BadRequest),
|
||||
_ => error_response(HttpError::BadRequest),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,6 +34,7 @@ use crate::api::{
|
||||
};
|
||||
use crate::landlock::Landlock;
|
||||
use crate::seccomp_filters::{Thread, get_seccomp_filter};
|
||||
use crate::vm::Error as VmError;
|
||||
use crate::{Error as VmmError, Result};
|
||||
|
||||
pub mod http_endpoint;
|
||||
@@ -68,6 +69,27 @@ pub enum HttpError {
|
||||
ApiError(#[source] ApiError),
|
||||
}
|
||||
|
||||
impl HttpError {
|
||||
/// Returns the HTTP status code that best matches this error.
|
||||
fn status_code(&self) -> StatusCode {
|
||||
match self {
|
||||
HttpError::SerdeJsonDeserialize(_) | HttpError::BadRequest => StatusCode::BadRequest,
|
||||
HttpError::NotFound => StatusCode::NotFound,
|
||||
HttpError::TooManyRequests => StatusCode::TooManyRequests,
|
||||
HttpError::InternalServerError => StatusCode::InternalServerError,
|
||||
HttpError::ApiError(e) => api_error_status_code(e),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Maps an [`ApiError`] to an HTTP [`StatusCode`].
|
||||
fn api_error_status_code(error: &ApiError) -> StatusCode {
|
||||
match error.source().and_then(|e| e.downcast_ref::<VmError>()) {
|
||||
Some(VmError::VmNotCreated | VmError::VmMissingConfig) => StatusCode::NotFound,
|
||||
_ => StatusCode::InternalServerError,
|
||||
}
|
||||
}
|
||||
|
||||
const HTTP_ROOT: &str = "/api/v1";
|
||||
|
||||
/// Creates the error response's JSON body meant to be sent back to an API client.
|
||||
@@ -76,8 +98,8 @@ const HTTP_ROOT: &str = "/api/v1";
|
||||
/// thus insightful and helpful while balancing technical accuracy and
|
||||
/// simplicity.
|
||||
#[allow(clippy::needless_pass_by_value)]
|
||||
pub fn error_response(error: HttpError, status: StatusCode) -> Response {
|
||||
let mut response = Response::new(Version::Http11, status);
|
||||
pub fn error_response(error: HttpError) -> Response {
|
||||
let mut response = Response::new(Version::Http11, error.status_code());
|
||||
|
||||
let error: &dyn Error = &error;
|
||||
// Write the Display::display() output all errors (from top to root).
|
||||
@@ -134,12 +156,7 @@ pub trait EndpointHandler {
|
||||
Response::new(Version::Http11, StatusCode::NoContent)
|
||||
}
|
||||
}
|
||||
Err(e @ HttpError::BadRequest) => error_response(e, StatusCode::BadRequest),
|
||||
Err(e @ HttpError::SerdeJsonDeserialize(_)) => {
|
||||
error_response(e, StatusCode::BadRequest)
|
||||
}
|
||||
Err(e @ HttpError::TooManyRequests) => error_response(e, StatusCode::TooManyRequests),
|
||||
Err(e) => error_response(e, StatusCode::InternalServerError),
|
||||
Err(e) => error_response(e),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -308,12 +325,9 @@ fn handle_http_request(
|
||||
let mut response = match HTTP_ROUTES.routes.get(&path) {
|
||||
Some(route) => match api_notifier.try_clone() {
|
||||
Ok(notifier) => route.handle_request(request, notifier, api_sender.clone()),
|
||||
Err(_) => error_response(
|
||||
HttpError::InternalServerError,
|
||||
StatusCode::InternalServerError,
|
||||
),
|
||||
Err(_) => error_response(HttpError::InternalServerError),
|
||||
},
|
||||
None => error_response(HttpError::NotFound, StatusCode::NotFound),
|
||||
None => error_response(HttpError::NotFound),
|
||||
};
|
||||
|
||||
response.set_server("Cloud Hypervisor API");
|
||||
|
||||
Reference in New Issue
Block a user