diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 423bc759e..2b0db473f 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,6 +15,10 @@ https://github.com/oxidecomputer/dropshot/compare/v0.11.0\...HEAD[Full list of commits] +=== Breaking Changes + +* https://github.com/oxidecomputer/dropshot/pull/959[#959] expands the settings available via `ConfigDropshot`. This adds two new fields `page_max_nitems` and `page_default_nitems` that will need to be added wherever a `ConfigDropshot` struct is manually constructed. The defaults are likely appropriate for most use cases. + == 0.11.0 (released 2024-08-21) https://github.com/oxidecomputer/dropshot/compare/v0.10.1\...v0.11.0[Full list of commits] diff --git a/dropshot/src/config.rs b/dropshot/src/config.rs index 40414ece9..a0429758a 100644 --- a/dropshot/src/config.rs +++ b/dropshot/src/config.rs @@ -4,6 +4,7 @@ use serde::Deserialize; use serde::Serialize; use std::net::SocketAddr; +use std::num::NonZeroU32; use std::path::PathBuf; /// Raw [`rustls::ServerConfig`] TLS configuration for use with @@ -49,6 +50,10 @@ pub struct ConfigDropshot { pub bind_address: SocketAddr, /// maximum allowed size of a request body, defaults to 1024 pub request_body_max_bytes: usize, + /// maximum size of any page of results + pub page_max_nitems: NonZeroU32, + /// default size for a page of results + pub page_default_nitems: NonZeroU32, /// Default behavior for HTTP handler functions with respect to clients /// disconnecting early. pub default_handler_task_mode: HandlerTaskMode, @@ -114,6 +119,8 @@ impl Default for ConfigDropshot { ConfigDropshot { bind_address: "127.0.0.1:0".parse().unwrap(), request_body_max_bytes: 1024, + page_max_nitems: NonZeroU32::new(10000).unwrap(), + page_default_nitems: NonZeroU32::new(100).unwrap(), default_handler_task_mode: HandlerTaskMode::Detached, log_headers: Default::default(), } diff --git a/dropshot/src/lib.rs b/dropshot/src/lib.rs index 620f3f1c0..76bbcf8ae 100644 --- a/dropshot/src/lib.rs +++ b/dropshot/src/lib.rs @@ -50,7 +50,7 @@ //! use dropshot::ConfigLoggingLevel; //! use dropshot::HandlerTaskMode; //! use dropshot::HttpServerStarter; -//! use std::sync::Arc; +//! use std::{num::NonZeroU32, sync::Arc}; //! //! #[tokio::main] //! async fn main() -> Result<(), String> { @@ -72,6 +72,8 @@ //! &ConfigDropshot { //! bind_address: "127.0.0.1:0".parse().unwrap(), //! request_body_max_bytes: 1024, +//! page_max_nitems: NonZeroU32::new(10000).unwrap(), +//! page_default_nitems: NonZeroU32::new(100).unwrap(), //! default_handler_task_mode: HandlerTaskMode::Detached, //! log_headers: Default::default(), //! }, diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 7f7935005..3db8ce1cb 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -32,7 +32,6 @@ use std::convert::TryFrom; use std::future::Future; use std::mem; use std::net::SocketAddr; -use std::num::NonZeroU32; use std::panic; use std::pin::Pin; use std::sync::Arc; @@ -64,7 +63,7 @@ pub struct DropshotState { /// caller-specific state pub private: C, /// static server configuration parameters - pub config: ServerConfig, + pub config: ConfigDropshot, /// request router pub router: HttpRouter, /// server-wide log handle @@ -84,29 +83,6 @@ impl DropshotState { } } -/// Stores static configuration associated with the server -/// TODO-cleanup merge with ConfigDropshot -#[derive(Debug)] -pub struct ServerConfig { - /// maximum allowed size of a request body - pub request_body_max_bytes: usize, - /// maximum size of any page of results - pub page_max_nitems: NonZeroU32, - /// default size for a page of results - pub page_default_nitems: NonZeroU32, - /// Default behavior for HTTP handler functions with respect to clients - /// disconnecting early. - pub default_handler_task_mode: HandlerTaskMode, - /// A list of header names to include as extra properties in the log - /// messages emitted by the per-request logger. Each header will, if - /// present, be included in the output with a "hdr_"-prefixed property name - /// in lower case that has all hyphens replaced with underscores; e.g., - /// "X-Forwarded-For" will be included as "hdr_x_forwarded_for". No attempt - /// is made to deal with headers that appear multiple times in a single - /// request. - pub log_headers: Vec, -} - pub struct HttpServerStarter { app_state: Arc>, local_addr: SocketAddr, @@ -131,22 +107,12 @@ impl HttpServerStarter { log: &Logger, tls: Option, ) -> Result, GenericError> { - let server_config = ServerConfig { - // We start aggressively to ensure test coverage. - request_body_max_bytes: config.request_body_max_bytes, - page_max_nitems: NonZeroU32::new(10000).unwrap(), - page_default_nitems: NonZeroU32::new(100).unwrap(), - default_handler_task_mode: config.default_handler_task_mode, - log_headers: config.log_headers.clone(), - }; - let handler_waitgroup = WaitGroup::new(); let starter = match &tls { Some(tls) => { let (starter, app_state, local_addr) = InnerHttpsServerStarter::new( - config, - server_config, + config.clone(), api, private, log, @@ -163,8 +129,7 @@ impl HttpServerStarter { None => { let (starter, app_state, local_addr) = InnerHttpServerStarter::new( - config, - server_config, + config.clone(), api, private, log, @@ -284,8 +249,7 @@ impl InnerHttpServerStarter { /// of `HttpServerStarter` (and await the result) to actually start the /// server. fn new( - config: &ConfigDropshot, - server_config: ServerConfig, + config: ConfigDropshot, api: ApiDescription, private: C, log: &Logger, @@ -296,7 +260,7 @@ impl InnerHttpServerStarter { let app_state = Arc::new(DropshotState { private, - config: server_config, + config, router: api.into_router(), log: log.new(o!("local_addr" => local_addr)), local_addr, @@ -569,8 +533,7 @@ impl InnerHttpsServerStarter { } fn new( - config: &ConfigDropshot, - server_config: ServerConfig, + config: ConfigDropshot, api: ApiDescription, private: C, log: &Logger, @@ -597,7 +560,7 @@ impl InnerHttpsServerStarter { let app_state = Arc::new(DropshotState { private, - config: server_config, + config, router: api.into_router(), log: logger, local_addr, diff --git a/dropshot/src/websocket.rs b/dropshot/src/websocket.rs index 5311e130c..78ff44680 100644 --- a/dropshot/src/websocket.rs +++ b/dropshot/src/websocket.rs @@ -296,10 +296,10 @@ impl JsonSchema for WebsocketUpgrade { mod tests { use crate::config::HandlerTaskMode; use crate::router::HttpRouter; - use crate::server::{DropshotState, ServerConfig}; + use crate::server::DropshotState; use crate::{ - ExclusiveExtractor, HttpError, RequestContext, RequestInfo, - WebsocketUpgrade, + ConfigDropshot, ExclusiveExtractor, HttpError, RequestContext, + RequestInfo, WebsocketUpgrade, }; use debug_ignore::DebugIgnore; use http::Request; @@ -324,13 +324,14 @@ mod tests { let rqctx = RequestContext { server: Arc::new(DropshotState { private: (), - config: ServerConfig { + config: ConfigDropshot { request_body_max_bytes: 0, page_max_nitems: NonZeroU32::new(1).unwrap(), page_default_nitems: NonZeroU32::new(1).unwrap(), default_handler_task_mode: HandlerTaskMode::CancelOnDisconnect, log_headers: Default::default(), + ..Default::default() }, router: HttpRouter::new(), log: log.clone(), diff --git a/dropshot/tests/test_config.rs b/dropshot/tests/test_config.rs index 17c3864e1..e699bde45 100644 --- a/dropshot/tests/test_config.rs +++ b/dropshot/tests/test_config.rs @@ -112,6 +112,7 @@ fn make_config( request_body_max_bytes: 1024, default_handler_task_mode, log_headers: Default::default(), + ..Default::default() } } diff --git a/dropshot/tests/test_tls.rs b/dropshot/tests/test_tls.rs index 233cfce2c..aff6a9caa 100644 --- a/dropshot/tests/test_tls.rs +++ b/dropshot/tests/test_tls.rs @@ -115,6 +115,7 @@ fn make_server( request_body_max_bytes: 1024, default_handler_task_mode: HandlerTaskMode::CancelOnDisconnect, log_headers: Default::default(), + ..Default::default() }; let config_tls = Some(ConfigTls::AsFile { cert_file: cert_file.to_path_buf(), @@ -428,6 +429,7 @@ async fn test_server_is_https() { request_body_max_bytes: 1024, default_handler_task_mode: HandlerTaskMode::CancelOnDisconnect, log_headers: Default::default(), + ..Default::default() }; let config_tls = Some(ConfigTls::AsFile { cert_file: cert_file.path().to_path_buf(),