Skip to content

Commit

Permalink
update to dropshot 0.15 (#7196)
Browse files Browse the repository at this point in the history
Co-authored-by: iliana etaoin <[email protected]>
  • Loading branch information
hawkw and iliana authored Jan 17, 2025
1 parent e2bd1d7 commit 8084358
Show file tree
Hide file tree
Showing 66 changed files with 316 additions and 262 deletions.
130 changes: 65 additions & 65 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ dns-server = { path = "dns-server" }
dns-server-api = { path = "dns-server-api" }
dns-service-client = { path = "clients/dns-service-client" }
dpd-client = { path = "clients/dpd-client" }
dropshot = { version = "0.13.0", features = [ "usdt-probes" ] }
dropshot = { version = "0.15.1", features = [ "usdt-probes" ] }
dyn-clone = "1.0.17"
either = "1.13.0"
expectorate = "1.1.0"
Expand Down
4 changes: 3 additions & 1 deletion clickhouse-admin/src/clickhouse_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,12 @@ impl From<ClickhouseCliError> for HttpError {
| ClickhouseCliError::ExecutionError(_) => {
let message = InlineErrorChain::new(&err).to_string();
HttpError {
status_code: http::StatusCode::INTERNAL_SERVER_ERROR,
status_code:
dropshot::ErrorStatusCode::INTERNAL_SERVER_ERROR,
error_code: Some(String::from("Internal")),
external_message: message.clone(),
internal_message: message,
headers: None,
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion clickhouse-admin/src/clickward.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ impl From<ClickwardError> for HttpError {
ClickwardError::Failure { .. } => {
let message = InlineErrorChain::new(&err).to_string();
HttpError {
status_code: http::StatusCode::INTERNAL_SERVER_ERROR,
status_code:
dropshot::ErrorStatusCode::INTERNAL_SERVER_ERROR,
error_code: Some(String::from("Internal")),
external_message: message.clone(),
internal_message: message,
headers: None,
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion cockroach-admin/src/cockroach_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@ impl From<CockroachCliError> for HttpError {
| CockroachCliError::ParseOutput { .. } => {
let message = InlineErrorChain::new(&err).to_string();
HttpError {
status_code: http::StatusCode::INTERNAL_SERVER_ERROR,
status_code:
dropshot::ErrorStatusCode::INTERNAL_SERVER_ERROR,
error_code: Some(String::from("Internal")),
external_message: message.clone(),
internal_message: message,
headers: None,
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion cockroach-admin/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,11 @@ impl ServerContext {
{err:#}",
);
Err(HttpError {
status_code: http::StatusCode::SERVICE_UNAVAILABLE,
status_code: dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
error_code: None,
external_message: message.clone(),
internal_message: message,
headers: None,
})
}
}
Expand Down
25 changes: 16 additions & 9 deletions common/src/api/external/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ impl From<Error> for HttpError {
};
HttpError::for_client_error(
Some(String::from("ObjectNotFound")),
http::StatusCode::NOT_FOUND,
dropshot::ClientErrorStatusCode::NOT_FOUND,
format!("not found: {}", message),
)
}
Expand All @@ -427,44 +427,47 @@ impl From<Error> for HttpError {
}

Error::Unauthenticated { internal_message } => HttpError {
status_code: http::StatusCode::UNAUTHORIZED,
status_code: dropshot::ErrorStatusCode::UNAUTHORIZED,
// TODO-polish We may want to rethink this error code. This is
// what HTTP calls it, but it's confusing.
error_code: Some(String::from("Unauthorized")),
external_message: String::from(
"credentials missing or invalid",
),
internal_message,
headers: None,
},

Error::InvalidRequest { message } => {
let (internal_message, external_message) =
message.into_internal_external();
HttpError {
status_code: http::StatusCode::BAD_REQUEST,
status_code: dropshot::ErrorStatusCode::BAD_REQUEST,
error_code: Some(String::from("InvalidRequest")),
external_message,
internal_message,
headers: None,
}
}

Error::InvalidValue { label, message } => {
let (internal_message, external_message) =
message.into_internal_external();
HttpError {
status_code: http::StatusCode::BAD_REQUEST,
status_code: dropshot::ErrorStatusCode::BAD_REQUEST,
error_code: Some(String::from("InvalidValue")),
external_message: format!(
"unsupported value for \"{}\": {}",
label, external_message
),
internal_message,
headers: None,
}
}

Error::Forbidden => HttpError::for_client_error(
Some(String::from("Forbidden")),
http::StatusCode::FORBIDDEN,
dropshot::ClientErrorStatusCode::FORBIDDEN,
String::from("Forbidden"),
),

Expand All @@ -485,13 +488,15 @@ impl From<Error> for HttpError {
// Need to construct an `HttpError` explicitly to present both
// an internal and an external message.
HttpError {
status_code: http::StatusCode::INSUFFICIENT_STORAGE,
status_code:
dropshot::ErrorStatusCode::INSUFFICIENT_STORAGE,
error_code: Some(String::from("InsufficientCapacity")),
external_message: format!(
"Insufficient capacity: {}",
external_message
),
internal_message,
headers: None,
}
}

Expand All @@ -503,27 +508,29 @@ impl From<Error> for HttpError {
let (internal_message, external_message) =
message.into_internal_external();
HttpError {
status_code: http::StatusCode::CONFLICT,
status_code: dropshot::ErrorStatusCode::CONFLICT,
error_code: Some(String::from("Conflict")),
external_message,
internal_message,
headers: None,
}
}

Error::NotFound { message } => {
let (internal_message, external_message) =
message.into_internal_external();
HttpError {
status_code: http::StatusCode::NOT_FOUND,
status_code: dropshot::ErrorStatusCode::NOT_FOUND,
error_code: Some(String::from("Not Found")),
external_message,
internal_message,
headers: None,
}
}

Error::Gone => HttpError::for_client_error(
Some(String::from("Gone")),
http::StatusCode::GONE,
dropshot::ClientErrorStatusCode::GONE,
String::from("Gone"),
),
}
Expand Down
2 changes: 1 addition & 1 deletion dns-server/examples/config.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[dropshot]
# 100 MiB. This ought to be large enough for a while, but not so large that
# we'll use tons of memory parsing incoming HTTP requests.
request_body_max_bytes = 104857600
default_request_body_max_bytes = 104857600

[log]
# Show log messages of this level and more severe
Expand Down
6 changes: 4 additions & 2 deletions dns-server/src/http_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,21 @@ impl From<UpdateError> for dropshot::HttpError {
let message = format!("{:#}", error);
match &error {
UpdateError::BadUpdateGeneration { .. } => dropshot::HttpError {
status_code: http::StatusCode::CONFLICT,
status_code: dropshot::ErrorStatusCode::CONFLICT,
error_code: Some(String::from(
ERROR_CODE_BAD_UPDATE_GENERATION,
)),
external_message: message.clone(),
internal_message: message,
headers: None,
},

UpdateError::UpdateInProgress { .. } => dropshot::HttpError {
status_code: http::StatusCode::CONFLICT,
status_code: dropshot::ErrorStatusCode::CONFLICT,
error_code: Some(String::from(ERROR_CODE_UPDATE_IN_PROGRESS)),
external_message: message.clone(),
internal_message: message,
headers: None,
},

UpdateError::InternalError(_) => {
Expand Down
2 changes: 1 addition & 1 deletion dns-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl TransientServer {
&dns_server::Config { bind_address: dns_bind_address },
&dropshot::ConfigDropshot {
bind_address: "[::1]:0".parse().unwrap(),
request_body_max_bytes: 4 * 1024 * 1024,
default_request_body_max_bytes: 4 * 1024 * 1024,
default_handler_task_mode: dropshot::HandlerTaskMode::Detached,
log_headers: vec![],
},
Expand Down
2 changes: 1 addition & 1 deletion dns-server/tests/basic_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ fn test_config(
dns_server::storage::Config { storage_path, keep_old_generations: 3 };
let config_dropshot = dropshot::ConfigDropshot {
bind_address: "[::1]:0".to_string().parse().unwrap(),
request_body_max_bytes: 1024,
default_request_body_max_bytes: 1024,
default_handler_task_mode: HandlerTaskMode::Detached,
log_headers: vec![],
};
Expand Down
8 changes: 8 additions & 0 deletions gateway-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ use schemars::JsonSchema;
use serde::Deserialize;
use uuid::Uuid;

/// This endpoint is used to upload SP and ROT Hubris archives as well as phase 1 host OS
/// images. The phase 1 image is 32 MiB, driven by the QSPI flash on hardware.
const SP_COMPONENT_UPDATE_MAX_BYTES: usize = 64 * 1024 * 1024;
/// The host phase 2 recovery image is currently (Dec 2024) ~130 MiB.
const HOST_PHASE2_MAX_BYTES: usize = 512 * 1024 * 1024;

#[dropshot::api_description]
pub trait GatewayApi {
type Context;
Expand Down Expand Up @@ -230,6 +236,7 @@ pub trait GatewayApi {
#[endpoint {
method = POST,
path = "/sp/{type}/{slot}/component/{component}/update",
request_body_max_bytes = SP_COMPONENT_UPDATE_MAX_BYTES,
}]
async fn sp_component_update(
rqctx: RequestContext<Self::Context>,
Expand Down Expand Up @@ -434,6 +441,7 @@ pub trait GatewayApi {
#[endpoint {
method = POST,
path = "/recovery/host-phase2",
request_body_max_bytes = HOST_PHASE2_MAX_BYTES,
}]
async fn recovery_host_phase2_upload(
rqctx: RequestContext<Self::Context>,
Expand Down
4 changes: 1 addition & 3 deletions gateway-test-utils/configs/config.test.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
host_phase2_recovery_image_cache_max_images = 1

[dropshot]
# We want to allow uploads of host phase 2 recovery images, which may be
# measured in the (small) hundreds of MiB. Set this to 512 MiB.
request_body_max_bytes = 536870912
default_request_body_max_bytes = 1048576

[switch]
# For tests, bind to port 0 (i.e., OS chooses an open port) instead of MGS's
Expand Down
4 changes: 1 addition & 3 deletions gateway/examples/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
host_phase2_recovery_image_cache_max_images = 1

[dropshot]
# We want to allow uploads of host phase 2 recovery images, which may be
# measured in the (small) hundreds of MiB. Set this to 1 GiB for testing.
request_body_max_bytes = 1_073_741_824
default_request_body_max_bytes = 1048576

[switch]
# Which interface is connected to our local sidecar SP (i.e., the SP that acts
Expand Down
2 changes: 1 addition & 1 deletion gateway/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl Config {

#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
pub struct PartialDropshotConfig {
pub request_body_max_bytes: usize,
pub default_request_body_max_bytes: usize,
}

#[derive(Debug, Error, SlogInlineError)]
Expand Down
19 changes: 10 additions & 9 deletions gateway/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl From<SpCommsError> for HttpError {
)),
..
} => http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"UpdateSlotBusy",
InlineErrorChain::new(&error).to_string(),
),
Expand All @@ -125,29 +125,29 @@ impl From<SpCommsError> for HttpError {
)),
..
} => http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"UpdateInProgress",
InlineErrorChain::new(&error).to_string(),
),
SpCommsError::SpAddressUnknown(_) => http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"SpAddressUnknown",
InlineErrorChain::new(&error).to_string(),
),
SpCommsError::Timeout { .. } => http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"Timeout ",
InlineErrorChain::new(&error).to_string(),
),
SpCommsError::SpCommunicationFailed { .. } => {
http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"SpCommunicationFailed",
InlineErrorChain::new(&error).to_string(),
)
}
SpCommsError::UpdateFailed { .. } => http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"UpdateFailed",
InlineErrorChain::new(&error).to_string(),
),
Expand All @@ -163,12 +163,12 @@ impl From<SpLookupError> for HttpError {
InlineErrorChain::new(&error).to_string(),
),
SpLookupError::DiscoveryNotYetComplete => http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"DiscoveryNotYetComplete",
InlineErrorChain::new(&error).to_string(),
),
SpLookupError::DiscoveryFailed { .. } => http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"DiscoveryFailed ",
InlineErrorChain::new(&error).to_string(),
),
Expand All @@ -181,7 +181,7 @@ impl From<SpLookupError> for HttpError {
// status code, we want to give our caller some information about what is going
// wrong (e.g., we timed out waiting for an SP).
pub(crate) fn http_err_with_message(
status_code: http::StatusCode,
status_code: dropshot::ErrorStatusCode,
error_code: &str,
message: String,
) -> HttpError {
Expand All @@ -190,5 +190,6 @@ pub(crate) fn http_err_with_message(
error_code: Some(error_code.to_string()),
external_message: message.clone(),
internal_message: message,
headers: None,
}
}
2 changes: 1 addition & 1 deletion gateway/src/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ impl GatewayApi for GatewayImpl {
let key = str::from_utf8(key).unwrap();
String::from_utf8(bytes).map_err(|_| {
http_err_with_message(
http::StatusCode::SERVICE_UNAVAILABLE,
dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE,
"InvalidCaboose",
format!("non-utf8 data returned for caboose key {key}"),
)
Expand Down
Loading

0 comments on commit 8084358

Please sign in to comment.