diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index ccfc5d0e6e5..f28352a7f8e 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -626,34 +626,6 @@ impl DataStore { }) } - /// Fetch an [`authz::Organization`] based on its id - pub async fn organization_lookup_by_id( - &self, - // TODO: OpContext, to verify actor has permission to lookup - organization_id: Uuid, - ) -> LookupResult { - use db::schema::organization::dsl; - // We only do this database lookup to verify that the Organization with - // this id exists and hasn't been deleted. - let _: Uuid = dsl::organization - .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(organization_id)) - .select(dsl::id) - .first_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Organization, - LookupType::ById(organization_id), - ), - ) - })?; - Ok(authz::FLEET - .organization(organization_id, LookupType::ById(organization_id))) - } - /// Look up the id for an organization based on its name /// /// Returns an [`authz::Organization`] (which makes the id available). @@ -882,32 +854,6 @@ impl DataStore { }) } - /// Fetch an [`authz::Project`] based on its id - pub async fn project_lookup_by_id( - &self, - project_id: Uuid, - ) -> LookupResult { - use db::schema::project::dsl; - let organization_id = dsl::project - .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(project_id)) - .select(dsl::organization_id) - .first_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Project, - LookupType::ById(project_id), - ), - ) - })?; - let authz_organization = - self.organization_lookup_by_id(organization_id).await?; - Ok(authz_organization.project(project_id, LookupType::ById(project_id))) - } - /// Look up the id for a Project based on its name /// /// Returns an [`authz::Project`] (which makes the id available). @@ -1074,35 +1020,6 @@ impl DataStore { }) } - /// Fetch an [`authz::Instance`] based on its id - pub async fn instance_lookup_by_id( - &self, - instance_id: Uuid, - ) -> LookupResult { - use db::schema::instance::dsl; - let project_id = dsl::instance - .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(instance_id)) - .select(dsl::project_id) - .first_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Instance, - LookupType::ById(instance_id), - ), - ) - })?; - let authz_project = self.project_lookup_by_id(project_id).await?; - Ok(authz_project.child_generic( - ResourceType::Instance, - instance_id, - LookupType::ById(instance_id), - )) - } - /// Look up the id for an Instance based on its name /// /// Returns an [`authz::Instance`] (which makes the id available). @@ -1381,35 +1298,6 @@ impl DataStore { }) } - /// Fetch an [`authz::Disk`] based on its id - pub async fn disk_lookup_by_id( - &self, - disk_id: Uuid, - ) -> LookupResult { - use db::schema::disk::dsl; - let project_id = dsl::disk - .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(disk_id)) - .select(dsl::project_id) - .first_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Disk, - LookupType::ById(disk_id), - ), - ) - })?; - let authz_project = self.project_lookup_by_id(project_id).await?; - Ok(authz_project.child_generic( - ResourceType::Disk, - disk_id, - LookupType::ById(disk_id), - )) - } - /// Look up the id for a Disk based on its name /// /// Returns an [`authz::Disk`] (which makes the id available). @@ -1420,7 +1308,7 @@ impl DataStore { // Projects), we don't do an authz check in the "lookup_by_path" functions // because we don't know if the caller has access to do the lookup. For // leaf resources (like Instances and Disks), though, we do. We could do - // the authz check here, and in disk_lookup_by_id() too. Should we? + // the authz check here. Should we? pub async fn disk_lookup_by_path( &self, organization_name: &Name, @@ -2144,78 +2032,6 @@ impl DataStore { // VPCs - /// Fetches a Vpc from the database and returns both the database row - /// and an [`authz::Vpc`] for doing authz checks - /// - /// See [`DataStore::organization_lookup_noauthz()`] for intended use cases - /// and caveats. - // TODO-security See the note on organization_lookup_noauthz(). - async fn vpc_lookup_noauthz( - &self, - authz_project: &authz::Project, - vpc_name: &Name, - ) -> LookupResult<(authz::Vpc, Vpc)> { - use db::schema::vpc::dsl; - dsl::vpc - .filter(dsl::time_deleted.is_null()) - .filter(dsl::project_id.eq(authz_project.id())) - .filter(dsl::name.eq(vpc_name.clone())) - .select(Vpc::as_select()) - .first_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Vpc, - LookupType::ByName(vpc_name.as_str().to_owned()), - ), - ) - }) - .map(|d| { - ( - authz_project.child_generic( - ResourceType::Vpc, - d.id(), - LookupType::from(&vpc_name.0), - ), - d, - ) - }) - } - - /// Look up the id for a Vpc based on its name - /// - /// Returns an [`authz::Vpc`] (which makes the id available). - /// - /// Like the other "lookup_by_path()" functions, this function does no authz - /// checks. - pub async fn vpc_lookup_by_path( - &self, - organization_name: &Name, - project_name: &Name, - vpc_name: &Name, - ) -> LookupResult { - let authz_project = self - .project_lookup_by_path(organization_name, project_name) - .await?; - self.vpc_lookup_noauthz(&authz_project, vpc_name).await.map(|(v, _)| v) - } - - /// Lookup a Vpc by name and return the full database record, along - /// with an [`authz::Vpc`] for subsequent authorization checks - pub async fn vpc_fetch( - &self, - opctx: &OpContext, - authz_project: &authz::Project, - name: &Name, - ) -> LookupResult<(authz::Vpc, Vpc)> { - let (authz_vpc, db_vpc) = - self.vpc_lookup_noauthz(authz_project, name).await?; - opctx.authorize(authz::Action::Read, &authz_vpc).await?; - Ok((authz_vpc, db_vpc)) - } - pub async fn project_list_vpcs( &self, opctx: &OpContext, @@ -2292,33 +2108,6 @@ impl DataStore { }) } - // TODO-security TODO-cleanup Remove this function. Update callers to use - // vpc_lookup_by_path() or vpc_fetch() instead. - pub async fn vpc_fetch_by_name( - &self, - project_id: &Uuid, - vpc_name: &Name, - ) -> LookupResult { - use db::schema::vpc::dsl; - - dsl::vpc - .filter(dsl::time_deleted.is_null()) - .filter(dsl::project_id.eq(*project_id)) - .filter(dsl::name.eq(vpc_name.clone())) - .select(Vpc::as_select()) - .get_result_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Vpc, - LookupType::ByName(vpc_name.as_str().to_owned()), - ), - ) - }) - } - pub async fn project_delete_vpc( &self, opctx: &OpContext, @@ -2489,81 +2278,6 @@ impl DataStore { .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } - /// Fetches a VpcSubnet from the database and returns both the database row - /// and an [`authz::VpcSubnet`] for doing authz checks - /// - /// See [`DataStore::organization_lookup_noauthz()`] for intended use cases - /// and caveats. - // TODO-security See the note on organization_lookup_noauthz(). - async fn vpc_subnet_lookup_noauthz( - &self, - authz_vpc: &authz::Vpc, - subnet_name: &Name, - ) -> LookupResult<(authz::VpcSubnet, VpcSubnet)> { - use db::schema::vpc_subnet::dsl; - dsl::vpc_subnet - .filter(dsl::time_deleted.is_null()) - .filter(dsl::vpc_id.eq(authz_vpc.id())) - .filter(dsl::name.eq(subnet_name.clone())) - .select(VpcSubnet::as_select()) - .first_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::VpcSubnet, - LookupType::ByName(subnet_name.as_str().to_owned()), - ), - ) - }) - .map(|d| { - ( - authz_vpc.child_generic( - ResourceType::VpcSubnet, - d.id(), - LookupType::from(&subnet_name.0), - ), - d, - ) - }) - } - - /// Look up the id for a VpcSubnet based on its name - /// - /// Returns an [`authz::VpcSubnet`] (which makes the id available). - /// - /// Like the other "lookup_by_path()" functions, this function does no authz - /// checks. - pub async fn vpc_subnet_lookup_by_path( - &self, - organization_name: &Name, - project_name: &Name, - vpc_name: &Name, - subnet_name: &Name, - ) -> LookupResult { - let authz_vpc = self - .vpc_lookup_by_path(organization_name, project_name, vpc_name) - .await?; - self.vpc_subnet_lookup_noauthz(&authz_vpc, subnet_name) - .await - .map(|(v, _)| v) - } - - /// Lookup a VpcSubnet by name and return the full database record, along - /// with an [`authz::VpcSubnet`] for subsequent authorization checks - pub async fn vpc_subnet_fetch( - &self, - opctx: &OpContext, - authz_vpc: &authz::Vpc, - name: &Name, - ) -> LookupResult<(authz::VpcSubnet, VpcSubnet)> { - let (authz_vpc_subnet, db_vpc_subnet) = - self.vpc_subnet_lookup_noauthz(authz_vpc, name).await?; - opctx.authorize(authz::Action::Read, &authz_vpc_subnet).await?; - Ok((authz_vpc_subnet, db_vpc_subnet)) - } - /// Insert a VPC Subnet, checking for unique IP address ranges. pub async fn vpc_create_subnet( &self, diff --git a/nexus/src/db/lookup.rs b/nexus/src/db/lookup.rs index bf9d232eb38..9f312c0f0d8 100644 --- a/nexus/src/db/lookup.rs +++ b/nexus/src/db/lookup.rs @@ -210,6 +210,11 @@ impl<'a> LookupPath<'a> { Vpc { key: Key::Id(Root { lookup_root: self }, id) } } + /// Select a resource of type VpcSubnet, identified by its id + pub fn vpc_subnet_id(self, id: Uuid) -> VpcSubnet<'a> { + VpcSubnet { key: Key::Id(Root { lookup_root: self }, id) } + } + /// Select a resource of type VpcRouter, identified by its id pub fn vpc_router_id(self, id: Uuid) -> VpcRouter<'a> { VpcRouter { key: Key::Id(Root { lookup_root: self }, id) } @@ -280,7 +285,14 @@ lookup_resource! { lookup_resource! { name = "Vpc", ancestors = [ "Organization", "Project" ], - children = [ "VpcRouter" ], + children = [ "VpcRouter", "VpcSubnet" ], + authz_kind = Generic +} + +lookup_resource! { + name = "VpcSubnet", + ancestors = [ "Organization", "Project", "Vpc" ], + children = [ ], authz_kind = Generic } diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 96dc03cfd44..eb26921d14a 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -999,9 +999,11 @@ impl Nexus { // // TODO Even worse, post-authz, we do two lookups here instead of one. // Maybe sagas should be able to emit `authz::Instance`-type objects. - let authz_instance = - self.db_datastore.instance_lookup_by_id(instance_id).await?; - self.db_datastore.instance_refetch(opctx, &authz_instance).await + let (.., db_instance) = LookupPath::new(opctx, &self.db_datastore) + .instance_id(instance_id) + .fetch() + .await?; + Ok(db_instance) } // TODO-correctness It's not totally clear what the semantics and behavior @@ -1276,10 +1278,12 @@ impl Nexus { migration_id: Uuid, dst_propolis_id: Uuid, ) -> UpdateResult { - let authz_instance = - self.db_datastore.instance_lookup_by_id(instance_id).await?; - let db_instance = - self.db_datastore.instance_refetch(opctx, &authz_instance).await?; + let (.., authz_instance, db_instance) = + LookupPath::new(opctx, &self.db_datastore) + .instance_id(instance_id) + .fetch() + .await + .unwrap(); let requested = InstanceRuntimeStateRequested { run_state: InstanceStateRequested::Migrating, migration_params: Some(InstanceRuntimeStateMigrateParams { @@ -1728,14 +1732,13 @@ impl Nexus { instance_name: &Name, params: ¶ms::NetworkInterfaceCreate, ) -> CreateResult { - let authz_project = self - .db_datastore - .project_lookup_by_path(organization_name, project_name) - .await?; - let (authz_instance, db_instance) = self - .db_datastore - .instance_fetch(opctx, &authz_project, instance_name) - .await?; + let (_, authz_project, authz_instance, db_instance) = + LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .instance_name(instance_name) + .fetch() + .await?; // TODO-completeness: We'd like to relax this once hot-plug is // supported. @@ -1757,14 +1760,13 @@ impl Nexus { // IDs for creating the network interface. let vpc_name = db::model::Name(params.vpc_name.clone()); let subnet_name = db::model::Name(params.subnet_name.clone()); - let (authz_vpc, _) = self - .db_datastore - .vpc_fetch(opctx, &authz_project, &vpc_name) - .await?; - let (authz_subnet, db_subnet) = self - .db_datastore - .vpc_subnet_fetch(opctx, &authz_vpc, &subnet_name) - .await?; + let (.., authz_vpc, authz_subnet, db_subnet) = + LookupPath::new(opctx, &self.db_datastore) + .project_id(authz_project.id()) + .vpc_name(&vpc_name) + .vpc_subnet_name(&subnet_name) + .fetch() + .await?; let mac = db::model::MacAddr::new()?; let interface_id = Uuid::new_v4(); let interface = db::model::IncompleteNetworkInterface::new( @@ -2019,15 +2021,13 @@ impl Nexus { project_name: &Name, vpc_name: &Name, ) -> LookupResult { - let authz_project = self - .db_datastore - .project_lookup_by_path(organization_name, project_name) + let (.., db_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .fetch() .await?; - Ok(self - .db_datastore - .vpc_fetch(opctx, &authz_project, vpc_name) - .await? - .1) + Ok(db_vpc) } pub async fn project_update_vpc( @@ -2038,9 +2038,11 @@ impl Nexus { vpc_name: &Name, params: ¶ms::VpcUpdate, ) -> UpdateResult { - let authz_vpc = self - .db_datastore - .vpc_lookup_by_path(organization_name, project_name, vpc_name) + let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .lookup_for(authz::Action::Modify) .await?; self.db_datastore .project_update_vpc(opctx, &authz_vpc, params.clone().into()) @@ -2054,14 +2056,14 @@ impl Nexus { project_name: &Name, vpc_name: &Name, ) -> DeleteResult { - let authz_project = self - .db_datastore - .project_lookup_by_path(organization_name, project_name) - .await?; - let (authz_vpc, db_vpc) = self - .db_datastore - .vpc_fetch(opctx, &authz_project, vpc_name) - .await?; + let (.., authz_vpc, db_vpc) = + LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .fetch() + .await?; + let authz_vpc_router = authz_vpc.child_generic( ResourceType::VpcRouter, db_vpc.system_router_id, @@ -2087,9 +2089,11 @@ impl Nexus { project_name: &Name, vpc_name: &Name, ) -> ListResultVec { - let authz_vpc = self - .db_datastore - .vpc_lookup_by_path(organization_name, project_name, vpc_name) + let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .lookup_for(authz::Action::Read) .await?; let rules = self .db_datastore @@ -2106,9 +2110,11 @@ impl Nexus { vpc_name: &Name, params: &VpcFirewallRuleUpdateParams, ) -> UpdateResult> { - let authz_vpc = self - .db_datastore - .vpc_lookup_by_path(organization_name, project_name, vpc_name) + let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .lookup_for(authz::Action::Modify) .await?; let rules = db::model::VpcFirewallRule::vec_from_params( authz_vpc.id(), @@ -2127,9 +2133,11 @@ impl Nexus { vpc_name: &Name, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { - let authz_vpc = self - .db_datastore - .vpc_lookup_by_path(organization_name, project_name, vpc_name) + let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .lookup_for(authz::Action::ListChildren) .await?; self.db_datastore.vpc_list_subnets(opctx, &authz_vpc, pagparams).await } @@ -2142,15 +2150,14 @@ impl Nexus { vpc_name: &Name, subnet_name: &Name, ) -> LookupResult { - let authz_vpc = self - .db_datastore - .vpc_lookup_by_path(organization_name, project_name, vpc_name) + let (.., db_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .vpc_subnet_name(subnet_name) + .fetch() .await?; - Ok(self - .db_datastore - .vpc_subnet_fetch(opctx, &authz_vpc, subnet_name) - .await? - .1) + Ok(db_vpc) } // TODO: When a subnet is created it should add a route entry into the VPC's @@ -2163,14 +2170,13 @@ impl Nexus { vpc_name: &Name, params: ¶ms::VpcSubnetCreate, ) -> CreateResult { - let authz_project = self - .db_datastore - .project_lookup_by_path(organization_name, project_name) - .await?; - let (authz_vpc, db_vpc) = self - .db_datastore - .vpc_fetch(opctx, &authz_project, vpc_name) - .await?; + let (.., authz_vpc, db_vpc) = + LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .fetch() + .await?; // Validate IPv4 range if !params.ipv4_block.network().is_private() { @@ -2322,14 +2328,12 @@ impl Nexus { vpc_name: &Name, subnet_name: &Name, ) -> DeleteResult { - let authz_subnet = self - .db_datastore - .vpc_subnet_lookup_by_path( - organization_name, - project_name, - vpc_name, - subnet_name, - ) + let (.., authz_subnet) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .vpc_subnet_name(subnet_name) + .lookup_for(authz::Action::Delete) .await?; self.db_datastore.vpc_delete_subnet(opctx, &authz_subnet).await } @@ -2343,14 +2347,12 @@ impl Nexus { subnet_name: &Name, params: ¶ms::VpcSubnetUpdate, ) -> UpdateResult { - let authz_subnet = self - .db_datastore - .vpc_subnet_lookup_by_path( - organization_name, - project_name, - vpc_name, - subnet_name, - ) + let (.., authz_subnet) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .vpc_subnet_name(subnet_name) + .lookup_for(authz::Action::Modify) .await?; self.db_datastore .vpc_update_subnet(&opctx, &authz_subnet, params.clone().into()) @@ -2366,14 +2368,12 @@ impl Nexus { subnet_name: &Name, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { - let authz_subnet = self - .db_datastore - .vpc_subnet_lookup_by_path( - organization_name, - project_name, - vpc_name, - subnet_name, - ) + let (.., authz_subnet) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .vpc_subnet_name(subnet_name) + .lookup_for(authz::Action::ListChildren) .await?; self.db_datastore .subnet_list_network_interfaces(opctx, &authz_subnet, pagparams) @@ -2388,9 +2388,11 @@ impl Nexus { vpc_name: &Name, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { - let authz_vpc = self - .db_datastore - .vpc_lookup_by_path(organization_name, project_name, vpc_name) + let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .lookup_for(authz::Action::ListChildren) .await?; let routers = self .db_datastore @@ -2426,9 +2428,11 @@ impl Nexus { kind: &VpcRouterKind, params: ¶ms::VpcRouterCreate, ) -> CreateResult { - let authz_vpc = self - .db_datastore - .vpc_lookup_by_path(organization_name, project_name, vpc_name) + let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .lookup_for(authz::Action::CreateChild) .await?; let id = Uuid::new_v4(); let router = db::model::VpcRouter::new( @@ -2846,7 +2850,10 @@ impl Nexus { new_state: &DiskRuntimeState, ) -> Result<(), Error> { let log = &self.log; - let authz_disk = self.db_datastore.disk_lookup_by_id(id).await?; + let (.., authz_disk) = LookupPath::new(&opctx, &self.db_datastore) + .disk_id(id) + .lookup_for(authz::Action::Modify) + .await?; let result = self .db_datastore @@ -3293,10 +3300,10 @@ impl TestInterfaces for Nexus { self.log.new(o!()), Arc::clone(&self.db_datastore), ); - let authz_instance = - self.db_datastore.instance_lookup_by_id(*id).await?; - let db_instance = - self.db_datastore.instance_refetch(&opctx, &authz_instance).await?; + let (.., db_instance) = LookupPath::new(&opctx, &self.db_datastore) + .instance_id(*id) + .fetch() + .await?; self.instance_sled(&db_instance).await } @@ -3308,14 +3315,14 @@ impl TestInterfaces for Nexus { self.log.new(o!()), Arc::clone(&self.db_datastore), ); - let authz_disk = self.db_datastore.disk_lookup_by_id(*id).await?; - let db_disk = - self.db_datastore.disk_refetch(&opctx, &authz_disk).await?; - let instance_id = db_disk.runtime().attach_instance_id.unwrap(); - let authz_instance = - self.db_datastore.instance_lookup_by_id(instance_id).await?; - let db_instance = - self.db_datastore.instance_refetch(&opctx, &authz_instance).await?; + let (.., db_disk) = LookupPath::new(&opctx, &self.db_datastore) + .disk_id(*id) + .fetch() + .await?; + let (.., db_instance) = LookupPath::new(&opctx, &self.db_datastore) + .instance_id(db_disk.runtime().attach_instance_id.unwrap()) + .fetch() + .await?; self.instance_sled(&db_instance).await } @@ -3332,9 +3339,11 @@ impl TestInterfaces for Nexus { Arc::clone(&self.db_datastore), ); - let authz_disk = self.db_datastore.disk_lookup_by_id(*disk_id).await?; - let db_disk = - self.db_datastore.disk_refetch(&opctx, &authz_disk).await?; + let (.., authz_disk, db_disk) = + LookupPath::new(&opctx, &self.db_datastore) + .disk_id(*disk_id) + .fetch() + .await?; let new_runtime = db_disk.runtime_state.faulted(); diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index 6cb886f9b4c..1daaa8384f1 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -11,9 +11,10 @@ use crate::context::OpContext; use crate::db::identity::{Asset, Resource}; +use crate::db::lookup::LookupPath; use crate::external_api::params; use crate::saga_interface::SagaContext; -use crate::{authn, db}; +use crate::{authn, authz, db}; use anyhow::anyhow; use chrono::Utc; use crucible_agent_client::{ @@ -334,20 +335,15 @@ async fn sic_create_custom_network_interfaces( let ids = sagactx.lookup::>("network_interface_ids")?; // Lookup authz objects, used in the call to create the NIC itself. - let authz_instance = datastore - .instance_lookup_by_id(instance_id) + let (.., authz_instance) = LookupPath::new(&opctx, &datastore) + .instance_id(instance_id) + .lookup_for(authz::Action::CreateChild) .await .map_err(ActionError::action_failed)?; - let authz_project = datastore - .project_lookup_by_id(saga_params.project_id) - .await - .map_err(ActionError::action_failed)?; - let (authz_vpc, db_vpc) = datastore - .vpc_fetch( - &opctx, - &authz_project, - &db::model::Name::from(interface_params[0].vpc_name.clone()), - ) + let (.., authz_vpc, db_vpc) = LookupPath::new(&opctx, &datastore) + .project_id(saga_params.project_id) + .vpc_name(&db::model::Name::from(interface_params[0].vpc_name.clone())) + .fetch() .await .map_err(ActionError::action_failed)?; @@ -374,12 +370,10 @@ async fn sic_create_custom_network_interfaces( // should probably either be in a transaction, or the // `subnet_create_network_interface` function/query needs some JOIN // on the `vpc_subnet` table. - let (authz_subnet, db_subnet) = datastore - .vpc_subnet_fetch( - &opctx, - &authz_vpc, - &db::model::Name::from(params.subnet_name.clone()), - ) + let (.., authz_subnet, db_subnet) = LookupPath::new(&opctx, &datastore) + .vpc_id(authz_vpc.id()) + .vpc_subnet_name(&db::model::Name::from(params.subnet_name.clone())) + .fetch() .await .map_err(ActionError::action_failed)?; let mac = @@ -480,22 +474,19 @@ async fn sic_create_default_network_interface( }; // Lookup authz objects, used in the call to actually create the NIC. - let authz_instance = datastore - .instance_lookup_by_id(instance_id) - .await - .map_err(ActionError::action_failed)?; - let authz_project = datastore - .project_lookup_by_id(saga_params.project_id) - .await - .map_err(ActionError::action_failed)?; - let (authz_vpc, _) = datastore - .vpc_fetch(&opctx, &authz_project, &internal_default_name.clone()) - .await - .map_err(ActionError::action_failed)?; - let (authz_subnet, db_subnet) = datastore - .vpc_subnet_fetch(&opctx, &authz_vpc, &internal_default_name) + let (.., authz_instance) = LookupPath::new(&opctx, &datastore) + .instance_id(instance_id) + .lookup_for(authz::Action::CreateChild) .await .map_err(ActionError::action_failed)?; + let (.., authz_vpc, authz_subnet, db_subnet) = + LookupPath::new(&opctx, &datastore) + .project_id(saga_params.project_id) + .vpc_name(&internal_default_name) + .vpc_subnet_name(&internal_default_name) + .fetch() + .await + .map_err(ActionError::action_failed)?; let mac = db::model::MacAddr::new().map_err(ActionError::action_failed)?; let interface_id = Uuid::new_v4(); @@ -699,21 +690,17 @@ async fn sic_delete_instance_record( ) -> Result<(), anyhow::Error> { let osagactx = sagactx.user_data(); let params = sagactx.saga_params(); + let datastore = osagactx.datastore(); let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); let instance_id = sagactx.lookup::("instance_id")?; let instance_name = sagactx.lookup::("instance_name")?; // We currently only support deleting an instance if it is stopped or // failed, so update the state accordingly to allow deletion. - let authz_project = osagactx - .datastore() - .project_lookup_by_id(params.project_id) - .await - .map_err(ActionError::action_failed)?; - - let (authz_instance, db_instance) = osagactx - .datastore() - .instance_fetch(&opctx, &authz_project, &instance_name) + let (.., authz_instance, db_instance) = LookupPath::new(&opctx, &datastore) + .project_id(params.project_id) + .instance_name(&instance_name) + .fetch() .await .map_err(ActionError::action_failed)?; @@ -729,8 +716,7 @@ async fn sic_delete_instance_record( ..db_instance.runtime_state }; - let updated = osagactx - .datastore() + let updated = datastore .instance_update_runtime(&instance_id, &runtime_state) .await .map_err(ActionError::action_failed)?; @@ -743,8 +729,7 @@ async fn sic_delete_instance_record( } // Actually delete the record. - osagactx - .datastore() + datastore .project_delete_instance(&opctx, &authz_instance) .await .map_err(ActionError::action_failed)?; @@ -758,6 +743,7 @@ async fn sic_instance_ensure( // TODO-correctness is this idempotent? let osagactx = sagactx.user_data(); let params = sagactx.saga_params(); + let datastore = osagactx.datastore(); let runtime_params = InstanceRuntimeStateRequested { run_state: InstanceStateRequested::Running, migration_params: None, @@ -766,15 +752,10 @@ async fn sic_instance_ensure( let instance_name = sagactx.lookup::("instance_name")?; let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); - let authz_project = osagactx - .datastore() - .project_lookup_by_id(params.project_id) - .await - .map_err(ActionError::action_failed)?; - - let (authz_instance, instance) = osagactx - .datastore() - .instance_fetch(&opctx, &authz_project, &instance_name) + let (.., authz_instance, db_instance) = LookupPath::new(&opctx, &datastore) + .project_id(params.project_id) + .instance_name(&instance_name) + .fetch() .await .map_err(ActionError::action_failed)?; @@ -783,7 +764,7 @@ async fn sic_instance_ensure( .instance_set_runtime( &opctx, &authz_instance, - &instance, + &db_instance, runtime_params, ) .await @@ -1352,8 +1333,9 @@ async fn sdc_finalize_disk_record( let disk_id = sagactx.lookup::("disk_id")?; let disk_created = sagactx.lookup::("created_disk")?; - let authz_disk = datastore - .disk_lookup_by_id(disk_id) + let (.., authz_disk) = LookupPath::new(&opctx, &datastore) + .disk_id(disk_id) + .lookup_for(authz::Action::Modify) .await .map_err(ActionError::action_failed)?; // TODO-security Review whether this can ever fail an authz check. We don't