From f24f5a9406b3e9d77f4570ea55822c5d8f73f966 Mon Sep 17 00:00:00 2001 From: Sean Klein <sean@oxide.computer> Date: Fri, 6 Jan 2023 11:15:09 -0500 Subject: [PATCH 1/4] Fix ip pool policy for instance allocation --- nexus/src/authz/omicron.polar | 4 + nexus/tests/integration_tests/instances.rs | 97 ++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 7fbffba837..b7d04304b5 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -373,6 +373,10 @@ resource IpPoolList { has_relation(fleet: Fleet, "parent_fleet", ip_pool_list: IpPoolList) if ip_pool_list.fleet = fleet; +# Any authenticated user can create a child of a provided IP Pools. +# This is necessary to use the pools when provisioning instances. +has_permission(_actor: AuthenticatedActor, "create_child", _ip_pool: IpPool); + # Describes the policy for accessing "/system/images" (in the API) resource GlobalImageList { permissions = [ diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 4e9079b39d..f5ff9987f5 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -11,6 +11,9 @@ use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::resource_helpers::create_disk; use nexus_test_utils::resource_helpers::create_ip_pool; +use nexus_test_utils::resource_helpers::create_local_user; +use nexus_test_utils::resource_helpers::create_silo; +use nexus_test_utils::resource_helpers::grant_iam; use nexus_test_utils::resource_helpers::object_create; use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::resource_helpers::populate_ip_pool; @@ -26,9 +29,11 @@ use omicron_common::api::external::InstanceState; use omicron_common::api::external::Ipv4Net; use omicron_common::api::external::Name; use omicron_common::api::external::NetworkInterface; +use omicron_nexus::authz::SiloRole; use omicron_nexus::external_api::shared::IpKind; use omicron_nexus::external_api::shared::IpRange; use omicron_nexus::external_api::shared::Ipv4Range; +use omicron_nexus::external_api::shared::SiloIdentityMode; use omicron_nexus::external_api::views; use omicron_nexus::TestInterfaces as _; use omicron_nexus::{external_api::params, Nexus}; @@ -2889,6 +2894,98 @@ async fn test_instance_ephemeral_ip_from_correct_pool( ); } +#[nexus_test] +async fn test_instance_create_in_silo(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + + // Create a silo with a Collaborator User + let silo = + create_silo(&client, "authz", true, SiloIdentityMode::LocalOnly).await; + let user_id = create_local_user( + client, + &silo, + &"unpriv".parse().unwrap(), + params::UserPassword::InvalidPassword, + ) + .await + .id; + grant_iam( + client, + "/system/silos/authz", + SiloRole::Collaborator, + user_id, + AuthnMode::PrivilegedUser, + ) + .await; + + // Populate IP Pool + populate_ip_pool(&client, "default", None).await; + + // Create test organization and projects. + NexusRequest::objects_post( + client, + "/organizations", + ¶ms::OrganizationCreate { + identity: IdentityMetadataCreateParams { + name: ORGANIZATION_NAME.parse().unwrap(), + description: String::new(), + }, + }, + ) + .authn_as(AuthnMode::SiloUser(user_id)) + .execute() + .await + .expect("failed to create Organization") + .parsed_body::<views::Organization>() + .expect("failed to parse new Organization"); + NexusRequest::objects_post( + client, + &format!("/organizations/{ORGANIZATION_NAME}/projects"), + ¶ms::ProjectCreate { + identity: IdentityMetadataCreateParams { + name: PROJECT_NAME.parse().unwrap(), + description: String::new(), + }, + }, + ) + .authn_as(AuthnMode::SiloUser(user_id)) + .execute() + .await + .expect("failed to create Project") + .parsed_body::<views::Project>() + .expect("failed to parse new Project"); + + // Create an instance using the authorization granted to the collaborator + // Silo User. + let instance_params = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("ip-pool-test")).unwrap(), + description: String::from("instance to test IP Pool authz"), + }, + ncpus: InstanceCpuCount::try_from(2).unwrap(), + memory: ByteCount::from_gibibytes_u32(4), + hostname: String::from("inst"), + user_data: vec![], + network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, + external_ips: vec![params::ExternalIpCreate::Ephemeral { + pool_name: Some(Name::try_from(String::from("default")).unwrap()), + }], + disks: vec![], + start: true, + }; + let url_instances = format!( + "/organizations/{}/projects/{}/instances", + ORGANIZATION_NAME, PROJECT_NAME + ); + NexusRequest::objects_post(client, &url_instances, &instance_params) + .authn_as(AuthnMode::SiloUser(user_id)) + .execute() + .await + .expect("Failed to create instance") + .parsed_body::<Instance>() + .expect("Failed to parse instance"); +} + async fn instance_get( client: &ClientTestContext, instance_url: &str, From d55ac1067974c070f59cfe33f1c720a27600e5fa Mon Sep 17 00:00:00 2001 From: Sean Klein <sean@oxide.computer> Date: Fri, 6 Jan 2023 12:50:11 -0500 Subject: [PATCH 2/4] review feedback --- nexus/src/authz/omicron.polar | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index b7d04304b5..3808a33251 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -373,9 +373,10 @@ resource IpPoolList { has_relation(fleet: Fleet, "parent_fleet", ip_pool_list: IpPoolList) if ip_pool_list.fleet = fleet; -# Any authenticated user can create a child of a provided IP Pools. +# Any authenticated user can create a child of a provided IP Pool. # This is necessary to use the pools when provisioning instances. -has_permission(_actor: AuthenticatedActor, "create_child", _ip_pool: IpPool); +has_permission(actor: AuthenticatedActor, "create_child", ip_pool: IpPool) + if actor.silo.fleet = ip_pool.fleet; # Describes the policy for accessing "/system/images" (in the API) resource GlobalImageList { From a11fc2689fd8c7c262c64710c4fd712751183f91 Mon Sep 17 00:00:00 2001 From: Sean Klein <sean@oxide.computer> Date: Fri, 6 Jan 2023 14:08:31 -0500 Subject: [PATCH 3/4] Unwrap --- nexus/src/authz/omicron.polar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 3808a33251..aa0014e256 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -376,7 +376,7 @@ has_relation(fleet: Fleet, "parent_fleet", ip_pool_list: IpPoolList) # Any authenticated user can create a child of a provided IP Pool. # This is necessary to use the pools when provisioning instances. has_permission(actor: AuthenticatedActor, "create_child", ip_pool: IpPool) - if actor.silo.fleet = ip_pool.fleet; + if actor.silo.unwrap().fleet = ip_pool.fleet; # Describes the policy for accessing "/system/images" (in the API) resource GlobalImageList { From 42d3b9265c7d1918b6fb0bbce61b0db439f1fefa Mon Sep 17 00:00:00 2001 From: Sean Klein <sean@oxide.computer> Date: Fri, 6 Jan 2023 14:45:31 -0500 Subject: [PATCH 4/4] less unwrap? --- nexus/src/authz/omicron.polar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index aa0014e256..bb114fb981 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -376,7 +376,7 @@ has_relation(fleet: Fleet, "parent_fleet", ip_pool_list: IpPoolList) # Any authenticated user can create a child of a provided IP Pool. # This is necessary to use the pools when provisioning instances. has_permission(actor: AuthenticatedActor, "create_child", ip_pool: IpPool) - if actor.silo.unwrap().fleet = ip_pool.fleet; + if silo in actor.silo and silo.fleet = ip_pool.fleet; # Describes the policy for accessing "/system/images" (in the API) resource GlobalImageList {