Exposing ServerConfig options

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]

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(),
         }

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(),
 //!             },

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<C: ServerContext> {
     /// caller-specific state
     pub private: C,
     /// static server configuration parameters
-    pub config: ServerConfig,
+    pub config: ConfigDropshot,
     /// request router
     pub router: HttpRouter<C>,
     /// server-wide log handle
@@ -84,29 +83,6 @@ impl<C: ServerContext> DropshotState<C> {
     }
 }
 
-/// 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<String>,
-}
-
 pub struct HttpServerStarter<C: ServerContext> {
     app_state: Arc<DropshotState<C>>,
     local_addr: SocketAddr,
@@ -131,22 +107,12 @@ impl<C: ServerContext> HttpServerStarter<C> {
         log: &Logger,
         tls: Option<ConfigTls>,
     ) -> Result<HttpServerStarter<C>, 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<C: ServerContext> HttpServerStarter<C> {
             None => {
                 let (starter, app_state, local_addr) =
                     InnerHttpServerStarter::new(
-                        config,
-                        server_config,
+                        config.clone(),
                         api,
                         private,
                         log,
@@ -284,8 +249,7 @@ impl<C: ServerContext> InnerHttpServerStarter<C> {
     /// of `HttpServerStarter` (and await the result) to actually start the
     /// server.
     fn new(
-        config: &ConfigDropshot,
-        server_config: ServerConfig,
+        config: ConfigDropshot,
         api: ApiDescription<C>,
         private: C,
         log: &Logger,
@@ -296,7 +260,7 @@ impl<C: ServerContext> InnerHttpServerStarter<C> {
 
         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<C: ServerContext> InnerHttpsServerStarter<C> {
     }
 
     fn new(
-        config: &ConfigDropshot,
-        server_config: ServerConfig,
+        config: ConfigDropshot,
         api: ApiDescription<C>,
         private: C,
         log: &Logger,
@@ -597,7 +560,7 @@ impl<C: ServerContext> InnerHttpsServerStarter<C> {
 
         let app_state = Arc::new(DropshotState {
             private,
-            config: server_config,
+            config,
             router: api.into_router(),
             log: logger,
             local_addr,

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(),

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()
     }
 }
 

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(),