update to dropshot 0.15.0

oxidecomputer · Dec 5, 2024 · 45b9156 · 45b9156
1 parent 5e71ab8
commit 45b9156
Show file tree

Hide file tree

Showing 34 changed files with 249 additions and 219 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -366,7 +366,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.0", features = [ "usdt-probes" ] }
 dyn-clone = "1.0.17"
 either = "1.13.0"
 expectorate = "1.1.0"
@@ -886,8 +886,3 @@ path = "workspace-hack"
 [patch."https://github.com/oxidecomputer/omicron"]
 omicron-uuid-kinds = { path = "uuid-kinds" }
 omicron-common = { path = "common" }
-
-# /!\ REMOVE BEFORE FLIGHT: Temporary patch to test dropshot breaking changes
-[patch.crates-io.dropshot]
-git = 'https://github.com/oxidecomputer/dropshot'
-branch = 'eliza/custom-error-httpresponse-result'
diff --git a/clickhouse-admin/src/clickhouse_cli.rs b/clickhouse-admin/src/clickhouse_cli.rs
@@ -60,6 +60,7 @@ impl From<ClickhouseCliError> for HttpError {
                     error_code: Some(String::from("Internal")),
                     external_message: message.clone(),
                     internal_message: message,
+                    headers: None,
                 }
             }
         }

diff --git a/clickhouse-admin/src/clickward.rs b/clickhouse-admin/src/clickward.rs
@@ -28,6 +28,7 @@ impl From<ClickwardError> for HttpError {
                     error_code: Some(String::from("Internal")),
                     external_message: message.clone(),
                     internal_message: message,
+                    headers: None,
                 }
             }
         }

diff --git a/cockroach-admin/src/cockroach_cli.rs b/cockroach-admin/src/cockroach_cli.rs
@@ -50,6 +50,7 @@ impl From<CockroachCliError> for HttpError {
                     error_code: Some(String::from("Internal")),
                     external_message: message.clone(),
                     internal_message: message,
+                    headers: None,
                 }
             }
         }

diff --git a/cockroach-admin/src/context.rs b/cockroach-admin/src/context.rs
@@ -57,6 +57,7 @@ impl ServerContext {
                     error_code: None,
                     external_message: message.clone(),
                     internal_message: message,
+                    headers: None,
                 })
             }
         }

diff --git a/common/src/api/external/error.rs b/common/src/api/external/error.rs
@@ -435,6 +435,7 @@ impl From<Error> for HttpError {
                     "credentials missing or invalid",
                 ),
                 internal_message,
+                headers: None,
             },
 
             Error::InvalidRequest { message } => {
@@ -445,6 +446,7 @@ impl From<Error> for HttpError {
                     error_code: Some(String::from("InvalidRequest")),
                     external_message,
                     internal_message,
+                    headers: None,
                 }
             }
 
@@ -459,6 +461,7 @@ impl From<Error> for HttpError {
                         label, external_message
                     ),
                     internal_message,
+                    headers: None,
                 }
             }
 
@@ -493,6 +496,7 @@ impl From<Error> for HttpError {
                         external_message
                     ),
                     internal_message,
+                    headers: None,
                 }
             }
 
@@ -508,6 +512,7 @@ impl From<Error> for HttpError {
                     error_code: Some(String::from("Conflict")),
                     external_message,
                     internal_message,
+                    headers: None,
                 }
             }
 
@@ -519,6 +524,7 @@ impl From<Error> for HttpError {
                     error_code: Some(String::from("Not Found")),
                     external_message,
                     internal_message,
+                    headers: None,
                 }
             }
 

diff --git a/dns-server/examples/config.toml b/dns-server/examples/config.toml
@@ -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

diff --git a/dns-server/src/http_server.rs b/dns-server/src/http_server.rs
@@ -70,13 +70,15 @@ impl From<UpdateError> for dropshot::HttpError {
                 )),
                 external_message: message.clone(),
                 internal_message: message,
