Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add db structures for Oxide service IP pools #1531

Merged
merged 10 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions common/src/sql/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,14 @@ CREATE TABLE omicron.public.ip_pool (
/* Optional ID of the project for which this pool is reserved. */
project_id UUID,

/*
* Optional rack ID, indicating this is a reserved pool for internal
* services on a specific rack.
* TODO(https://github.com/oxidecomputer/omicron/issues/1276): This
* should probably point to an AZ or fleet, not a rack.
*/
rack_id UUID,

/* The collection's child-resource generation number */
rcgen INT8 NOT NULL
);
Expand All @@ -981,6 +989,15 @@ CREATE UNIQUE INDEX ON omicron.public.ip_pool (
) WHERE
time_deleted IS NULL;

/*
* Index ensuring uniqueness of IP pools by rack ID
*/
CREATE UNIQUE INDEX ON omicron.public.ip_pool (
bnaecker marked this conversation as resolved.
Show resolved Hide resolved
rack_id
) WHERE
rack_id IS NOT NULL AND
time_deleted IS NULL;

/*
* IP Pools are made up of a set of IP ranges, which are start/stop addresses.
* Note that these need not be CIDR blocks or well-behaved subnets with a
Expand Down
7 changes: 7 additions & 0 deletions nexus/db-model/src/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ pub struct IpPool {
/// An optional ID of the project for which this pool is reserved.
pub project_id: Option<Uuid>,

/// An optional ID of the rack for which this pool is reserved.
// TODO(https://github.com/oxidecomputer/omicron/issues/1276): This
// should probably point to an AZ or fleet, not a rack.
pub rack_id: Option<Uuid>,

/// Child resource generation number, for optimistic concurrency control of
/// the contained ranges.
pub rcgen: i64,
Expand All @@ -40,13 +45,15 @@ impl IpPool {
pub fn new(
pool_identity: &external::IdentityMetadataCreateParams,
project_id: Option<Uuid>,
rack_id: Option<Uuid>,
) -> Self {
Self {
identity: IpPoolIdentity::new(
Uuid::new_v4(),
pool_identity.clone(),
),
project_id,
rack_id,
rcgen: 0,
}
}
Expand Down
1 change: 1 addition & 0 deletions nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ table! {
time_modified -> Timestamptz,
time_deleted -> Nullable<Timestamptz>,
project_id -> Nullable<Uuid>,
rack_id -> Nullable<Uuid>,
rcgen -> Int8,
}
}
Expand Down
114 changes: 106 additions & 8 deletions nexus/src/app/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ use ipnetwork::IpNetwork;
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::ListResultVec;
use omicron_common::api::external::LookupResult;
use omicron_common::api::external::ResourceType;
use omicron_common::api::external::UpdateResult;
use uuid::Uuid;

Expand All @@ -26,7 +28,16 @@ impl super::Nexus {
opctx: &OpContext,
new_pool: &params::IpPoolCreate,
) -> CreateResult<db::model::IpPool> {
self.db_datastore.ip_pool_create(opctx, new_pool).await
self.db_datastore.ip_pool_create(opctx, new_pool, None).await
}

pub async fn ip_pool_services_create(
&self,
opctx: &OpContext,
new_pool: &params::IpPoolCreate,
rack_id: Uuid,
) -> CreateResult<db::model::IpPool> {
self.db_datastore.ip_pool_create(opctx, new_pool, Some(rack_id)).await
}

pub async fn ip_pools_list_by_name(
Expand Down Expand Up @@ -91,10 +102,18 @@ impl super::Nexus {
pool_name: &Name,
pagparams: &DataPageParams<'_, IpNetwork>,
) -> ListResultVec<db::model::IpPoolRange> {
let (.., authz_pool) = LookupPath::new(opctx, &self.db_datastore)
.ip_pool_name(pool_name)
.lookup_for(authz::Action::ListChildren)
.await?;
let (.., authz_pool, db_pool) =
LookupPath::new(opctx, &self.db_datastore)
.ip_pool_name(pool_name)
.fetch_for(authz::Action::ListChildren)
.await?;
if db_pool.rack_id.is_some() {
return Err(Error::not_found_by_name(
ResourceType::IpPool,
pool_name,
));
}

self.db_datastore
.ip_pool_list_ranges(opctx, &authz_pool, pagparams)
.await
Expand All @@ -111,6 +130,12 @@ impl super::Nexus {
.ip_pool_name(pool_name)
.fetch_for(authz::Action::Modify)
.await?;
if db_pool.rack_id.is_some() {
return Err(Error::not_found_by_name(
ResourceType::IpPool,
pool_name,
));
}
self.db_datastore
.ip_pool_add_range(opctx, &authz_pool, &db_pool, range)
.await
Expand All @@ -122,10 +147,83 @@ impl super::Nexus {
pool_name: &Name,
range: &IpRange,
) -> DeleteResult {
let (.., authz_pool) = LookupPath::new(opctx, &self.db_datastore)
.ip_pool_name(pool_name)
.lookup_for(authz::Action::Modify)
let (.., authz_pool, db_pool) =
LookupPath::new(opctx, &self.db_datastore)
.ip_pool_name(pool_name)
.fetch_for(authz::Action::Modify)
.await?;
if db_pool.rack_id.is_some() {
return Err(Error::not_found_by_name(
ResourceType::IpPool,
pool_name,
));
}
self.db_datastore.ip_pool_delete_range(opctx, &authz_pool, range).await
}

// The "ip_pool_service_..." functions look up IP pools for Oxide service usage,
// rather than for VMs. As such, they're identified by rack UUID, not
// by pool names.
//
// TODO(https://github.com/oxidecomputer/omicron/issues/1276): Should be
// AZ UUID, probably.

pub async fn ip_pool_service_fetch(
&self,
opctx: &OpContext,
rack_id: Uuid,
) -> LookupResult<db::model::IpPool> {
let (authz_pool, db_pool) = self
.db_datastore
.ip_pools_lookup_by_rack_id(opctx, rack_id)
.await?;
opctx.authorize(authz::Action::Read, &authz_pool).await?;
Ok(db_pool)
}

pub async fn ip_pool_service_list_ranges(
&self,
opctx: &OpContext,
rack_id: Uuid,
pagparams: &DataPageParams<'_, IpNetwork>,
) -> ListResultVec<db::model::IpPoolRange> {
let (authz_pool, ..) = self
.db_datastore
.ip_pools_lookup_by_rack_id(opctx, rack_id)
.await?;
opctx.authorize(authz::Action::Read, &authz_pool).await?;
self.db_datastore
.ip_pool_list_ranges(opctx, &authz_pool, pagparams)
.await
}

pub async fn ip_pool_service_add_range(
&self,
opctx: &OpContext,
rack_id: Uuid,
range: &IpRange,
) -> UpdateResult<db::model::IpPoolRange> {
let (authz_pool, db_pool) = self
.db_datastore
.ip_pools_lookup_by_rack_id(opctx, rack_id)
.await?;
opctx.authorize(authz::Action::Modify, &authz_pool).await?;
self.db_datastore
.ip_pool_add_range(opctx, &authz_pool, &db_pool, range)
.await
}

pub async fn ip_pool_service_delete_range(
&self,
opctx: &OpContext,
rack_id: Uuid,
range: &IpRange,
) -> DeleteResult {
let (authz_pool, ..) = self
.db_datastore
.ip_pools_lookup_by_rack_id(opctx, rack_id)
.await?;
opctx.authorize(authz::Action::Modify, &authz_pool).await?;
self.db_datastore.ip_pool_delete_range(opctx, &authz_pool, range).await
}
}
2 changes: 2 additions & 0 deletions nexus/src/authz/omicron.polar
Original file line number Diff line number Diff line change
Expand Up @@ -436,3 +436,5 @@ has_permission(_actor: AuthenticatedActor, "query", _resource: Database);
# The "db-init" user is the only one with the "init" role.
has_permission(actor: AuthenticatedActor, "modify", _resource: Database)
if actor = USER_DB_INIT;
has_permission(actor: AuthenticatedActor, "create_child", _resource: IpPoolList)
if actor = USER_DB_INIT;
Comment on lines +439 to +440
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sure if this is the best way of doing this, but without this line, I can't create the Nexus-owned IP pool during the populate step.

66 changes: 65 additions & 1 deletion nexus/src/db/datastore/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use omicron_common::api::external::DataPageParams;
use omicron_common::api::external::DeleteResult;
use omicron_common::api::external::Error;
use omicron_common::api::external::ListResultVec;
use omicron_common::api::external::LookupResult;
use omicron_common::api::external::LookupType;
use omicron_common::api::external::ResourceType;
use omicron_common::api::external::UpdateResult;
Expand All @@ -49,6 +50,7 @@ impl DataStore {
.authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST)
.await?;
paginated(dsl::ip_pool, dsl::name, pagparams)
.filter(dsl::rack_id.is_null())
.filter(dsl::time_deleted.is_null())
.select(db::model::IpPool::as_select())
.get_results_async(self.pool_authorized(opctx).await?)
Expand All @@ -67,17 +69,71 @@ impl DataStore {
.authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST)
.await?;
paginated(dsl::ip_pool, dsl::id, pagparams)
.filter(dsl::rack_id.is_null())
.filter(dsl::time_deleted.is_null())
.select(db::model::IpPool::as_select())
.get_results_async(self.pool_authorized(opctx).await?)
.await
.map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
}

/// Looks up an IP pool by a particular Rack ID.
///
/// An index exists to look up pools by rack ID, but it is not a primary
/// key, which requires this lookup function to be used instead of the
/// [`LookupPath`] utility.
pub async fn ip_pools_lookup_by_rack_id(
&self,
opctx: &OpContext,
rack_id: Uuid,
) -> LookupResult<(authz::IpPool, IpPool)> {
use db::schema::ip_pool::dsl;

// Ensure the caller has the ability to look up these IP pools.
// If they don't, return "not found" instead of "forbidden".
opctx
.authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST)
.await
.map_err(|e| match e {
Error::Forbidden => {
LookupType::ByCompositeId(format!("Rack ID: {rack_id}"))
.into_not_found(ResourceType::IpPool)
}
_ => e,
})?;

// Look up this IP pool by rack ID.
let (authz_pool, pool) = dsl::ip_pool
.filter(dsl::rack_id.eq(Some(rack_id)))
.filter(dsl::time_deleted.is_null())
.select(IpPool::as_select())
.get_result_async(self.pool_authorized(opctx).await?)
.await
.map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
.map(|ip_pool| {
(
authz::IpPool::new(
authz::FLEET,
ip_pool.id(),
LookupType::ByCompositeId(format!(
"Rack ID: {rack_id}"
)),
),
ip_pool,
)
})?;
Ok((authz_pool, pool))
}

/// Creates a new IP pool.
///
/// - If `rack_id` is provided, this IP pool is used for Oxide
/// services.
pub async fn ip_pool_create(
&self,
opctx: &OpContext,
new_pool: &params::IpPoolCreate,
rack_id: Option<Uuid>,
) -> CreateResult<IpPool> {
use db::schema::ip_pool::dsl;
opctx
Expand All @@ -86,6 +142,12 @@ impl DataStore {
let project_id = match new_pool.project.clone() {
None => None,
Some(project) => {
if let Some(_) = &rack_id {
return Err(Error::invalid_request(
"Internal Service IP pools cannot be project-scoped",
));
}

let (.., authz_project) = LookupPath::new(opctx, self)
.organization_name(&Name(project.organization))
.project_name(&Name(project.project))
Expand All @@ -94,7 +156,7 @@ impl DataStore {
Some(authz_project.id())
}
};
let pool = IpPool::new(&new_pool.identity, project_id);
let pool = IpPool::new(&new_pool.identity, project_id, rack_id);
let pool_name = pool.name().as_str().to_string();
diesel::insert_into(dsl::ip_pool)
.values(pool)
Expand Down Expand Up @@ -143,6 +205,7 @@ impl DataStore {
// in between the above check for children and this query.
let now = Utc::now();
let updated_rows = diesel::update(dsl::ip_pool)
.filter(dsl::rack_id.is_null())
.filter(dsl::time_deleted.is_null())
.filter(dsl::id.eq(authz_pool.id()))
.filter(dsl::rcgen.eq(db_pool.rcgen))
Expand Down Expand Up @@ -174,6 +237,7 @@ impl DataStore {
use db::schema::ip_pool::dsl;
opctx.authorize(authz::Action::Modify, authz_pool).await?;
diesel::update(dsl::ip_pool)
.filter(dsl::rack_id.is_null())
.filter(dsl::id.eq(authz_pool.id()))
.filter(dsl::time_deleted.is_null())
.set(updates)
Expand Down
3 changes: 2 additions & 1 deletion nexus/src/db/queries/external_ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,8 @@ mod tests {
name: String::from(name).parse().unwrap(),
description: format!("ip pool {}", name),
},
None,
/* project_id= */ None,
/* rack_id= */ None,
);
pool.project_id = project_id;

Expand Down
Loading