From 021b4df8dab5e4a8039a975e6359cd248fc562ea Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 29 Nov 2022 22:56:32 -0500 Subject: [PATCH 1/2] Revise 322 PoC to have fewer query params --- common/src/api/external/mod.rs | 2 +- nexus/db-model/src/name.rs | 11 - nexus/src/app/instance.rs | 45 ++-- nexus/src/app/project.rs | 26 +- nexus/src/external_api/http_entrypoints.rs | 225 ++++++++--------- nexus/tests/integration_tests/instances.rs | 6 +- nexus/types/src/external_api/params.rs | 96 ++------ openapi/nexus.json | 273 ++++----------------- 8 files changed, 213 insertions(+), 471 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index bb2d0b2e788..7ae77794494 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -268,7 +268,7 @@ impl Name { } } -#[derive(Serialize, Deserialize, Display)] +#[derive(Serialize, Deserialize, Display, Clone)] #[display("{0}")] #[serde(untagged)] pub enum NameOrId { diff --git a/nexus/db-model/src/name.rs b/nexus/db-model/src/name.rs index 57235bdf011..96603530333 100644 --- a/nexus/db-model/src/name.rs +++ b/nexus/db-model/src/name.rs @@ -63,14 +63,3 @@ where String::from_sql(bytes)?.parse().map(Name).map_err(|e| e.into()) } } - -/// Newtype wrapper around [external::NameOrId]. This type isn't actually -/// stored in the database, but exists as a convenience for the API. -#[derive(JsonSchema, Serialize, Deserialize, RefCast, Display)] -#[serde(transparent)] -#[repr(transparent)] -#[display("{0}")] -pub struct NameOrId(pub external::NameOrId); - -NewtypeFrom! { () pub struct NameOrId(external::NameOrId); } -NewtypeDeref! { () pub struct NameOrId(external::NameOrId); } diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index c8b933d58f0..1289f9183c4 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -32,6 +32,7 @@ use omicron_common::api::external::InstanceState; 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::external::UpdateResult; use omicron_common::api::external::Vni; use omicron_common::api::internal::nexus; @@ -59,13 +60,16 @@ impl super::Nexus { instance_selector: params::InstanceSelector, ) -> LookupResult { match instance_selector { - params::InstanceSelector::InstanceId { instance_id } => { - Ok(instance_id) + params::InstanceSelector { instance: NameOrId::Id(id), .. } => { + // TODO: 400 if project or organization are present + Ok(id) } - params::InstanceSelector::InstanceAndProjectId { - instance_name, - project_id, + params::InstanceSelector { + instance: NameOrId::Name(instance_name), + project: Some(NameOrId::Id(project_id)), + .. } => { + // TODO: 400 if organization is present let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) .project_id(project_id) @@ -74,10 +78,10 @@ impl super::Nexus { .await?; Ok(authz_instance.id()) } - params::InstanceSelector::InstanceProjectAndOrgId { - instance_name, - project_name, - organization_id, + params::InstanceSelector { + instance: NameOrId::Name(instance_name), + project: Some(NameOrId::Name(project_name)), + organization: Some(NameOrId::Id(organization_id)), } => { let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) @@ -88,10 +92,10 @@ impl super::Nexus { .await?; Ok(authz_instance.id()) } - params::InstanceSelector::InstanceProjectAndOrg { - instance_name, - project_name, - organization_name, + params::InstanceSelector { + instance: NameOrId::Name(instance_name), + project: Some(NameOrId::Name(project_name)), + organization: Some(NameOrId::Name(organization_name)), } => { let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) @@ -102,14 +106,15 @@ impl super::Nexus { .await?; Ok(authz_instance.id()) } - params::InstanceSelector::None {} => Err(Error::InvalidRequest { + // TODO: Add a better error message + _ => Err(Error::InvalidRequest { message: " - Unable to resolve instance. Expected one of - - instance_id - - instance_name, project_id - - instance_name, project_name, organization_id - - instance_name, project_name, organization_name - " + Unable to resolve instance. Expected one of + - instance: Uuid + - instance: Name, project: Uuid + - instance: Name, project: Name, organization: Uuid + - instance: Name, project: Name, organization: Name + " .to_string(), }), } diff --git a/nexus/src/app/project.rs b/nexus/src/app/project.rs index c0eeb4c95dd..e5178a6537e 100644 --- a/nexus/src/app/project.rs +++ b/nexus/src/app/project.rs @@ -20,6 +20,7 @@ use omicron_common::api::external::Error; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; +use omicron_common::api::external::NameOrId; use omicron_common::api::external::UpdateResult; use uuid::Uuid; @@ -30,10 +31,13 @@ impl super::Nexus { project_selector: params::ProjectSelector, ) -> LookupResult { match project_selector { - params::ProjectSelector::ProjectId { project_id } => Ok(project_id), - params::ProjectSelector::ProjectAndOrgId { - project_name, - organization_id, + params::ProjectSelector { project: NameOrId::Id(id), .. } => { + // TODO: 400 if organization is present + Ok(id) + } + params::ProjectSelector { + project: NameOrId::Name(project_name), + organization: Some(NameOrId::Id(organization_id)), } => { let (.., authz_project) = LookupPath::new(opctx, &self.db_datastore) @@ -43,9 +47,9 @@ impl super::Nexus { .await?; Ok(authz_project.id()) } - params::ProjectSelector::ProjectAndOrg { - project_name, - organization_name, + params::ProjectSelector { + project: NameOrId::Name(project_name), + organization: Some(NameOrId::Name(organization_name)), } => { let (.., authz_project) = LookupPath::new(opctx, &self.db_datastore) @@ -55,12 +59,12 @@ impl super::Nexus { .await?; Ok(authz_project.id()) } - params::ProjectSelector::None {} => Err(Error::InvalidRequest { + _ => Err(Error::InvalidRequest { message: " Unable to resolve project. Expected one of - - project_id - - project_name, organization_id - - project_name, organization_name + - project: Uuid + - project: Name, organization: Uuid + - project: Name, organization: Name " .to_string(), }), diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 3199914d6f9..428e9192681 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -18,7 +18,6 @@ use crate::authz; use crate::context::OpContext; use crate::db; use crate::db::model::Name; -use crate::db::model::NameOrId; use crate::external_api::shared; use crate::ServerContext; use dropshot::ApiDescription; @@ -61,6 +60,7 @@ use omicron_common::api::external::Disk; use omicron_common::api::external::Error; use omicron_common::api::external::Instance; use omicron_common::api::external::InternalContext; +use omicron_common::api::external::NameOrId; use omicron_common::api::external::NetworkInterface; use omicron_common::api::external::RouterRoute; use omicron_common::api::external::RouterRouteCreateParams; @@ -2015,7 +2015,7 @@ struct InstanceListQueryParams { #[serde(flatten)] pagination: PaginatedByName, #[serde(flatten)] - selector: params::ProjectSelector, + selector: Option, } #[endpoint { @@ -2032,24 +2032,30 @@ async fn v1_instance_list( let query = query_params.into_inner(); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - let project_id = - nexus.project_lookup_id(&opctx, query.selector).await?; - let instances = nexus - .project_list_instances( - &opctx, - project_id, - &data_page_params_for(&rqctx, &query.pagination)? - .map_name(|n| Name::ref_cast(n)), - ) - .await? - .into_iter() - .map(|i| i.into()) - .collect(); - Ok(HttpResponseOk(ScanByName::results_page( - &query.pagination, - instances, - &marker_for_name, - )?)) + if let Some(selector) = query.selector { + let project_id = nexus.project_lookup_id(&opctx, selector).await?; + let instances = nexus + .project_list_instances( + &opctx, + project_id, + &data_page_params_for(&rqctx, &query.pagination)? + .map_name(|n| Name::ref_cast(n)), + ) + .await? + .into_iter() + .map(|i| i.into()) + .collect(); + Ok(HttpResponseOk(ScanByName::results_page( + &query.pagination, + instances, + &marker_for_name, + )?)) + } else { + Err(HttpError::for_bad_request( + Some("missing_param".to_string()), + "missing required query parameter: project".to_string(), + )) + } }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } @@ -2076,9 +2082,11 @@ async fn instance_list( let project_id = nexus .project_lookup_id( &opctx, - params::ProjectSelector::ProjectAndOrg { - project_name: project_name.clone().into(), - organization_name: organization_name.clone().into(), + params::ProjectSelector { + project: NameOrId::Name(project_name.clone().into()), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2102,6 +2110,12 @@ async fn instance_list( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +#[derive(Deserialize, JsonSchema)] +struct InstanceCreateParams { + #[serde(flatten)] + selector: params::ProjectSelector, +} + #[endpoint { method = POST, path = "/v1/instances", @@ -2109,7 +2123,7 @@ async fn instance_list( }] async fn v1_instance_create( rqctx: Arc>>, - query_params: Query, + query_params: Query, new_instance: TypedBody, ) -> Result, HttpError> { let apictx = rqctx.context(); @@ -2157,9 +2171,11 @@ async fn instance_create( let project_id = nexus .project_lookup_id( &opctx, - params::ProjectSelector::ProjectAndOrg { - project_name: project_name.clone().into(), - organization_name: organization_name.clone().into(), + params::ProjectSelector { + project: NameOrId::Name(project_name.clone().into()), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2186,7 +2202,7 @@ struct InstanceLookupPathParam { #[derive(Deserialize, JsonSchema)] struct InstanceQueryParams { #[serde(flatten)] - selector: params::ProjectSelector, + selector: Option, } #[endpoint { @@ -2208,7 +2224,7 @@ async fn v1_instance_view( let instance_id = nexus .instance_lookup_id( &opctx, - query.selector.to_instance_selector(path.instance.into()), + params::InstanceSelector::new(path.instance, &query.selector), ) .await?; let instance = nexus.instance_fetch(&opctx, &instance_id).await?; @@ -2246,17 +2262,12 @@ async fn instance_view( let instance_id = nexus .instance_lookup_id( &opctx, - params::InstanceSelector::InstanceProjectAndOrg { - instance_name: omicron_common::api::external::Name::from( - instance_name.clone(), - ), - project_name: omicron_common::api::external::Name::from( - project_name.clone(), - ), - organization_name: - omicron_common::api::external::Name::from( - organization_name.clone(), - ), + params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2307,7 +2318,7 @@ async fn v1_instance_delete( let instance_id = nexus .instance_lookup_id( &opctx, - query.selector.to_instance_selector(path.instance.into()), + params::InstanceSelector::new(path.instance, &query.selector), ) .await?; nexus.project_destroy_instance(&opctx, &instance_id).await?; @@ -2337,17 +2348,12 @@ async fn instance_delete( let instance_id = nexus .instance_lookup_id( &opctx, - params::InstanceSelector::InstanceProjectAndOrg { - instance_name: omicron_common::api::external::Name::from( - instance_name.clone(), - ), - project_name: omicron_common::api::external::Name::from( - project_name.clone(), - ), - organization_name: - omicron_common::api::external::Name::from( - organization_name.clone(), - ), + params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2379,7 +2385,7 @@ async fn v1_instance_migrate( let instance_id = nexus .instance_lookup_id( &opctx, - query.selector.to_instance_selector(path.instance.into()), + params::InstanceSelector::new(path.instance, &query.selector), ) .await?; let instance = nexus @@ -2418,17 +2424,12 @@ async fn instance_migrate( let instance_id = nexus .instance_lookup_id( &opctx, - params::InstanceSelector::InstanceProjectAndOrg { - instance_name: omicron_common::api::external::Name::from( - instance_name.clone(), - ), - project_name: omicron_common::api::external::Name::from( - project_name.clone(), - ), - organization_name: - omicron_common::api::external::Name::from( - organization_name.clone(), - ), + params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2463,7 +2464,7 @@ async fn v1_instance_reboot( let instance_id = nexus .instance_lookup_id( &opctx, - query.selector.to_instance_selector(path.instance.into()), + params::InstanceSelector::new(path.instance, &query.selector), ) .await?; let instance = nexus.instance_reboot(&opctx, instance_id).await?; @@ -2493,17 +2494,12 @@ async fn instance_reboot( let instance_id = nexus .instance_lookup_id( &opctx, - params::InstanceSelector::InstanceProjectAndOrg { - instance_name: omicron_common::api::external::Name::from( - instance_name.clone(), - ), - project_name: omicron_common::api::external::Name::from( - project_name.clone(), - ), - organization_name: - omicron_common::api::external::Name::from( - organization_name.clone(), - ), + params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2533,7 +2529,7 @@ async fn v1_instance_start( let instance_id = nexus .instance_lookup_id( &opctx, - query.selector.to_instance_selector(path.instance.into()), + params::InstanceSelector::new(path.instance, &query.selector), ) .await?; let instance = nexus.instance_start(&opctx, instance_id).await?; @@ -2563,17 +2559,12 @@ async fn instance_start( let instance_id = nexus .instance_lookup_id( &opctx, - params::InstanceSelector::InstanceProjectAndOrg { - instance_name: omicron_common::api::external::Name::from( - instance_name.clone(), - ), - project_name: omicron_common::api::external::Name::from( - project_name.clone(), - ), - organization_name: - omicron_common::api::external::Name::from( - organization_name.clone(), - ), + params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2602,7 +2593,7 @@ async fn v1_instance_stop( let instance_id = nexus .instance_lookup_id( &opctx, - query.selector.to_instance_selector(path.instance.into()), + params::InstanceSelector::new(path.instance, &query.selector), ) .await?; let instance = nexus.instance_stop(&opctx, instance_id).await?; @@ -2632,17 +2623,12 @@ async fn instance_stop( let instance_id = nexus .instance_lookup_id( &opctx, - params::InstanceSelector::InstanceProjectAndOrg { - instance_name: omicron_common::api::external::Name::from( - instance_name.clone(), - ), - project_name: omicron_common::api::external::Name::from( - project_name.clone(), - ), - organization_name: - omicron_common::api::external::Name::from( - organization_name.clone(), - ), + params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2655,7 +2641,7 @@ async fn instance_stop( #[derive(Deserialize, JsonSchema)] pub struct InstanceSerialConsoleParams { #[serde(flatten)] - selector: params::ProjectSelector, + selector: Option, #[serde(flatten)] pub console_params: params::InstanceSerialConsoleRequest, @@ -2680,7 +2666,7 @@ async fn v1_instance_serial_console( let instance_id = nexus .instance_lookup_id( &opctx, - query.selector.to_instance_selector(path.instance.into()), + params::InstanceSelector::new(path.instance, &query.selector), ) .await?; let console = nexus @@ -2717,17 +2703,12 @@ async fn instance_serial_console( let instance_id = nexus .instance_lookup_id( &opctx, - params::InstanceSelector::InstanceProjectAndOrg { - instance_name: omicron_common::api::external::Name::from( - instance_name.clone(), - ), - project_name: omicron_common::api::external::Name::from( - project_name.clone(), - ), - organization_name: - omicron_common::api::external::Name::from( - organization_name.clone(), - ), + params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; @@ -2762,7 +2743,7 @@ async fn v1_instance_serial_console_stream( let instance_id = nexus .instance_lookup_id( &opctx, - query.selector.to_instance_selector(path.instance.into()), + params::InstanceSelector::new(path.instance, &query.selector), ) .await?; nexus.instance_serial_console_stream(&opctx, conn, &instance_id).await?; @@ -2790,16 +2771,12 @@ async fn instance_serial_console_stream( let instance_id = nexus .instance_lookup_id( &opctx, - params::InstanceSelector::InstanceProjectAndOrg { - instance_name: omicron_common::api::external::Name::from( - instance_name.clone(), - ), - project_name: omicron_common::api::external::Name::from( - project_name.clone(), - ), - organization_name: omicron_common::api::external::Name::from( - organization_name.clone(), - ), + params::InstanceSelector { + instance: NameOrId::Name(instance_name.clone().into()), + project: Some(NameOrId::Name(project_name.clone().into())), + organization: Some(NameOrId::Name( + organization_name.clone().into(), + )), }, ) .await?; diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 5dec931188b..594c9573640 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -149,7 +149,7 @@ async fn test_v1_instance_access(cptestctx: &ControlPlaneTestContext) { let fetched_instance = instance_get( &client, format!( - "/v1/instances/{}?project_id={}", + "/v1/instances/{}?project={}", instance.identity.name, project.identity.id ) .as_str(), @@ -161,7 +161,7 @@ async fn test_v1_instance_access(cptestctx: &ControlPlaneTestContext) { let fetched_instance = instance_get( &client, format!( - "/v1/instances/{}?project_name={}&organization_id={}", + "/v1/instances/{}?project={}&organization={}", instance.identity.name, project.identity.name, org.identity.id ) .as_str(), @@ -173,7 +173,7 @@ async fn test_v1_instance_access(cptestctx: &ControlPlaneTestContext) { let fetched_instance = instance_get( &client, format!( - "/v1/instances/{}?project_name={}&organization_name={}", + "/v1/instances/{}?project={}&organization={}", instance.identity.name, project.identity.name, org.identity.name ) .as_str(), diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 151a71f0a5a..dd57a3935dc 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -19,87 +19,33 @@ use std::{net::IpAddr, str::FromStr}; use uuid::Uuid; #[derive(Deserialize, JsonSchema)] -#[serde(untagged)] -pub enum ProjectSelector { - ProjectId { - project_id: Uuid, - }, - ProjectAndOrgId { - project_name: Name, - organization_id: Uuid, - }, - ProjectAndOrg { - // FIXME: There's a bug in schemars or serde which causes project_name to be emitted twice - #[schemars(skip)] - project_name: Name, - organization_name: Name, - }, - None {}, +pub struct ProjectSelector { + pub project: NameOrId, + pub organization: Option, } -impl ProjectSelector { - pub fn to_instance_selector(self, instance: NameOrId) -> InstanceSelector { - match instance { - NameOrId::Id(instance_id) => { - InstanceSelector::InstanceId { instance_id } - } - NameOrId::Name(instance_name) => match self { - ProjectSelector::ProjectId { project_id } => { - InstanceSelector::InstanceAndProjectId { - instance_name, - project_id, - } - } - ProjectSelector::ProjectAndOrgId { - project_name, - organization_id, - } => InstanceSelector::InstanceProjectAndOrgId { - instance_name, - project_name, - organization_id, - }, - ProjectSelector::ProjectAndOrg { - project_name, - organization_name, - } => InstanceSelector::InstanceProjectAndOrg { - instance_name, - project_name, - organization_name, - }, - ProjectSelector::None {} => InstanceSelector::None {}, - }, +#[derive(Deserialize, JsonSchema)] +pub struct InstanceSelector { + pub instance: NameOrId, + pub project: Option, + pub organization: Option, +} + +impl InstanceSelector { + pub fn new( + instance: NameOrId, + project_selector: &Option, + ) -> InstanceSelector { + InstanceSelector { + instance, + organization: project_selector + .as_ref() + .and_then(|s| s.organization.clone()), + project: project_selector.as_ref().map(|s| s.project.clone()), } } } -#[derive(Deserialize, JsonSchema)] -#[serde(untagged)] -pub enum InstanceSelector { - InstanceId { - instance_id: Uuid, - }, - InstanceAndProjectId { - instance_name: Name, - project_id: Uuid, - }, - InstanceProjectAndOrgId { - // FIXME: There's a bug in schemars or serde which causes instance_name to be emitted multiple times - #[schemars(skip)] - instance_name: Name, - project_name: Name, - organization_id: Uuid, - }, - InstanceProjectAndOrg { - #[schemars(skip)] - instance_name: Name, - // FIXME: There's a bug in schemars or serde which causes project_name to be emitted multiple times - #[schemars(skip)] - project_name: Name, - organization_name: Name, - }, - None {}, -} - // Silos /// Create-time parameters for a [`Silo`](crate::external_api::views::Silo) diff --git a/openapi/nexus.json b/openapi/nexus.json index 3f6ad4d8e31..a0769ea7fac 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -7675,53 +7675,35 @@ }, { "in": "query", - "name": "page_token", - "description": "Token returned by previous call to retrieve the subsequent page", - "schema": { - "nullable": true, - "type": "string" - }, - "style": "form" - }, - { - "in": "query", - "name": "sort_by", - "schema": { - "$ref": "#/components/schemas/NameSortMode" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_id", + "name": "organization", "schema": { - "type": "string", - "format": "uuid" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_id", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", "schema": { - "type": "string", - "format": "uuid" + "nullable": true, + "type": "string" }, "style": "form" }, { "in": "query", - "name": "project_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "sort_by", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameSortMode" }, "style": "form" } @@ -7754,35 +7736,18 @@ "parameters": [ { "in": "query", - "name": "project_id", + "name": "organization", "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", - "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", + "required": true, "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" } @@ -7826,35 +7791,17 @@ "parameters": [ { "in": "query", - "name": "project_id", + "name": "organization", "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", - "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, @@ -7896,35 +7843,17 @@ "parameters": [ { "in": "query", - "name": "project_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", + "name": "organization", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, @@ -7961,35 +7890,17 @@ "parameters": [ { "in": "query", - "name": "project_id", + "name": "organization", "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", - "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, @@ -8043,35 +7954,17 @@ "parameters": [ { "in": "query", - "name": "project_id", + "name": "organization", "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", - "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, @@ -8161,35 +8054,17 @@ }, { "in": "query", - "name": "project_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", + "name": "organization", "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", - "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" } @@ -8233,35 +8108,17 @@ }, { "in": "query", - "name": "project_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", + "name": "organization", "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", - "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" } @@ -8289,35 +8146,17 @@ "parameters": [ { "in": "query", - "name": "project_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", + "name": "organization", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, @@ -8361,35 +8200,17 @@ "parameters": [ { "in": "query", - "name": "project_id", - "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "organization_id", + "name": "organization", "schema": { - "type": "string", - "format": "uuid" - }, - "style": "form" - }, - { - "in": "query", - "name": "project_name", - "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, { "in": "query", - "name": "organization_name", + "name": "project", "schema": { - "$ref": "#/components/schemas/Name" + "$ref": "#/components/schemas/NameOrId" }, "style": "form" }, From 613275e6859ba33ae01bfaaa5bf419e6c9307adb Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 29 Nov 2022 23:12:43 -0500 Subject: [PATCH 2/2] Ensure list requires project --- nexus/src/external_api/http_entrypoints.rs | 44 ++++++++++------------ openapi/nexus.json | 1 + 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 428e9192681..4cbc23ad709 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -2015,7 +2015,7 @@ struct InstanceListQueryParams { #[serde(flatten)] pagination: PaginatedByName, #[serde(flatten)] - selector: Option, + selector: params::ProjectSelector, } #[endpoint { @@ -2032,30 +2032,24 @@ async fn v1_instance_list( let query = query_params.into_inner(); let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; - if let Some(selector) = query.selector { - let project_id = nexus.project_lookup_id(&opctx, selector).await?; - let instances = nexus - .project_list_instances( - &opctx, - project_id, - &data_page_params_for(&rqctx, &query.pagination)? - .map_name(|n| Name::ref_cast(n)), - ) - .await? - .into_iter() - .map(|i| i.into()) - .collect(); - Ok(HttpResponseOk(ScanByName::results_page( - &query.pagination, - instances, - &marker_for_name, - )?)) - } else { - Err(HttpError::for_bad_request( - Some("missing_param".to_string()), - "missing required query parameter: project".to_string(), - )) - } + let project_id = + nexus.project_lookup_id(&opctx, query.selector).await?; + let instances = nexus + .project_list_instances( + &opctx, + project_id, + &data_page_params_for(&rqctx, &query.pagination)? + .map_name(|n| Name::ref_cast(n)), + ) + .await? + .into_iter() + .map(|i| i.into()) + .collect(); + Ok(HttpResponseOk(ScanByName::results_page( + &query.pagination, + instances, + &marker_for_name, + )?)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } diff --git a/openapi/nexus.json b/openapi/nexus.json index a0769ea7fac..8e730cb17be 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -7694,6 +7694,7 @@ { "in": "query", "name": "project", + "required": true, "schema": { "$ref": "#/components/schemas/NameOrId" },