diff --git a/nexus/src/app/disk.rs b/nexus/src/app/disk.rs index 2f045272437..327fcd627e2 100644 --- a/nexus/src/app/disk.rs +++ b/nexus/src/app/disk.rs @@ -9,6 +9,7 @@ use crate::authn; 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; @@ -20,26 +21,58 @@ use omicron_common::api::external::Error; use omicron_common::api::external::InternalContext; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; +use omicron_common::api::external::NameOrId; use omicron_common::api::internal::nexus::DiskRuntimeState; +use ref_cast::RefCast; use sled_agent_client::Client as SledAgentClient; use std::sync::Arc; use uuid::Uuid; impl super::Nexus { // Disks + pub fn disk_lookup<'a>( + &'a self, + opctx: &'a OpContext, + disk_selector: &'a params::DiskSelector, + ) -> LookupResult> { + match disk_selector { + params::DiskSelector { + disk: NameOrId::Id(id), + project_selector: None, + } => { + let disk = + LookupPath::new(opctx, &self.db_datastore).disk_id(*id); + Ok(disk) + } + params::DiskSelector { + disk: NameOrId::Name(name), + project_selector: Some(project_selector), + } => { + let disk = self + .project_lookup(opctx, project_selector)? + .disk_name(Name::ref_cast(name)); + Ok(disk) + } + params::DiskSelector { + disk: NameOrId::Id(_), + project_selector: Some(_), + } => Err(Error::invalid_request( + "when providing disk as an ID, project should not be specified", + )), + _ => Err(Error::invalid_request( + "disk should either be UUID or project should be specified", + )), + } + } pub async fn project_create_disk( self: &Arc, opctx: &OpContext, - organization_name: &Name, - project_name: &Name, + project_lookup: &lookup::Project<'_>, params: ¶ms::DiskCreate, ) -> CreateResult { - let (.., authz_project) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .lookup_for(authz::Action::CreateChild) - .await?; + let (.., authz_project) = + project_lookup.lookup_for(authz::Action::CreateChild).await?; match ¶ms.disk_source { params::DiskSource::Blank { block_size } => { @@ -225,49 +258,30 @@ impl super::Nexus { Ok(disk_created) } - pub async fn project_list_disks( + pub async fn project_list_disks_by_id( &self, opctx: &OpContext, - organization_name: &Name, - project_name: &Name, - pagparams: &DataPageParams<'_, Name>, + project_lookup: &lookup::Project<'_>, + pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { - let (.., authz_project) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .lookup_for(authz::Action::ListChildren) - .await?; + let (.., authz_project) = + project_lookup.lookup_for(authz::Action::ListChildren).await?; self.db_datastore - .project_list_disks(opctx, &authz_project, pagparams) + .project_list_disks_by_id(opctx, &authz_project, pagparams) .await } - pub async fn disk_fetch( - &self, - opctx: &OpContext, - organization_name: &Name, - project_name: &Name, - disk_name: &Name, - ) -> LookupResult { - let (.., db_disk) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .disk_name(disk_name) - .fetch() - .await?; - Ok(db_disk) - } - - pub async fn disk_fetch_by_id( + pub async fn project_list_disks_by_name( &self, opctx: &OpContext, - disk_id: &Uuid, - ) -> LookupResult { - let (.., db_disk) = LookupPath::new(opctx, &self.db_datastore) - .disk_id(*disk_id) - .fetch() - .await?; - Ok(db_disk) + project_lookup: &lookup::Project<'_>, + pagparams: &DataPageParams<'_, Name>, + ) -> ListResultVec { + let (.., authz_project) = + project_lookup.lookup_for(authz::Action::ListChildren).await?; + self.db_datastore + .project_list_disks_by_name(opctx, &authz_project, pagparams) + .await } /// Modifies the runtime state of the Disk as requested. This generally @@ -372,17 +386,10 @@ impl super::Nexus { pub async fn project_delete_disk( self: &Arc, - opctx: &OpContext, - organization_name: &Name, - project_name: &Name, - disk_name: &Name, + disk_lookup: &lookup::Disk<'_>, ) -> DeleteResult { - let (.., authz_disk) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .disk_name(disk_name) - .lookup_for(authz::Action::Delete) - .await?; + let (.., authz_disk) = + disk_lookup.lookup_for(authz::Action::Delete).await?; let saga_params = sagas::disk_delete::Params { disk_id: authz_disk.id() }; diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 44bb4b9f9be..782c6c91975 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -478,7 +478,7 @@ impl super::Nexus { // Gather disk information and turn that into DiskRequests let disks = self .db_datastore - .instance_list_disks( + .instance_list_disks_by_name( &opctx, &authz_instance, &DataPageParams { @@ -699,23 +699,31 @@ impl super::Nexus { } } - /// Lists disks attached to the instance. - pub async fn instance_list_disks( + /// Lists disks attached to the instance by id. + pub async fn instance_list_disks_by_id( &self, opctx: &OpContext, - organization_name: &Name, - project_name: &Name, - instance_name: &Name, + instance_lookup: &lookup::Instance<'_>, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + let (.., authz_instance) = + instance_lookup.lookup_for(authz::Action::ListChildren).await?; + self.db_datastore + .instance_list_disks_by_id(opctx, &authz_instance, pagparams) + .await + } + + /// Lists disks attached to the instance by name. + pub async fn instance_list_disks_by_name( + &self, + opctx: &OpContext, + instance_lookup: &lookup::Instance<'_>, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { - let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .instance_name(instance_name) - .lookup_for(authz::Action::ListChildren) - .await?; + let (.., authz_instance) = + instance_lookup.lookup_for(authz::Action::ListChildren).await?; self.db_datastore - .instance_list_disks(opctx, &authz_instance, pagparams) + .instance_list_disks_by_name(opctx, &authz_instance, pagparams) .await } @@ -723,24 +731,34 @@ impl super::Nexus { pub async fn instance_attach_disk( &self, opctx: &OpContext, - organization_name: &Name, - project_name: &Name, - instance_name: &Name, - disk_name: &Name, + instance_lookup: &lookup::Instance<'_>, + disk: NameOrId, ) -> UpdateResult { - let (.., authz_project, authz_disk, _) = - LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .disk_name(disk_name) - .fetch() - .await?; - let (.., authz_instance, _) = - LookupPath::new(opctx, &self.db_datastore) - .project_id(authz_project.id()) - .instance_name(instance_name) - .fetch() - .await?; + let (.., authz_project, authz_instance) = + instance_lookup.lookup_for(authz::Action::Modify).await?; + let (.., authz_project_disk, authz_disk) = self + .disk_lookup( + opctx, + ¶ms::DiskSelector::new( + None, + Some(authz_project.id().into()), + disk, + ), + )? + .lookup_for(authz::Action::Modify) + .await?; + + // TODO-v1: Write test to verify this case + // Because both instance and disk can be provided by ID it's possible for someone + // to specify resources from different projects. The lookups would resolve the resources + // (assuming the user had sufficient permissions on both) without verifying the shared hierarchy. + // To mitigate that we verify that their parent projects have the same ID. + if authz_project.id() != authz_project_disk.id() { + return Err(Error::InvalidRequest { + message: "disk must be in the same project as the instance" + .to_string(), + }); + } // TODO(https://github.com/oxidecomputer/omicron/issues/811): // Disk attach is only implemented for instances that are not @@ -771,25 +789,22 @@ impl super::Nexus { pub async fn instance_detach_disk( &self, opctx: &OpContext, - organization_name: &Name, - project_name: &Name, - instance_name: &Name, - disk_name: &Name, + instance_lookup: &lookup::Instance<'_>, + disk: NameOrId, ) -> UpdateResult { - let (.., authz_project, authz_disk, _) = - LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .disk_name(disk_name) - .fetch() - .await?; - let (.., authz_instance, _) = - LookupPath::new(opctx, &self.db_datastore) - .project_id(authz_project.id()) - .instance_name(instance_name) - .fetch() - .await?; - + let (.., authz_project, authz_instance) = + instance_lookup.lookup_for(authz::Action::Modify).await?; + let (.., authz_disk) = self + .disk_lookup( + opctx, + ¶ms::DiskSelector::new( + None, + Some(authz_project.id().into()), + disk, + ), + )? + .lookup_for(authz::Action::Modify) + .await?; // TODO(https://github.com/oxidecomputer/omicron/issues/811): // Disk detach is only implemented for instances that are not // currently running. This operation therefore can operate exclusively diff --git a/nexus/src/app/sagas/disk_create.rs b/nexus/src/app/sagas/disk_create.rs index 9a678ec5f75..efda83b1f4b 100644 --- a/nexus/src/app/sagas/disk_create.rs +++ b/nexus/src/app/sagas/disk_create.rs @@ -585,7 +585,7 @@ pub(crate) mod test { use crate::{ app::saga::create_saga_dag, app::sagas::disk_create::Params, app::sagas::disk_create::SagaDiskCreate, authn::saga::Serialized, - context::OpContext, db, db::datastore::DataStore, external_api::params, + context::OpContext, db::datastore::DataStore, external_api::params, }; use async_bb8_diesel::{AsyncRunQueryDsl, OptionalExtension}; use diesel::{ExpressionMethods, QueryDsl, SelectableHelper}; @@ -599,7 +599,6 @@ pub(crate) mod test { use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Name; use omicron_sled_agent::sim::SledAgent; - use ref_cast::RefCast; use std::num::NonZeroU32; use uuid::Uuid; @@ -863,20 +862,15 @@ pub(crate) mod test { async fn destroy_disk(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; let opctx = test_opctx(&cptestctx); + let disk_selector = params::DiskSelector::new( + Some(Name::try_from(ORG_NAME.to_string()).unwrap().into()), + Some(Name::try_from(PROJECT_NAME.to_string()).unwrap().into()), + Name::try_from(DISK_NAME.to_string()).unwrap().into(), + ); + let disk_lookup = nexus.disk_lookup(&opctx, &disk_selector).unwrap(); nexus - .project_delete_disk( - &opctx, - db::model::Name::ref_cast( - &Name::try_from(ORG_NAME.to_string()).unwrap(), - ), - db::model::Name::ref_cast( - &Name::try_from(PROJECT_NAME.to_string()).unwrap(), - ), - db::model::Name::ref_cast( - &Name::try_from(DISK_NAME.to_string()).unwrap(), - ), - ) + .project_delete_disk(&disk_lookup) .await .expect("Failed to delete disk"); } diff --git a/nexus/src/app/sagas/disk_delete.rs b/nexus/src/app/sagas/disk_delete.rs index e0c78e94dd5..3634217a3a9 100644 --- a/nexus/src/app/sagas/disk_delete.rs +++ b/nexus/src/app/sagas/disk_delete.rs @@ -88,7 +88,7 @@ async fn sdd_delete_volume( pub(crate) mod test { use crate::{ app::saga::create_saga_dag, app::sagas::disk_delete::Params, - app::sagas::disk_delete::SagaDiskDelete, context::OpContext, db, + app::sagas::disk_delete::SagaDiskDelete, context::OpContext, }; use dropshot::test_util::ClientTestContext; use nexus_test_utils::resource_helpers::create_ip_pool; @@ -96,8 +96,8 @@ pub(crate) mod test { use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils::resource_helpers::DiskTest; use nexus_test_utils_macros::nexus_test; + use nexus_types::external_api::params; use omicron_common::api::external::Name; - use ref_cast::RefCast; use std::num::NonZeroU32; use uuid::Uuid; @@ -125,15 +125,17 @@ pub(crate) mod test { let nexus = &cptestctx.server.apictx.nexus; let opctx = test_opctx(&cptestctx); + let project_selector = params::ProjectSelector::new( + Some(Name::try_from(ORG_NAME.to_string()).unwrap().into()), + Name::try_from(PROJECT_NAME.to_string()).unwrap().into(), + ); + let project_lookup = + nexus.project_lookup(&opctx, &project_selector).unwrap(); + nexus .project_create_disk( &opctx, - db::model::Name::ref_cast( - &Name::try_from(ORG_NAME.to_string()).unwrap(), - ), - db::model::Name::ref_cast( - &Name::try_from(PROJECT_NAME.to_string()).unwrap(), - ), + &project_lookup, &crate::app::sagas::disk_create::test::new_disk_create_params(), ) .await diff --git a/nexus/src/db/datastore/disk.rs b/nexus/src/db/datastore/disk.rs index cc1a3cf7216..33a6b257cf1 100644 --- a/nexus/src/db/datastore/disk.rs +++ b/nexus/src/db/datastore/disk.rs @@ -42,8 +42,27 @@ use omicron_common::bail_unless; use uuid::Uuid; impl DataStore { - /// List disks associated with a given instance. - pub async fn instance_list_disks( + /// List disks associated with a given instance by id. + pub async fn instance_list_disks_by_id( + &self, + opctx: &OpContext, + authz_instance: &authz::Instance, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + use db::schema::disk::dsl; + + opctx.authorize(authz::Action::ListChildren, authz_instance).await?; + + paginated(dsl::disk, dsl::id, &pagparams) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::attach_instance_id.eq(authz_instance.id())) + .select(Disk::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } + /// List disks associated with a given instance by name. + pub async fn instance_list_disks_by_name( &self, opctx: &OpContext, authz_instance: &authz::Instance, @@ -110,7 +129,24 @@ impl DataStore { Ok(disk) } - pub async fn project_list_disks( + pub async fn project_list_disks_by_id( + &self, + opctx: &OpContext, + authz_project: &authz::Project, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, authz_project).await?; + + use db::schema::disk::dsl; + paginated(dsl::disk, dsl::id, &pagparams) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::project_id.eq(authz_project.id())) + .select(Disk::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } + pub async fn project_list_disks_by_name( &self, opctx: &OpContext, authz_project: &authz::Project, diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 0337d6c8ac8..71f70d76be3 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -146,6 +146,11 @@ pub fn external_api() -> NexusApiDescription { api.register(disk_delete)?; api.register(disk_metrics_list)?; + api.register(disk_list_v1)?; + api.register(disk_create_v1)?; + api.register(disk_view_v1)?; + api.register(disk_delete_v1)?; + api.register(instance_list)?; api.register(instance_create)?; api.register(instance_view)?; @@ -155,6 +160,9 @@ pub fn external_api() -> NexusApiDescription { api.register(instance_reboot)?; api.register(instance_start)?; api.register(instance_stop)?; + api.register(instance_disk_list)?; + api.register(instance_disk_attach)?; + api.register(instance_disk_detach)?; api.register(instance_serial_console)?; api.register(instance_serial_console_stream)?; @@ -166,6 +174,9 @@ pub fn external_api() -> NexusApiDescription { api.register(instance_reboot_v1)?; api.register(instance_start_v1)?; api.register(instance_stop_v1)?; + api.register(instance_disk_list_v1)?; + api.register(instance_disk_attach_v1)?; + api.register(instance_disk_detach_v1)?; api.register(instance_serial_console_v1)?; api.register(instance_serial_console_stream_v1)?; @@ -176,10 +187,6 @@ pub fn external_api() -> NexusApiDescription { api.register(image_view_by_id)?; api.register(image_delete)?; - api.register(instance_disk_list)?; - api.register(instance_disk_attach)?; - api.register(instance_disk_detach)?; - api.register(snapshot_list)?; api.register(snapshot_create)?; api.register(snapshot_view)?; @@ -2296,11 +2303,62 @@ async fn ip_pool_service_range_remove( // Disks +#[endpoint { + method = GET, + path = "/v1/disks", + tags = ["disks"], +}] +async fn disk_list_v1( + rqctx: Arc>>, + query_params: Query>, +) -> Result>, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let query = query_params.into_inner(); + let pag_params = data_page_params_for(&rqctx, &query)?; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let disks = match name_or_id_pagination(&query, &pag_params)? { + PaginatedBy::Id(pag_params, selector) => { + let project_lookup = nexus.project_lookup(&opctx, &selector)?; + nexus + .project_list_disks_by_id( + &opctx, + &project_lookup, + &pag_params, + ) + .await? + } + PaginatedBy::Name(pag_params, selector) => { + let project_lookup = nexus.project_lookup(&opctx, &selector)?; + nexus + .project_list_disks_by_name( + &opctx, + &project_lookup, + &pag_params.map_name(|n| Name::ref_cast(n)), + ) + .await? + } + } + .into_iter() + .map(|disk| disk.into()) + .collect(); + Ok(HttpResponseOk(ScanByNameOrId::results_page( + &query, + disks, + &marker_for_name_or_id, + )?)) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /// List disks +/// Use `GET /v1/disks` instead #[endpoint { method = GET, path = "/organizations/{organization_name}/projects/{project_name}/disks", - tags = ["disks"] + tags = ["disks"], + deprecated = true }] async fn disk_list( rqctx: Arc>>, @@ -2311,15 +2369,17 @@ async fn disk_list( let nexus = &apictx.nexus; let query = query_params.into_inner(); let path = path_params.into_inner(); - let organization_name = &path.organization_name; - let project_name = &path.project_name; + let project_selector = params::ProjectSelector::new( + Some(path.organization_name.into()), + path.project_name.into(), + ); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; + let authz_project = nexus.project_lookup(&opctx, &project_selector)?; let disks = nexus - .project_list_disks( + .project_list_disks_by_name( &opctx, - organization_name, - project_name, + &authz_project, &data_page_params_for(&rqctx, &query)? .map_name(|n| Name::ref_cast(n)), ) @@ -2337,11 +2397,37 @@ async fn disk_list( } /// Create a disk +#[endpoint { + method = POST, + path = "/v1/disks", + tags = ["disks"] +}] +async fn disk_create_v1( + rqctx: Arc>>, + query_params: Query, + new_disk: TypedBody, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let query = query_params.into_inner(); + let params = new_disk.into_inner(); + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let project_lookup = nexus.project_lookup(&opctx, &query)?; + let disk = + nexus.project_create_disk(&opctx, &project_lookup, ¶ms).await?; + Ok(HttpResponseCreated(disk.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + // TODO-correctness See note about instance create. This should be async. +/// Use `POST /v1/disks` instead #[endpoint { method = POST, path = "/organizations/{organization_name}/projects/{project_name}/disks", - tags = ["disks"] + tags = ["disks"], + deprecated = true }] async fn disk_create( rqctx: Arc>>, @@ -2351,24 +2437,49 @@ async fn disk_create( let apictx = rqctx.context(); let nexus = &apictx.nexus; let path = path_params.into_inner(); - let organization_name = &path.organization_name; - let project_name = &path.project_name; let new_disk_params = &new_disk.into_inner(); + let project_selector = params::ProjectSelector::new( + Some(path.organization_name.into()), + path.project_name.into(), + ); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; + let project_lookup = nexus.project_lookup(&opctx, &project_selector)?; let disk = nexus - .project_create_disk( - &opctx, - &organization_name, - &project_name, - &new_disk_params, - ) + .project_create_disk(&opctx, &project_lookup, &new_disk_params) .await?; Ok(HttpResponseCreated(disk.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +#[endpoint { + method = GET, + path = "/v1/disks/{disk}", + tags = ["disks"] +}] +async fn disk_view_v1( + rqctx: Arc>>, + path_params: Path, + query_params: Query, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let disk_selector = params::DiskSelector { + disk: path.disk, + project_selector: query.project_selector, + }; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let (.., disk) = + nexus.disk_lookup(&opctx, &disk_selector)?.fetch().await?; + Ok(HttpResponseOk(disk.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /// Path parameters for Disk requests #[derive(Deserialize, JsonSchema)] struct DiskPathParam { @@ -2378,10 +2489,12 @@ struct DiskPathParam { } /// Fetch a disk +/// Use `GET /v1/disks/{disk}` instead #[endpoint { method = GET, path = "/organizations/{organization_name}/projects/{project_name}/disks/{disk_name}", tags = ["disks"], + deprecated = true }] async fn disk_view( rqctx: Arc>>, @@ -2390,24 +2503,27 @@ async fn disk_view( let apictx = rqctx.context(); let nexus = &apictx.nexus; let path = path_params.into_inner(); - let organization_name = &path.organization_name; - let project_name = &path.project_name; - let disk_name = &path.disk_name; + let disk_selector = params::DiskSelector::new( + Some(path.organization_name.into()), + Some(path.project_name.into()), + path.disk_name.into(), + ); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let disk = nexus - .disk_fetch(&opctx, &organization_name, &project_name, &disk_name) - .await?; + let (.., disk) = + nexus.disk_lookup(&opctx, &disk_selector)?.fetch().await?; Ok(HttpResponseOk(disk.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } /// Fetch a disk by id +/// Use `GET /v1/disks/{disk}` instead #[endpoint { method = GET, path = "/by-id/disks/{id}", tags = ["disks"], + deprecated = true }] async fn disk_view_by_id( rqctx: Arc>>, @@ -2416,20 +2532,50 @@ async fn disk_view_by_id( let apictx = rqctx.context(); let nexus = &apictx.nexus; let path = path_params.into_inner(); - let id = &path.id; + let disk_selector = params::DiskSelector::new(None, None, path.id.into()); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let disk = nexus.disk_fetch_by_id(&opctx, id).await?; + let (.., disk) = + nexus.disk_lookup(&opctx, &disk_selector)?.fetch().await?; Ok(HttpResponseOk(disk.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } /// Delete a disk +#[endpoint { + method = DELETE, + path = "/v1/disks/{disk}", + tags = ["disks"], +}] +async fn disk_delete_v1( + rqctx: Arc>>, + path_params: Path, + query_params: Query, +) -> Result { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let disk_selector = params::DiskSelector { + disk: path.disk, + project_selector: query.project_selector, + }; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let disk_lookup = nexus.disk_lookup(&opctx, &disk_selector)?; + nexus.project_delete_disk(&disk_lookup).await?; + Ok(HttpResponseDeleted()) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Use `DELETE /v1/disks/{disk}` instead #[endpoint { method = DELETE, path = "/organizations/{organization_name}/projects/{project_name}/disks/{disk_name}", tags = ["disks"], + deprecated = true }] async fn disk_delete( rqctx: Arc>>, @@ -2438,19 +2584,15 @@ async fn disk_delete( let apictx = rqctx.context(); let nexus = &apictx.nexus; let path = path_params.into_inner(); - let organization_name = &path.organization_name; - let project_name = &path.project_name; - let disk_name = &path.disk_name; + let disk_selector = params::DiskSelector::new( + Some(path.organization_name.into()), + Some(path.project_name.into()), + path.disk_name.into(), + ); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - nexus - .project_delete_disk( - &opctx, - &organization_name, - &project_name, - &disk_name, - ) - .await?; + let disk_lookup = nexus.disk_lookup(&opctx, &disk_selector)?; + nexus.project_delete_disk(&disk_lookup).await?; Ok(HttpResponseDeleted()) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2483,28 +2625,25 @@ async fn disk_metrics_list( ) -> Result>, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.nexus; - let path = path_params.into_inner(); - let organization_name = &path.inner.organization_name; - let project_name = &path.inner.project_name; - let disk_name = &path.inner.disk_name; - let metric_name = path.metric_name; - let query = query_params.into_inner(); let limit = rqctx.page_limit(&query)?; - + let disk_selector = params::DiskSelector::new( + Some(path.inner.organization_name.into()), + Some(path.inner.project_name.into()), + path.inner.disk_name.into(), + ); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - - // This ensures the user is authorized on Action::Read for this disk - let disk = nexus - .disk_fetch(&opctx, organization_name, project_name, disk_name) + let (.., authz_disk) = nexus + .disk_lookup(&opctx, &disk_selector)? + .lookup_for(authz::Action::Read) .await?; - let upstairs_uuid = disk.id(); + let result = nexus .select_timeseries( - &format!("crucible_upstairs:{}", metric_name), - &[&format!("upstairs_uuid=={}", upstairs_uuid)], + &format!("crucible_upstairs:{}", path.metric_name), + &[&format!("upstairs_uuid=={}", authz_disk.id())], query, limit, ) @@ -3210,12 +3349,74 @@ async fn instance_serial_console_stream( Ok(()) } +#[endpoint { + method = GET, + path = "/v1/instances/{instance}/disks", + tags = ["instances"], +}] +async fn instance_disk_list_v1( + rqctx: Arc>>, + query_params: Query>, + path_params: Path, +) -> Result>, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let query = query_params.into_inner(); + let path = path_params.into_inner(); + let pag_params = data_page_params_for(&rqctx, &query)?; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let disks = match name_or_id_pagination(&query, &pag_params)? { + PaginatedBy::Id(pag_params, selector) => { + let instance_selector = params::InstanceSelector { + project_selector: selector.project_selector, + instance: path.instance, + }; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + nexus + .instance_list_disks_by_id( + &opctx, + &instance_lookup, + &pag_params, + ) + .await? + } + PaginatedBy::Name(pag_params, selector) => { + let instance_selector = params::InstanceSelector { + project_selector: selector.project_selector, + instance: path.instance, + }; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + nexus + .instance_list_disks_by_name( + &opctx, + &instance_lookup, + &pag_params.map_name(|n| Name::ref_cast(n)), + ) + .await? + } + } + .into_iter() + .map(|d| d.into()) + .collect(); + Ok(HttpResponseOk(ScanByNameOrId::results_page( + &query, + disks, + &marker_for_name_or_id, + )?)) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /// List an instance's disks -// TODO-scalability needs to be paginated +/// Use `GET /v1/instances/{instance}/disks` instead #[endpoint { method = GET, path = "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks", tags = ["instances"], + deprecated = true }] async fn instance_disk_list( rqctx: Arc>>, @@ -3226,17 +3427,19 @@ async fn instance_disk_list( let nexus = &apictx.nexus; let query = query_params.into_inner(); let path = path_params.into_inner(); - let organization_name = &path.organization_name; - let project_name = &path.project_name; - let instance_name = &path.instance_name; + let instance_selector = params::InstanceSelector::new( + Some(path.organization_name.into()), + Some(path.project_name.into()), + path.instance_name.into(), + ); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; let disks = nexus - .instance_list_disks( + .instance_list_disks_by_name( &opctx, - &organization_name, - &project_name, - &instance_name, + &instance_lookup, &data_page_params_for(&rqctx, &query)? .map_name(|n| Name::ref_cast(n)), ) @@ -3253,11 +3456,44 @@ async fn instance_disk_list( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +#[endpoint { + method = POST, + path = "/v1/instances/{instance}/disks/attach", + tags = ["instances"], +}] +async fn instance_disk_attach_v1( + rqctx: Arc>>, + path_params: Path, + query_params: Query, + disk_to_attach: TypedBody, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let disk = disk_to_attach.into_inner().disk; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let instance_selector = params::InstanceSelector { + project_selector: query.project_selector, + instance: path.instance, + }; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let disk = + nexus.instance_attach_disk(&opctx, &instance_lookup, disk).await?; + Ok(HttpResponseAccepted(disk.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /// Attach a disk to an instance +/// Use `POST /v1/instances/{instance}/disks/attach` instead #[endpoint { method = POST, path = "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/attach", tags = ["instances"], + deprecated = true }] async fn instance_disk_attach( rqctx: Arc>>, @@ -3267,30 +3503,62 @@ async fn instance_disk_attach( let apictx = rqctx.context(); let nexus = &apictx.nexus; let path = path_params.into_inner(); - let organization_name = &path.organization_name; - let project_name = &path.project_name; - let instance_name = &path.instance_name; + let disk = disk_to_attach.into_inner(); + let instance_selector = params::InstanceSelector::new( + Some(path.organization_name.clone().into()), + Some(path.project_name.clone().into()), + path.instance_name.into(), + ); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; let disk = nexus - .instance_attach_disk( - &opctx, - &organization_name, - &project_name, - &instance_name, - &disk_to_attach.into_inner().name.into(), - ) + .instance_attach_disk(&opctx, &instance_lookup, disk.name.into()) .await?; Ok(HttpResponseAccepted(disk.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +#[endpoint { + method = POST, + path = "/v1/instances/{instance}/disks/detach", + tags = ["instances"], +}] +async fn instance_disk_detach_v1( + rqctx: Arc>>, + path_params: Path, + query_params: Query, + disk_to_detach: TypedBody, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let disk = disk_to_detach.into_inner().disk; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let instance_selector = params::InstanceSelector { + project_selector: query.project_selector, + instance: path.instance, + }; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; + let disk = + nexus.instance_detach_disk(&opctx, &instance_lookup, disk).await?; + Ok(HttpResponseAccepted(disk.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /// Detach a disk from an instance +/// Use `POST /v1/disks/{disk}/detach` instead #[endpoint { method = POST, path = "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/detach", tags = ["instances"], + deprecated = true }] async fn instance_disk_detach( rqctx: Arc>>, @@ -3300,19 +3568,18 @@ async fn instance_disk_detach( let apictx = rqctx.context(); let nexus = &apictx.nexus; let path = path_params.into_inner(); - let organization_name = &path.organization_name; - let project_name = &path.project_name; - let instance_name = &path.instance_name; + let disk = disk_to_detach.into_inner(); + let instance_selector = params::InstanceSelector::new( + Some(path.organization_name.clone().into()), + Some(path.project_name.clone().into()), + path.instance_name.into(), + ); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; + let instance_lookup = + nexus.instance_lookup(&opctx, &instance_selector)?; let disk = nexus - .instance_detach_disk( - &opctx, - &organization_name, - &project_name, - &instance_name, - &disk_to_detach.into_inner().name.into(), - ) + .instance_detach_disk(&opctx, &instance_lookup, disk.name.into()) .await?; Ok(HttpResponseAccepted(disk.into())) }; diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs index ae631019d59..2ddacc27470 100644 --- a/nexus/tests/integration_tests/disks.rs +++ b/nexus/tests/integration_tests/disks.rs @@ -48,28 +48,36 @@ const PROJECT_NAME: &str = "springfield-squidport-disks"; const DISK_NAME: &str = "just-rainsticks"; const INSTANCE_NAME: &str = "just-rainsticks"; -fn get_project_url() -> String { - format!("/organizations/{}/projects/{}", ORG_NAME, PROJECT_NAME) -} - fn get_disks_url() -> String { - format!("{}/disks", get_project_url()) + format!("/v1/disks?organization={}&project={}", ORG_NAME, PROJECT_NAME) } -fn get_instances_url() -> String { - format!("{}/instances", get_project_url()) +fn get_disk_url(disk_name: &str) -> String { + format!( + "/v1/disks/{disk_name}?organization={}&project={}", + ORG_NAME, PROJECT_NAME + ) } fn get_instance_disks_url(instance_name: &str) -> String { - format!("{}/{}/disks", get_instances_url(), instance_name) + format!( + "/v1/instances/{instance_name}/disks?organization={}&project={}", + ORG_NAME, PROJECT_NAME + ) } fn get_disk_attach_url(instance_name: &str) -> String { - format!("{}/attach", get_instance_disks_url(instance_name)) + format!( + "/v1/instances/{instance_name}/disks/attach?organization={}&project={}", + ORG_NAME, PROJECT_NAME + ) } fn get_disk_detach_url(instance_name: &str) -> String { - format!("{}/detach", get_instance_disks_url(instance_name)) + format!( + "/v1/instances/{instance_name}/disks/detach?organization={}&project={}", + ORG_NAME, PROJECT_NAME + ) } async fn create_org_and_project(client: &ClientTestContext) -> Uuid { @@ -93,7 +101,7 @@ async fn test_disk_not_found_before_creation( assert_eq!(disks.len(), 0); // Make sure we get a 404 if we fetch one. - let disk_url = format!("{}/{}", disks_url, DISK_NAME); + let disk_url = get_disk_url(DISK_NAME); let error = NexusRequest::new( RequestBuilder::new(client, Method::GET, &disk_url) .expect_status(Some(StatusCode::NOT_FOUND)), @@ -128,10 +136,14 @@ async fn test_disk_not_found_before_creation( async fn set_instance_state( client: &ClientTestContext, - instance_url: &str, + instance_name: &str, state: &str, ) -> Instance { - let url = format!("{}/{}", instance_url, state); + let url = format!( + "/v1/instances/{instance_name}/{state}?organization={}&project={}", + ORG_NAME, PROJECT_NAME + ); + NexusRequest::new( RequestBuilder::new(client, Method::POST, &url) .body(None as Option<&serde_json::Value>) @@ -161,7 +173,7 @@ async fn test_disk_create_attach_detach_delete( let disks_url = get_disks_url(); // Create a disk. - let disk_url = format!("{}/{}", disks_url, DISK_NAME); + let disk_url = get_disk_url(DISK_NAME); let disk = create_disk(&client, ORG_NAME, PROJECT_NAME, DISK_NAME).await; assert_eq!(disk.identity.name, DISK_NAME); assert_eq!(disk.identity.description, "sells rainsticks"); @@ -198,12 +210,8 @@ async fn test_disk_create_attach_detach_delete( // // Instances must be stopped before disks can be attached - this // is an artificial limitation without hotplug support. - let instance1_url = format!( - "/organizations/{}/projects/{}/instances/{}", - ORG_NAME, PROJECT_NAME, INSTANCE_NAME - ); let instance_next = - set_instance_state(&client, &instance1_url, "stop").await; + set_instance_state(&client, INSTANCE_NAME, "stop").await; instance_simulate(nexus, &instance_next.identity.id).await; // Verify that there are no disks attached to the instance, and specifically @@ -305,7 +313,7 @@ async fn test_disk_create_disk_that_already_exists_fails( size: ByteCount::from_gibibytes_u32(1), }; let _ = create_disk(&client, ORG_NAME, PROJECT_NAME, DISK_NAME).await; - let disk_url = format!("{}/{}", disks_url, DISK_NAME); + let disk_url = get_disk_url(DISK_NAME); let disk = disk_get(&client, &disk_url).await; // Attempt to create a second disk with a conflicting name. @@ -340,7 +348,7 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { let disks_url = get_disks_url(); // Create a disk. - let disk_url = format!("{}/{}", disks_url, DISK_NAME); + let disk_url = get_disk_url(DISK_NAME); let disk = create_disk(client, ORG_NAME, PROJECT_NAME, DISK_NAME).await; // Create an instance to attach the disk. @@ -350,12 +358,8 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { // // Instances must be stopped before disks can be attached - this // is an artificial limitation without hotplug support. - let instance_url = format!( - "/organizations/{}/projects/{}/instances/{}", - ORG_NAME, PROJECT_NAME, INSTANCE_NAME - ); let instance_next = - set_instance_state(&client, &instance_url, "stop").await; + set_instance_state(&client, INSTANCE_NAME, "stop").await; instance_simulate(nexus, &instance_next.identity.id).await; // Verify that there are no disks attached to the instance, and specifically @@ -392,12 +396,7 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { // fail and the disk should remain attached to the first instance. let instance2 = create_instance(&client, ORG_NAME, PROJECT_NAME, "instance2").await; - let instance2_url = format!( - "/organizations/{}/projects/{}/instances/{}", - ORG_NAME, PROJECT_NAME, "instance2" - ); - let instance_next = - set_instance_state(&client, &instance2_url, "stop").await; + let instance_next = set_instance_state(&client, "instance2", "stop").await; instance_simulate(nexus, &instance_next.identity.id).await; let url_instance2_attach_disk = @@ -407,8 +406,8 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { let error: HttpErrorResponseBody = NexusRequest::new( RequestBuilder::new(client, Method::POST, &url_instance2_attach_disk) - .body(Some(¶ms::DiskIdentifier { - name: disk.identity.name.clone(), + .body(Some(¶ms::DiskPath { + disk: disk.identity.name.clone().into(), })) .expect_status(Some(StatusCode::BAD_REQUEST)), ) @@ -463,8 +462,8 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { // instance (the first one). let error: HttpErrorResponseBody = NexusRequest::new( RequestBuilder::new(client, Method::POST, &url_instance_attach_disk) - .body(Some(¶ms::DiskIdentifier { - name: disk.identity.name.clone(), + .body(Some(¶ms::DiskPath { + disk: disk.identity.name.clone().into(), })) .expect_status(Some(StatusCode::BAD_REQUEST)), ) @@ -1004,7 +1003,7 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { } // Delete the first disk, freeing up 7 gibibytes. - let disk_url = format!("{}/{}", disks_url, "disk-one"); + let disk_url = get_disk_url("disk-one"); NexusRequest::new( RequestBuilder::new(client, Method::DELETE, &disk_url) .expect_status(Some(StatusCode::NO_CONTENT)), @@ -1179,7 +1178,7 @@ async fn test_disk_metrics(cptestctx: &ControlPlaneTestContext) { // Whenever we grab this URL, get the surrounding few seconds of metrics. let metric_url = |metric_type: &str| { - let disk_url = format!("{}/{}", get_disks_url(), DISK_NAME); + let disk_url = format!("/organizations/{ORG_NAME}/projects/{PROJECT_NAME}/disks/{DISK_NAME}"); format!( "{disk_url}/metrics/{metric_type}?start_time={:?}&end_time={:?}", Utc::now() - chrono::Duration::seconds(2), @@ -1221,10 +1220,12 @@ async fn test_disk_metrics_paginated(cptestctx: &ControlPlaneTestContext) { create_org_and_project(client).await; create_disk(&client, ORG_NAME, PROJECT_NAME, DISK_NAME).await; create_instance_with_disk(client).await; + let disk_url = format!( + "/organizations/{ORG_NAME}/projects/{PROJECT_NAME}/disks/{DISK_NAME}" + ); for metric in &ALL_METRICS { - let collection_url = - format!("{}/{DISK_NAME}/metrics/{metric}", get_disks_url()); + let collection_url = format!("{}/metrics/{metric}", disk_url); let initial_params = format!( "start_time={:?}&end_time={:?}", Utc::now() - chrono::Duration::seconds(2), @@ -1296,7 +1297,7 @@ async fn disk_post( ) -> Disk { NexusRequest::new( RequestBuilder::new(client, Method::POST, url) - .body(Some(¶ms::DiskIdentifier { name: disk_name })) + .body(Some(¶ms::DiskPath { disk: disk_name.into() })) .expect_status(Some(StatusCode::ACCEPTED)), ) .authn_as(AuthnMode::PrivilegedUser) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index c08db6b02cb..fa83cd73dbb 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -104,7 +104,7 @@ lazy_static! { pub static ref DEMO_PROJECT_URL: String = format!("/v1/projects/{}?organization={}", *DEMO_PROJECT_NAME, *DEMO_ORG_NAME); pub static ref DEMO_PROJECT_SELECTOR: String = - format!("?organization={}&project={}", *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); + format!("organization={}&project={}", *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); pub static ref DEMO_PROJECT_POLICY_URL: String = format!("/v1/projects/{}/policy?organization={}", *DEMO_PROJECT_NAME, *DEMO_ORG_NAME); pub static ref DEMO_PROJECT_URL_DISKS: String = @@ -193,8 +193,11 @@ lazy_static! { // Disk used for testing pub static ref DEMO_DISK_NAME: Name = "demo-disk".parse().unwrap(); + // TODO: Once we can test a URL multiple times we should also a case to exercise authz for disks filtered by instances + pub static ref DEMO_DISKS_URL: String = + format!("/v1/disks?{}", *DEMO_PROJECT_SELECTOR); pub static ref DEMO_DISK_URL: String = - format!("{}/{}", *DEMO_PROJECT_URL_DISKS, *DEMO_DISK_NAME); + format!("/v1/disks/{}?{}", *DEMO_DISK_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_DISK_CREATE: params::DiskCreate = params::DiskCreate { identity: IdentityMetadataCreateParams { @@ -211,8 +214,10 @@ lazy_static! { }; pub static ref DEMO_DISK_METRICS_URL: String = format!( - "{}/metrics/activated?start_time={:?}&end_time={:?}", - *DEMO_DISK_URL, + "/organizations/{}/projects/{}/disks/{}/metrics/activated?start_time={:?}&end_time={:?}", + *DEMO_ORG_NAME, + *DEMO_PROJECT_NAME, + *DEMO_DISK_NAME, Utc::now(), Utc::now(), ); @@ -223,28 +228,30 @@ lazy_static! { lazy_static! { // Instance used for testing pub static ref DEMO_INSTANCE_NAME: Name = "demo-instance".parse().unwrap(); + pub static ref DEMO_INSTANCE_SELECTOR: String = format!("{}&instance={}", *DEMO_PROJECT_SELECTOR, *DEMO_INSTANCE_NAME); pub static ref DEMO_INSTANCE_URL: String = - format!("/v1/instances/{}?organization={}&project={}", *DEMO_INSTANCE_NAME, *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); + format!("/v1/instances/{}?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_INSTANCE_START_URL: String = - format!("/v1/instances/{}/start?organization={}&project={}", *DEMO_INSTANCE_NAME, *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); + format!("/v1/instances/{}/start?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_INSTANCE_STOP_URL: String = - format!("/v1/instances/{}/stop?organization={}&project={}", *DEMO_INSTANCE_NAME, *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); + format!("/v1/instances/{}/stop?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_INSTANCE_REBOOT_URL: String = - format!("/v1/instances/{}/reboot?organization={}&project={}", *DEMO_INSTANCE_NAME, *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); + format!("/v1/instances/{}/reboot?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_INSTANCE_MIGRATE_URL: String = - format!("/v1/instances/{}/migrate?organization={}&project={}", *DEMO_INSTANCE_NAME, *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); + format!("/v1/instances/{}/migrate?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_INSTANCE_SERIAL_URL: String = - format!("/v1/instances/{}/serial-console?organization={}&project={}", *DEMO_INSTANCE_NAME, *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); + format!("/v1/instances/{}/serial-console?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_INSTANCE_SERIAL_STREAM_URL: String = - format!("/v1/instances/{}/serial-console/stream?organization={}&project={}", *DEMO_INSTANCE_NAME, *DEMO_ORG_NAME, *DEMO_PROJECT_NAME); + format!("/v1/instances/{}/serial-console/stream?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); - // To be migrated... pub static ref DEMO_INSTANCE_DISKS_URL: String = - format!("/organizations/{}/projects/{}/instances/{}/disks", *DEMO_ORG_NAME, *DEMO_PROJECT_NAME, *DEMO_INSTANCE_NAME); + format!("/v1/instances/{}/disks?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_INSTANCE_DISKS_ATTACH_URL: String = - format!("{}/attach", *DEMO_INSTANCE_DISKS_URL); + format!("/v1/instances/{}/disks/attach?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_INSTANCE_DISKS_DETACH_URL: String = - format!("{}/detach", *DEMO_INSTANCE_DISKS_URL); + format!("/v1/instances/{}/disks/detach?{}", *DEMO_INSTANCE_NAME, *DEMO_PROJECT_SELECTOR); + + // To be migrated... pub static ref DEMO_INSTANCE_NICS_URL: String = format!("/organizations/{}/projects/{}/instances/{}/network-interfaces", *DEMO_ORG_NAME, *DEMO_PROJECT_NAME, *DEMO_INSTANCE_NAME); pub static ref DEMO_INSTANCE_EXTERNAL_IPS_URL: String = @@ -1111,7 +1118,7 @@ lazy_static! { /* Disks */ VerifyEndpoint { - url: &*DEMO_PROJECT_URL_DISKS, + url: &*DEMO_DISKS_URL, visibility: Visibility::Protected, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ @@ -1122,15 +1129,6 @@ lazy_static! { ], }, - VerifyEndpoint { - url: "/by-id/disks/{id}", - visibility: Visibility::Protected, - unprivileged_access: UnprivilegedAccess::None, - allowed_methods: vec![ - AllowedMethod::Get, - ], - }, - VerifyEndpoint { url: &*DEMO_DISK_URL, visibility: Visibility::Protected, @@ -1156,28 +1154,30 @@ lazy_static! { unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, - ], + ] }, + VerifyEndpoint { url: &*DEMO_INSTANCE_DISKS_ATTACH_URL, visibility: Visibility::Protected, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Post( - serde_json::to_value(params::DiskIdentifier { - name: DEMO_DISK_NAME.clone() + serde_json::to_value(params::DiskPath { + disk: DEMO_DISK_NAME.clone().into() }).unwrap() ) ], }, + VerifyEndpoint { url: &*DEMO_INSTANCE_DISKS_DETACH_URL, visibility: Visibility::Protected, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Post( - serde_json::to_value(params::DiskIdentifier { - name: DEMO_DISK_NAME.clone() + serde_json::to_value(params::DiskPath { + disk: DEMO_DISK_NAME.clone().into() }).unwrap() ) ], diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 2a60bb5b6be..8a15a4d339a 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -249,9 +249,9 @@ lazy_static! { }, // Create a Disk in the Project SetupReq::Post { - url: &*DEMO_PROJECT_URL_DISKS, + url: &*DEMO_DISKS_URL, body: serde_json::to_value(&*DEMO_DISK_CREATE).unwrap(), - id_routes: vec!["/by-id/disks/{id}"], + id_routes: vec!["/v1/disks/{id}"], }, // Create an Instance in the Project SetupReq::Post { diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index 65931358f7d..8c1b793a07f 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -1,11 +1,15 @@ API operations found with tag "disks" OPERATION ID URL PATH disk_create /organizations/{organization_name}/projects/{project_name}/disks +disk_create_v1 /v1/disks disk_delete /organizations/{organization_name}/projects/{project_name}/disks/{disk_name} +disk_delete_v1 /v1/disks/{disk} disk_list /organizations/{organization_name}/projects/{project_name}/disks +disk_list_v1 /v1/disks disk_metrics_list /organizations/{organization_name}/projects/{project_name}/disks/{disk_name}/metrics/{metric_name} disk_view /organizations/{organization_name}/projects/{project_name}/disks/{disk_name} disk_view_by_id /by-id/disks/{id} +disk_view_v1 /v1/disks/{disk} API operations found with tag "hidden" OPERATION ID URL PATH @@ -32,8 +36,11 @@ instance_create_v1 /v1/instances instance_delete /organizations/{organization_name}/projects/{project_name}/instances/{instance_name} instance_delete_v1 /v1/instances/{instance} instance_disk_attach /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/attach +instance_disk_attach_v1 /v1/instances/{instance}/disks/attach instance_disk_detach /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/detach +instance_disk_detach_v1 /v1/instances/{instance}/disks/detach instance_disk_list /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks +instance_disk_list_v1 /v1/instances/{instance}/disks instance_external_ip_list /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/external-ips instance_list /organizations/{organization_name}/projects/{project_name}/instances instance_list_v1 /v1/instances diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index 1f6005f6886..653d8046f04 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -1,7 +1,9 @@ API endpoints with no coverage in authz tests: organization_delete (delete "/organizations/{organization_name}") project_delete (delete "/organizations/{organization_name}/projects/{project_name}") +disk_delete (delete "/organizations/{organization_name}/projects/{project_name}/disks/{disk_name}") instance_delete (delete "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}") +disk_view_by_id (get "/by-id/disks/{id}") instance_view_by_id (get "/by-id/instances/{id}") organization_view_by_id (get "/by-id/organizations/{id}") project_view_by_id (get "/by-id/projects/{id}") @@ -11,8 +13,11 @@ organization_view (get "/organizations/{organization_n organization_policy_view (get "/organizations/{organization_name}/policy") project_list (get "/organizations/{organization_name}/projects") project_view (get "/organizations/{organization_name}/projects/{project_name}") +disk_list (get "/organizations/{organization_name}/projects/{project_name}/disks") +disk_view (get "/organizations/{organization_name}/projects/{project_name}/disks/{disk_name}") instance_list (get "/organizations/{organization_name}/projects/{project_name}/instances") instance_view (get "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}") +instance_disk_list (get "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks") instance_serial_console (get "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/serial-console") instance_serial_console_stream (get "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/serial-console/stream") project_policy_view (get "/organizations/{organization_name}/projects/{project_name}/policy") @@ -25,7 +30,10 @@ login_saml (post "/login/{silo_name}/saml/{provi logout (post "/logout") organization_create (post "/organizations") project_create (post "/organizations/{organization_name}/projects") +disk_create (post "/organizations/{organization_name}/projects/{project_name}/disks") instance_create (post "/organizations/{organization_name}/projects/{project_name}/instances") +instance_disk_attach (post "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/attach") +instance_disk_detach (post "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/detach") instance_migrate (post "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/migrate") instance_reboot (post "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/reboot") instance_start (post "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/start") diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index efcd31f2473..7a69b497096 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -18,6 +18,7 @@ use serde::{ use std::{net::IpAddr, str::FromStr}; use uuid::Uuid; +// TODO-v1: Post migration rename `*Path` to `*Identifier` #[derive(Deserialize, JsonSchema)] pub struct OrganizationPath { pub organization: NameOrId, @@ -33,7 +34,12 @@ pub struct InstancePath { pub instance: NameOrId, } -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq)] +#[derive(Serialize, Deserialize, JsonSchema)] +pub struct DiskPath { + pub disk: NameOrId, +} + +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)] pub struct OrganizationSelector { pub organization: NameOrId, } @@ -68,12 +74,33 @@ impl ProjectSelector { } } -#[derive(Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq)] pub struct OptionalProjectSelector { #[serde(flatten)] pub project_selector: Option, } +#[derive(Deserialize, JsonSchema)] +pub struct DiskSelector { + #[serde(flatten)] + pub project_selector: Option, + pub disk: NameOrId, +} + +impl DiskSelector { + pub fn new( + organization: Option, + project: Option, + disk: NameOrId, + ) -> Self { + DiskSelector { + project_selector: project + .map(|p| ProjectSelector::new(organization, p)), + disk, + } + } +} + #[derive(Deserialize, JsonSchema)] pub struct InstanceSelector { #[serde(flatten)] @@ -945,6 +972,7 @@ pub struct DiskCreate { pub size: ByteCount, } +/// TODO-v1: Delete this /// Parameters for the [`Disk`](omicron_common::api::external::Disk) to be /// attached or detached to an instance #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] @@ -952,6 +980,7 @@ pub struct DiskIdentifier { pub name: Name, } +/// TODO-v1: Delete this /// Parameters for the /// [`NetworkInterface`](omicron_common::api::external::NetworkInterface) to be /// attached or detached to an instance. diff --git a/openapi/nexus.json b/openapi/nexus.json index a1c546824e2..8446c1cead4 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -16,6 +16,7 @@ "disks" ], "summary": "Fetch a disk by id", + "description": "Use `GET /v1/disks/{disk}` instead", "operationId": "disk_view_by_id", "parameters": [ { @@ -45,7 +46,8 @@ "5XX": { "$ref": "#/components/responses/Error" } - } + }, + "deprecated": true } }, "/by-id/images/{id}": { @@ -1355,6 +1357,7 @@ "disks" ], "summary": "List disks", + "description": "Use `GET /v1/disks` instead", "operationId": "disk_list", "parameters": [ { @@ -1421,13 +1424,14 @@ "$ref": "#/components/responses/Error" } }, + "deprecated": true, "x-dropshot-pagination": true }, "post": { "tags": [ "disks" ], - "summary": "Create a disk", + "summary": "Use `POST /v1/disks` instead", "operationId": "disk_create", "parameters": [ { @@ -1476,7 +1480,8 @@ "5XX": { "$ref": "#/components/responses/Error" } - } + }, + "deprecated": true } }, "/organizations/{organization_name}/projects/{project_name}/disks/{disk_name}": { @@ -1485,6 +1490,7 @@ "disks" ], "summary": "Fetch a disk", + "description": "Use `GET /v1/disks/{disk}` instead", "operationId": "disk_view", "parameters": [ { @@ -1529,13 +1535,14 @@ "5XX": { "$ref": "#/components/responses/Error" } - } + }, + "deprecated": true }, "delete": { "tags": [ "disks" ], - "summary": "Delete a disk", + "summary": "Use `DELETE /v1/disks/{disk}` instead", "operationId": "disk_delete", "parameters": [ { @@ -1573,7 +1580,8 @@ "5XX": { "$ref": "#/components/responses/Error" } - } + }, + "deprecated": true } }, "/organizations/{organization_name}/projects/{project_name}/disks/{disk_name}/metrics/{metric_name}": { @@ -2145,6 +2153,7 @@ "instances" ], "summary": "List an instance's disks", + "description": "Use `GET /v1/instances/{instance}/disks` instead", "operationId": "instance_disk_list", "parameters": [ { @@ -2217,6 +2226,7 @@ "$ref": "#/components/responses/Error" } }, + "deprecated": true, "x-dropshot-pagination": true } }, @@ -2226,6 +2236,7 @@ "instances" ], "summary": "Attach a disk to an instance", + "description": "Use `POST /v1/instances/{instance}/disks/attach` instead", "operationId": "instance_disk_attach", "parameters": [ { @@ -2280,7 +2291,8 @@ "5XX": { "$ref": "#/components/responses/Error" } - } + }, + "deprecated": true } }, "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/detach": { @@ -2289,6 +2301,7 @@ "instances" ], "summary": "Detach a disk from an instance", + "description": "Use `POST /v1/disks/{disk}/detach` instead", "operationId": "instance_disk_detach", "parameters": [ { @@ -2343,7 +2356,8 @@ "5XX": { "$ref": "#/components/responses/Error" } - } + }, + "deprecated": true } }, "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/external-ips": { @@ -7323,6 +7337,220 @@ "x-dropshot-pagination": true } }, + "/v1/disks": { + "get": { + "tags": [ + "disks" + ], + "operationId": "disk_list_v1", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "organization", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/NameOrIdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DiskResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": true + }, + "post": { + "tags": [ + "disks" + ], + "summary": "Create a disk", + "operationId": "disk_create_v1", + "parameters": [ + { + "in": "query", + "name": "organization", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "project", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DiskCreate" + } + } + }, + "required": true + }, + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Disk" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/v1/disks/{disk}": { + "get": { + "tags": [ + "disks" + ], + "operationId": "disk_view_v1", + "parameters": [ + { + "in": "path", + "name": "disk", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "organization", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Disk" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "delete": { + "tags": [ + "disks" + ], + "summary": "Delete a disk", + "operationId": "disk_delete_v1", + "parameters": [ + { + "in": "path", + "name": "disk", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "organization", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "responses": { + "204": { + "description": "successful deletion" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/instances": { "get": { "tags": [ @@ -7539,6 +7767,204 @@ } } }, + "/v1/instances/{instance}/disks": { + "get": { + "tags": [ + "instances" + ], + "operationId": "instance_disk_list_v1", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "organization", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/NameOrIdSortMode" + } + }, + { + "in": "path", + "name": "instance", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DiskResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": true + } + }, + "/v1/instances/{instance}/disks/attach": { + "post": { + "tags": [ + "instances" + ], + "operationId": "instance_disk_attach_v1", + "parameters": [ + { + "in": "path", + "name": "instance", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "organization", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DiskPath" + } + } + }, + "required": true + }, + "responses": { + "202": { + "description": "successfully enqueued operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Disk" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/v1/instances/{instance}/disks/detach": { + "post": { + "tags": [ + "instances" + ], + "operationId": "instance_disk_detach_v1", + "parameters": [ + { + "in": "path", + "name": "instance", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "organization", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DiskPath" + } + } + }, + "required": true + }, + "responses": { + "202": { + "description": "successfully enqueued operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Disk" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/instances/{instance}/migrate": { "post": { "tags": [ @@ -9132,7 +9558,7 @@ ] }, "DiskIdentifier": { - "description": "Parameters for the [`Disk`](omicron_common::api::external::Disk) to be attached or detached to an instance", + "description": "TODO-v1: Delete this Parameters for the [`Disk`](omicron_common::api::external::Disk) to be attached or detached to an instance", "type": "object", "properties": { "name": { @@ -9143,6 +9569,17 @@ "name" ] }, + "DiskPath": { + "type": "object", + "properties": { + "disk": { + "$ref": "#/components/schemas/NameOrId" + } + }, + "required": [ + "disk" + ] + }, "DiskResultsPage": { "description": "A single page of results", "type": "object", @@ -10844,6 +11281,27 @@ "pattern": "^(?![0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$)^[a-z][a-z0-9-]*[a-zA-Z0-9]$", "maxLength": 63 }, + "NameOrId": { + "oneOf": [ + { + "title": "id", + "allOf": [ + { + "type": "string", + "format": "uuid" + } + ] + }, + { + "title": "name", + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + } + ] + }, "NetworkInterface": { "description": "A `NetworkInterface` represents a virtual network interface device.", "type": "object", @@ -13659,27 +14117,6 @@ "write", "write_bytes" ] - }, - "NameOrId": { - "oneOf": [ - { - "title": "id", - "allOf": [ - { - "type": "string", - "format": "uuid" - } - ] - }, - { - "title": "name", - "allOf": [ - { - "$ref": "#/components/schemas/Name" - } - ] - } - ] } } },