+                headers: None,
             },
 
             UpdateError::UpdateInProgress { .. } => dropshot::HttpError {
                 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(_) => {

diff --git a/gateway/examples/config.toml b/gateway/examples/config.toml
@@ -11,7 +11,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 = 1_073_741_824
 
 [switch]
 # Which interface is connected to our local sidecar SP (i.e., the SP that acts

diff --git a/gateway/src/error.rs b/gateway/src/error.rs
@@ -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,
     }
 }
diff --git a/illumos-utils/src/lib.rs b/illumos-utils/src/lib.rs
@@ -82,6 +82,7 @@ impl From<ExecutionError> for HttpError {
             error_code: Some(String::from("Internal")),
             external_message: message.clone(),
             internal_message: message,
+            headers: None,
         }
     }
 }

diff --git a/nexus-config/src/nexus_config.rs b/nexus-config/src/nexus_config.rs
@@ -902,10 +902,10 @@ mod test {
             external_dns_servers = [ "1.1.1.1", "9.9.9.9" ]
             [deployment.dropshot_external]
             bind_address = "10.1.2.3:4567"
-            request_body_max_bytes = 1024
+            default_request_body_max_bytes = 1024
             [deployment.dropshot_internal]
             bind_address = "10.1.2.3:4568"
-            request_body_max_bytes = 1024
+            default_request_body_max_bytes = 1024
             [deployment.internal_dns]
             type = "from_subnet"
             subnet.net = "::/56"
@@ -1176,10 +1176,10 @@ mod test {
             external_dns_servers = [ "1.1.1.1", "9.9.9.9" ]
             [deployment.dropshot_external]
             bind_address = "10.1.2.3:4567"
-            request_body_max_bytes = 1024
+            default_request_body_max_bytes = 1024
             [deployment.dropshot_internal]
             bind_address = "10.1.2.3:4568"
-            request_body_max_bytes = 1024
+            default_request_body_max_bytes = 1024
             [deployment.internal_dns]
             type = "from_subnet"
             subnet.net = "::/56"
@@ -1262,10 +1262,10 @@ mod test {
             external_dns_servers = [ "1.1.1.1", "9.9.9.9" ]
             [deployment.dropshot_external]
             bind_address = "10.1.2.3:4567"
-            request_body_max_bytes = 1024
+            default_request_body_max_bytes = 1024
             [deployment.dropshot_internal]
             bind_address = "10.1.2.3:4568"
-            request_body_max_bytes = 1024
+            default_request_body_max_bytes = 1024
             [deployment.internal_dns]
             type = "from_subnet"
             subnet.net = "::/56"
@@ -1319,10 +1319,10 @@ mod test {
             external_dns_servers = [ "1.1.1.1", "9.9.9.9" ]
             [deployment.dropshot_external]
             bind_address = "10.1.2.3:4567"
-            request_body_max_bytes = 1024
+            default_request_body_max_bytes = 1024
             [deployment.dropshot_internal]
             bind_address = "10.1.2.3:4568"
-            request_body_max_bytes = 1024
+            default_request_body_max_bytes = 1024
             [deployment.internal_dns]
             type = "from_subnet"
             subnet.net = "::/56"

diff --git a/nexus/examples/config-second.toml b/nexus/examples/config-second.toml
@@ -60,7 +60,7 @@ bind_address = "127.0.0.1:12222"
 #
 # This should be brought back down to a more reasonable value once per-endpoint
 # request body limits are implemented.
-request_body_max_bytes = 3221225472
+default_request_body_max_bytes = 3221225472
 # To have Nexus's external HTTP endpoint use TLS, uncomment the line below.  You
 # will also need to provide an initial TLS certificate during rack
 # initialization.  If you're using this config file, you're probably running a
@@ -73,7 +73,7 @@ request_body_max_bytes = 3221225472
 # This config file uses 12223 to avoid colliding with the usual 12221 that's
 # used by `omicron-dev run-all`
 bind_address = "[::1]:12223"
-request_body_max_bytes = 1048576
+default_request_body_max_bytes = 1048576
 
 #[deployment.internal_dns]
 ## These values are overridden at the bottom of this file.

diff --git a/nexus/examples/config.toml b/nexus/examples/config.toml
@@ -46,7 +46,7 @@ bind_address = "127.0.0.1:12220"
 #
 # This should be brought back down to a more reasonable value once per-endpoint
 # request body limits are implemented.
-request_body_max_bytes = 3221225472
+default_request_body_max_bytes = 3221225472
 # To have Nexus's external HTTP endpoint use TLS, uncomment the line below.  You
 # will also need to provide an initial TLS certificate during rack
 # initialization.  If you're using this config file, you're probably running a
@@ -57,7 +57,7 @@ request_body_max_bytes = 3221225472
 [deployment.dropshot_internal]
 # IP Address and TCP port on which to listen for the internal API
 bind_address = "[::1]:12221"
-request_body_max_bytes = 1048576
+default_request_body_max_bytes = 1048576
 
 [deployment.internal_dns]
 # Example address.

diff --git a/nexus/src/app/background/init.rs b/nexus/src/app/background/init.rs
@@ -1098,7 +1098,7 @@ pub mod test {
             },
             &dropshot::ConfigDropshot {
                 bind_address: "[::1]:0".parse().unwrap(),
-                request_body_max_bytes: 8 * 1024,
+                default_request_body_max_bytes: 8 * 1024,
                 default_handler_task_mode: HandlerTaskMode::Detached,
                 log_headers: vec![],
             },

diff --git a/nexus/test-utils/src/http_testing.rs b/nexus/test-utils/src/http_testing.rs
@@ -349,7 +349,15 @@ impl<'a> RequestBuilder<'a> {
         if let Some(allowed_headers) = self.allowed_headers {
             for header_name in headers.keys() {
                 ensure!(
-                    allowed_headers.contains(header_name),
+                    allowed_headers.contains(header_name)
+                        || (
+                            // Dropshot adds `allow` headers to its 405 Method
+                            // Not Allowed responses, per RFC 9110. If we expect
+                            // a 405 we should also inherently expect `allow`.
+                            self.expected_status
+                                == Some(http::StatusCode::METHOD_NOT_ALLOWED)
+                                && header_name == http::header::ALLOW
+                        ),
                     "response contained unexpected header {:?}",
                     header_name
                 );

diff --git a/nexus/tests/config.test.toml b/nexus/tests/config.test.toml
@@ -50,11 +50,11 @@ external_dns_servers = ["1.1.1.1", "9.9.9.9"]
 # available port) because the test suite will be running many servers
 # concurrently.
 bind_address = "127.0.0.1:0"
-request_body_max_bytes = 8388608
+default_request_body_max_bytes = 8388608
 
 [deployment.dropshot_internal]
 bind_address = "127.0.0.1:0"
-request_body_max_bytes = 8388608
+default_request_body_max_bytes = 8388608
 
 #
 # NOTE: for the test suite, the internal DNS address will be replaced with one

diff --git a/sled-agent/src/boot_disk_os_writer.rs b/sled-agent/src/boot_disk_os_writer.rs
@@ -140,6 +140,7 @@ impl From<&BootDiskOsWriteError> for HttpError {
                     error_code: None,
                     external_message: message.clone(),
                     internal_message: message,
+                    headers: None,
                 }
             }
         }

diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs
@@ -708,6 +708,7 @@ impl SledAgentApi for SledAgentImpl {
                 error_code: None,
                 external_message: message.clone(),
                 internal_message: message,
+                headers: None,
             }
         })?;
         Ok(HttpResponseUpdatedNoContent())
@@ -759,6 +760,7 @@ impl SledAgentApi for SledAgentImpl {
                     error_code: None,
                     external_message: message.clone(),
                     internal_message: message,
+                    headers: None,
                 });
             }
             None => {
@@ -768,6 +770,7 @@ impl SledAgentApi for SledAgentImpl {
                     error_code: None,
                     external_message: message.clone(),
                     internal_message: message,
+                    headers: None,
                 });
             }
         };

