Skip to content

Commit

Permalink
[nexus] Endpoint to list IP pools for silo, add is_default to silo-…
Browse files Browse the repository at this point in the history
…scoped IP pools list (#4843)

Fixes #4752
Fixes #4763 

The main trick here is introducing `views::SiloIpPool`, which is the
same as `views::IpPool` except it also has `is_default` on it. It only
makes sense in the context of a particular silo because `is_default` is
only defined for a (pool, silo) pair, not for a pool alone.

- [x] Add `GET /v1/system/silos/{silo}/ip-pools`
- [x] `/v1/ip-pools` and `/v1/ip-pools/{pool}` should return
`SiloIpPool` too
- [x] Tests for `/v1/system/silos/{silo}/ip-pools`
- [x] We can't have both `SiloIpPool` and `IpPoolSilo`, cleaned up by
changing:
  - `views::IpPoolSilo` -> `views::SiloIpSiloLink`
  - `params::IpPoolSiloLink` -> `views::IpPoolLinkSilo`
  • Loading branch information
david-crespo authored Jan 20, 2024
1 parent d8bbf6d commit 205382f
Show file tree
Hide file tree
Showing 13 changed files with 431 additions and 137 deletions.
4 changes: 2 additions & 2 deletions end-to-end-tests/src/bin/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use end_to_end_tests::helpers::{generate_name, get_system_ip_pool};
use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError};
use oxide_client::types::{
ByteCount, DeviceAccessTokenRequest, DeviceAuthRequest, DeviceAuthVerify,
DiskCreate, DiskSource, IpPoolCreate, IpPoolSiloLink, IpRange, Ipv4Range,
DiskCreate, DiskSource, IpPoolCreate, IpPoolLinkSilo, IpRange, Ipv4Range,
NameOrId, SiloQuotasUpdate,
};
use oxide_client::{
Expand Down Expand Up @@ -51,7 +51,7 @@ async fn main() -> Result<()> {
client
.ip_pool_silo_link()
.pool(pool_name)
.body(IpPoolSiloLink {
.body(IpPoolLinkSilo {
silo: NameOrId::Name(params.silo_name().parse().unwrap()),
is_default: true,
})
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-model/src/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub struct IpPoolResource {
pub is_default: bool,
}

impl From<IpPoolResource> for views::IpPoolSilo {
impl From<IpPoolResource> for views::IpPoolSiloLink {
fn from(assoc: IpPoolResource) -> Self {
Self {
ip_pool_id: assoc.ip_pool_id,
Expand Down
86 changes: 40 additions & 46 deletions nexus/db-queries/src/db/datastore/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,47 +79,6 @@ impl DataStore {
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}

/// List IP pools linked to the current silo
pub async fn silo_ip_pools_list(
&self,
opctx: &OpContext,
pagparams: &PaginatedBy<'_>,
) -> ListResultVec<db::model::IpPool> {
use db::schema::ip_pool;
use db::schema::ip_pool_resource;

// From the developer user's point of view, we treat IP pools linked to
// their silo as silo resources, so they can list them if they can list
// silo children
let authz_silo =
opctx.authn.silo_required().internal_context("listing IP pools")?;
opctx.authorize(authz::Action::ListChildren, &authz_silo).await?;

let silo_id = authz_silo.id();

match pagparams {
PaginatedBy::Id(pagparams) => {
paginated(ip_pool::table, ip_pool::id, pagparams)
}
PaginatedBy::Name(pagparams) => paginated(
ip_pool::table,
ip_pool::name,
&pagparams.map_name(|n| Name::ref_cast(n)),
),
}
.inner_join(ip_pool_resource::table)
.filter(
ip_pool_resource::resource_type
.eq(IpPoolResourceType::Silo)
.and(ip_pool_resource::resource_id.eq(silo_id)),
)
.filter(ip_pool::time_deleted.is_null())
.select(db::model::IpPool::as_select())
.get_results_async(&*self.pool_connection_authorized(opctx).await?)
.await
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}

/// Look up whether the given pool is available to users in the current
/// silo, i.e., whether there is an entry in the association table linking
/// the pool with that silo
Expand Down Expand Up @@ -400,6 +359,37 @@ impl DataStore {
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}

/// Returns (IpPool, IpPoolResource) so we can know in the calling code
/// whether the pool is default for the silo
pub async fn silo_ip_pool_list(
&self,
opctx: &OpContext,
authz_silo: &authz::Silo,
pagparams: &PaginatedBy<'_>,
) -> ListResultVec<(IpPool, IpPoolResource)> {
use db::schema::ip_pool;
use db::schema::ip_pool_resource;

match pagparams {
PaginatedBy::Id(pagparams) => {
paginated(ip_pool::table, ip_pool::id, pagparams)
}
PaginatedBy::Name(pagparams) => paginated(
ip_pool::table,
ip_pool::name,
&pagparams.map_name(|n| Name::ref_cast(n)),
),
}
.inner_join(ip_pool_resource::table)
.filter(ip_pool_resource::resource_id.eq(authz_silo.id()))
.filter(ip_pool_resource::resource_type.eq(IpPoolResourceType::Silo))
.filter(ip_pool::time_deleted.is_null())
.select(<(IpPool, IpPoolResource)>::as_select())
.load_async(&*self.pool_connection_authorized(opctx).await?)
.await
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}

pub async fn ip_pool_link_silo(
&self,
opctx: &OpContext,
Expand Down Expand Up @@ -867,8 +857,11 @@ mod test {
.await
.expect("Should list IP pools");
assert_eq!(all_pools.len(), 0);

let authz_silo = opctx.authn.silo_required().unwrap();

let silo_pools = datastore
.silo_ip_pools_list(&opctx, &pagbyid)
.silo_ip_pool_list(&opctx, &authz_silo, &pagbyid)
.await
.expect("Should list silo IP pools");
assert_eq!(silo_pools.len(), 0);
Expand All @@ -893,7 +886,7 @@ mod test {
.expect("Should list IP pools");
assert_eq!(all_pools.len(), 1);
let silo_pools = datastore
.silo_ip_pools_list(&opctx, &pagbyid)
.silo_ip_pool_list(&opctx, &authz_silo, &pagbyid)
.await
.expect("Should list silo IP pools");
assert_eq!(silo_pools.len(), 0);
Expand Down Expand Up @@ -929,11 +922,12 @@ mod test {

// now it shows up in the silo list
let silo_pools = datastore
.silo_ip_pools_list(&opctx, &pagbyid)
.silo_ip_pool_list(&opctx, &authz_silo, &pagbyid)
.await
.expect("Should list silo IP pools");
assert_eq!(silo_pools.len(), 1);
assert_eq!(silo_pools[0].id(), pool1_for_silo.id());
assert_eq!(silo_pools[0].0.id(), pool1_for_silo.id());
assert_eq!(silo_pools[0].1.is_default, false);

// linking an already linked silo errors due to PK conflict
let err = datastore
Expand Down Expand Up @@ -998,7 +992,7 @@ mod test {

// and silo pools list is empty again
let silo_pools = datastore
.silo_ip_pools_list(&opctx, &pagbyid)
.silo_ip_pool_list(&opctx, &authz_silo, &pagbyid)
.await
.expect("Should list silo IP pools");
assert_eq!(silo_pools.len(), 0);
Expand Down
47 changes: 38 additions & 9 deletions nexus/src/app/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use omicron_common::api::external::CreateResult;
use omicron_common::api::external::DataPageParams;
use omicron_common::api::external::DeleteResult;
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;
Expand Down Expand Up @@ -74,12 +75,20 @@ impl super::Nexus {
}

/// List IP pools in current silo
pub(crate) async fn silo_ip_pools_list(
pub(crate) async fn current_silo_ip_pool_list(
&self,
opctx: &OpContext,
pagparams: &PaginatedBy<'_>,
) -> ListResultVec<db::model::IpPool> {
self.db_datastore.silo_ip_pools_list(opctx, pagparams).await
) -> ListResultVec<(db::model::IpPool, db::model::IpPoolResource)> {
let authz_silo =
opctx.authn.silo_required().internal_context("listing IP pools")?;

// From the developer user's point of view, we treat IP pools linked to
// their silo as silo resources, so they can list them if they can list
// silo children
opctx.authorize(authz::Action::ListChildren, &authz_silo).await?;

self.db_datastore.silo_ip_pool_list(opctx, &authz_silo, pagparams).await
}

// Look up pool by name or ID, but only return it if it's linked to the
Expand All @@ -88,19 +97,19 @@ impl super::Nexus {
&'a self,
opctx: &'a OpContext,
pool: &'a NameOrId,
) -> LookupResult<db::model::IpPool> {
) -> LookupResult<(db::model::IpPool, db::model::IpPoolResource)> {
let (authz_pool, pool) =
self.ip_pool_lookup(opctx, pool)?.fetch().await?;

// 404 if no link is found in the current silo
let link = self.db_datastore.ip_pool_fetch_link(opctx, pool.id()).await;
if link.is_err() {
return Err(authz_pool.not_found());
match link {
Ok(link) => Ok((pool, link)),
Err(_) => Err(authz_pool.not_found()),
}

Ok(pool)
}

/// List silos for a given pool
pub(crate) async fn ip_pool_silo_list(
&self,
opctx: &OpContext,
Expand All @@ -109,14 +118,34 @@ impl super::Nexus {
) -> ListResultVec<db::model::IpPoolResource> {
let (.., authz_pool) =
pool_lookup.lookup_for(authz::Action::ListChildren).await?;

// check ability to list silos in general
opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?;

self.db_datastore.ip_pool_silo_list(opctx, &authz_pool, pagparams).await
}

// List pools for a given silo
pub(crate) async fn silo_ip_pool_list(
&self,
opctx: &OpContext,
silo_lookup: &lookup::Silo<'_>,
pagparams: &PaginatedBy<'_>,
) -> ListResultVec<(db::model::IpPool, db::model::IpPoolResource)> {
let (.., authz_silo) =
silo_lookup.lookup_for(authz::Action::Read).await?;
// check ability to list pools in general
opctx
.authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST)
.await?;
self.db_datastore.silo_ip_pool_list(opctx, &authz_silo, pagparams).await
}

pub(crate) async fn ip_pool_link_silo(
&self,
opctx: &OpContext,
pool_lookup: &lookup::IpPool<'_>,
silo_link: &params::IpPoolSiloLink,
silo_link: &params::IpPoolLinkSilo,
) -> CreateResult<db::model::IpPoolResource> {
let (authz_pool,) =
pool_lookup.lookup_for(authz::Action::Modify).await?;
Expand Down
Loading

0 comments on commit 205382f

Please sign in to comment.