From afbda07b89a4fbd93aa6f08a584212238a5be740 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 9 Dec 2022 15:17:10 -0500 Subject: [PATCH 1/2] Update instance lookup to return lookups instead of authz refs --- nexus/src/app/instance.rs | 115 ++++----- nexus/src/external_api/http_entrypoints.rs | 272 ++++++++------------- 2 files changed, 147 insertions(+), 240 deletions(-) diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 23fbdebb3f3..2911839ef89 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -15,6 +15,7 @@ use crate::cidata::InstanceCiData; use crate::context::OpContext; use crate::db; use crate::db::identity::Resource; +use crate::db::lookup; use crate::db::lookup::LookupPath; use crate::db::queries::network_interface; use crate::external_api::params; @@ -36,6 +37,7 @@ use omicron_common::api::external::NameOrId; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::Vni; use omicron_common::api::internal::nexus; +use ref_cast::RefCast; use sled_agent_client::types::InstanceRuntimeStateMigrateParams; use sled_agent_client::types::InstanceRuntimeStateRequested; use sled_agent_client::types::InstanceStateRequested; @@ -54,20 +56,17 @@ use uuid::Uuid; const MAX_KEYS_PER_INSTANCE: u32 = 8; impl super::Nexus { - pub async fn instance_lookup( - &self, - opctx: &OpContext, - instance_selector: params::InstanceSelector, - ) -> LookupResult { + pub fn instance_lookup<'a>( + &'a self, + opctx: &'a OpContext, + instance_selector: &'a params::InstanceSelector, + ) -> LookupResult> { match instance_selector { params::InstanceSelector { instance: NameOrId::Id(id), .. } => { // TODO: 400 if project or organization are present - let (.., authz_instance) = - LookupPath::new(opctx, &self.db_datastore) - .instance_id(id) - .lookup_for(authz::Action::Read) - .await?; - Ok(authz_instance) + let instance = + LookupPath::new(opctx, &self.db_datastore).instance_id(*id); + Ok(instance) } params::InstanceSelector { instance: NameOrId::Name(instance_name), @@ -75,41 +74,32 @@ impl super::Nexus { .. } => { // TODO: 400 if organization is present - let (.., authz_instance) = - LookupPath::new(opctx, &self.db_datastore) - .project_id(project_id) - .instance_name(&Name(instance_name)) - .lookup_for(authz::Action::Read) - .await?; - Ok(authz_instance) + let instance = LookupPath::new(opctx, &self.db_datastore) + .project_id(*project_id) + .instance_name(Name::ref_cast(instance_name)); + Ok(instance) } params::InstanceSelector { instance: NameOrId::Name(instance_name), project: Some(NameOrId::Name(project_name)), organization: Some(NameOrId::Id(organization_id)), } => { - let (.., authz_instance) = - LookupPath::new(opctx, &self.db_datastore) - .organization_id(organization_id) - .project_name(&Name(project_name)) - .instance_name(&Name(instance_name.clone())) - .lookup_for(authz::Action::Read) - .await?; - Ok(authz_instance) + let instance = LookupPath::new(opctx, &self.db_datastore) + .organization_id(*organization_id) + .project_name(Name::ref_cast(project_name)) + .instance_name(Name::ref_cast(instance_name)); + Ok(instance) } params::InstanceSelector { instance: NameOrId::Name(instance_name), project: Some(NameOrId::Name(project_name)), organization: Some(NameOrId::Name(organization_name)), } => { - let (.., authz_instance) = - LookupPath::new(opctx, &self.db_datastore) - .organization_name(&Name(organization_name)) - .project_name(&Name(project_name)) - .instance_name(&Name(instance_name.clone())) - .lookup_for(authz::Action::Read) - .await?; - Ok(authz_instance) + let instance = LookupPath::new(opctx, &self.db_datastore) + .organization_name(Name::ref_cast(organization_name)) + .project_name(Name::ref_cast(project_name)) + .instance_name(Name::ref_cast(instance_name)); + Ok(instance) } // TODO: Add a better error message _ => Err(Error::InvalidRequest { @@ -269,30 +259,19 @@ impl super::Nexus { .await } - pub async fn instance_fetch( - &self, - opctx: &OpContext, - authz_instance: &authz::Instance, - ) -> LookupResult { - let (.., db_instance) = LookupPath::new(opctx, &self.db_datastore) - .instance_id(authz_instance.id()) - .fetch() - .await?; - Ok(db_instance) - } - // This operation may only occur on stopped instances, which implies that // the attached disks do not have any running "upstairs" process running // within the sled. pub async fn project_destroy_instance( &self, opctx: &OpContext, - authz_instance: &authz::Instance, + instance_lookup: &lookup::Instance<'_>, ) -> DeleteResult { // TODO-robustness We need to figure out what to do with Destroyed // instances? Presumably we need to clean them up at some point, but // not right away so that callers can see that they've been destroyed. - opctx.authorize(authz::Action::Delete, authz_instance).await?; + let (.., authz_instance) = + instance_lookup.lookup_for(authz::Action::Delete).await?; self.db_datastore .project_delete_instance(opctx, &authz_instance) @@ -310,10 +289,11 @@ impl super::Nexus { pub async fn project_instance_migrate( self: &Arc, opctx: &OpContext, - authz_instance: &authz::Instance, + instance_lookup: &lookup::Instance<'_>, params: params::InstanceMigrate, ) -> UpdateResult { - opctx.authorize(authz::Action::Modify, authz_instance).await?; + let (.., authz_instance) = + instance_lookup.lookup_for(authz::Action::Modify).await?; // Kick off the migration saga let saga_params = sagas::instance_migrate::Params { @@ -367,7 +347,7 @@ impl super::Nexus { pub async fn instance_reboot( &self, opctx: &OpContext, - authz_instance: &authz::Instance, + instance_lookup: &lookup::Instance<'_>, ) -> UpdateResult { // To implement reboot, we issue a call to the sled agent to set a // runtime state of "reboot". We cannot simply stop the Instance and @@ -380,11 +360,7 @@ impl super::Nexus { // even if the whole rack powered off while this was going on, we would // never lose track of the fact that this Instance was supposed to be // running. - let (.., authz_instance, db_instance) = - LookupPath::new(opctx, &self.db_datastore) - .instance_id(authz_instance.id()) - .fetch() - .await?; + let (.., authz_instance, db_instance) = instance_lookup.fetch().await?; let requested = InstanceRuntimeStateRequested { run_state: InstanceStateRequested::Reboot, migration_params: None, @@ -403,13 +379,9 @@ impl super::Nexus { pub async fn instance_start( &self, opctx: &OpContext, - authz_instance: &authz::Instance, + instance_lookup: &lookup::Instance<'_>, ) -> UpdateResult { - let (.., authz_instance, db_instance) = - LookupPath::new(opctx, &self.db_datastore) - .instance_id(authz_instance.id()) - .fetch() - .await?; + let (.., authz_instance, db_instance) = instance_lookup.fetch().await?; let requested = InstanceRuntimeStateRequested { run_state: InstanceStateRequested::Running, migration_params: None, @@ -428,13 +400,9 @@ impl super::Nexus { pub async fn instance_stop( &self, opctx: &OpContext, - authz_instance: &authz::Instance, + instance_lookup: &lookup::Instance<'_>, ) -> UpdateResult { - let (.., authz_instance, db_instance) = - LookupPath::new(opctx, &self.db_datastore) - .instance_id(authz_instance.id()) - .fetch() - .await?; + let (.., authz_instance, db_instance) = instance_lookup.fetch().await?; let requested = InstanceRuntimeStateRequested { run_state: InstanceStateRequested::Stopped, migration_params: None, @@ -1126,11 +1094,10 @@ impl super::Nexus { /// provided they are still in the sled-agent's cache. pub(crate) async fn instance_serial_console_data( &self, - opctx: &OpContext, - authz_instance: &authz::Instance, + instance_lookup: &lookup::Instance<'_>, params: ¶ms::InstanceSerialConsoleRequest, ) -> Result { - let db_instance = self.instance_fetch(opctx, authz_instance).await?; + let (.., db_instance) = instance_lookup.fetch().await?; let sa = self.instance_sled(&db_instance).await?; let data = sa @@ -1152,11 +1119,11 @@ impl super::Nexus { pub(crate) async fn instance_serial_console_stream( &self, - opctx: &OpContext, conn: dropshot::WebsocketConnection, - authz_instance: &authz::Instance, + instance_lookup: &lookup::Instance<'_>, ) -> Result<(), Error> { - let instance = self.instance_fetch(opctx, authz_instance).await?; + // TODO: Technically the stream is two way so the user of this method can modify the instance in some way. Should we use different permissions? + let (.., instance) = instance_lookup.fetch().await?; let ip_addr = instance .runtime_state .propolis_ip diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 8da76a36770..62898fcbf50 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -2214,13 +2214,11 @@ async fn instance_view_v1( let query = query_params.into_inner(); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector::new(path.instance, &query.selector), - ) - .await?; - let instance = nexus.instance_fetch(&opctx, &authz_instance).await?; + let instance_selector = + params::InstanceSelector::new(path.instance, &query.selector); + let instance_selector = + nexus.instance_lookup(&opctx, &instance_selector)?; + let (.., instance) = instance_selector.fetch().await?; Ok(HttpResponseOk(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2250,21 +2248,16 @@ async fn instance_view( let organization_name = &path.organization_name; let project_name = &path.project_name; let instance_name = &path.instance_name; + let instance_selector = params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector { - instance: NameOrId::Name(instance_name.clone().into()), - project: Some(NameOrId::Name(project_name.clone().into())), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; - let instance = nexus.instance_fetch(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let (.., instance) = instance_lookup.fetch().await?; Ok(HttpResponseOk(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2286,17 +2279,17 @@ async fn instance_view_by_id( let id = &path.id; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus + let (.., instance) = nexus .instance_lookup( &opctx, - params::InstanceSelector { + ¶ms::InstanceSelector { instance: NameOrId::Id(*id), project: None, organization: None, }, - ) + )? + .fetch() .await?; - let instance = nexus.instance_fetch(&opctx, &authz_instance).await?; Ok(HttpResponseOk(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2316,15 +2309,13 @@ async fn instance_delete_v1( let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); + let instance_selector = + params::InstanceSelector::new(path.instance, &query.selector); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector::new(path.instance, &query.selector), - ) - .await?; - nexus.project_destroy_instance(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + nexus.project_destroy_instance(&opctx, &instance_lookup).await?; Ok(HttpResponseDeleted()) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2346,21 +2337,16 @@ async fn instance_delete( let organization_name = &path.organization_name; let project_name = &path.project_name; let instance_name = &path.instance_name; + let instance_selector = params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector { - instance: NameOrId::Name(instance_name.clone().into()), - project: Some(NameOrId::Name(project_name.clone().into())), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; - nexus.project_destroy_instance(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + nexus.project_destroy_instance(&opctx, &instance_lookup).await?; Ok(HttpResponseDeleted()) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2383,18 +2369,16 @@ async fn instance_migrate_v1( let path = path_params.into_inner(); let query = query_params.into_inner(); let migrate_instance_params = migrate_params.into_inner(); + let instance_selector = + params::InstanceSelector::new(path.instance, &query.selector); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector::new(path.instance, &query.selector), - ) - .await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; let instance = nexus .project_instance_migrate( &opctx, - &authz_instance, + &instance_lookup, migrate_instance_params, ) .await?; @@ -2422,24 +2406,19 @@ async fn instance_migrate( let project_name = &path.project_name; let instance_name = &path.instance_name; let migrate_instance_params = migrate_params.into_inner(); + let instance_selector = params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector { - instance: NameOrId::Name(instance_name.clone().into()), - project: Some(NameOrId::Name(project_name.clone().into())), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; let instance = nexus .project_instance_migrate( &opctx, - &authz_instance, + &instance_lookup, migrate_instance_params, ) .await?; @@ -2462,15 +2441,13 @@ async fn instance_reboot_v1( let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); + let instance_selector = + params::InstanceSelector::new(path.instance, &query.selector); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector::new(path.instance, &query.selector), - ) - .await?; - let instance = nexus.instance_reboot(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let instance = nexus.instance_reboot(&opctx, &instance_lookup).await?; Ok(HttpResponseOk(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2492,21 +2469,16 @@ async fn instance_reboot( let organization_name = &path.organization_name; let project_name = &path.project_name; let instance_name = &path.instance_name; + let instance_selector = params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector { - instance: NameOrId::Name(instance_name.clone().into()), - project: Some(NameOrId::Name(project_name.clone().into())), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; - let instance = nexus.instance_reboot(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let instance = nexus.instance_reboot(&opctx, &instance_lookup).await?; Ok(HttpResponseAccepted(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2527,15 +2499,13 @@ async fn instance_start_v1( let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); + let instance_selector = + params::InstanceSelector::new(path.instance, &query.selector); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector::new(path.instance, &query.selector), - ) - .await?; - let instance = nexus.instance_start(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let instance = nexus.instance_start(&opctx, &instance_lookup).await?; Ok(HttpResponseOk(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2557,21 +2527,16 @@ async fn instance_start( let organization_name = &path.organization_name; let project_name = &path.project_name; let instance_name = &path.instance_name; + let instance_selector = params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector { - instance: NameOrId::Name(instance_name.clone().into()), - project: Some(NameOrId::Name(project_name.clone().into())), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; - let instance = nexus.instance_start(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let instance = nexus.instance_start(&opctx, &instance_lookup).await?; Ok(HttpResponseAccepted(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2591,15 +2556,13 @@ async fn instance_stop_v1( let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); + let instance_selector = + params::InstanceSelector::new(path.instance, &query.selector); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector::new(path.instance, &query.selector), - ) - .await?; - let instance = nexus.instance_stop(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let instance = nexus.instance_stop(&opctx, &instance_lookup).await?; Ok(HttpResponseOk(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2621,21 +2584,16 @@ async fn instance_stop( let organization_name = &path.organization_name; let project_name = &path.project_name; let instance_name = &path.instance_name; + let instance_selector = params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector { - instance: NameOrId::Name(instance_name.clone().into()), - project: Some(NameOrId::Name(project_name.clone().into())), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; - let instance = nexus.instance_stop(&opctx, &authz_instance).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let instance = nexus.instance_stop(&opctx, &instance_lookup).await?; Ok(HttpResponseAccepted(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2664,18 +2622,15 @@ async fn instance_serial_console_v1( let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); + let instance_selector = + params::InstanceSelector::new(path.instance, &query.selector); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector::new(path.instance, &query.selector), - ) - .await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; let data = nexus .instance_serial_console_data( - &opctx, - &authz_instance, + &instance_lookup, &query.console_params, ) .await?; @@ -2701,24 +2656,18 @@ async fn instance_serial_console( let organization_name = &path.organization_name; let project_name = &path.project_name; let instance_name = &path.instance_name; + let instance_selector = params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector { - instance: NameOrId::Name(instance_name.clone().into()), - project: Some(NameOrId::Name(project_name.clone().into())), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; let data = nexus .instance_serial_console_data( - &opctx, - &authz_instance, + &instance_lookup, &query_params.into_inner(), ) .await?; @@ -2743,13 +2692,10 @@ async fn instance_serial_console_stream_v1( let path = path_params.into_inner(); let query = query_params.into_inner(); let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector::new(path.instance, &query.selector), - ) - .await?; - nexus.instance_serial_console_stream(&opctx, conn, &authz_instance).await?; + let instance_selector = + params::InstanceSelector::new(path.instance, &query.selector); + let instance_lookup = nexus.instance_lookup(&opctx, &instance_selector)?; + nexus.instance_serial_console_stream(conn, &instance_lookup).await?; Ok(()) } @@ -2771,19 +2717,13 @@ async fn instance_serial_console_stream( let project_name = &path.project_name; let instance_name = &path.instance_name; let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_instance = nexus - .instance_lookup( - &opctx, - params::InstanceSelector { - instance: NameOrId::Name(instance_name.clone().into()), - project: Some(NameOrId::Name(project_name.clone().into())), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; - nexus.instance_serial_console_stream(&opctx, conn, &authz_instance).await?; + let instance_selector = params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; + let instance_lookup = nexus.instance_lookup(&opctx, &instance_selector)?; + nexus.instance_serial_console_stream(conn, &instance_lookup).await?; Ok(()) } From eb9335db6a27b86e70c38930e89caf1925d6e1aa Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 9 Dec 2022 15:42:40 -0500 Subject: [PATCH 2/2] Convert project lookup to return lookup::Project --- nexus/src/app/instance.rs | 10 +++-- nexus/src/app/project.rs | 43 ++++++++----------- nexus/src/external_api/http_entrypoints.rs | 49 ++++++++++------------ 3 files changed, 46 insertions(+), 56 deletions(-) diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 2911839ef89..ec4f0f574ac 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -118,10 +118,11 @@ impl super::Nexus { pub async fn project_create_instance( self: &Arc, opctx: &OpContext, - authz_project: &authz::Project, + project_lookup: &lookup::Project<'_>, params: ¶ms::InstanceCreate, ) -> CreateResult { - opctx.authorize(authz::Action::CreateChild, authz_project).await?; + let (.., authz_project) = + project_lookup.lookup_for(authz::Action::CreateChild).await?; // Validate parameters if params.disks.len() > MAX_DISKS_PER_INSTANCE as usize { @@ -250,10 +251,11 @@ impl super::Nexus { pub async fn project_list_instances( &self, opctx: &OpContext, - authz_project: &authz::Project, + project_lookup: &lookup::Project<'_>, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { - opctx.authorize(authz::Action::ListChildren, authz_project).await?; + let (.., authz_project) = + project_lookup.lookup_for(authz::Action::ListChildren).await?; self.db_datastore .project_list_instances(opctx, &authz_project, pagparams) .await diff --git a/nexus/src/app/project.rs b/nexus/src/app/project.rs index 917fc93dfb6..07c4c73ceec 100644 --- a/nexus/src/app/project.rs +++ b/nexus/src/app/project.rs @@ -7,6 +7,7 @@ use crate::authz; use crate::context::OpContext; use crate::db; +use crate::db::lookup; use crate::db::lookup::LookupPath; use crate::db::model::Name; use crate::external_api::params; @@ -22,47 +23,39 @@ use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::NameOrId; use omicron_common::api::external::UpdateResult; +use ref_cast::RefCast; use uuid::Uuid; impl super::Nexus { - pub async fn project_lookup( - &self, - opctx: &OpContext, - project_selector: params::ProjectSelector, - ) -> LookupResult { + pub fn project_lookup<'a>( + &'a self, + opctx: &'a OpContext, + project_selector: &'a params::ProjectSelector, + ) -> LookupResult> { match project_selector { params::ProjectSelector { project: NameOrId::Id(id), .. } => { // TODO: 400 if organization is present - let (.., authz_project) = - LookupPath::new(opctx, &self.db_datastore) - .project_id(id) - .lookup_for(authz::Action::Read) - .await?; - Ok(authz_project) + let project = + LookupPath::new(opctx, &self.db_datastore).project_id(*id); + Ok(project) } params::ProjectSelector { project: NameOrId::Name(project_name), organization: Some(NameOrId::Id(organization_id)), } => { - let (.., authz_project) = - LookupPath::new(opctx, &self.db_datastore) - .organization_id(organization_id) - .project_name(&Name(project_name)) - .lookup_for(authz::Action::Read) - .await?; - Ok(authz_project) + let project = LookupPath::new(opctx, &self.db_datastore) + .organization_id(*organization_id) + .project_name(Name::ref_cast(project_name)); + Ok(project) } params::ProjectSelector { project: NameOrId::Name(project_name), organization: Some(NameOrId::Name(organization_name)), } => { - let (.., authz_project) = - LookupPath::new(opctx, &self.db_datastore) - .organization_name(&Name(organization_name)) - .project_name(&Name(project_name)) - .lookup_for(authz::Action::Read) - .await?; - Ok(authz_project) + let project = LookupPath::new(opctx, &self.db_datastore) + .organization_name(Name::ref_cast(organization_name)) + .project_name(Name::ref_cast(project_name)); + Ok(project) } _ => Err(Error::InvalidRequest { message: " diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 62898fcbf50..50b3435f5c8 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -2032,8 +2032,7 @@ async fn instance_list_v1( let query = query_params.into_inner(); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_project = - nexus.project_lookup(&opctx, query.selector).await?; + let authz_project = nexus.project_lookup(&opctx, &query.selector)?; let instances = nexus .project_list_instances( &opctx, @@ -2071,23 +2070,17 @@ async fn instance_list( let path = path_params.into_inner(); let organization_name = &path.organization_name; let project_name = &path.project_name; + let project_selector = params::ProjectSelector { + project: NameOrId::Name(project_name.clone().into()), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let authz_project = nexus - .project_lookup( - &opctx, - params::ProjectSelector { - project: NameOrId::Name(project_name.clone().into()), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, - ) - .await?; + let project_lookup = nexus.project_lookup(&opctx, &project_selector)?; let instances = nexus .project_list_instances( &opctx, - &authz_project, + &project_lookup, &data_page_params_for(&rqctx, &query)? .map_name(|n| Name::ref_cast(n)), ) @@ -2126,9 +2119,13 @@ async fn instance_create_v1( let new_instance_params = &new_instance.into_inner(); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let project_id = nexus.project_lookup(&opctx, query.selector).await?; + let project_lookup = nexus.project_lookup(&opctx, &query.selector)?; let instance = nexus - .project_create_instance(&opctx, &project_id, &new_instance_params) + .project_create_instance( + &opctx, + &project_lookup, + &new_instance_params, + ) .await?; Ok(HttpResponseCreated(instance.into())) }; @@ -2159,22 +2156,20 @@ async fn instance_create( let organization_name = &path.organization_name; let project_name = &path.project_name; let new_instance_params = &new_instance.into_inner(); + let project_selector = params::ProjectSelector { + project: NameOrId::Name(project_name.clone().into()), + organization: Some(NameOrId::Name(organization_name.clone().into())), + }; let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let project_id = nexus - .project_lookup( + let project_lookup = nexus.project_lookup(&opctx, &project_selector)?; + let instance = nexus + .project_create_instance( &opctx, - params::ProjectSelector { - project: NameOrId::Name(project_name.clone().into()), - organization: Some(NameOrId::Name( - organization_name.clone().into(), - )), - }, + &project_lookup, + &new_instance_params, ) .await?; - let instance = nexus - .project_create_instance(&opctx, &project_id, &new_instance_params) - .await?; Ok(HttpResponseCreated(instance.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await