From a6ce6d6cd36c490990640d44b027ba0df6c996e5 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 13:33:03 -0800 Subject: [PATCH 1/9] error function cleanup, part 1 --- common/src/api/external/error.rs | 47 +++++++++--------------------- nexus/src/authz/mod.rs | 1 + nexus/src/db/error.rs | 50 ++++++++++++++++++++++---------- 3 files changed, 50 insertions(+), 48 deletions(-) diff --git a/common/src/api/external/error.rs b/common/src/api/external/error.rs index de0a60246b..4507e01277 100644 --- a/common/src/api/external/error.rs +++ b/common/src/api/external/error.rs @@ -74,8 +74,6 @@ pub enum LookupType { ByName(String), /** a specific id was requested */ ById(Uuid), - /** some other lookup type was used */ - Other(String), } impl LookupType { @@ -133,14 +131,6 @@ impl Error { LookupType::ById(*id).into_not_found(type_name) } - /** - * Generates an [`Error::ObjectNotFound`] error for some other kind of - * lookup. - */ - pub fn not_found_other(type_name: ResourceType, message: String) -> Error { - LookupType::Other(message).into_not_found(type_name) - } - /** * Generates an [`Error::InternalError`] error with the specific message * @@ -186,29 +176,20 @@ impl From for HttpError { fn from(error: Error) -> HttpError { match error { Error::ObjectNotFound { type_name: t, lookup_type: lt } => { - if let LookupType::Other(message) = lt { - HttpError::for_client_error( - Some(String::from("ObjectNotFound")), - http::StatusCode::NOT_FOUND, - message, - ) - } else { - /* TODO-cleanup is there a better way to express this? */ - let (lookup_field, lookup_value) = match lt { - LookupType::ByName(name) => ("name", name), - LookupType::ById(id) => ("id", id.to_string()), - LookupType::Other(_) => panic!("unhandled other"), - }; - let message = format!( - "not found: {} with {} \"{}\"", - t, lookup_field, lookup_value - ); - HttpError::for_client_error( - Some(String::from("ObjectNotFound")), - http::StatusCode::NOT_FOUND, - message, - ) - } + /* TODO-cleanup is there a better way to express this? */ + let (lookup_field, lookup_value) = match lt { + LookupType::ByName(name) => ("name", name), + LookupType::ById(id) => ("id", id.to_string()), + }; + let message = format!( + "not found: {} with {} \"{}\"", + t, lookup_field, lookup_value + ); + HttpError::for_client_error( + Some(String::from("ObjectNotFound")), + http::StatusCode::NOT_FOUND, + message, + ) } Error::ObjectAlreadyExists { type_name: t, object_name: n } => { diff --git a/nexus/src/authz/mod.rs b/nexus/src/authz/mod.rs index 5cf56b37f3..3688abe4a3 100644 --- a/nexus/src/authz/mod.rs +++ b/nexus/src/authz/mod.rs @@ -163,6 +163,7 @@ mod actor; mod api_resources; +pub use api_resources::ApiResource; pub use api_resources::Fleet; pub use api_resources::FleetChild; pub use api_resources::Organization; diff --git a/nexus/src/db/error.rs b/nexus/src/db/error.rs index 5a7018637d..7cd9f8999c 100644 --- a/nexus/src/db/error.rs +++ b/nexus/src/db/error.rs @@ -81,14 +81,35 @@ pub fn diesel_pool_result_optional( } /// Converts a Diesel pool error to an external error when operating on a -/// particular resource -pub fn public_error_from_diesel_pool( +/// particular resource, identified by resource type and lookup type +/// NOTE: If you already have an `authz::ApiResource`, you should use +/// [`public_error_from_diesel_pool_authz()`] instead. Eventually, the only +/// uses of this function should be in the DataStore functions that actually +/// look up a record for the first time. +pub fn public_error_from_diesel_pool_lookup( error: PoolError, resource_type: ResourceType, lookup_type: LookupType, ) -> PublicError { - public_error_from_diesel_pool_helper(error, |error| { - public_error_from_diesel(error, resource_type, lookup_type) + public_error_from_diesel_pool_lookup_helper(error, |error| { + public_error_from_diesel(error, || PublicError::ObjectNotFound { + type_name: resource_type, + lookup_type, + }) + }) +} + +/// Converts a Diesel pool error to an external error when operating on a +/// particular resource, identified by ApiResource +pub fn public_error_from_diesel_pool_authz( + error: PoolError, + resource: &R, +) -> PublicError +where + R: crate::authz::ApiResource, +{ + public_error_from_diesel_pool_lookup_helper(error, |error| { + public_error_from_diesel(error, || resource.not_found()) }) } @@ -99,7 +120,7 @@ pub fn public_error_from_diesel_pool_create( resource_type: ResourceType, object_name: &str, ) -> PublicError { - public_error_from_diesel_pool_helper(error, |error| { + public_error_from_diesel_pool_lookup_helper(error, |error| { public_error_from_diesel_create(error, resource_type, object_name) }) } @@ -110,7 +131,7 @@ pub fn public_error_from_diesel_pool_create( pub fn public_error_from_diesel_pool_shouldnt_fail( error: PoolError, ) -> PublicError { - public_error_from_diesel_pool_helper(error, |diesel_error| { + public_error_from_diesel_pool_lookup_helper(error, |diesel_error| { PublicError::internal_error(&format!( "unexpected database error: {:#}", diesel_error @@ -123,7 +144,7 @@ pub fn public_error_from_diesel_pool_shouldnt_fail( /// `PoolError::Connection(ConnectionError::Query(diesel_error))` to /// `make_query_error(diesel_error)`, allowing the caller to decide how to /// format a message for that case. -fn public_error_from_diesel_pool_helper( +fn public_error_from_diesel_pool_lookup_helper( error: PoolError, make_query_error: F, ) -> PublicError @@ -145,16 +166,15 @@ where } /// Converts a Diesel error to an external error. -fn public_error_from_diesel( +fn public_error_from_diesel( error: DieselError, - resource_type: ResourceType, - lookup_type: LookupType, -) -> PublicError { + make_not_found_error: F, +) -> PublicError +where + F: FnOnce() -> PublicError, +{ match error { - DieselError::NotFound => PublicError::ObjectNotFound { - type_name: resource_type, - lookup_type, - }, + DieselError::NotFound => make_not_found_error(), DieselError::DatabaseError(kind, info) => { PublicError::internal_error(&format_database_error(kind, &*info)) } From bec9f6a74ff163a81800c115ca64593b8288ffa3 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 13:36:35 -0800 Subject: [PATCH 2/9] error function cleanup, part 2 --- nexus/src/db/datastore.rs | 214 +++++++++++--------------------------- nexus/src/nexus.rs | 9 +- 2 files changed, 59 insertions(+), 164 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 1ae13a9944..41a2cb152e 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -66,7 +66,9 @@ use uuid::Uuid; use crate::db::{ self, error::{ - public_error_from_diesel_pool, public_error_from_diesel_pool_create, + public_error_from_diesel_pool_authz, + public_error_from_diesel_pool_create, + public_error_from_diesel_pool_lookup, public_error_from_diesel_pool_shouldnt_fail, TransactionError, }, model::{ @@ -168,13 +170,7 @@ impl DataStore { .select(Sled::as_select()) .load_async(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Sled, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn sled_fetch(&self, id: Uuid) -> LookupResult { @@ -185,7 +181,7 @@ impl DataStore { .first_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Sled, LookupType::ById(id), @@ -538,7 +534,7 @@ impl DataStore { .get_result_async::(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Organization, LookupType::ByName(name.as_str().to_owned()), @@ -594,7 +590,7 @@ impl DataStore { .get_result_async::<(Uuid, Generation)>(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Organization, LookupType::ByName(name.as_str().to_owned()), @@ -617,13 +613,7 @@ impl DataStore { .first_async::(self.pool()) .await, ) - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Project, - LookupType::Other("by organization_id".to_string()), - ) - })?; + .map_err(public_error_from_diesel_pool_shouldnt_fail)?; if project_found.is_some() { return Err(Error::InvalidRequest { message: "organization to be deleted contains a project" @@ -640,7 +630,7 @@ impl DataStore { .execute_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Organization, LookupType::ById(id), @@ -668,13 +658,7 @@ impl DataStore { .select(Organization::as_select()) .load_async::(self.pool_authorized(opctx).await?) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Organization, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn organizations_list_by_name( @@ -689,13 +673,7 @@ impl DataStore { .select(Organization::as_select()) .load_async::(self.pool_authorized(opctx).await?) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Organization, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } /// Updates a organization by name (clobbering update -- no etag) @@ -717,13 +695,7 @@ impl DataStore { .returning(Organization::as_returning()) .get_result_async(self.pool_authorized(opctx).await?) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Organization, - LookupType::ByName(name.as_str().to_owned()), - ) - }) + .map_err(|e| public_error_from_diesel_pool_authz(e, &authz_org)) } /// Create a project @@ -780,7 +752,7 @@ impl DataStore { .first_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Project, LookupType::ByName(project_name.as_str().to_owned()), @@ -838,6 +810,7 @@ impl DataStore { name: &Name, ) -> DeleteResult { use db::schema::project::dsl; + let now = Utc::now(); diesel::update(dsl::project) .filter(dsl::time_deleted.is_null()) @@ -848,7 +821,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Project, LookupType::ByName(name.as_str().to_owned()), @@ -913,7 +886,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Project, LookupType::ByName(name.as_str().to_owned()), @@ -998,13 +971,7 @@ impl DataStore { .select(Instance::as_select()) .load_async::(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Instance, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn instance_fetch( @@ -1020,7 +987,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Instance, LookupType::ById(*instance_id), @@ -1043,7 +1010,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Instance, LookupType::ByName(instance_name.as_str().to_owned()), @@ -1080,7 +1047,7 @@ impl DataStore { UpdateStatus::NotUpdatedButExists => false, }) .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Instance, LookupType::ById(*instance_id), @@ -1122,7 +1089,7 @@ impl DataStore { .execute_and_check(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Instance, LookupType::ById(*instance_id), @@ -1161,13 +1128,7 @@ impl DataStore { .select(Disk::as_select()) .load_async::(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Disk, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn project_create_disk(&self, disk: Disk) -> CreateResult { @@ -1217,13 +1178,7 @@ impl DataStore { .select(Disk::as_select()) .load_async::(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Disk, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn disk_update_runtime( @@ -1246,7 +1201,7 @@ impl DataStore { UpdateStatus::NotUpdatedButExists => false, }) .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Disk, LookupType::ById(*disk_id), @@ -1266,7 +1221,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Disk, LookupType::ById(*disk_id), @@ -1289,7 +1244,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Disk, LookupType::ByName(disk_name.as_str().to_owned()), @@ -1346,7 +1301,7 @@ impl DataStore { .execute_and_check(pool) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Disk, LookupType::ById(*disk_id), @@ -1482,7 +1437,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::NetworkInterface, LookupType::ById(*network_interface_id), @@ -1534,7 +1489,7 @@ impl DataStore { .first_async::(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Oximeter, LookupType::ById(id), @@ -1551,13 +1506,7 @@ impl DataStore { paginated(dsl::oximeter, dsl::id, page_params) .load_async::(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Oximeter, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } // Create a record for a new producer endpoint @@ -1677,7 +1626,7 @@ impl DataStore { .execute_and_check(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::SagaDbg, LookupType::ById(saga_id.0.into()), @@ -1717,7 +1666,7 @@ impl DataStore { .load_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::SagaDbg, LookupType::ById(sec_id.0), @@ -1736,7 +1685,7 @@ impl DataStore { .load_async::(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::SagaDbg, LookupType::ById(id.0 .0), @@ -1762,13 +1711,7 @@ impl DataStore { .select(Vpc::as_select()) .load_async(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Vpc, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn project_create_vpc(&self, vpc: Vpc) -> Result { @@ -1806,7 +1749,7 @@ impl DataStore { .execute_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Vpc, LookupType::ById(*vpc_id), @@ -1830,7 +1773,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Vpc, LookupType::ByName(vpc_name.as_str().to_owned()), @@ -1856,7 +1799,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Vpc, LookupType::ById(*vpc_id), @@ -1878,13 +1821,7 @@ impl DataStore { .select(VpcFirewallRule::as_select()) .load_async(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::VpcFirewallRule, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn vpc_delete_all_firewall_rules( @@ -1902,7 +1839,7 @@ impl DataStore { .execute_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Vpc, LookupType::ById(*vpc_id), @@ -1963,7 +1900,7 @@ impl DataStore { TxnError::CustomError( FirewallUpdateError::CollectionNotFound, ) => Error::not_found_by_id(ResourceType::Vpc, vpc_id), - TxnError::Pool(e) => public_error_from_diesel_pool( + TxnError::Pool(e) => public_error_from_diesel_pool_lookup( e, ResourceType::VpcFirewallRule, LookupType::ById(*vpc_id), @@ -1984,14 +1921,9 @@ impl DataStore { .select(VpcSubnet::as_select()) .load_async(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::VpcSubnet, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } + pub async fn vpc_subnet_fetch_by_name( &self, vpc_id: &Uuid, @@ -2007,7 +1939,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::VpcSubnet, LookupType::ByName(subnet_name.as_str().to_owned()), @@ -2051,7 +1983,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::VpcSubnet, LookupType::ById(*subnet_id), @@ -2074,7 +2006,7 @@ impl DataStore { .execute_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::VpcSubnet, LookupType::ById(*subnet_id), @@ -2096,13 +2028,7 @@ impl DataStore { .select(NetworkInterface::as_select()) .load_async::(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::NetworkInterface, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn vpc_list_routers( @@ -2118,13 +2044,7 @@ impl DataStore { .select(VpcRouter::as_select()) .load_async::(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::VpcRouter, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn vpc_router_fetch_by_name( @@ -2142,7 +2062,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::VpcRouter, LookupType::ByName(router_name.as_str().to_owned()), @@ -2186,7 +2106,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::VpcRouter, LookupType::ById(*router_id), @@ -2209,7 +2129,7 @@ impl DataStore { .execute_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::VpcRouter, LookupType::ById(*router_id), @@ -2231,13 +2151,7 @@ impl DataStore { .select(RouterRoute::as_select()) .load_async::(self.pool()) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::RouterRoute, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn router_route_fetch_by_name( @@ -2255,7 +2169,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::RouterRoute, LookupType::ByName(route_name.as_str().to_owned()), @@ -2304,7 +2218,7 @@ impl DataStore { .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::RouterRoute, LookupType::ById(*route_id), @@ -2327,7 +2241,7 @@ impl DataStore { .execute_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::RouterRoute, LookupType::ById(*route_id), @@ -2431,13 +2345,7 @@ impl DataStore { .select(UserBuiltin::as_select()) .load_async::(self.pool_authorized(opctx).await?) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::User, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn user_builtin_fetch( @@ -2461,7 +2369,7 @@ impl DataStore { .first_async::(self.pool_authorized(opctx).await?) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::User, LookupType::ByName(name.as_str().to_owned()), @@ -2527,13 +2435,7 @@ impl DataStore { .select(RoleBuiltin::as_select()) .load_async::(self.pool_authorized(opctx).await?) .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ResourceType::Role, - LookupType::Other("Listing All".to_string()), - ) - }) + .map_err(public_error_from_diesel_pool_shouldnt_fail) } pub async fn role_builtin_fetch( @@ -2563,7 +2465,7 @@ impl DataStore { .first_async::(self.pool_authorized(opctx).await?) .await .map_err(|e| { - public_error_from_diesel_pool( + public_error_from_diesel_pool_lookup( e, ResourceType::Role, LookupType::ByName(String::from(name)), diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 0cf079968c..59cd7d1b60 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -1295,14 +1295,7 @@ impl Nexus { } } - Err(Error::not_found_other( - ResourceType::Disk, - format!( - "disk \"{}\" is not attached to instance \"{}\"", - disk_name.as_str(), - instance_name.as_str() - ), - )) + Err(Error::not_found_by_name(ResourceType::Disk, disk_name)) } /** From 8e21a57a435acd11e1f89bcfe8f6d4bcaf962e9a Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 13:52:16 -0800 Subject: [PATCH 3/9] renamed wrong error handling function --- nexus/src/db/error.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/nexus/src/db/error.rs b/nexus/src/db/error.rs index 7cd9f8999c..6f57da2146 100644 --- a/nexus/src/db/error.rs +++ b/nexus/src/db/error.rs @@ -91,10 +91,12 @@ pub fn public_error_from_diesel_pool_lookup( resource_type: ResourceType, lookup_type: LookupType, ) -> PublicError { - public_error_from_diesel_pool_lookup_helper(error, |error| { - public_error_from_diesel(error, || PublicError::ObjectNotFound { - type_name: resource_type, - lookup_type, + public_error_from_diesel_pool_helper(error, |error| { + public_error_from_diesel_lookup_helper(error, || { + PublicError::ObjectNotFound { + type_name: resource_type, + lookup_type, + } }) }) } @@ -108,8 +110,8 @@ pub fn public_error_from_diesel_pool_authz( where R: crate::authz::ApiResource, { - public_error_from_diesel_pool_lookup_helper(error, |error| { - public_error_from_diesel(error, || resource.not_found()) + public_error_from_diesel_pool_helper(error, |error| { + public_error_from_diesel_lookup_helper(error, || resource.not_found()) }) } @@ -120,7 +122,7 @@ pub fn public_error_from_diesel_pool_create( resource_type: ResourceType, object_name: &str, ) -> PublicError { - public_error_from_diesel_pool_lookup_helper(error, |error| { + public_error_from_diesel_pool_helper(error, |error| { public_error_from_diesel_create(error, resource_type, object_name) }) } @@ -131,7 +133,7 @@ pub fn public_error_from_diesel_pool_create( pub fn public_error_from_diesel_pool_shouldnt_fail( error: PoolError, ) -> PublicError { - public_error_from_diesel_pool_lookup_helper(error, |diesel_error| { + public_error_from_diesel_pool_helper(error, |diesel_error| { PublicError::internal_error(&format!( "unexpected database error: {:#}", diesel_error @@ -144,7 +146,7 @@ pub fn public_error_from_diesel_pool_shouldnt_fail( /// `PoolError::Connection(ConnectionError::Query(diesel_error))` to /// `make_query_error(diesel_error)`, allowing the caller to decide how to /// format a message for that case. -fn public_error_from_diesel_pool_lookup_helper( +fn public_error_from_diesel_pool_helper( error: PoolError, make_query_error: F, ) -> PublicError @@ -165,8 +167,9 @@ where } } -/// Converts a Diesel error to an external error. -fn public_error_from_diesel( +/// Converts a Diesel error to an external error, handling "NotFound" using +/// `make_not_found_error`. +fn public_error_from_diesel_lookup_helper( error: DieselError, make_not_found_error: F, ) -> PublicError From 81b7c459b8cc1cb4c3bbc28f223ad153ffc7f54b Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 13:33:16 -0800 Subject: [PATCH 4/9] authz: protect project delete --- nexus/src/db/datastore.rs | 14 +++----- nexus/src/external_api/http_entrypoints.rs | 3 +- nexus/src/nexus.rs | 10 +++--- nexus/tests/integration_tests/basic.rs | 36 +++++++++---------- .../tests/integration_tests/organizations.rs | 8 +++-- 5 files changed, 34 insertions(+), 37 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 41a2cb152e..f9c2372431 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -806,26 +806,22 @@ impl DataStore { */ pub async fn project_delete( &self, - organization_id: &Uuid, - name: &Name, + opctx: &OpContext, + authz_project: &authz::Project, ) -> DeleteResult { use db::schema::project::dsl; + opctx.authorize(authz::Action::Delete, authz_project).await?; let now = Utc::now(); diesel::update(dsl::project) .filter(dsl::time_deleted.is_null()) - .filter(dsl::organization_id.eq(*organization_id)) - .filter(dsl::name.eq(name.clone())) + .filter(dsl::id.eq(authz_project.id())) .set(dsl::time_deleted.eq(now)) .returning(Project::as_returning()) .get_result_async(self.pool()) .await .map_err(|e| { - public_error_from_diesel_pool_lookup( - e, - ResourceType::Project, - LookupType::ByName(name.as_str().to_owned()), - ) + public_error_from_diesel_pool_authz(e, authz_project) })?; Ok(()) } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 40b9594d0f..ba3e33e3a1 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -506,7 +506,8 @@ async fn organization_projects_delete_project( let organization_name = ¶ms.organization_name; let project_name = ¶ms.project_name; let handler = async { - nexus.project_delete(&organization_name, &project_name).await?; + let opctx = OpContext::for_external_api(&rqctx).await?; + nexus.project_delete(&opctx, &organization_name, &project_name).await?; Ok(HttpResponseDeleted()) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 59cd7d1b60..2465e420fe 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -632,15 +632,15 @@ impl Nexus { pub async fn project_delete( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, ) -> DeleteResult { - let organization_id = self + let authz_project = self .db_datastore - .organization_lookup_path(organization_name) - .await? - .id(); - self.db_datastore.project_delete(&organization_id, project_name).await + .project_lookup_path(organization_name, project_name) + .await?; + self.db_datastore.project_delete(opctx, &authz_project).await } pub async fn project_update( diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index b8b7de0995..41a047e5e2 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -312,33 +312,31 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) { * Delete "simproject2". We'll make sure that's reflected in the other * requests. */ - client - .make_request_no_body( - Method::DELETE, - "/organizations/test-org/projects/simproject2", - StatusCode::NO_CONTENT, - ) - .await - .expect("expected success"); + NexusRequest::object_delete( + client, + "/organizations/test-org/projects/simproject2", + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("expected request to fail"); /* * Having deleted "simproject2", verify "GET", "PUT", and "DELETE" on * "/organizations/test-org/projects/simproject2". */ - client - .make_request_error( - Method::GET, - "/organizations/test-org/projects/simproject2", + for method in [Method::GET, Method::DELETE] { + NexusRequest::expect_failure( + client, StatusCode::NOT_FOUND, - ) - .await; - client - .make_request_error( - Method::DELETE, + method, "/organizations/test-org/projects/simproject2", - StatusCode::NOT_FOUND, ) - .await; + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request"); + } client .make_request_error_body( Method::PUT, diff --git a/nexus/tests/integration_tests/organizations.rs b/nexus/tests/integration_tests/organizations.rs index 3cb26103aa..aab6f3187b 100644 --- a/nexus/tests/integration_tests/organizations.rs +++ b/nexus/tests/integration_tests/organizations.rs @@ -5,8 +5,6 @@ use nexus_test_utils::http_testing::{AuthnMode, NexusRequest}; use omicron_nexus::external_api::views::Organization; -use dropshot::test_util::object_delete; - use http::method::Method; use http::StatusCode; use nexus_test_utils::resource_helpers::{ @@ -168,7 +166,11 @@ async fn test_organizations(cptestctx: &ControlPlaneTestContext) { .expect("failed to make request"); // Delete the project, then delete the organization - object_delete(&client, &project_url).await; + NexusRequest::object_delete(&client, &project_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request"); NexusRequest::object_delete(&client, &o2_url) .authn_as(AuthnMode::PrivilegedUser) .execute() From 6befd2e8c7470cdc3d801e83770ec435d80420be Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 14:12:37 -0800 Subject: [PATCH 5/9] authz: protect project update --- nexus/src/db/datastore.rs | 27 ++++++++-------------- nexus/src/external_api/http_entrypoints.rs | 2 ++ nexus/src/nexus.rs | 14 ++++------- nexus/tests/integration_tests/basic.rs | 25 ++++++++++++-------- nexus/tests/integration_tests/projects.rs | 26 +++++++++++++++++++++ 5 files changed, 58 insertions(+), 36 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index f9c2372431..05ca9aaae2 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -809,16 +809,16 @@ impl DataStore { opctx: &OpContext, authz_project: &authz::Project, ) -> DeleteResult { - use db::schema::project::dsl; - opctx.authorize(authz::Action::Delete, authz_project).await?; + + use db::schema::project::dsl; let now = Utc::now(); diesel::update(dsl::project) .filter(dsl::time_deleted.is_null()) .filter(dsl::id.eq(authz_project.id())) .set(dsl::time_deleted.eq(now)) .returning(Project::as_returning()) - .get_result_async(self.pool()) + .get_result_async(self.pool_authorized(opctx).await?) .await .map_err(|e| { public_error_from_diesel_pool_authz(e, authz_project) @@ -864,30 +864,23 @@ impl DataStore { .map_err(public_error_from_diesel_pool_shouldnt_fail) } - /// Updates a project by name (clobbering update -- no etag) + /// Updates a project (clobbering update -- no etag) pub async fn project_update( &self, - organization_id: &Uuid, - name: &Name, + authz_project: &authz::Project, updates: ProjectUpdate, ) -> UpdateResult { - use db::schema::project::dsl; + opctx.authorize(authz::Action::Modify, authz_project).await?; + use db::schema::project::dsl; diesel::update(dsl::project) .filter(dsl::time_deleted.is_null()) - .filter(dsl::organization_id.eq(*organization_id)) - .filter(dsl::name.eq(name.clone())) + .filter(dsl::id.eq(authz_project.id())) .set(updates) .returning(Project::as_returning()) - .get_result_async(self.pool()) + .get_result_async(self.pool_authorized(opctx).await?) .await - .map_err(|e| { - public_error_from_diesel_pool_lookup( - e, - ResourceType::Project, - LookupType::ByName(name.as_str().to_owned()), - ) - }) + .map_err(|e| public_error_from_diesel_pool_authz(e, authz_project)) } /* diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index ba3e33e3a1..46c5915cfe 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -539,8 +539,10 @@ async fn organization_projects_put_project( let organization_name = &path.organization_name; let project_name = &path.project_name; let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; let newproject = nexus .project_update( + &opctx, &organization_name, &project_name, &updated_project.into_inner(), diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 2465e420fe..35d5b35c48 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -645,21 +645,17 @@ impl Nexus { pub async fn project_update( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, new_params: ¶ms::ProjectUpdate, ) -> UpdateResult { - let organization_id = self + let authz_project = self .db_datastore - .organization_lookup_path(organization_name) - .await? - .id(); + .project_lookup_path(organization_name, project_name) + .await?; self.db_datastore - .project_update( - &organization_id, - project_name, - new_params.clone().into(), - ) + .project_update(&authz_project, new_params.clone().into()) .await } diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index 41a047e5e2..b626fdc90a 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -337,19 +337,24 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) { .await .expect("failed to make request"); } - client - .make_request_error_body( + NexusRequest::new( + RequestBuilder::new( + client, Method::PUT, "/organizations/test-org/projects/simproject2", - params::ProjectUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: None, - }, - }, - StatusCode::NOT_FOUND, ) - .await; + .body(Some(params::ProjectUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: None, + }, + })) + .expect_status(Some(StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request"); /* * Similarly, verify "GET /organizations/test-org/projects" diff --git a/nexus/tests/integration_tests/projects.rs b/nexus/tests/integration_tests/projects.rs index 8a2392e578..ba6a4048a6 100644 --- a/nexus/tests/integration_tests/projects.rs +++ b/nexus/tests/integration_tests/projects.rs @@ -94,4 +94,30 @@ async fn test_projects(cptestctx: &ControlPlaneTestContext) { .all_items; assert_eq!(projects.len(), 1); assert_eq!(projects[0].identity.name, p1_name); + + // Unauthenticated and unauthorized users should not be able to delete or + // modify a Project. + NexusRequest::expect_failure( + &client, + http::StatusCode::NOT_FOUND, + http::Method::DELETE, + &p1_url, + ) + .execute() + .await + .expect("failed to make request"); + NexusRequest::new( + RequestBuilder::new(&client, http::Method::PUT, &projects_url) + .body(Some(params::ProjectUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: None, + }, + })) + .expect_status(Some(http::StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("failed to make request"); } From ebff5725a1e1da2385afde2b6228b33cc73c29d1 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 14:38:02 -0800 Subject: [PATCH 6/9] fixes, update tests --- nexus/src/db/datastore.rs | 1 + nexus/src/nexus.rs | 2 +- nexus/test-utils/src/http_testing.rs | 16 +++++++ nexus/tests/integration_tests/basic.rs | 58 ++++++++++++++++++----- nexus/tests/integration_tests/projects.rs | 7 ++- 5 files changed, 70 insertions(+), 14 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 05ca9aaae2..18d67a86c7 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -867,6 +867,7 @@ impl DataStore { /// Updates a project (clobbering update -- no etag) pub async fn project_update( &self, + opctx: &OpContext, authz_project: &authz::Project, updates: ProjectUpdate, ) -> UpdateResult { diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 35d5b35c48..ff35230165 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -655,7 +655,7 @@ impl Nexus { .project_lookup_path(organization_name, project_name) .await?; self.db_datastore - .project_update(&authz_project, new_params.clone().into()) + .project_update(opctx, &authz_project, new_params.clone().into()) .await } diff --git a/nexus/test-utils/src/http_testing.rs b/nexus/test-utils/src/http_testing.rs index 9a78153916..6bcd66ebfe 100644 --- a/nexus/test-utils/src/http_testing.rs +++ b/nexus/test-utils/src/http_testing.rs @@ -479,6 +479,22 @@ impl<'a> NexusRequest<'a> { ) } + /// Returns a new `NexusRequest` suitable for `PUT $uri` + pub fn object_put( + testctx: &'a ClientTestContext, + uri: &str, + body: Option<&B>, + ) -> Self + where + B: serde::Serialize, + { + NexusRequest::new( + RequestBuilder::new(testctx, http::Method::PUT, uri) + .body(body) + .expect_status(Some(http::StatusCode::OK)), + ) + } + /// Convenience constructor for failure cases pub fn expect_failure( testctx: &'a ClientTestContext, diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index b626fdc90a..9ee395509e 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -343,7 +343,7 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) { Method::PUT, "/organizations/test-org/projects/simproject2", ) - .body(Some(params::ProjectUpdate { + .body(Some(¶ms::ProjectUpdate { identity: IdentityMetadataUpdateParams { name: None, description: None, @@ -386,25 +386,61 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) { ); /* - * Update "simproject3". We'll make sure that's reflected in the other - * requests. + * Unprivileged users should not be able to update a Project. */ let project_update = params::ProjectUpdate { identity: IdentityMetadataUpdateParams { name: None, - description: Some("Li'l lightnin'".to_string()), + description: None, }, }; - let mut response = client - .make_request( + NexusRequest::new( + RequestBuilder::new( + client, Method::PUT, "/organizations/test-org/projects/simproject3", - Some(project_update), - StatusCode::OK, ) - .await - .expect("expected success"); - let project: Project = read_json(&mut response).await; + .body(Some(&project_update)) + .expect_status(Some(StatusCode::NOT_FOUND)), + ) + .execute() + .await + .expect("failed to make request"); + NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + "/organizations/test-org/projects/simproject3", + ) + .body(Some(&project_update)) + .expect_status(Some(StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("failed to make request"); + + /* + * Update "simproject3". We'll make sure that's reflected in the other + * requests. + */ + let project_update = params::ProjectUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("Li'l lightnin'".to_string()), + }, + }; + let project = NexusRequest::object_put( + client, + "/organizations/test-org/projects/simproject3", + Some(&project_update), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("expected success") + .parsed_body::() + .expect("failed to parse Project from PUT response"); assert_eq!(project.identity.id, new_project_ids[2]); assert_eq!(project.identity.name, "simproject3"); assert_eq!(project.identity.description, "Li'l lightnin'"); diff --git a/nexus/tests/integration_tests/projects.rs b/nexus/tests/integration_tests/projects.rs index ba6a4048a6..6920f25942 100644 --- a/nexus/tests/integration_tests/projects.rs +++ b/nexus/tests/integration_tests/projects.rs @@ -4,7 +4,10 @@ use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; +use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::resource_helpers::project_get; +use omicron_common::api::external::IdentityMetadataUpdateParams; +use omicron_nexus::external_api::params; use omicron_nexus::external_api::views::Project; use nexus_test_utils::resource_helpers::{create_organization, create_project}; @@ -107,8 +110,8 @@ async fn test_projects(cptestctx: &ControlPlaneTestContext) { .await .expect("failed to make request"); NexusRequest::new( - RequestBuilder::new(&client, http::Method::PUT, &projects_url) - .body(Some(params::ProjectUpdate { + RequestBuilder::new(&client, http::Method::PUT, &p1_url) + .body(Some(¶ms::ProjectUpdate { identity: IdentityMetadataUpdateParams { name: None, description: None, From 4c65796d19d08169cabfb72610c6b8e8a51120cb Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 14:48:29 -0800 Subject: [PATCH 7/9] fix tests --- nexus/tests/integration_tests/basic.rs | 38 ++++++++++++++------------ 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index 9ee395509e..22ad43c99f 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -464,27 +464,31 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) { description: Some("little lightning".to_string()), }, }; - let mut response = client - .make_request( - Method::PUT, - "/organizations/test-org/projects/simproject3", - Some(project_update), - StatusCode::OK, - ) - .await - .expect("failed to make request to server"); - let project: Project = read_json(&mut response).await; + let project = NexusRequest::object_put( + client, + "/organizations/test-org/projects/simproject3", + Some(&project_update), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("expected success") + .parsed_body::() + .expect("failed to parse Project from PUT response"); assert_eq!(project.identity.id, new_project_ids[2]); assert_eq!(project.identity.name, "lil-lightnin"); assert_eq!(project.identity.description, "little lightning"); - client - .make_request_error( - Method::GET, - "/organizations/test-org/projects/simproject3", - StatusCode::NOT_FOUND, - ) - .await; + NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + Method::GET, + "/organizations/test-org/projects/simproject3", + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("expected success"); /* * Try to create a project with a name that conflicts with an existing one. From e191e6cc7352ff1ca984436db3d3dae04243f4eb Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 14:52:12 -0800 Subject: [PATCH 8/9] finish test --- nexus/tests/integration_tests/projects.rs | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/nexus/tests/integration_tests/projects.rs b/nexus/tests/integration_tests/projects.rs index 6920f25942..fc71598039 100644 --- a/nexus/tests/integration_tests/projects.rs +++ b/nexus/tests/integration_tests/projects.rs @@ -109,6 +109,30 @@ async fn test_projects(cptestctx: &ControlPlaneTestContext) { .execute() .await .expect("failed to make request"); + NexusRequest::expect_failure( + &client, + http::StatusCode::NOT_FOUND, + http::Method::DELETE, + &p1_url, + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("failed to make request"); + + NexusRequest::new( + RequestBuilder::new(&client, http::Method::PUT, &p1_url) + .body(Some(¶ms::ProjectUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: None, + }, + })) + .expect_status(Some(http::StatusCode::NOT_FOUND)), + ) + .execute() + .await + .expect("failed to make request"); NexusRequest::new( RequestBuilder::new(&client, http::Method::PUT, &p1_url) .body(Some(¶ms::ProjectUpdate { From e07d05f0565ec97b2986206bf52e7777874fc1d3 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 26 Jan 2022 16:02:23 -0800 Subject: [PATCH 9/9] fix nit --- nexus/tests/integration_tests/basic.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index 22ad43c99f..55648c53f5 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -10,7 +10,6 @@ */ use dropshot::test_util::objects_list_page; -use dropshot::test_util::read_json; use dropshot::test_util::ClientTestContext; use dropshot::HttpErrorResponseBody; use http::method::Method;