From 9e2e06300c9aa34c8c9c3873567275be567b7127 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 5 Apr 2024 09:32:13 -0500 Subject: [PATCH 01/11] Remove ServerConfig --- dropshot/examples/basic.rs | 2 +- dropshot/examples/file_server.rs | 2 +- dropshot/examples/https.rs | 2 +- dropshot/examples/index.rs | 2 +- dropshot/examples/module-basic.rs | 2 +- dropshot/examples/module-shared-context.rs | 2 +- dropshot/examples/multipart.rs | 2 +- dropshot/examples/multiple-servers.rs | 2 +- dropshot/examples/pagination-basic.rs | 2 +- .../examples/pagination-multiple-resources.rs | 2 +- .../examples/pagination-multiple-sorts.rs | 2 +- dropshot/examples/request-headers.rs | 2 +- dropshot/examples/self-referential.rs | 2 +- dropshot/examples/websocket.rs | 2 +- dropshot/examples/well-tagged.rs | 2 +- dropshot/src/config.rs | 7 +++ dropshot/src/lib.rs | 6 ++- dropshot/src/server.rs | 47 +++++-------------- dropshot/src/test_util.rs | 4 +- dropshot/src/websocket.rs | 8 ++-- dropshot/tests/common/mod.rs | 2 +- dropshot/tests/test_config.rs | 11 +++-- dropshot/tests/test_tls.rs | 6 ++- 23 files changed, 54 insertions(+), 67 deletions(-) diff --git a/dropshot/examples/basic.rs b/dropshot/examples/basic.rs index dbd7ea4d6..eaba81336 100644 --- a/dropshot/examples/basic.rs +++ b/dropshot/examples/basic.rs @@ -44,7 +44,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = - HttpServerStarter::new(&config_dropshot, api, api_context, &log) + HttpServerStarter::new(config_dropshot, api, api_context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/file_server.rs b/dropshot/examples/file_server.rs index df9392496..8a5c9f4dd 100644 --- a/dropshot/examples/file_server.rs +++ b/dropshot/examples/file_server.rs @@ -44,7 +44,7 @@ async fn main() -> Result<(), String> { let context = FileServerContext { base: PathBuf::from(".") }; // Set up the server. - let server = HttpServerStarter::new(&config_dropshot, api, context, &log) + let server = HttpServerStarter::new(config_dropshot, api, context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/https.rs b/dropshot/examples/https.rs index a0b4348d3..cb650986a 100644 --- a/dropshot/examples/https.rs +++ b/dropshot/examples/https.rs @@ -86,7 +86,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = HttpServerStarter::new_with_tls( - &config_dropshot, + config_dropshot, api, api_context, &log, diff --git a/dropshot/examples/index.rs b/dropshot/examples/index.rs index 48d3b74e8..4823cedb6 100644 --- a/dropshot/examples/index.rs +++ b/dropshot/examples/index.rs @@ -35,7 +35,7 @@ async fn main() -> Result<(), String> { api.register(index).unwrap(); // Set up the server. - let server = HttpServerStarter::new(&config_dropshot, api, (), &log) + let server = HttpServerStarter::new(config_dropshot, api, (), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/module-basic.rs b/dropshot/examples/module-basic.rs index 4b95d6f04..25bd4b443 100644 --- a/dropshot/examples/module-basic.rs +++ b/dropshot/examples/module-basic.rs @@ -36,7 +36,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = - HttpServerStarter::new(&config_dropshot, api, api_context, &log) + HttpServerStarter::new(config_dropshot, api, api_context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/module-shared-context.rs b/dropshot/examples/module-shared-context.rs index 3d9ecdf04..a219131c1 100644 --- a/dropshot/examples/module-shared-context.rs +++ b/dropshot/examples/module-shared-context.rs @@ -43,7 +43,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = HttpServerStarter::new( - &config_dropshot, + config_dropshot, api, api_context.clone(), &log, diff --git a/dropshot/examples/multipart.rs b/dropshot/examples/multipart.rs index 8080d7aa1..1f79c580d 100644 --- a/dropshot/examples/multipart.rs +++ b/dropshot/examples/multipart.rs @@ -34,7 +34,7 @@ async fn main() -> Result<(), String> { api.register(index).unwrap(); // Set up the server. - let server = HttpServerStarter::new(&config_dropshot, api, (), &log) + let server = HttpServerStarter::new(config_dropshot, api, (), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/multiple-servers.rs b/dropshot/examples/multiple-servers.rs index 1f11098c5..61a447efd 100644 --- a/dropshot/examples/multiple-servers.rs +++ b/dropshot/examples/multiple-servers.rs @@ -219,7 +219,7 @@ impl SharedMultiServerContext { log: log.clone(), }; let server = - HttpServerStarter::new(&config_dropshot, api, context, &log) + HttpServerStarter::new(config_dropshot, api, context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); let shutdown_handle = server.wait_for_shutdown(); diff --git a/dropshot/examples/pagination-basic.rs b/dropshot/examples/pagination-basic.rs index 342087e1d..3e444c0ce 100644 --- a/dropshot/examples/pagination-basic.rs +++ b/dropshot/examples/pagination-basic.rs @@ -128,7 +128,7 @@ async fn main() -> Result<(), String> { .map_err(|error| format!("failed to create logger: {}", error))?; let mut api = ApiDescription::new(); api.register(example_list_projects).unwrap(); - let server = HttpServerStarter::new(&config_dropshot, api, ctx, &log) + let server = HttpServerStarter::new(config_dropshot, api, ctx, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); server.await diff --git a/dropshot/examples/pagination-multiple-resources.rs b/dropshot/examples/pagination-multiple-resources.rs index f4afea277..d9c139f62 100644 --- a/dropshot/examples/pagination-multiple-resources.rs +++ b/dropshot/examples/pagination-multiple-resources.rs @@ -290,7 +290,7 @@ async fn main() -> Result<(), String> { api.register(example_list_projects).unwrap(); api.register(example_list_disks).unwrap(); api.register(example_list_instances).unwrap(); - let server = HttpServerStarter::new(&config_dropshot, api, ctx, &log) + let server = HttpServerStarter::new(config_dropshot, api, ctx, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/pagination-multiple-sorts.rs b/dropshot/examples/pagination-multiple-sorts.rs index 7f673f741..88fea3fe9 100644 --- a/dropshot/examples/pagination-multiple-sorts.rs +++ b/dropshot/examples/pagination-multiple-sorts.rs @@ -304,7 +304,7 @@ async fn main() -> Result<(), String> { .map_err(|error| format!("failed to create logger: {}", error))?; let mut api = ApiDescription::new(); api.register(example_list_projects).unwrap(); - let server = HttpServerStarter::new(&config_dropshot, api, ctx, &log) + let server = HttpServerStarter::new(config_dropshot, api, ctx, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/request-headers.rs b/dropshot/examples/request-headers.rs index a32916d52..dae322897 100644 --- a/dropshot/examples/request-headers.rs +++ b/dropshot/examples/request-headers.rs @@ -33,7 +33,7 @@ async fn main() -> Result<(), String> { let api_context = (); let server = - HttpServerStarter::new(&config_dropshot, api, api_context, &log) + HttpServerStarter::new(config_dropshot, api, api_context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); server.await diff --git a/dropshot/examples/self-referential.rs b/dropshot/examples/self-referential.rs index fb7d7c2f4..37bc20a60 100644 --- a/dropshot/examples/self-referential.rs +++ b/dropshot/examples/self-referential.rs @@ -34,7 +34,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = - HttpServerStarter::new(&config_dropshot, api, api_context, &log) + HttpServerStarter::new(config_dropshot, api, api_context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); let shutdown = server.wait_for_shutdown(); diff --git a/dropshot/examples/websocket.rs b/dropshot/examples/websocket.rs index 5fa128cd9..75c25c223 100644 --- a/dropshot/examples/websocket.rs +++ b/dropshot/examples/websocket.rs @@ -37,7 +37,7 @@ async fn main() -> Result<(), String> { api.register(example_api_websocket_counter).unwrap(); // Set up the server. - let server = HttpServerStarter::new(&config_dropshot, api, (), &log) + let server = HttpServerStarter::new(config_dropshot, api, (), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/well-tagged.rs b/dropshot/examples/well-tagged.rs index 9284ede6c..4501e5ede 100644 --- a/dropshot/examples/well-tagged.rs +++ b/dropshot/examples/well-tagged.rs @@ -99,7 +99,7 @@ async fn main() -> Result<(), String> { api.register(get_fryism).unwrap(); // Set up the server. - let server = HttpServerStarter::new(&config_dropshot, api, (), &log) + let server = HttpServerStarter::new(config_dropshot, api, (), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/src/config.rs b/dropshot/src/config.rs index 7b9dc532b..ef4cd9883 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, @@ -106,6 +111,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, } } diff --git a/dropshot/src/lib.rs b/dropshot/src/lib.rs index 4bf15b012..c891cff8e 100644 --- a/dropshot/src/lib.rs +++ b/dropshot/src/lib.rs @@ -49,7 +49,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> { @@ -68,9 +68,11 @@ //! // Start the server. //! let server = //! HttpServerStarter::new( -//! &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, //! }, //! api, diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 795530cfe..0bca23b4b 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -31,7 +31,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; @@ -63,7 +62,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 @@ -83,21 +82,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, -} - pub struct HttpServerStarter { app_state: Arc>, local_addr: SocketAddr, @@ -107,7 +91,7 @@ pub struct HttpServerStarter { impl HttpServerStarter { pub fn new( - config: &ConfigDropshot, + config: ConfigDropshot, api: ApiDescription, private: C, log: &Logger, @@ -116,27 +100,19 @@ impl HttpServerStarter { } pub fn new_with_tls( - config: &ConfigDropshot, + config: ConfigDropshot, api: ApiDescription, private: C, 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, - }; - let handler_waitgroup = WaitGroup::new(); let starter = match &tls { Some(tls) => { let (starter, app_state, local_addr) = InnerHttpsServerStarter::new( config, - server_config, + // server_config, api, private, log, @@ -154,7 +130,7 @@ impl HttpServerStarter { let (starter, app_state, local_addr) = InnerHttpServerStarter::new( config, - server_config, + // server_config, api, private, log, @@ -274,8 +250,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, @@ -286,7 +261,8 @@ impl InnerHttpServerStarter { let app_state = Arc::new(DropshotState { private, - config: server_config, + // config: server_config, + config, router: api.into_router(), log: log.new(o!("local_addr" => local_addr)), local_addr, @@ -559,8 +535,7 @@ impl InnerHttpsServerStarter { } fn new( - config: &ConfigDropshot, - server_config: ServerConfig, + config: ConfigDropshot, api: ApiDescription, private: C, log: &Logger, @@ -587,7 +562,7 @@ impl InnerHttpsServerStarter { let app_state = Arc::new(DropshotState { private, - config: server_config, + config, router: api.into_router(), log: logger, local_addr, @@ -1115,7 +1090,7 @@ mod test { let log_context = LogContext::new("test server", &config_logging); let log = &log_context.log; - let server = HttpServerStarter::new(&config_dropshot, api, 0, log) + let server = HttpServerStarter::new(config_dropshot, api, 0, log) .unwrap() .start(); diff --git a/dropshot/src/test_util.rs b/dropshot/src/test_util.rs index f94eeb35a..df7803074 100644 --- a/dropshot/src/test_util.rs +++ b/dropshot/src/test_util.rs @@ -480,7 +480,7 @@ impl TestContext { pub fn new( api: ApiDescription, private: Context, - config_dropshot: &ConfigDropshot, + config_dropshot: ConfigDropshot, log_context: Option, log: Logger, ) -> TestContext { @@ -492,7 +492,7 @@ impl TestContext { // Set up the server itself. let server = - HttpServerStarter::new(&config_dropshot, api, private, &log) + HttpServerStarter::new(config_dropshot, api, private, &log) .unwrap() .start(); diff --git a/dropshot/src/websocket.rs b/dropshot/src/websocket.rs index dfcea7530..282ca6319 100644 --- a/dropshot/src/websocket.rs +++ b/dropshot/src/websocket.rs @@ -296,10 +296,9 @@ 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,12 +323,13 @@ 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, + ..Default::default() }, router: HttpRouter::new(), log: log.clone(), diff --git a/dropshot/tests/common/mod.rs b/dropshot/tests/common/mod.rs index 9bade6b7d..408c063b9 100644 --- a/dropshot/tests/common/mod.rs +++ b/dropshot/tests/common/mod.rs @@ -39,7 +39,7 @@ pub fn test_setup_with_context( let logctx = create_log_context(test_name); let log = logctx.log.new(o!()); - TestContext::new(api, ctx, &config_dropshot, Some(logctx), log) + TestContext::new(api, ctx, config_dropshot, Some(logctx), log) } pub fn create_log_context(test_name: &str) -> LogContext { diff --git a/dropshot/tests/test_config.rs b/dropshot/tests/test_config.rs index c70108c9e..5af62a32f 100644 --- a/dropshot/tests/test_config.rs +++ b/dropshot/tests/test_config.rs @@ -84,7 +84,7 @@ fn test_config_bad_request_body_max_bytes_too_large() { fn make_server( context: T, - config: &ConfigDropshot, + config: ConfigDropshot, log: &Logger, tls: Option, api_description: Option>, @@ -111,6 +111,7 @@ fn make_config( ), request_body_max_bytes: 1024, default_handler_task_mode, + ..Default::default() } } @@ -206,7 +207,7 @@ async fn test_config_bind_address_http() { bind_port, HandlerTaskMode::CancelOnDisconnect, ); - make_server(0, &config, &self.log, None, None).start() + make_server(0, config, &self.log, None, None).start() } fn log(&self) -> &slog::Logger { @@ -274,7 +275,7 @@ async fn test_config_bind_address_https() { bind_port, HandlerTaskMode::CancelOnDisconnect, ); - make_server(0, &config, &self.log, tls, None).start() + make_server(0, config, &self.log, tls, None).start() } fn log(&self) -> &Logger { @@ -350,7 +351,7 @@ async fn test_config_bind_address_https_buffer() { bind_port, HandlerTaskMode::CancelOnDisconnect, ); - make_server(0, &config, &self.log, tls, None).start() + make_server(0, config, &self.log, tls, None).start() } fn log(&self) -> &Logger { @@ -478,7 +479,7 @@ impl TestConfigBindServer api.register(track_cancel_endpoint).unwrap(); let server = - make_server(context, &config, &self.log, None, Some(api)).start(); + make_server(context, config, &self.log, None, Some(api)).start(); self.bound_port.store(server.local_addr().port(), Ordering::SeqCst); diff --git a/dropshot/tests/test_tls.rs b/dropshot/tests/test_tls.rs index d06f62d21..a223a2e05 100644 --- a/dropshot/tests/test_tls.rs +++ b/dropshot/tests/test_tls.rs @@ -114,13 +114,14 @@ fn make_server( bind_address: "127.0.0.1:0".parse().unwrap(), request_body_max_bytes: 1024, default_handler_task_mode: HandlerTaskMode::CancelOnDisconnect, + ..Default::default() }; let config_tls = Some(ConfigTls::AsFile { cert_file: cert_file.to_path_buf(), key_file: key_file.to_path_buf(), }); HttpServerStarter::new_with_tls( - &config, + config, dropshot::ApiDescription::new(), 0, log, @@ -426,6 +427,7 @@ async fn test_server_is_https() { bind_address: "127.0.0.1:0".parse().unwrap(), request_body_max_bytes: 1024, default_handler_task_mode: HandlerTaskMode::CancelOnDisconnect, + ..Default::default() }; let config_tls = Some(ConfigTls::AsFile { cert_file: cert_file.path().to_path_buf(), @@ -434,7 +436,7 @@ async fn test_server_is_https() { let mut api = dropshot::ApiDescription::new(); api.register(tls_check_handler).unwrap(); let server = - HttpServerStarter::new_with_tls(&config, api, 0, &log, config_tls) + HttpServerStarter::new_with_tls(config, api, 0, &log, config_tls) .unwrap() .start(); let port = server.local_addr().port(); From 6e9e0f18f45182a6f0d75a545ee9ca43767ee974 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 5 Apr 2024 09:45:38 -0500 Subject: [PATCH 02/11] Fmt --- dropshot/examples/module-shared-context.rs | 12 ++++-------- dropshot/src/websocket.rs | 3 ++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/dropshot/examples/module-shared-context.rs b/dropshot/examples/module-shared-context.rs index a219131c1..fba44b1b3 100644 --- a/dropshot/examples/module-shared-context.rs +++ b/dropshot/examples/module-shared-context.rs @@ -42,14 +42,10 @@ async fn main() -> Result<(), String> { let api_context = Arc::new(ExampleContext::new()); // Set up the server. - let server = HttpServerStarter::new( - config_dropshot, - api, - api_context.clone(), - &log, - ) - .map_err(|error| format!("failed to create server: {}", error))? - .start(); + let server = + HttpServerStarter::new(config_dropshot, api, api_context.clone(), &log) + .map_err(|error| format!("failed to create server: {}", error))? + .start(); // Wait for the server to stop. Note that there's not any code to shut down // this server, so we should never get past this point. diff --git a/dropshot/src/websocket.rs b/dropshot/src/websocket.rs index 282ca6319..55668395b 100644 --- a/dropshot/src/websocket.rs +++ b/dropshot/src/websocket.rs @@ -298,7 +298,8 @@ mod tests { use crate::router::HttpRouter; use crate::server::DropshotState; use crate::{ - ConfigDropshot, ExclusiveExtractor, HttpError, RequestContext, RequestInfo, WebsocketUpgrade + ConfigDropshot, ExclusiveExtractor, HttpError, RequestContext, + RequestInfo, WebsocketUpgrade, }; use debug_ignore::DebugIgnore; use http::Request; From 49723bf28f9a6d1726c204b0d0116939fc9af275 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Thu, 25 Apr 2024 08:18:11 -0500 Subject: [PATCH 03/11] Cleanup --- dropshot/src/server.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 0bca23b4b..cf09d3cd6 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -112,7 +112,6 @@ impl HttpServerStarter { let (starter, app_state, local_addr) = InnerHttpsServerStarter::new( config, - // server_config, api, private, log, @@ -130,7 +129,6 @@ impl HttpServerStarter { let (starter, app_state, local_addr) = InnerHttpServerStarter::new( config, - // server_config, api, private, log, @@ -261,7 +259,6 @@ 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)), From 8637cde7c3b55eedf29207385f83757f173ce425 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Thu, 25 Apr 2024 08:27:41 -0500 Subject: [PATCH 04/11] Revert ownership change --- dropshot/examples/basic.rs | 2 +- dropshot/examples/file_server.rs | 2 +- dropshot/examples/https.rs | 2 +- dropshot/examples/index.rs | 2 +- dropshot/examples/module-basic.rs | 2 +- dropshot/examples/module-shared-context.rs | 2 +- dropshot/examples/multipart.rs | 2 +- dropshot/examples/multiple-servers.rs | 2 +- dropshot/examples/pagination-basic.rs | 2 +- dropshot/examples/pagination-multiple-resources.rs | 2 +- dropshot/examples/pagination-multiple-sorts.rs | 2 +- dropshot/examples/request-headers.rs | 2 +- dropshot/examples/self-referential.rs | 2 +- dropshot/examples/websocket.rs | 2 +- dropshot/examples/well-tagged.rs | 2 +- dropshot/src/server.rs | 10 +++++----- dropshot/src/test_util.rs | 2 +- dropshot/tests/test_config.rs | 10 +++++----- dropshot/tests/test_tls.rs | 4 ++-- 19 files changed, 28 insertions(+), 28 deletions(-) diff --git a/dropshot/examples/basic.rs b/dropshot/examples/basic.rs index eaba81336..dbd7ea4d6 100644 --- a/dropshot/examples/basic.rs +++ b/dropshot/examples/basic.rs @@ -44,7 +44,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = - HttpServerStarter::new(config_dropshot, api, api_context, &log) + HttpServerStarter::new(&config_dropshot, api, api_context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/file_server.rs b/dropshot/examples/file_server.rs index 8a5c9f4dd..df9392496 100644 --- a/dropshot/examples/file_server.rs +++ b/dropshot/examples/file_server.rs @@ -44,7 +44,7 @@ async fn main() -> Result<(), String> { let context = FileServerContext { base: PathBuf::from(".") }; // Set up the server. - let server = HttpServerStarter::new(config_dropshot, api, context, &log) + let server = HttpServerStarter::new(&config_dropshot, api, context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/https.rs b/dropshot/examples/https.rs index cb650986a..a0b4348d3 100644 --- a/dropshot/examples/https.rs +++ b/dropshot/examples/https.rs @@ -86,7 +86,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = HttpServerStarter::new_with_tls( - config_dropshot, + &config_dropshot, api, api_context, &log, diff --git a/dropshot/examples/index.rs b/dropshot/examples/index.rs index 4823cedb6..48d3b74e8 100644 --- a/dropshot/examples/index.rs +++ b/dropshot/examples/index.rs @@ -35,7 +35,7 @@ async fn main() -> Result<(), String> { api.register(index).unwrap(); // Set up the server. - let server = HttpServerStarter::new(config_dropshot, api, (), &log) + let server = HttpServerStarter::new(&config_dropshot, api, (), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/module-basic.rs b/dropshot/examples/module-basic.rs index 25bd4b443..4b95d6f04 100644 --- a/dropshot/examples/module-basic.rs +++ b/dropshot/examples/module-basic.rs @@ -36,7 +36,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = - HttpServerStarter::new(config_dropshot, api, api_context, &log) + HttpServerStarter::new(&config_dropshot, api, api_context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/module-shared-context.rs b/dropshot/examples/module-shared-context.rs index fba44b1b3..2a1c02b97 100644 --- a/dropshot/examples/module-shared-context.rs +++ b/dropshot/examples/module-shared-context.rs @@ -43,7 +43,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = - HttpServerStarter::new(config_dropshot, api, api_context.clone(), &log) + HttpServerStarter::new(&config_dropshot, api, api_context.clone(), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/multipart.rs b/dropshot/examples/multipart.rs index 1f79c580d..8080d7aa1 100644 --- a/dropshot/examples/multipart.rs +++ b/dropshot/examples/multipart.rs @@ -34,7 +34,7 @@ async fn main() -> Result<(), String> { api.register(index).unwrap(); // Set up the server. - let server = HttpServerStarter::new(config_dropshot, api, (), &log) + let server = HttpServerStarter::new(&config_dropshot, api, (), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/multiple-servers.rs b/dropshot/examples/multiple-servers.rs index 61a447efd..1f11098c5 100644 --- a/dropshot/examples/multiple-servers.rs +++ b/dropshot/examples/multiple-servers.rs @@ -219,7 +219,7 @@ impl SharedMultiServerContext { log: log.clone(), }; let server = - HttpServerStarter::new(config_dropshot, api, context, &log) + HttpServerStarter::new(&config_dropshot, api, context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); let shutdown_handle = server.wait_for_shutdown(); diff --git a/dropshot/examples/pagination-basic.rs b/dropshot/examples/pagination-basic.rs index 3e444c0ce..342087e1d 100644 --- a/dropshot/examples/pagination-basic.rs +++ b/dropshot/examples/pagination-basic.rs @@ -128,7 +128,7 @@ async fn main() -> Result<(), String> { .map_err(|error| format!("failed to create logger: {}", error))?; let mut api = ApiDescription::new(); api.register(example_list_projects).unwrap(); - let server = HttpServerStarter::new(config_dropshot, api, ctx, &log) + let server = HttpServerStarter::new(&config_dropshot, api, ctx, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); server.await diff --git a/dropshot/examples/pagination-multiple-resources.rs b/dropshot/examples/pagination-multiple-resources.rs index d9c139f62..f4afea277 100644 --- a/dropshot/examples/pagination-multiple-resources.rs +++ b/dropshot/examples/pagination-multiple-resources.rs @@ -290,7 +290,7 @@ async fn main() -> Result<(), String> { api.register(example_list_projects).unwrap(); api.register(example_list_disks).unwrap(); api.register(example_list_instances).unwrap(); - let server = HttpServerStarter::new(config_dropshot, api, ctx, &log) + let server = HttpServerStarter::new(&config_dropshot, api, ctx, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/pagination-multiple-sorts.rs b/dropshot/examples/pagination-multiple-sorts.rs index 88fea3fe9..7f673f741 100644 --- a/dropshot/examples/pagination-multiple-sorts.rs +++ b/dropshot/examples/pagination-multiple-sorts.rs @@ -304,7 +304,7 @@ async fn main() -> Result<(), String> { .map_err(|error| format!("failed to create logger: {}", error))?; let mut api = ApiDescription::new(); api.register(example_list_projects).unwrap(); - let server = HttpServerStarter::new(config_dropshot, api, ctx, &log) + let server = HttpServerStarter::new(&config_dropshot, api, ctx, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/request-headers.rs b/dropshot/examples/request-headers.rs index dae322897..a32916d52 100644 --- a/dropshot/examples/request-headers.rs +++ b/dropshot/examples/request-headers.rs @@ -33,7 +33,7 @@ async fn main() -> Result<(), String> { let api_context = (); let server = - HttpServerStarter::new(config_dropshot, api, api_context, &log) + HttpServerStarter::new(&config_dropshot, api, api_context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); server.await diff --git a/dropshot/examples/self-referential.rs b/dropshot/examples/self-referential.rs index 37bc20a60..fb7d7c2f4 100644 --- a/dropshot/examples/self-referential.rs +++ b/dropshot/examples/self-referential.rs @@ -34,7 +34,7 @@ async fn main() -> Result<(), String> { // Set up the server. let server = - HttpServerStarter::new(config_dropshot, api, api_context, &log) + HttpServerStarter::new(&config_dropshot, api, api_context, &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); let shutdown = server.wait_for_shutdown(); diff --git a/dropshot/examples/websocket.rs b/dropshot/examples/websocket.rs index 75c25c223..5fa128cd9 100644 --- a/dropshot/examples/websocket.rs +++ b/dropshot/examples/websocket.rs @@ -37,7 +37,7 @@ async fn main() -> Result<(), String> { api.register(example_api_websocket_counter).unwrap(); // Set up the server. - let server = HttpServerStarter::new(config_dropshot, api, (), &log) + let server = HttpServerStarter::new(&config_dropshot, api, (), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/examples/well-tagged.rs b/dropshot/examples/well-tagged.rs index 4501e5ede..9284ede6c 100644 --- a/dropshot/examples/well-tagged.rs +++ b/dropshot/examples/well-tagged.rs @@ -99,7 +99,7 @@ async fn main() -> Result<(), String> { api.register(get_fryism).unwrap(); // Set up the server. - let server = HttpServerStarter::new(config_dropshot, api, (), &log) + let server = HttpServerStarter::new(&config_dropshot, api, (), &log) .map_err(|error| format!("failed to create server: {}", error))? .start(); diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index cf09d3cd6..075dab63f 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -91,7 +91,7 @@ pub struct HttpServerStarter { impl HttpServerStarter { pub fn new( - config: ConfigDropshot, + config: &ConfigDropshot, api: ApiDescription, private: C, log: &Logger, @@ -100,7 +100,7 @@ impl HttpServerStarter { } pub fn new_with_tls( - config: ConfigDropshot, + config: &ConfigDropshot, api: ApiDescription, private: C, log: &Logger, @@ -111,7 +111,7 @@ impl HttpServerStarter { Some(tls) => { let (starter, app_state, local_addr) = InnerHttpsServerStarter::new( - config, + config.clone(), api, private, log, @@ -128,7 +128,7 @@ impl HttpServerStarter { None => { let (starter, app_state, local_addr) = InnerHttpServerStarter::new( - config, + config.clone(), api, private, log, @@ -1087,7 +1087,7 @@ mod test { let log_context = LogContext::new("test server", &config_logging); let log = &log_context.log; - let server = HttpServerStarter::new(config_dropshot, api, 0, log) + let server = HttpServerStarter::new(&config_dropshot, api, 0, log) .unwrap() .start(); diff --git a/dropshot/src/test_util.rs b/dropshot/src/test_util.rs index df7803074..73b0de61e 100644 --- a/dropshot/src/test_util.rs +++ b/dropshot/src/test_util.rs @@ -492,7 +492,7 @@ impl TestContext { // Set up the server itself. let server = - HttpServerStarter::new(config_dropshot, api, private, &log) + HttpServerStarter::new(&config_dropshot, api, private, &log) .unwrap() .start(); diff --git a/dropshot/tests/test_config.rs b/dropshot/tests/test_config.rs index 5af62a32f..b4d356503 100644 --- a/dropshot/tests/test_config.rs +++ b/dropshot/tests/test_config.rs @@ -84,7 +84,7 @@ fn test_config_bad_request_body_max_bytes_too_large() { fn make_server( context: T, - config: ConfigDropshot, + config: &ConfigDropshot, log: &Logger, tls: Option, api_description: Option>, @@ -207,7 +207,7 @@ async fn test_config_bind_address_http() { bind_port, HandlerTaskMode::CancelOnDisconnect, ); - make_server(0, config, &self.log, None, None).start() + make_server(0, &config, &self.log, None, None).start() } fn log(&self) -> &slog::Logger { @@ -275,7 +275,7 @@ async fn test_config_bind_address_https() { bind_port, HandlerTaskMode::CancelOnDisconnect, ); - make_server(0, config, &self.log, tls, None).start() + make_server(0, &config, &self.log, tls, None).start() } fn log(&self) -> &Logger { @@ -351,7 +351,7 @@ async fn test_config_bind_address_https_buffer() { bind_port, HandlerTaskMode::CancelOnDisconnect, ); - make_server(0, config, &self.log, tls, None).start() + make_server(0, &config, &self.log, tls, None).start() } fn log(&self) -> &Logger { @@ -479,7 +479,7 @@ impl TestConfigBindServer api.register(track_cancel_endpoint).unwrap(); let server = - make_server(context, config, &self.log, None, Some(api)).start(); + make_server(context, &config, &self.log, None, Some(api)).start(); self.bound_port.store(server.local_addr().port(), Ordering::SeqCst); diff --git a/dropshot/tests/test_tls.rs b/dropshot/tests/test_tls.rs index a223a2e05..f41f8e8fd 100644 --- a/dropshot/tests/test_tls.rs +++ b/dropshot/tests/test_tls.rs @@ -121,7 +121,7 @@ fn make_server( key_file: key_file.to_path_buf(), }); HttpServerStarter::new_with_tls( - config, + &config, dropshot::ApiDescription::new(), 0, log, @@ -436,7 +436,7 @@ async fn test_server_is_https() { let mut api = dropshot::ApiDescription::new(); api.register(tls_check_handler).unwrap(); let server = - HttpServerStarter::new_with_tls(config, api, 0, &log, config_tls) + HttpServerStarter::new_with_tls(&config, api, 0, &log, config_tls) .unwrap() .start(); let port = server.local_addr().port(); From 264a1efa609833219a676427a8f300e2bf49a6b8 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Thu, 25 Apr 2024 08:28:45 -0500 Subject: [PATCH 05/11] Fix doc --- dropshot/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dropshot/src/lib.rs b/dropshot/src/lib.rs index c891cff8e..a61f3952a 100644 --- a/dropshot/src/lib.rs +++ b/dropshot/src/lib.rs @@ -68,7 +68,7 @@ //! // Start the server. //! let server = //! HttpServerStarter::new( -//! ConfigDropshot { +//! &ConfigDropshot { //! bind_address: "127.0.0.1:0".parse().unwrap(), //! request_body_max_bytes: 1024, //! page_max_nitems: NonZeroU32::new(10000).unwrap(), From 9788f79f72d90a539f8bfc3d676a948b6994504c Mon Sep 17 00:00:00 2001 From: augustuswm Date: Thu, 25 Apr 2024 09:56:59 -0500 Subject: [PATCH 06/11] Fmt --- dropshot/examples/module-shared-context.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/dropshot/examples/module-shared-context.rs b/dropshot/examples/module-shared-context.rs index 2a1c02b97..3d9ecdf04 100644 --- a/dropshot/examples/module-shared-context.rs +++ b/dropshot/examples/module-shared-context.rs @@ -42,10 +42,14 @@ async fn main() -> Result<(), String> { let api_context = Arc::new(ExampleContext::new()); // Set up the server. - let server = - HttpServerStarter::new(&config_dropshot, api, api_context.clone(), &log) - .map_err(|error| format!("failed to create server: {}", error))? - .start(); + let server = HttpServerStarter::new( + &config_dropshot, + api, + api_context.clone(), + &log, + ) + .map_err(|error| format!("failed to create server: {}", error))? + .start(); // Wait for the server to stop. Note that there's not any code to shut down // this server, so we should never get past this point. From 40ba009fcd3ee4c26301684db64583418d4681d4 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Thu, 25 Apr 2024 10:00:15 -0500 Subject: [PATCH 07/11] Add breaking change note --- CHANGELOG.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 992b4f81e..179e1a4bc 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,6 +15,10 @@ https://github.com/oxidecomputer/dropshot/compare/v0.10.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. + == 0.10.0 (released 2024-02-06) https://github.com/oxidecomputer/dropshot/compare/v0.9.0\...v0.10.0[Full list of commits] From 737d45bea56388c5b881f694cc638751d4f5233b Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Wed, 21 Aug 2024 15:55:42 -0500 Subject: [PATCH 08/11] Update changelog --- CHANGELOG.adoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 72c35ec1d..241c5ca0a 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. + == 0.11.0 (released 2024-08-21) https://github.com/oxidecomputer/dropshot/compare/v0.10.1\...v0.11.0[Full list of commits] @@ -53,10 +57,6 @@ https://github.com/oxidecomputer/dropshot/compare/v0.10.0\...v0.10.1[Full list o * https://github.com/oxidecomputer/dropshot/pull/1005[#1005] Update edition to 2021. * https://github.com/oxidecomputer/dropshot/pull/988[#988] Add a spurious, trailing newline to OpenAPI output. -=== 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. - == 0.10.0 (released 2024-02-06) https://github.com/oxidecomputer/dropshot/compare/v0.9.0\...v0.10.0[Full list of commits] From 07b84dea77b8e3aca8d192403383800aa1e987d0 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Wed, 21 Aug 2024 15:55:48 -0500 Subject: [PATCH 09/11] Fmt --- dropshot/src/websocket.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dropshot/src/websocket.rs b/dropshot/src/websocket.rs index abf48bcba..78ff44680 100644 --- a/dropshot/src/websocket.rs +++ b/dropshot/src/websocket.rs @@ -328,7 +328,8 @@ mod tests { 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, + default_handler_task_mode: + HandlerTaskMode::CancelOnDisconnect, log_headers: Default::default(), ..Default::default() }, From a1609d875fa6ba22f21302c0262ac6c6a5b41c9d Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Wed, 21 Aug 2024 15:58:39 -0500 Subject: [PATCH 10/11] Revert test change --- dropshot/src/test_util.rs | 2 +- dropshot/tests/common/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dropshot/src/test_util.rs b/dropshot/src/test_util.rs index 73b0de61e..f94eeb35a 100644 --- a/dropshot/src/test_util.rs +++ b/dropshot/src/test_util.rs @@ -480,7 +480,7 @@ impl TestContext { pub fn new( api: ApiDescription, private: Context, - config_dropshot: ConfigDropshot, + config_dropshot: &ConfigDropshot, log_context: Option, log: Logger, ) -> TestContext { diff --git a/dropshot/tests/common/mod.rs b/dropshot/tests/common/mod.rs index ffa449d95..2fe33a659 100644 --- a/dropshot/tests/common/mod.rs +++ b/dropshot/tests/common/mod.rs @@ -39,7 +39,7 @@ pub fn test_setup_with_context( let logctx = create_log_context(test_name); let log = logctx.log.new(o!()); - TestContext::new(api, ctx, config_dropshot, Some(logctx), log) + TestContext::new(api, ctx, &config_dropshot, Some(logctx), log) } pub fn create_log_context(test_name: &str) -> LogContext { From 7d0851df397a4f86c7c40a7a69412ade0038e05f Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Wed, 21 Aug 2024 16:00:46 -0500 Subject: [PATCH 11/11] Update breaking change note --- CHANGELOG.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 241c5ca0a..2b0db473f 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -17,7 +17,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.11.0\...HEAD[Full list of c === 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. +* 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)