diff --git a/sled-agent/src/sim/http_entrypoints.rs b/sled-agent/src/sim/http_entrypoints.rs
@@ -667,6 +667,7 @@ fn method_unimplemented<T>() -> Result<T, HttpError> {
             .to_string(),
         internal_message: "Method not implemented in sled-agent-sim"
             .to_string(),
+        headers: None,
     })
 }
 

diff --git a/smf/clickhouse-admin-keeper/config.toml b/smf/clickhouse-admin-keeper/config.toml
@@ -1,6 +1,6 @@
 [dropshot]
 # 1 MiB; we don't expect any requests of more than nominal size.
-request_body_max_bytes = 1048576
+default_request_body_max_bytes = 1048576
 
 [log]
 # Show log messages of this level and more severe

diff --git a/smf/clickhouse-admin-server/config.toml b/smf/clickhouse-admin-server/config.toml
@@ -1,6 +1,6 @@
 [dropshot]
 # 1 MiB; we don't expect any requests of more than nominal size.
-request_body_max_bytes = 1048576
+default_request_body_max_bytes = 1048576
 
 [log]
 # Show log messages of this level and more severe

diff --git a/smf/clickhouse-admin-single/config.toml b/smf/clickhouse-admin-single/config.toml
@@ -1,6 +1,6 @@
 [dropshot]
 # 1 MiB; we don't expect any requests of more than nominal size.
