diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs index 3cfbfd47157..e1ca6035d85 100644 --- a/nexus/src/authz/api_resources.rs +++ b/nexus/src/authz/api_resources.rs @@ -479,3 +479,4 @@ impl ApiResourceError for ProjectChild { pub type Disk = ProjectChild; pub type Instance = ProjectChild; +pub type Vpc = ProjectChild; diff --git a/nexus/src/authz/mod.rs b/nexus/src/authz/mod.rs index 27d25396df6..cdc73768ecd 100644 --- a/nexus/src/authz/mod.rs +++ b/nexus/src/authz/mod.rs @@ -170,6 +170,7 @@ pub use api_resources::FleetChild; pub use api_resources::Instance; pub use api_resources::Organization; pub use api_resources::Project; +pub use api_resources::Vpc; pub use api_resources::FLEET; mod context; diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 39c1f00d0a0..591c677ef7d 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -2067,30 +2067,111 @@ impl DataStore { // VPCs + /// Fetches a Vpc from the database and returns both the database row + /// and an [`authz::Vpc`] for doing authz checks + /// + /// See [`DataStore::organization_lookup_noauthz()`] for intended use cases + /// and caveats. + // TODO-security See the note on organization_lookup_noauthz(). + async fn vpc_lookup_noauthz( + &self, + authz_project: &authz::Project, + vpc_name: &Name, + ) -> LookupResult<(authz::Vpc, Vpc)> { + use db::schema::vpc::dsl; + dsl::vpc + .filter(dsl::time_deleted.is_null()) + .filter(dsl::project_id.eq(authz_project.id())) + .filter(dsl::name.eq(vpc_name.clone())) + .select(Vpc::as_select()) + .first_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Vpc, + LookupType::ByName(vpc_name.as_str().to_owned()), + ), + ) + }) + .map(|d| { + ( + authz_project.child_generic( + ResourceType::Vpc, + d.id(), + LookupType::from(&vpc_name.0), + ), + d, + ) + }) + } + + /// Look up the id for a Vpc based on its name + /// + /// Returns an [`authz::Vpc`] (which makes the id available). + /// + /// Like the other "lookup_by_path()" functions, this function does no authz + /// checks. + pub async fn vpc_lookup_by_path( + &self, + organization_name: &Name, + project_name: &Name, + vpc_name: &Name, + ) -> LookupResult { + let authz_project = self + .project_lookup_by_path(organization_name, project_name) + .await?; + self.vpc_lookup_noauthz(&authz_project, vpc_name).await.map(|(v, _)| v) + } + + /// Lookup a Vpc by name and return the full database record, along + /// with an [`authz::Vpc`] for subsequent authorization checks + pub async fn vpc_fetch( + &self, + opctx: &OpContext, + authz_project: &authz::Project, + name: &Name, + ) -> LookupResult<(authz::Vpc, Vpc)> { + let (authz_vpc, db_vpc) = + self.vpc_lookup_noauthz(authz_project, name).await?; + opctx.authorize(authz::Action::Read, &authz_vpc).await?; + Ok((authz_vpc, db_vpc)) + } + pub async fn project_list_vpcs( &self, - project_id: &Uuid, + opctx: &OpContext, + authz_project: &authz::Project, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { - use db::schema::vpc::dsl; + opctx.authorize(authz::Action::ListChildren, authz_project).await?; + use db::schema::vpc::dsl; paginated(dsl::vpc, dsl::name, &pagparams) .filter(dsl::time_deleted.is_null()) - .filter(dsl::project_id.eq(*project_id)) + .filter(dsl::project_id.eq(authz_project.id())) .select(Vpc::as_select()) - .load_async(self.pool()) + .load_async(self.pool_authorized(opctx).await?) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } - pub async fn project_create_vpc(&self, vpc: Vpc) -> Result { + pub async fn project_create_vpc( + &self, + opctx: &OpContext, + authz_project: &authz::Project, + vpc: Vpc, + ) -> Result { use db::schema::vpc::dsl; + assert_eq!(authz_project.id(), vpc.project_id); + opctx.authorize(authz::Action::CreateChild, authz_project).await?; + + // TODO-correctness Shouldn't this use "insert_resource"? let name = vpc.name().clone(); let vpc = diesel::insert_into(dsl::vpc) .values(vpc) - .on_conflict(dsl::id) - .do_nothing() .returning(Vpc::as_returning()) .get_result_async(self.pool()) .await @@ -2105,29 +2186,30 @@ impl DataStore { pub async fn project_update_vpc( &self, - vpc_id: &Uuid, + opctx: &OpContext, + authz_vpc: &authz::Vpc, updates: VpcUpdate, - ) -> Result<(), Error> { - use db::schema::vpc::dsl; + ) -> UpdateResult { + opctx.authorize(authz::Action::Modify, authz_vpc).await?; + use db::schema::vpc::dsl; diesel::update(dsl::vpc) .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(*vpc_id)) + .filter(dsl::id.eq(authz_vpc.id())) .set(updates) - .execute_async(self.pool()) + .returning(Vpc::as_returning()) + .get_result_async(self.pool_authorized(opctx).await?) .await .map_err(|e| { public_error_from_diesel_pool( e, - ErrorHandler::NotFoundByLookup( - ResourceType::Vpc, - LookupType::ById(*vpc_id), - ), + ErrorHandler::NotFoundByResource(authz_vpc), ) - })?; - Ok(()) + }) } + // TODO-security TODO-cleanup Remove this function. Update callers to use + // vpc_lookup_by_path() or vpc_fetch() instead. pub async fn vpc_fetch_by_name( &self, project_id: &Uuid, @@ -2153,7 +2235,13 @@ impl DataStore { }) } - pub async fn project_delete_vpc(&self, vpc_id: &Uuid) -> DeleteResult { + pub async fn project_delete_vpc( + &self, + opctx: &OpContext, + authz_vpc: &authz::Vpc, + ) -> DeleteResult { + opctx.authorize(authz::Action::Delete, authz_vpc).await?; + use db::schema::vpc::dsl; // Note that we don't ensure the firewall rules are empty here, because @@ -2165,18 +2253,15 @@ impl DataStore { let now = Utc::now(); diesel::update(dsl::vpc) .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(*vpc_id)) + .filter(dsl::id.eq(authz_vpc.id())) .set(dsl::time_deleted.eq(now)) .returning(Vpc::as_returning()) - .get_result_async(self.pool()) + .get_result_async(self.pool_authorized(opctx).await?) .await .map_err(|e| { public_error_from_diesel_pool( e, - ErrorHandler::NotFoundByLookup( - ResourceType::Vpc, - LookupType::ById(*vpc_id), - ), + ErrorHandler::NotFoundByResource(authz_vpc), ) })?; Ok(()) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index b4b481f8601..e9d195e95ec 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1130,8 +1130,10 @@ async fn project_vpcs_get( let organization_name = &path.organization_name; let project_name = &path.project_name; let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; let vpcs = nexus .project_list_vpcs( + &opctx, &organization_name, &project_name, &data_page_params_for(&rqctx, &query)? @@ -1176,8 +1178,9 @@ async fn project_vpcs_get_vpc( let project_name = &path.project_name; let vpc_name = &path.vpc_name; let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; let vpc = nexus - .project_lookup_vpc(&organization_name, &project_name, &vpc_name) + .vpc_fetch(&opctx, &organization_name, &project_name, &vpc_name) .await?; Ok(HttpResponseOk(vpc.into())) }; @@ -1204,8 +1207,10 @@ async fn project_vpcs_post( let project_name = &path.project_name; let new_vpc_params = &new_vpc.into_inner(); let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; let vpc = nexus .project_create_vpc( + &opctx, &organization_name, &project_name, &new_vpc_params, @@ -1228,20 +1233,22 @@ async fn project_vpcs_put_vpc( rqctx: Arc>>, path_params: Path, updated_vpc: TypedBody, -) -> Result { +) -> Result, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.nexus; let path = path_params.into_inner(); let handler = async { - nexus + let opctx = OpContext::for_external_api(&rqctx).await?; + let newvpc = nexus .project_update_vpc( + &opctx, &path.organization_name, &path.project_name, &path.vpc_name, &updated_vpc.into_inner(), ) .await?; - Ok(HttpResponseUpdatedNoContent()) + Ok(HttpResponseOk(newvpc.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } @@ -1265,8 +1272,14 @@ async fn project_vpcs_delete_vpc( let project_name = &path.project_name; let vpc_name = &path.vpc_name; let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; nexus - .project_delete_vpc(&organization_name, &project_name, &vpc_name) + .project_delete_vpc( + &opctx, + &organization_name, + &project_name, + &vpc_name, + ) .await?; Ok(HttpResponseDeleted()) }; diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 4adb092b7d7..54bbb302e26 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -578,6 +578,7 @@ impl Nexus { // Create a default VPC associated with the project. let _ = self .project_create_vpc( + opctx, &organization_name, &new_project.identity.name.clone().into(), ¶ms::VpcCreate { @@ -1540,82 +1541,35 @@ impl Nexus { .map(|_| ()) } - /** - * Creates a new network interface for this instance - */ - pub async fn instance_create_network_interface( - &self, - organization_name: &Name, - project_name: &Name, - instance_name: &Name, - vpc_name: &Name, - subnet_name: &Name, - params: ¶ms::NetworkInterfaceCreate, - ) -> CreateResult { - let authz_instance = self - .db_datastore - .instance_lookup_by_path( - organization_name, - project_name, - instance_name, - ) - .await?; - let vpc = self - .db_datastore - .vpc_fetch_by_name(&authz_instance.project().id(), vpc_name) - .await?; - let subnet = self - .db_datastore - .vpc_subnet_fetch_by_name(&vpc.id(), subnet_name) - .await?; - - let mac = db::model::MacAddr::new()?; - - let interface_id = Uuid::new_v4(); - // Request an allocation - let ip = None; - let interface = db::model::IncompleteNetworkInterface::new( - interface_id, - authz_instance.id(), - // TODO-correctness: vpc_id here is used for name uniqueness. Should - // interface names be unique to the subnet's VPC or to the - // VPC associated with the instance's default interface? - vpc.id(), - subnet, - mac, - ip, - params.clone(), - ); - self.db_datastore.instance_create_network_interface(interface).await - } - pub async fn project_list_vpcs( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { - let project_id = self + let authz_project = self .db_datastore .project_lookup_by_path(organization_name, project_name) - .await? - .id(); - let vpcs = - self.db_datastore.project_list_vpcs(&project_id, pagparams).await?; + .await?; + let vpcs = self + .db_datastore + .project_list_vpcs(&opctx, &authz_project, pagparams) + .await?; Ok(vpcs) } pub async fn project_create_vpc( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, params: ¶ms::VpcCreate, ) -> CreateResult { - let project_id = self + let authz_project = self .db_datastore .project_lookup_by_path(organization_name, project_name) - .await? - .id(); + .await?; let vpc_id = Uuid::new_v4(); let system_router_id = Uuid::new_v4(); let default_route_id = Uuid::new_v4(); @@ -1666,11 +1620,14 @@ impl Nexus { self.db_datastore.router_create_route(route).await?; let vpc = db::model::Vpc::new( vpc_id, - project_id, + authz_project.id(), system_router_id, params.clone(), )?; - let vpc = self.db_datastore.project_create_vpc(vpc).await?; + let vpc = self + .db_datastore + .project_create_vpc(opctx, &authz_project, vpc) + .await?; // Allocate the first /64 sub-range from the requested or created // prefix. @@ -1698,23 +1655,26 @@ impl Nexus { ipv6_block, ); - // Create the subnet record in the database. Overlapping IP ranges should be translated - // into an internal error. That implies that there's already an existing VPC Subnet, but - // we're explicitly creating the _first_ VPC in the project. Something is wrong, and likely - // a bug in our code. + // Create the subnet record in the database. Overlapping IP ranges + // should be translated into an internal error. That implies that + // there's already an existing VPC Subnet, but we're explicitly creating + // the _first_ VPC in the project. Something is wrong, and likely a bug + // in our code. let _ = self.db_datastore.vpc_create_subnet(subnet).await.map_err(|err| { match err { SubnetError::OverlappingIpRange => { warn!( self.log, - "failed to create default VPC Subnet, found overlapping IP address ranges"; + "failed to create default VPC Subnet, \ + found overlapping IP address ranges"; "vpc_id" => ?vpc_id, "subnet_id" => ?default_subnet_id, "ipv4_block" => ?*defaults::DEFAULT_VPC_SUBNET_IPV4_BLOCK, "ipv6_block" => ?ipv6_block, ); external::Error::internal_error( - "Failed to create default VPC Subnet, found overlapping IP address ranges" + "Failed to create default VPC Subnet, \ + found overlapping IP address ranges" ) }, SubnetError::External(e) => e, @@ -1736,6 +1696,27 @@ impl Nexus { Ok(()) } + pub async fn vpc_fetch( + &self, + opctx: &OpContext, + organization_name: &Name, + project_name: &Name, + vpc_name: &Name, + ) -> LookupResult { + let authz_project = self + .db_datastore + .project_lookup_by_path(organization_name, project_name) + .await?; + Ok(self + .db_datastore + .vpc_fetch(opctx, &authz_project, vpc_name) + .await? + .1) + } + + // TODO-security TODO-cleanup Remove this function. Callers should use + // vpc_lookup_by_path() / vpc_fetch() instead, or we should create a more + // useful pattern for looking up records by path (e.g., *_fetch_by_path()). pub async fn project_lookup_vpc( &self, organization_name: &Name, @@ -1752,41 +1733,44 @@ impl Nexus { pub async fn project_update_vpc( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, vpc_name: &Name, params: ¶ms::VpcUpdate, - ) -> UpdateResult<()> { - let project_id = self - .db_datastore - .project_lookup_by_path(organization_name, project_name) - .await? - .id(); - let vpc = - self.db_datastore.vpc_fetch_by_name(&project_id, vpc_name).await?; - Ok(self + ) -> UpdateResult { + let authz_vpc = self .db_datastore - .project_update_vpc(&vpc.id(), params.clone().into()) - .await?) + .vpc_lookup_by_path(organization_name, project_name, vpc_name) + .await?; + self.db_datastore + .project_update_vpc(opctx, &authz_vpc, params.clone().into()) + .await } pub async fn project_delete_vpc( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, vpc_name: &Name, ) -> DeleteResult { - let vpc = self - .project_lookup_vpc(organization_name, project_name, vpc_name) + let authz_project = self + .db_datastore + .project_lookup_by_path(organization_name, project_name) + .await?; + let (authz_vpc, db_vpc) = self + .db_datastore + .vpc_fetch(opctx, &authz_project, vpc_name) .await?; // TODO: This should eventually use a saga to call the // networking subsystem to have it clean up the networking resources - self.db_datastore.vpc_delete_router(&vpc.system_router_id).await?; - self.db_datastore.project_delete_vpc(&vpc.id()).await?; + self.db_datastore.vpc_delete_router(&db_vpc.system_router_id).await?; + self.db_datastore.project_delete_vpc(opctx, &authz_vpc).await?; // Delete all firewall rules after deleting the VPC, to ensure no // firewall rules get added between rules deletion and VPC deletion. - self.db_datastore.vpc_delete_all_firewall_rules(&vpc.id()).await + self.db_datastore.vpc_delete_all_firewall_rules(&authz_vpc.id()).await } pub async fn vpc_list_firewall_rules( diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index ea004cd5a88..637b49e809e 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -2,6 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +use crate::http_testing::RequestBuilder; use crate::ControlPlaneTestContext; use super::http_testing::dropshot_compat::objects_post; @@ -151,14 +152,14 @@ pub async fn create_vpc( project_name: &str, vpc_name: &str, ) -> Vpc { - objects_post( + object_create( &client, format!( "/organizations/{}/projects/{}/vpcs", &organization_name, &project_name ) .as_str(), - params::VpcCreate { + ¶ms::VpcCreate { identity: IdentityMetadataCreateParams { name: vpc_name.parse().unwrap(), description: "vpc description".to_string(), @@ -179,25 +180,32 @@ pub async fn create_vpc_with_error( vpc_name: &str, status: StatusCode, ) -> HttpErrorResponseBody { - client - .make_request_error_body( + NexusRequest::new( + RequestBuilder::new( + client, Method::POST, format!( "/organizations/{}/projects/{}/vpcs", &organization_name, &project_name ) .as_str(), - params::VpcCreate { - identity: IdentityMetadataCreateParams { - name: vpc_name.parse().unwrap(), - description: String::from("vpc description"), - }, - ipv6_prefix: None, - dns_name: "abc".parse().unwrap(), - }, - status, ) - .await + .body(Some(¶ms::VpcCreate { + identity: IdentityMetadataCreateParams { + name: vpc_name.parse().unwrap(), + description: String::from("vpc description"), + }, + ipv6_prefix: None, + dns_name: "abc".parse().unwrap(), + })) + .expect_status(Some(status)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap() } pub async fn create_router( diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index cfa6f70b326..528eb2f9aa1 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -107,6 +107,11 @@ lazy_static! { url: &*DEMO_ORG_PROJECTS_URL, body: serde_json::to_value(&*DEMO_PROJECT_CREATE).unwrap(), }, + // Create a VPC in the Project + SetupReq { + url: &*DEMO_PROJECT_URL_VPCS, + body: serde_json::to_value(&*DEMO_VPC_CREATE).unwrap(), + }, // Create a Disk in the Project SetupReq { url: &*DEMO_PROJECT_URL_DISKS, @@ -141,6 +146,8 @@ lazy_static! { format!("{}/disks", *DEMO_PROJECT_URL); static ref DEMO_PROJECT_URL_INSTANCES: String = format!("{}/instances", *DEMO_PROJECT_URL); + static ref DEMO_PROJECT_URL_VPCS: String = + format!("{}/vpcs", *DEMO_PROJECT_URL); static ref DEMO_PROJECT_CREATE: params::ProjectCreate = params::ProjectCreate { identity: IdentityMetadataCreateParams { @@ -149,6 +156,20 @@ lazy_static! { }, }; + // VPC used for testing + static ref DEMO_VPC_NAME: Name = "demo-vpc".parse().unwrap(); + static ref DEMO_VPC_URL: String = + format!("{}/{}", *DEMO_PROJECT_URL_VPCS, *DEMO_VPC_NAME); + static ref DEMO_VPC_CREATE: params::VpcCreate = + params::VpcCreate { + identity: IdentityMetadataCreateParams { + name: DEMO_VPC_NAME.clone(), + description: "".parse().unwrap(), + }, + ipv6_prefix: None, + dns_name: DEMO_VPC_NAME.clone(), + }; + // Disk used for testing static ref DEMO_DISK_NAME: Name = "demo-disk".parse().unwrap(); static ref DEMO_DISK_URL: String = @@ -353,6 +374,36 @@ lazy_static! { ], }, + /* VPCs */ + VerifyEndpoint { + url: &*DEMO_PROJECT_URL_VPCS, + visibility: Visibility::Protected, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_VPC_CREATE).unwrap() + ), + ], + }, + + VerifyEndpoint { + url: &*DEMO_VPC_URL, + visibility: Visibility::Protected, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(¶ms::VpcUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + dns_name: None, + }).unwrap() + ), + AllowedMethod::Delete, + ], + }, + /* Disks */ VerifyEndpoint { diff --git a/nexus/tests/integration_tests/vpc_firewall.rs b/nexus/tests/integration_tests/vpc_firewall.rs index ef9d8245aa0..c70bffd0745 100644 --- a/nexus/tests/integration_tests/vpc_firewall.rs +++ b/nexus/tests/integration_tests/vpc_firewall.rs @@ -2,8 +2,15 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +use dropshot::test_util::object_get; use http::method::Method; use http::StatusCode; +use nexus_test_utils::http_testing::{AuthnMode, NexusRequest}; +use nexus_test_utils::resource_helpers::{ + create_organization, create_project, create_vpc, +}; +use nexus_test_utils::ControlPlaneTestContext; +use nexus_test_utils_macros::nexus_test; use omicron_common::api::external::{ IdentityMetadata, L4Port, L4PortRange, VpcFirewallRule, VpcFirewallRuleAction, VpcFirewallRuleDirection, VpcFirewallRuleFilter, @@ -15,14 +22,6 @@ use omicron_nexus::external_api::views::Vpc; use std::convert::TryFrom; use uuid::Uuid; -use dropshot::test_util::{object_delete, object_get}; - -use nexus_test_utils::resource_helpers::{ - create_organization, create_project, create_vpc, -}; -use nexus_test_utils::ControlPlaneTestContext; -use nexus_test_utils_macros::nexus_test; - #[nexus_test] async fn test_vpc_firewall(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; @@ -37,7 +36,13 @@ async fn test_vpc_firewall(cptestctx: &ControlPlaneTestContext) { // Each project has a default VPC. Make sure it has the default rules. let default_vpc_url = format!("{}/default", vpcs_url); - let default_vpc = object_get::(client, &default_vpc_url).await; + let default_vpc: Vpc = NexusRequest::object_get(client, &default_vpc_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); let default_vpc_firewall = format!("{}/firewall/rules", default_vpc_url); let rules = object_get::(client, &default_vpc_firewall) @@ -127,7 +132,14 @@ async fn test_vpc_firewall(cptestctx: &ControlPlaneTestContext) { .await; // Delete a VPC and ensure we can't read its firewall anymore - object_delete(client, format!("{}/{}", vpcs_url, other_vpc).as_str()).await; + NexusRequest::object_delete( + client, + format!("{}/{}", vpcs_url, other_vpc).as_str(), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); client .make_request_error( Method::GET, diff --git a/nexus/tests/integration_tests/vpcs.rs b/nexus/tests/integration_tests/vpcs.rs index 87b26e998eb..7322973750a 100644 --- a/nexus/tests/integration_tests/vpcs.rs +++ b/nexus/tests/integration_tests/vpcs.rs @@ -2,23 +2,24 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +use dropshot::test_util::ClientTestContext; +use dropshot::HttpErrorResponseBody; use http::method::Method; use http::StatusCode; -use omicron_common::api::external::IdentityMetadataCreateParams; -use omicron_common::api::external::IdentityMetadataUpdateParams; -use omicron_common::api::external::Ipv6Net; -use omicron_nexus::external_api::{params, views::Vpc}; - -use dropshot::test_util::object_get; -use dropshot::test_util::objects_list_page; -use dropshot::test_util::ClientTestContext; - +use nexus_test_utils::http_testing::AuthnMode; +use nexus_test_utils::http_testing::NexusRequest; +use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::identity_eq; +use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::resource_helpers::{ create_organization, create_project, create_vpc, create_vpc_with_error, }; use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; +use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_common::api::external::IdentityMetadataUpdateParams; +use omicron_common::api::external::Ipv6Net; +use omicron_nexus::external_api::{params, views::Vpc}; #[nexus_test] async fn test_vpcs(cptestctx: &ControlPlaneTestContext) { @@ -42,40 +43,48 @@ async fn test_vpcs(cptestctx: &ControlPlaneTestContext) { assert_eq!(vpcs[0].dns_name, "default"); let default_vpc = vpcs.remove(0); - /* Make sure we get a 404 if we fetch one. */ + /* Make sure we get a 404 if we fetch or delete one. */ let vpc_url = format!("{}/just-rainsticks", vpcs_url); - - let error = client - .make_request_error(Method::GET, &vpc_url, StatusCode::NOT_FOUND) - .await; - assert_eq!(error.message, "not found: vpc with name \"just-rainsticks\""); - - /* Ditto if we try to delete one. */ - let error = client - .make_request_error(Method::DELETE, &vpc_url, StatusCode::NOT_FOUND) - .await; - assert_eq!(error.message, "not found: vpc with name \"just-rainsticks\""); + for method in &[Method::GET, Method::DELETE] { + let error: HttpErrorResponseBody = NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + method.clone(), + &vpc_url, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!( + error.message, + "not found: vpc with name \"just-rainsticks\"" + ); + } /* * Make sure creating a VPC fails if we specify an IPv6 prefix that is * not a valid ULA range. */ let bad_prefix = Ipv6Net("2000:1000::/48".parse().unwrap()); - let _ = client - .make_request_error_body( - Method::POST, - &vpcs_url, - params::VpcCreate { + NexusRequest::new( + RequestBuilder::new(client, Method::POST, &vpcs_url) + .expect_status(Some(StatusCode::BAD_REQUEST)) + .body(Some(¶ms::VpcCreate { identity: IdentityMetadataCreateParams { name: "just-rainsticks".parse().unwrap(), description: String::from("vpc description"), }, ipv6_prefix: Some(bad_prefix), dns_name: "abc".parse().unwrap(), - }, - StatusCode::BAD_REQUEST, - ) - .await; + })), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); /* Create a VPC. */ let vpc_name = "just-rainsticks"; @@ -127,12 +136,24 @@ async fn test_vpcs(cptestctx: &ControlPlaneTestContext) { }, dns_name: Some("def".parse().unwrap()), }; - vpc_put(&client, &vpc_url, update_params).await; + let updated_vpc = vpc_put(&client, &vpc_url, update_params).await; + assert_eq!(updated_vpc.identity.name, "new-name"); + assert_eq!(updated_vpc.identity.description, "another description"); + assert_eq!(updated_vpc.dns_name, "def"); // fetching by old name fails - let error = client - .make_request_error(Method::GET, &vpc_url, StatusCode::NOT_FOUND) - .await; + let error: HttpErrorResponseBody = NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + Method::GET, + &vpc_url, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); assert_eq!(error.message, "not found: vpc with name \"just-rainsticks\""); // new url with new name @@ -145,15 +166,25 @@ async fn test_vpcs(cptestctx: &ControlPlaneTestContext) { assert_eq!(vpc.dns_name, "def"); /* Delete the VPC. */ - client - .make_request_no_body(Method::DELETE, &vpc_url, StatusCode::NO_CONTENT) + NexusRequest::object_delete(client, &vpc_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() .await .unwrap(); /* Now we expect a 404 on fetch */ - let error = client - .make_request_error(Method::GET, &vpc_url, StatusCode::NOT_FOUND) - .await; + let error: HttpErrorResponseBody = NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + Method::GET, + &vpc_url, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); assert_eq!(error.message, "not found: vpc with name \"new-name\""); /* And the list should be empty (aside from default VPC) again */ @@ -163,27 +194,31 @@ async fn test_vpcs(cptestctx: &ControlPlaneTestContext) { } async fn vpcs_list(client: &ClientTestContext, vpcs_url: &str) -> Vec { - objects_list_page::(client, vpcs_url).await.items + objects_list_page_authz::(client, vpcs_url).await.items } async fn vpc_get(client: &ClientTestContext, vpc_url: &str) -> Vpc { - object_get::(client, vpc_url).await + NexusRequest::object_get(client, vpc_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap() } async fn vpc_put( client: &ClientTestContext, vpc_url: &str, params: params::VpcUpdate, -) { - client - .make_request( - Method::PUT, - &vpc_url, - Some(params), - StatusCode::NO_CONTENT, - ) +) -> Vpc { + NexusRequest::object_put(client, vpc_url, Some(¶ms)) + .authn_as(AuthnMode::PrivilegedUser) + .execute() .await - .unwrap(); + .unwrap() + .parsed_body() + .unwrap() } fn vpcs_eq(vpc1: &Vpc, vpc2: &Vpc) { diff --git a/openapi/nexus.json b/openapi/nexus.json index a88e86dd099..1b415ad8301 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1889,8 +1889,15 @@ "required": true }, "responses": { - "204": { - "description": "resource updated" + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Vpc" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" diff --git a/tools/oxapi_demo b/tools/oxapi_demo index 94abf753fc9..4571c5c0e1e 100755 --- a/tools/oxapi_demo +++ b/tools/oxapi_demo @@ -260,7 +260,7 @@ function cmd_project_list_disks function cmd_project_list_vpcs { [[ $# != 2 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME" - do_curl "/organizations/$1/projects/$2/vpcs" + do_curl_authn "/organizations/$1/projects/$2/vpcs" } function cmd_instance_create_demo @@ -353,20 +353,19 @@ function cmd_vpc_create_demo { [[ $# != 4 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME DNS_NAME" mkjson name="$3" dns_name="$4" description="a vpc called $3" | - do_curl "/organizations/$1/projects/$2/vpcs" -X POST -T - + do_curl_authn "/organizations/$1/projects/$2/vpcs" -X POST -T - } - function cmd_vpc_get { [[ $# != 3 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME" - do_curl "/organizations/$1/projects/$2/vpcs/$3" + do_curl_authn "/organizations/$1/projects/$2/vpcs/$3" } function cmd_vpc_delete { [[ $# != 3 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME" - do_curl "/organizations/$1/projects/$2/vpcs/$3" -X DELETE + do_curl_authn "/organizations/$1/projects/$2/vpcs/$3" -X DELETE } function cmd_vpc_subnets_list