Bump dropshot from b562f4f to fef3a0e

common/src/api/external/error.rs

@@ -255,17 +255,65 @@ impl From<Error> for HttpError {
     }
 }
 
-impl<T: std::fmt::Debug> From<progenitor::progenitor_client::Error<T>>
-    for crate::api::external::Error
-{
+pub trait ClientError: std::fmt::Debug {
+    fn message(&self) -> String;
+}
+
+// TODO-security this `From` may give us a shortcut in situtations where we
+// want more robust consideration of errors. For example, while some errors
+// from other services may directly result in errors that percolate to the
+// external client, others may require, for example, retries with an alternate
+// service instance or additional interpretation to sanitize the output error.
+// This should be removed to avoid leaking data.
+impl<T: ClientError> From<progenitor::progenitor_client::Error<T>> for Error {
     fn from(e: progenitor::progenitor_client::Error<T>) -> Self {
-        match e.status() {
-            Some(status_code) if status_code.is_client_error() => {
-                crate::api::external::Error::InvalidRequest {
-                    message: e.to_string(),
+        match e {
+            // This error indicates a problem with the request to the remote
+            // service that did not result in an HTTP response code, but rather
+            // pertained to local (i.e. client-side) encoding or network
+            // communication.
+            progenitor::progenitor_client::Error::CommunicationError(ee) => {
+                Error::internal_error(&format!("CommunicationError: {}", ee))
+            }
+
+            // This error represents an expected error from the remote service.
+            progenitor::progenitor_client::Error::ErrorResponse(rv) => {
+                let message = rv.message();
+
+                match rv.status() {
+                    http::StatusCode::SERVICE_UNAVAILABLE => {
+                        Error::unavail(&message)
+                    }
+                    status if status.is_client_error() => {
+                        Error::invalid_request(&message)
+                    }
+                    _ => Error::internal_error(&message),
                 }
             }
-            _ => crate::api::external::Error::internal_error(&e.to_string()),
+
+            // This error indicates that the body returned by the client didn't
+            // match what was documented in the OpenAPI description for the
+            // service. This could only happen for us in the case of a severe
+            // logic/encoding bug in the remote service or due to a failure of
+            // our version constraints (i.e. that the call was to a newer
+            // service with an incompatible response).
+            progenitor::progenitor_client::Error::InvalidResponsePayload(
+                ee,
+            ) => Error::internal_error(&format!(
+                "InvalidResponsePayload: {}",
+                ee,
+            )),
+
+            // This error indicates that the client generated a response code
+            // that was not described in the OpenAPI description for the
+            // service; this could be a success or failure response, but either
+            // way it indicates a logic or version error as above.
+            progenitor::progenitor_client::Error::UnexpectedResponse(r) => {
+                Error::internal_error(&format!(
+                    "UnexpectedResponse: status code {}",
+                    r.status(),
+                ))
+            }
         }
     }
 }

nexus-client/src/lib.rs

@@ -13,6 +13,12 @@ use omicron_common::generate_logging_api;
 
 generate_logging_api!("../openapi/nexus-internal.json");
 
+impl omicron_common::api::external::ClientError for types::Error {
+    fn message(&self) -> String {
+        self.message.clone()
+    }
+}
+
 impl From<types::Generation> for omicron_common::api::external::Generation {
     fn from(s: types::Generation) -> Self {
         Self::try_from(s.0 as i64).unwrap()

nexus/Cargo.toml

@@ -13,7 +13,7 @@ async-bb8-diesel = { git = "https://github.com/oxidecomputer/async-bb8-diesel",
 async-trait = "0.1.51"
 bb8 = "0.7.1"
 cookie = "0.16"
-crucible-agent-client = { git = "https://github.com/oxidecomputer/crucible", rev = "79e30b132f398351213d929402173d37cdc60b81" }
+crucible-agent-client = { git = "https://github.com/oxidecomputer/crucible", rev = "3e7e49eeb88fa8ad74375b0642aabd4224b1f2cb" }
 # Tracking pending 2.0 version.
 diesel = { git = "https://github.com/diesel-rs/diesel", rev = "ce77c382", features = ["postgres", "r2d2", "chrono", "serde_json", "network-address", "uuid"] }
 futures = "0.3.21"

nexus/src/sagas.rs

@@ -732,7 +732,22 @@ async fn delete_regions(
             let url = format!("http://{}", dataset.address());
             let client = CrucibleAgentClient::new(&url);
             let id = RegionId(region.id().to_string());
-            client.region_delete(&id).await
+            client.region_delete(&id).await.map_err(|e| match e {
+                crucible_agent_client::Error::ErrorResponse(rv) => {
+                    match rv.status() {
+                        http::StatusCode::SERVICE_UNAVAILABLE => {
+                            Error::unavail(&rv.message)
+                        }
+                        status if status.is_client_error() => {
+                            Error::invalid_request(&rv.message)
+                        }
+                        _ => Error::internal_error(&rv.message),
+                    }
+                }
+                _ => Error::internal_error(
+                    "unexpected failure during `region_delete`",
+                ),
+            })
         })
         // Execute the allocation requests concurrently.
         .buffer_unordered(std::cmp::min(

nexus/test-utils/src/lib.rs

@@ -238,7 +238,7 @@ impl oximeter::Producer for IntegrationProducer {
         &mut self,
     ) -> Result<
         Box<(dyn Iterator<Item = oximeter::types::Sample> + 'static)>,
-        oximeter::Error,
+        oximeter::MetricsError,
     > {
         use oximeter::Metric;
         let sample = oximeter::types::Sample::new(&self.target, &self.metric);

openapi/bootstrap-agent.json

@@ -33,13 +33,50 @@
                 }
               }
             }
+          },
+          "4XX": {
+            "$ref": "#/components/responses/Error"
+          },
+          "5XX": {
+            "$ref": "#/components/responses/Error"
           }
         }
       }
     }
   },
   "components": {
+    "responses": {
+      "Error": {
+        "description": "Error",
+        "content": {
+          "application/json": {
+            "schema": {
+              "$ref": "#/components/schemas/Error"
+            }
+          }
+        }
+      }
+    },
     "schemas": {
+      "Error": {
+        "description": "Error information from a response.",
+        "type": "object",
+        "properties": {
+          "error_code": {
+            "type": "string"
+          },
+          "message": {
+            "type": "string"
+          },
+          "request_id": {
+            "type": "string"
+          }
+        },
+        "required": [
+          "message",
+          "request_id"
+        ]
+      },
       "ShareRequest": {
         "description": "Identity signed by local RoT and Oxide certificate chain.",
         "type": "object",