-request_body_max_bytes = 1048576
+default_request_body_max_bytes = 1048576
 
 [log]
 # Show log messages of this level and more severe

diff --git a/smf/cockroach-admin/config.toml b/smf/cockroach-admin/config.toml
@@ -1,6 +1,6 @@
 [dropshot]
 # 1 MiB; we don't expect any requests of more than nominal size.
-request_body_max_bytes = 1048576
+default_request_body_max_bytes = 1048576
 
 [log]
 # Show log messages of this level and more severe

diff --git a/smf/external-dns/config.toml b/smf/external-dns/config.toml
@@ -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 it.
-request_body_max_bytes = 104857600
+default_request_body_max_bytes = 104857600
 
 [log]
 # Show log messages of this level and more severe

diff --git a/smf/internal-dns/config.toml b/smf/internal-dns/config.toml
@@ -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 it.
-request_body_max_bytes = 104857600
+default_request_body_max_bytes = 104857600
 
 [log]
 # Show log messages of this level and more severe

diff --git a/smf/mgs-sim/config.toml b/smf/mgs-sim/config.toml
@@ -11,7 +11,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 = 536870912
 
 [switch]
 # Which interface is connected to our local sidecar SP (i.e., the SP that acts

diff --git a/smf/sled-agent/gimlet-standalone/config.toml b/smf/sled-agent/gimlet-standalone/config.toml
@@ -44,7 +44,7 @@ data_links = ["net0", "net1"]
 [dropshot]
 # Host OS images are just over 800 MiB currently; set this to 2 GiB to give some
 # breathing room.
-request_body_max_bytes = 2_147_483_648
+default_request_body_max_bytes = 2_147_483_648
 
 [log]
 level = "info"

diff --git a/smf/sled-agent/gimlet/config.toml b/smf/sled-agent/gimlet/config.toml
@@ -40,7 +40,7 @@ data_links = ["cxgbe0", "cxgbe1"]
 [dropshot]
 # Host OS images are just over 800 MiB currently; set this to 2 GiB to give some
 # breathing room.
-request_body_max_bytes = 2_147_483_648
+default_request_body_max_bytes = 2_147_483_648
 
 [log]
 level = "info"