diff --git a/nexus/db-model/src/ip_pool.rs b/nexus/db-model/src/ip_pool.rs index 819be85ec8e..c105a662d8c 100644 --- a/nexus/db-model/src/ip_pool.rs +++ b/nexus/db-model/src/ip_pool.rs @@ -24,6 +24,37 @@ use omicron_common::api::external; use std::net::IpAddr; use uuid::Uuid; +impl_enum_type!( + IpPoolReservationTypeEnum: + + #[derive( + AsExpression, + Clone, + Copy, + Debug, + Eq, + FromSqlRow, + PartialEq, + schemars::JsonSchema, + serde::Deserialize, + serde::Serialize, + )] + pub enum IpPoolReservationType; + + ExternalSilos => b"external_silos" + OxideInternal => b"oxide_internal" +); + +impl ::std::fmt::Display for IpPoolReservationType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let s = match self { + IpPoolReservationType::ExternalSilos => "external_silos", + IpPoolReservationType::OxideInternal => "oxide_internal", + }; + f.write_str(s) + } +} + impl_enum_type!( IpVersionEnum: @@ -77,7 +108,9 @@ impl From for shared::IpVersion { /// IP pools can be external or internal. External IP pools can be associated /// with a silo or project so that instance IP allocation draws from that pool /// instead of a system pool. -#[derive(Queryable, Insertable, Selectable, Clone, Debug, Resource)] +#[derive( + Queryable, Insertable, Selectable, Clone, Debug, Resource, PartialEq, +)] #[diesel(table_name = ip_pool)] pub struct IpPool { #[diesel(embed)] @@ -89,12 +122,19 @@ pub struct IpPool { /// Child resource generation number, for optimistic concurrency control of /// the contained ranges. pub rcgen: i64, + + /// Indicates what the pool is reserved for. + pub reservation_type: IpPoolReservationType, } impl IpPool { + /// Create a new IP Pool. + /// + /// The pool is reserved for external customer Silos. pub fn new( pool_identity: &external::IdentityMetadataCreateParams, ip_version: IpVersion, + reservation_type: IpPoolReservationType, ) -> Self { Self { identity: IpPoolIdentity::new( @@ -103,19 +143,28 @@ impl IpPool { ), ip_version, rcgen: 0, + reservation_type, } } + /// Create a new IPv4 IP Pool. + /// + /// The pool is reserved for external customer Silos. pub fn new_v4( pool_identity: &external::IdentityMetadataCreateParams, + reservation_type: IpPoolReservationType, ) -> Self { - Self::new(pool_identity, IpVersion::V4) + Self::new(pool_identity, IpVersion::V4, reservation_type) } + /// Create a new IPv6 IP Pool. + /// + /// The pool is reserved for external customer Silos. pub fn new_v6( pool_identity: &external::IdentityMetadataCreateParams, + reservation_type: IpPoolReservationType, ) -> Self { - Self::new(pool_identity, IpVersion::V6) + Self::new(pool_identity, IpVersion::V6, reservation_type) } } diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 49e0f9b84d3..4e2003a9ff6 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(196, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(197, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(197, "add-ip-pool-reservation-type-column"), KnownVersion::new(196, "user-provision-type-for-silo-user-and-group"), KnownVersion::new(195, "tuf-pruned-index"), KnownVersion::new(194, "tuf-pruned"), diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index f31ae4181fa..bcc12654491 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -13,10 +13,10 @@ use crate::db::collection_insert::DatastoreCollection; use crate::db::datastore::SERVICE_IPV4_POOL_NAME; use crate::db::datastore::SERVICE_IPV6_POOL_NAME; use crate::db::identity::Resource; -use crate::db::model::ExternalIp; use crate::db::model::IpKind; use crate::db::model::IpPool; use crate::db::model::IpPoolRange; +use crate::db::model::IpPoolReservationType; use crate::db::model::IpPoolResource; use crate::db::model::IpPoolResourceType; use crate::db::model::IpPoolUpdate; @@ -24,11 +24,15 @@ use crate::db::model::Name; use crate::db::pagination::Paginator; use crate::db::pagination::paginated; use crate::db::queries::ip_pool::FilterOverlappingIpRanges; +use crate::db::raw_query_builder::QueryBuilder; +use crate::db::raw_query_builder::SelectableSql; +use crate::db::raw_query_builder::TypedSqlQuery; use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; use diesel::prelude::*; use diesel::result::DatabaseErrorKind; use diesel::result::Error as DieselError; +use diesel::sql_types; use ipnetwork::IpNetwork; use nexus_db_errors::ErrorHandler; use nexus_db_errors::OptionalError; @@ -42,7 +46,10 @@ use nexus_db_model::InternetGatewayIpPool; use nexus_db_model::IpVersion; use nexus_db_model::Project; use nexus_db_model::Vpc; +use nexus_db_schema::enums::IpKindEnum; +use nexus_db_schema::enums::IpPoolReservationTypeEnum; use nexus_types::external_api::shared::IpRange; +use nexus_types::silo::INTERNAL_SILO_ID; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; @@ -87,50 +94,89 @@ impl ServiceIpPools { } } -// Constraint used to ensure we don't set a default IP Pool for the internal -// silo. -const INTERNAL_SILO_DEFAULT_CONSTRAINT: &'static str = - "internal_silo_has_no_default_pool"; +// Error message emitted when a user attempts to link an IP Pool and internal +// Silo, but the pool is already reserved for internal use, or vice versa. +const BAD_SILO_LINK_ERROR: &str = "IP Pools cannot be both linked to external \ + Silos and reserved for internal Oxide usage."; -// Error message emitted when we attempt to set a default IP Pool for the -// internal silo. -const INTERNAL_SILO_DEFAULT_ERROR: &'static str = - "The internal Silo cannot have a default IP Pool"; +// Error message emitted when a user attempts to unlink an IP Pool from a Silo +// while the pool has external IP addresses allocated from it. +const POOL_HAS_IPS_ERROR: &str = + "IP addresses from this pool are in use in the linked silo"; + +// Error message emitted when a user attempts to unlink an IP Pool from the +// Oxide internal Silo, without at least one other IP Pool linked to it. +const LAST_POOL_ERROR: &str = "Cannot delete the last IP Pool reserved for \ + Oxide internal usage. Create and reserve at least one more IP Pool \ + before deleting this one."; impl DataStore { - /// List IP Pools - pub async fn ip_pools_list( + /// List IP Pools by their reservation type and optionally IP version, paginated. + pub async fn ip_pools_list_paginated( &self, opctx: &OpContext, + reservation_type: IpPoolReservationType, + version: Option, pagparams: &PaginatedBy<'_>, ) -> ListResultVec { use nexus_db_schema::schema::ip_pool; - opctx .authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST) .await?; - match pagparams { - PaginatedBy::Id(pagparams) => { - paginated(ip_pool::table, ip_pool::id, pagparams) + let mut query = match pagparams { + PaginatedBy::Id(by_id) => { + paginated(ip_pool::table, ip_pool::id, by_id) } - PaginatedBy::Name(pagparams) => paginated( + PaginatedBy::Name(by_name) => paginated( ip_pool::table, ip_pool::name, - &pagparams.map_name(|n| Name::ref_cast(n)), + &by_name.map_name(|n| Name::ref_cast(n)), ), - } - .filter(ip_pool::name.ne(SERVICE_IPV4_POOL_NAME)) - .filter(ip_pool::name.ne(SERVICE_IPV6_POOL_NAME)) - .filter(ip_pool::time_deleted.is_null()) - .select(IpPool::as_select()) - .get_results_async(&*self.pool_connection_authorized(opctx).await?) + }; + query = match version { + Some(ver) => query.filter(ip_pool::ip_version.eq(ver)), + None => query, + }; + query + .filter(ip_pool::time_deleted.is_null()) + .filter(ip_pool::reservation_type.eq(reservation_type)) + .select(IpPool::as_select()) + .get_results_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + + /// List IP Pools + /// + /// This returns the pools available for external customer use. + pub async fn ip_pools_list( + &self, + opctx: &OpContext, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + self.ip_pools_list_paginated( + opctx, + IpPoolReservationType::ExternalSilos, + None, + pagparams, + ) .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 + // + // TODO-correctness: This seems difficult to use without TOCTOU issues. It's + // currently used to ensure there's a link between a Silo and an IP Pool + // when allocating an external address for an instance in that Silo. But + // that works by checking that the link exists, and then in a separate + // query, allocating an address out of it. Suppose the silo was unlinked + // after this check, but before the external address allocation query ran. + // Then one could end up with an address from an unlinked IP Pool, which + // seems wrong. + // + // See https://github.com/oxidecomputer/omicron/issues/8992 pub async fn ip_pool_fetch_link( &self, opctx: &OpContext, @@ -224,9 +270,9 @@ impl DataStore { /// This is useful when you need to handle resources like external IPs where /// the actual address might be from either IP version. // - // NOTE: It'd be better to do one roundtrip to the DB, but this is - // rarely-used right now. We also want to return the authz and database - // objects, so we need the lookup-path mechanism. + // TODO-remove: Use list_ip_pools_for_internal instead. + // + // See https://github.com/oxidecomputer/omicron/issues/8947. pub async fn ip_pools_service_lookup_both_versions( &self, opctx: &OpContext, @@ -243,6 +289,11 @@ impl DataStore { /// names. There are separate IP Pools for IPv4 and IPv6 address ranges. /// /// This method may require an index by Availability Zone in the future. + // + // TODO-remove: Use ip_pools_list_paginated with the right enum type + // instead. + // + // See https://github.com/oxidecomputer/omicron/issues/8947. pub async fn ip_pools_service_lookup( &self, opctx: &OpContext, @@ -282,6 +333,10 @@ impl DataStore { }) } + /// Delete an IP Pool, and any links between it an any Silos. + /// + /// This fails if there are still IP Ranges in the pool, or if we're + /// deleting the last pool reserved for Oxide use. pub async fn ip_pool_delete( &self, opctx: &OpContext, @@ -311,6 +366,20 @@ impl DataStore { )); } + // Add a small subquery, if needed, to ensure that we don't delete this + // IP Pool if it's the last reserved pool. There has to always be at + // least one of these. + let enough_reserved_pools = if matches!( + db_pool.reservation_type, + IpPoolReservationType::ExternalSilos + ) { + diesel::dsl::sql::("TRUE") + } else { + diesel::dsl::sql::(&count_reserved_pools_subquery( + db_pool.reservation_type, + )) + }; + // Delete the pool, conditional on the rcgen not having changed. This // protects the delete from occuring if clients created a new IP range // in between the above check for children and this query. @@ -319,14 +388,21 @@ impl DataStore { .filter(dsl::time_deleted.is_null()) .filter(dsl::id.eq(authz_pool.id())) .filter(dsl::rcgen.eq(db_pool.rcgen)) + .filter(enough_reserved_pools) .set(dsl::time_deleted.eq(now)) .execute_async(&*conn) .await - .map_err(|e| { - public_error_from_diesel( + .map_err(|e| match e { + DieselError::DatabaseError( + DatabaseErrorKind::Unknown, + ref info, + ) if info.message().ends_with("invalid bool value") => { + Error::invalid_request(LAST_POOL_ERROR) + } + _ => public_error_from_diesel( e, ErrorHandler::NotFoundByResource(authz_pool), - ) + ), })?; if updated_rows == 0 { @@ -351,31 +427,32 @@ impl DataStore { /// Check whether the pool is internal by checking that it exists and is /// associated with the internal silo + // + // TODO-remove: This should go away when we let operators reserve any IP + // Pools for internal Oxide usage. The pool belongs to them even in that + // case, and so we should show it to them. + // + // See https://github.com/oxidecomputer/omicron/issues/8947. pub async fn ip_pool_is_internal( &self, opctx: &OpContext, authz_pool: &authz::IpPool, ) -> LookupResult { use nexus_db_schema::schema::ip_pool; - ip_pool::table - .filter(ip_pool::id.eq(authz_pool.id())) - .filter( - ip_pool::name - .eq(SERVICE_IPV4_POOL_NAME) - .or(ip_pool::name.eq(SERVICE_IPV6_POOL_NAME)), - ) + .find(authz_pool.id()) .filter(ip_pool::time_deleted.is_null()) - .select(ip_pool::id) - .first_async::( + .select( + ip_pool::reservation_type + .ne(IpPoolReservationType::ExternalSilos), + ) + .first_async::( &*self.pool_connection_authorized(opctx).await?, ) .await .optional() - // if there is a result, the pool is associated with the internal silo, - // which makes it the internal pool - .map(|result| Ok(result.is_some())) - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))? + .map(|result| result.unwrap_or(false)) + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } pub async fn ip_pool_update( @@ -402,6 +479,67 @@ impl DataStore { }) } + /// Reserve an IP Pool for a specific use. + pub async fn ip_pool_reserve( + &self, + opctx: &OpContext, + authz_pool: &authz::IpPool, + db_pool: &IpPool, + reservation_type: IpPoolReservationType, + ) -> UpdateResult<()> { + if db_pool.reservation_type == reservation_type { + return Err(Error::invalid_request(format!( + "IP Pool already has reservation type '{}'", + reservation_type, + ))); + } + let n_rows = reserve_ip_pool_query(db_pool, reservation_type) + .execute_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| match e { + DieselError::DatabaseError( + DatabaseErrorKind::Unknown, + ref info, + ) => { + let message = info.message(); + if message.ends_with("invalid bool value") { + Error::invalid_request(BAD_SILO_LINK_ERROR) + } else if message.contains("division by zero") { + Error::invalid_request(POOL_HAS_IPS_ERROR) + } else if message.starts_with("could not parse") + && message.contains("as type int") + { + Error::invalid_request(LAST_POOL_ERROR) + } else { + public_error_from_diesel(e, ErrorHandler::Server) + } + } + _ => public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource(authz_pool), + ), + })?; + if n_rows == 0 { + Err(Error::invalid_request( + "update failed due to concurrent modification", + )) + } else { + Ok(()) + } + } + + /// Unreserve an IP. + /// + /// TODO(ben) Remove this, use reserve with specific type for everything. + pub async fn ip_pool_revoke( + &self, + _opctx: &OpContext, + _authz_pool: &authz::IpPool, + _db_pool: &IpPool, + ) -> UpdateResult<()> { + todo!("remove me"); + } + /// Return the number of IPs allocated from and the capacity of the provided /// IP Pool. pub async fn ip_pool_utilization( @@ -526,6 +664,7 @@ impl DataStore { Ok(count) } + /// List Silos linked to the given IP Pool. pub async fn ip_pool_silo_list( &self, opctx: &OpContext, @@ -555,6 +694,8 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + /// List IP Pools linked to the given Silo. + /// /// 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( @@ -586,31 +727,29 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + /// Insert a link between an IP Pool and a Silo. pub async fn ip_pool_link_silo( &self, opctx: &OpContext, ip_pool_resource: IpPoolResource, ) -> CreateResult { - use nexus_db_schema::schema::ip_pool_resource::dsl; + if ip_pool_resource.resource_id == INTERNAL_SILO_ID { + return Err(Error::internal_error( + "IP Pools should not be linked to the internal silo. \ + Set the `reservation_type` column to an internal \ + variant instead.", + )); + } opctx .authorize(authz::Action::CreateChild, &authz::IP_POOL_LIST) .await?; let conn = self.pool_connection_authorized(opctx).await?; - - let result = diesel::insert_into(dsl::ip_pool_resource) - .values(ip_pool_resource) + let result = link_ip_pool_to_external_silo_query(&ip_pool_resource) .get_result_async(&*conn) .await .map_err(|e| { match e { - // Catch the check constraint ensuring the internal silo has - // no default pool - DieselError::DatabaseError(DatabaseErrorKind::CheckViolation, ref info) - if info.constraint_name() == Some(INTERNAL_SILO_DEFAULT_CONSTRAINT) => - { - Error::invalid_request(INTERNAL_SILO_DEFAULT_ERROR) - } DieselError::DatabaseError(DatabaseErrorKind::UniqueViolation, _) => { public_error_from_diesel( e, @@ -625,6 +764,28 @@ impl DataStore { ) ) } + // Handle intentional errors in the query. + DieselError::DatabaseError(DatabaseErrorKind::Unknown, ref info) => { + let is_uuid_cast_error = |msg: &str, sentinel: &str| -> bool { + // We're unfortunately allocating here, but this + // error path isn't expected to be common. + let expected = format!( + "uuid: incorrect UUID length: {}", + sentinel, + ); + msg.ends_with(&expected) + }; + let msg = info.message(); + if is_uuid_cast_error(msg, BAD_SILO_LINK_SENTINEL) { + Error::invalid_request(BAD_SILO_LINK_ERROR) + } else if is_uuid_cast_error(msg, IP_POOL_DELETED_SENTINEL) { + Error::not_found_by_id(ResourceType::IpPool, &ip_pool_resource.ip_pool_id) + } else if is_uuid_cast_error(msg, SILO_DELETED_SENTINEL) { + Error::not_found_by_id(ResourceType::Silo, &ip_pool_resource.resource_id) + } else { + public_error_from_diesel(e, ErrorHandler::Server) + } + } _ => public_error_from_diesel(e, ErrorHandler::Server), } })?; @@ -642,6 +803,11 @@ impl DataStore { Ok(result) } + // TODO-correctness: This seems like it should be in a transaction. At + // least, the nested-loops can mostly be re-expressed as a join between the + // silos, projects, vpcs, and Internet gateway tables. + // + // See https://github.com/oxidecomputer/omicron/issues/8992. async fn link_default_gateway( &self, opctx: &OpContext, @@ -735,6 +901,10 @@ impl DataStore { Ok(()) } + // TODO-correctness: This should probably be in a transaction, collecting + // all the Internet gateway IDs via a JOIN and then soft-deleting them all. + // + // See https://github.com/oxidecomputer/omicron/issues/8992. async fn unlink_ip_pool_gateway( &self, opctx: &OpContext, @@ -908,145 +1078,21 @@ impl DataStore { IpPoolResourceUpdateError::FailedToUnsetDefault(err), )) => public_error_from_diesel(err, ErrorHandler::Server), Some(TxnError::Database(err)) => { - match err { - // Catch the check constraint ensuring the internal silo has - // no default pool - DieselError::DatabaseError( - DatabaseErrorKind::CheckViolation, - ref info, - ) if info.constraint_name() - == Some(INTERNAL_SILO_DEFAULT_CONSTRAINT) => - { - Error::invalid_request(INTERNAL_SILO_DEFAULT_ERROR) - } - _ => { - public_error_from_diesel(err, ErrorHandler::Server) - } - } - } - None => { - match e { - // Catch the check constraint ensuring the internal silo has - // no default pool - DieselError::DatabaseError( - DatabaseErrorKind::CheckViolation, - ref info, - ) if info.constraint_name() - == Some(INTERNAL_SILO_DEFAULT_CONSTRAINT) => - { - Error::invalid_request(INTERNAL_SILO_DEFAULT_ERROR) - } - _ => public_error_from_diesel( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::IpPoolResource, - // TODO: would be nice to put the actual names and/or ids in - // here but LookupType on each of the two silos doesn't have - // a nice to_string yet or a way of composing them - LookupType::ByCompositeId( - "(pool, silo)".to_string(), - ), - ), - ), - } + public_error_from_diesel(err, ErrorHandler::Server) } + None => public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::IpPoolResource, + // TODO: would be nice to put the actual names and/or ids in + // here but LookupType on each of the two silos doesn't have + // a nice to_string yet or a way of composing them + LookupType::ByCompositeId("(pool, silo)".to_string()), + ), + ), }) } - /// Ephemeral and snat IPs are associated with a silo through an instance, - /// so in order to see if there are any such IPs outstanding in the given - /// silo, we have to join IP -> Instance -> Project -> Silo - async fn ensure_no_instance_ips_outstanding( - &self, - opctx: &OpContext, - authz_pool: &authz::IpPool, - authz_silo: &authz::Silo, - ) -> Result<(), Error> { - use nexus_db_schema::schema::external_ip; - use nexus_db_schema::schema::instance; - use nexus_db_schema::schema::project; - - let existing_ips = external_ip::table - .inner_join( - instance::table - .on(external_ip::parent_id.eq(instance::id.nullable())), - ) - .inner_join(project::table.on(instance::project_id.eq(project::id))) - .filter(external_ip::is_service.eq(false)) - .filter(external_ip::parent_id.is_not_null()) - .filter(external_ip::time_deleted.is_null()) - .filter(external_ip::ip_pool_id.eq(authz_pool.id())) - // important, floating IPs are handled separately - .filter(external_ip::kind.eq(IpKind::Ephemeral).or(external_ip::kind.eq(IpKind::SNat))) - .filter(instance::time_deleted.is_null()) - // we have to join through IPs to instances to projects to get the silo ID - .filter(project::silo_id.eq(authz_silo.id())) - .select(ExternalIp::as_select()) - .limit(1) - .load_async::( - &*self.pool_connection_authorized(opctx).await?, - ) - .await - .map_err(|e| { - Error::internal_error(&format!( - "error checking for outstanding IPs before deleting IP pool association to resource: {:?}", - e - )) - })?; - - if !existing_ips.is_empty() { - return Err(Error::invalid_request( - "IP addresses from this pool are in use in the linked silo", - )); - } - - Ok(()) - } - - /// Floating IPs are associated with a silo through a project, so this one - /// is a little simpler than ephemeral. We join IP -> Project -> Silo. - async fn ensure_no_floating_ips_outstanding( - &self, - opctx: &OpContext, - authz_pool: &authz::IpPool, - authz_silo: &authz::Silo, - ) -> Result<(), Error> { - use nexus_db_schema::schema::external_ip; - use nexus_db_schema::schema::project; - - let existing_ips = external_ip::table - .inner_join(project::table.on(external_ip::project_id.eq(project::id.nullable()))) - .filter(external_ip::is_service.eq(false)) - .filter(external_ip::time_deleted.is_null()) - // all floating IPs have a project - .filter(external_ip::project_id.is_not_null()) - .filter(external_ip::ip_pool_id.eq(authz_pool.id())) - .filter(external_ip::kind.eq(IpKind::Floating)) - // we have to join through IPs to projects to get the silo ID - .filter(project::silo_id.eq(authz_silo.id())) - .filter(project::time_deleted.is_null()) - .select(ExternalIp::as_select()) - .limit(1) - .load_async::( - &*self.pool_connection_authorized(opctx).await?, - ) - .await - .map_err(|e| { - Error::internal_error(&format!( - "error checking for outstanding IPs before deleting IP pool association to resource: {:?}", - e - )) - })?; - - if !existing_ips.is_empty() { - return Err(Error::invalid_request( - "IP addresses from this pool are in use in the linked silo", - )); - } - - Ok(()) - } - /// Delete IP pool assocation with resource unless there are outstanding /// IPs allocated from the pool in the associated silo pub async fn ip_pool_unlink_silo( @@ -1055,33 +1101,44 @@ impl DataStore { authz_pool: &authz::IpPool, authz_silo: &authz::Silo, ) -> DeleteResult { - use nexus_db_schema::schema::ip_pool_resource; - opctx.authorize(authz::Action::Modify, authz_pool).await?; opctx.authorize(authz::Action::Modify, authz_silo).await?; - // We can only delete the association if there are no IPs allocated - // from this pool in the associated resource. - self.ensure_no_instance_ips_outstanding(opctx, authz_pool, authz_silo) - .await?; - self.ensure_no_floating_ips_outstanding(opctx, authz_pool, authz_silo) - .await?; + if authz_silo.id() == INTERNAL_SILO_ID { + return Err(Error::internal_error( + "Cannot unlink an internally-reserved IP Pool. \ + Use the `reservation_type` column instead.", + )); + } let conn = self.pool_connection_authorized(opctx).await?; + unlink_ip_pool_from_external_silo_query( + authz_pool.id(), + authz_silo.id(), + ) + .execute_async(&*conn) + .await + .map_err(|e| match e { + DieselError::DatabaseError( + DatabaseErrorKind::Unknown, + ref info, + ) => { + let msg = info.message(); + // Intentional bool-parsing error, which we use to detect + // when there are still external IPs in the Silo. + if msg.contains("could not parse") + && msg.ends_with("invalid bool value") + { + Error::invalid_request(POOL_HAS_IPS_ERROR) + } else { + public_error_from_diesel(e, ErrorHandler::Server) + } + } + _ => public_error_from_diesel(e, ErrorHandler::Server), + })?; - diesel::delete(ip_pool_resource::table) - .filter(ip_pool_resource::ip_pool_id.eq(authz_pool.id())) - .filter(ip_pool_resource::resource_id.eq(authz_silo.id())) - .execute_async(&*conn) - .await - .map(|_rows_deleted| ()) - .map_err(|e| { - Error::internal_error(&format!( - "error deleting IP pool association to resource: {:?}", - e - )) - })?; - + // TODO-correctness: We probably want to do this in the same transaction + // as above. See https://github.com/oxidecomputer/omicron/issues/8992. self.unlink_ip_pool_gateway( opctx, authz_silo.id(), @@ -1313,27 +1370,431 @@ impl DataStore { } } +// Sentinel we try to cast as a UUID in the database, when linking an IP Pool to +// a Silo of the wrong "type" -- i.e., linking to an external Silo if the Pool +// is already linked to our internal Silo, or vice versa. +const BAD_SILO_LINK_SENTINEL: &str = "bad-link-type"; + +// Sentinel we try to cast as a UUID in the database when the IP Pool is +// deleted between selecting it and trying to insert the link. +const IP_POOL_DELETED_SENTINEL: &str = "ip-pool-deleted"; + +// Sentinel we try to cast as a UUID in the database when the Silo is deleted +// between selecting it and trying to insert the link. +const SILO_DELETED_SENTINEL: &str = "silo-deleted"; + +// Sentinel we try to cast as an integer when removing the reservation on the +// last internal pool of a given type. +const LAST_POOL_SENTINEL: &str = "last-pool"; + +// Query to conditionally link an IP Pool to an external customer Silo. +// +// This method returns a SQL query to conditionally insert a link between an IP +// Pool and a Silo. It maintains the invariant that a pool can be reserved for +// Oxide internal usage XOR linked to customer silos. It also checks that the +// pool and silo still exist when the query is run. +// +// The full query is: +// +// ```sql +// WITH +// -- Select the IP Pool by ID, used to ensure it still exists when we run +// -- this query. Also select the reservation type, and fail if the pool is +// -- currently reserved for Oxide. +// ip_pool AS ( +// SELECT CAST(IF( +// reservation_type != 'external_silos', +// 'bad-link-type', +// $1) +// AS UUID) +// FROM ip_pool +// WHERE id = $2 AND time_deleted IS NULL +// ) +// -- Select the Silo by ID, used to ensure it still exists when we run this +// -- query +// silo AS (SELECT id FROM silo WHERE id = $3 AND time_deleted IS NULL), +// INSERT +// INTO +// ip_pool_resource (ip_pool_id, resource_type, resource_id, is_default) +// SELECT +// -- If the pool exists, take its ID as a string. If it does not exist, take +// -- the string `'ip-pool-deleted'`. Attempt to cast the result to a UUID. +// -- This is the "true or cast error" trick we use in many places. +// CAST(COALESCE(CAST(ip.id AS STRING), 'ip-pool-deleted') AS UUID), +// -- The resource type, always 'silo' here. +// $4, +// -- If the silo exists, take its ID as a string. If it does not exist, take +// -- the string `'silo-deleted'`. Attempt to cast the result to a UUID. +// -- This is the "true or cast error" trick we use in many places. +// CAST(COALESCE(CAST(s.id AS STRING), 'silo-deleted') AS UUID), +// $5 +// FROM +// (SELECT 1) AS dummy +// LEFT JOIN ip_pool AS ip ON true +// LEFT JOIN silo AS s ON true +// RETURNING +// * +// ``` +fn link_ip_pool_to_external_silo_query( + ip_pool_resource: &IpPoolResource, +) -> TypedSqlQuery> { + let mut builder = QueryBuilder::new(); + builder + .sql("WITH ip_pool AS (SELECT CAST(IF(reservation_type != ") + .param() + .bind::( + IpPoolReservationType::ExternalSilos, + ) + .sql(", '") + .sql(BAD_SILO_LINK_SENTINEL) + .sql("', ") + .param() + .bind::(ip_pool_resource.ip_pool_id.to_string()) + .sql(") AS UUID) AS id FROM ip_pool WHERE id = ") + .param() + .bind::(ip_pool_resource.ip_pool_id) + .sql( + " \ + AND time_deleted IS NULL), \ + silo AS (SELECT id FROM silo WHERE id = ", + ) + .param() + .bind::(ip_pool_resource.resource_id) + .sql( + " AND time_deleted IS NULL) \ + INSERT INTO ip_pool_resource (\ + ip_pool_id, \ + resource_type, \ + resource_id, \ + is_default\ + ) SELECT CAST(COALESCE(CAST(ip.id AS STRING), '", + ) + .sql(IP_POOL_DELETED_SENTINEL) + .sql("') AS UUID), ") + .param() + .bind::( + ip_pool_resource.resource_type, + ) + .sql(", CAST(COALESCE(CAST(s.id AS STRING), '") + .sql(SILO_DELETED_SENTINEL) + .sql("') AS UUID), ") + .param() + .bind::(ip_pool_resource.is_default) + .sql( + " FROM (SELECT 1) AS dummy \ + LEFT JOIN ip_pool AS ip ON TRUE \ + LEFT JOIN silo AS s ON TRUE \ + RETURNING *", + ); + builder.query() +} + +// Query to conditionally unlink an IP Pool from an external / customer Silo. +// +// This deletes the link iff there are no outstanding instance external IPs or +// floating IPs allocated out of the pool, to objects in the Silo. +// +// The full query is: +// +// ``` +// -- This CTE returns one row if there are any external IPs attached to +// -- instances, in any projects in the Silo. +// WITH instance_ips AS ( +// SELECT 1 +// FROM external_ip +// INNER JOIN instance ON external_ip.parent_id = instance.id +// INNER JOIN project ON instance.project_id = project.id +// WHERE +// external_ip.is_service = FALSE AND +// external_ip.parent_id IS NOT NULL AND +// external_ip.time_deleted IS NULL AND +// external_ip.ip_pool_id = $1 AND +// external_ip.kind != 'floating' AND +// instance.time_deleted IS NULL AND +// project.silo_id = $2 +// LIMIT 1 +// ), +// -- This CTE returns one row if there are any Floating IPs in the Silo, +// -- whether they're attached or not. +// floating_ips AS ( +// SELECT 1 +// FROM external_ip +// INNER JOIN project ON external_ip.project_id = project.id +// WHERE +// external_ip.is_service = FALSE AND +// external_ip.time_deleted IS NULL AND +// external_ip.project_id IS NOT NULL AND +// external_ip.ip_pool_id = $3 AND +// external_ip.kind = 'floating' AND +// project.silo_id = $4 AND +// project.time_deleted IS NULL +// LIMIT 1 +// ) +// -- Delete the requested link by primary key, but conditionally. +// DELETE FROM ip_pool_resource +// WHERE +// ip_pool_id = $7 AND +// resource_type = 'silo' AND +// resource_id = $8 AND +// -- If there are any external IPs, this generates an error casting 'eips' to +// -- a boolean, which we detect and handle. +// CAST(IF(EXISTS( +// SELECT 1 FROM instance_ips +// UNION ALL +// SELECT 1 FROM floating_ips +// ), 'eips', 'true') AS BOOL) +// ``` +fn unlink_ip_pool_from_external_silo_query( + ip_pool_id: Uuid, + silo_id: Uuid, +) -> TypedSqlQuery<()> { + let mut builder = QueryBuilder::new(); + builder + .sql( + "WITH instance_ips AS (\ + SELECT 1 \ + FROM external_ip \ + INNER JOIN instance ON external_ip.parent_id = instance.id \ + INNER JOIN project ON instance.project_id = project.id \ + WHERE \ + external_ip.is_service = FALSE AND \ + external_ip.parent_id IS NOT NULL AND \ + external_ip.time_deleted IS NULL AND \ + external_ip.ip_pool_id = ", + ) + .param() + .bind::(ip_pool_id) + .sql(" AND external_ip.kind != ") + .param() + .bind::(IpKind::Floating) + .sql(" AND instance.time_deleted IS NULL AND project.silo_id = ") + .param() + .bind::(silo_id) + .sql( + " LIMIT 1), floating_ips AS (\ + SELECT 1 \ + FROM external_ip \ + INNER JOIN project ON external_ip.project_id = project.id \ + WHERE \ + external_ip.is_service = FALSE AND \ + external_ip.time_deleted IS NULL AND \ + external_ip.project_id IS NOT NULL AND \ + external_ip.ip_pool_id = ", + ) + .param() + .bind::(ip_pool_id) + .sql(" AND external_ip.kind = ") + .param() + .bind::(IpKind::Floating) + .sql(" AND project.silo_id = ") + .param() + .bind::(silo_id) + .sql( + " AND project.time_deleted IS NULL LIMIT 1) \ + DELETE FROM ip_pool_resource \ + WHERE ip_pool_id = ", + ) + .param() + .bind::(ip_pool_id) + .sql(" AND resource_type = ") + .param() + .bind::( + IpPoolResourceType::Silo, + ) + .sql(" AND resource_id = ") + .param() + .bind::(silo_id) + .sql( + " AND \ + CAST(IF(\ + EXISTS(\ + SELECT 1 FROM instance_ips \ + UNION ALL \ + SELECT 1 FROM floating_ips\ + ), \ + 'has-eips', \ + 'true'\ + ) AS BOOL)", + ); + builder.query() +} + +// Generate a small helper subquery which fails with a bool-cast error if there +// are fewer than 2 IP Pools reserved for the provided use. It must be internal. +fn count_reserved_pools_subquery( + reservation_type: IpPoolReservationType, +) -> String { + assert!(!matches!(reservation_type, IpPoolReservationType::ExternalSilos)); + format!( + "CAST(IF(\ + (SELECT COUNT(1) \ + FROM ip_pool \ + WHERE time_deleted IS NULL AND reservation_type = '{}' LIMIT 2\ + ) >= 2, \ + 'true', \ + '{}') \ + AS BOOL)", + reservation_type, LAST_POOL_SENTINEL, + ) +} + +// Conditionally reserve an IP Pool for a specific use. +// +// # Panics +// +// Panics if the current and new reservation type are the same. +fn reserve_ip_pool_query( + pool: &IpPool, + reservation_type: IpPoolReservationType, +) -> TypedSqlQuery<()> { + assert_ne!(pool.reservation_type, reservation_type); + match pool.reservation_type { + IpPoolReservationType::ExternalSilos => { + reserve_external_ip_pool_query(pool, reservation_type) + } + IpPoolReservationType::OxideInternal => { + reserve_internal_ip_pool_query(pool, reservation_type) + } + } +} + +// Query to conditionally reserve an IP Pool that is currently reserved for +// external silo use. +// +// Checks that the pool is not currently linked to any silos first. Note that +// this means there cannot be any silo-specific resources using the pool. +fn reserve_external_ip_pool_query( + ip_pool: &IpPool, + new_reservation_type: IpPoolReservationType, +) -> TypedSqlQuery<()> { + let mut builder = QueryBuilder::new(); + builder + .sql("UPDATE ip_pool SET reservation_type = ") + .param() + .bind::(new_reservation_type) + .sql(", time_modified = NOW() WHERE id = ") + .param() + .bind::(ip_pool.id()) + .sql(" AND time_deleted IS NULL AND reservation_type = ") + .param() + .bind::(ip_pool.reservation_type) + .sql( + " AND CAST(IF(EXISTS(\ + SELECT 1 \ + FROM ip_pool_resource \ + WHERE ip_pool_id = ", + ) + .param() + .bind::(ip_pool.id()) + .sql(" LIMIT 1), '") + .sql(BAD_SILO_LINK_SENTINEL) + .sql("', 'TRUE') AS BOOL)"); + builder.query() +} + +// Query to conditionally reserve an IP Pool that is currently reserved for Oxide +// internal use. +// +// Checks that: +// +// - There are no external IPs in use by Oxide resources. +// - There is at least one other internal pool of the same reservation type. +fn reserve_internal_ip_pool_query( + ip_pool: &IpPool, + new_reservation_type: IpPoolReservationType, +) -> TypedSqlQuery<()> { + let mut builder = QueryBuilder::new(); + builder + .sql("UPDATE ip_pool SET reservation_type = ") + .param() + .bind::(new_reservation_type) + .sql(", time_modified = NOW() WHERE id = ") + .param() + .bind::(ip_pool.id()) + .sql(" AND time_deleted IS NULL AND reservation_type = ") + .param() + .bind::(ip_pool.reservation_type) + // Generate div-by-zero error if there are IPs + .sql( + " AND (\ + SELECT CAST(IF(EXISTS(\ + SELECT 1 \ + FROM external_ip \ + WHERE ip_pool_id = ", + ) + .param() + .bind::(ip_pool.id()) + .sql( + " AND \ + external_ip.is_service AND \ + time_deleted IS NULL \ + LIMIT 1\ + ), 1/0, 1) AS BOOL))", + ) + // Generate int-cast error if this is the last pool of this reservation + // type. + .sql( + " AND CAST(IF(\ + (SELECT COUNT(1) \ + FROM ip_pool \ + WHERE time_deleted IS NULL \ + AND reservation_type = ", + ) + .param() + .bind::(ip_pool.reservation_type) + .sql( + " \ + LIMIT 2\ + ) >= 2, \ + '1', ", + ) + .param() + .bind::(LAST_POOL_SENTINEL) + .sql(") AS INT) = 1"); + builder.query() +} + #[cfg(test)] mod test { + use std::net::Ipv4Addr; use std::num::NonZeroU32; use crate::authz; - use crate::db::datastore::ip_pool::INTERNAL_SILO_DEFAULT_ERROR; + use crate::db::datastore::ip_pool::{ + BAD_SILO_LINK_ERROR, LAST_POOL_ERROR, POOL_HAS_IPS_ERROR, + link_ip_pool_to_external_silo_query, reserve_ip_pool_query, + unlink_ip_pool_from_external_silo_query, + }; + use crate::db::explain::ExplainableAsync as _; use crate::db::model::{ IpPool, IpPoolResource, IpPoolResourceType, Project, }; + use crate::db::pagination::Paginator; use crate::db::pub_test_utils::TestDatabase; + use crate::db::raw_query_builder::expectorate_query_contents; use assert_matches::assert_matches; - use nexus_db_model::{IpPoolIdentity, IpVersion}; + use async_bb8_diesel::AsyncRunQueryDsl as _; + use diesel::{ + ExpressionMethods as _, QueryDsl as _, SelectableHelper as _, + }; + use nexus_db_lookup::LookupPath; + use nexus_db_model::{IpPoolIdentity, IpPoolReservationType, IpVersion}; + use nexus_sled_agent_shared::inventory::ZoneKind; + use nexus_types::deployment::{ + OmicronZoneExternalFloatingIp, OmicronZoneExternalIp, + }; use nexus_types::external_api::params; use nexus_types::identity::Resource; + use nexus_types::silo::INTERNAL_SILO_ID; use omicron_common::address::{IpRange, Ipv4Range, Ipv6Range}; use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::external::{ DataPageParams, Error, IdentityMetadataCreateParams, LookupType, }; use omicron_test_utils::dev; - use uuid::Uuid; + use omicron_uuid_kinds::{ + ExternalIpUuid, GenericUuid as _, OmicronZoneUuid, + }; #[tokio::test] async fn test_default_ip_pools() { @@ -1375,7 +1836,14 @@ mod test { description: "".to_string(), }; let pool1_for_silo = datastore - .ip_pool_create(&opctx, IpPool::new(&identity, IpVersion::V4)) + .ip_pool_create( + &opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::ExternalSilos, + ), + ) .await .expect("Failed to create IP pool"); @@ -1461,7 +1929,14 @@ mod test { description: "".to_string(), }; let second_silo_default = datastore - .ip_pool_create(&opctx, IpPool::new(&identity, IpVersion::V4)) + .ip_pool_create( + &opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::ExternalSilos, + ), + ) .await .expect("Failed to create pool"); let err = datastore @@ -1481,11 +1956,44 @@ mod test { // now remove the association and we should get nothing again let authz_silo = authz::Silo::new(authz::Fleet, silo_id, LookupType::ById(silo_id)); + let q = + nexus_db_schema::schema::ip_pool_resource::dsl::ip_pool_resource + .select(IpPoolResource::as_select()) + .filter( + nexus_db_schema::schema::ip_pool_resource::dsl::resource_id + .eq(authz_silo.id()), + ) + .get_results_async( + &*datastore + .pool_connection_authorized(opctx) + .await + .unwrap(), + ) + .await + .unwrap(); + println!("{q:#?}"); datastore .ip_pool_unlink_silo(&opctx, &authz_pool1_for_silo, &authz_silo) .await .expect("Failed to unlink IP pool from silo"); + let q = + nexus_db_schema::schema::ip_pool_resource::dsl::ip_pool_resource + .select(IpPoolResource::as_select()) + .filter( + nexus_db_schema::schema::ip_pool_resource::dsl::resource_id + .eq(authz_silo.id()), + ) + .get_results_async( + &*datastore + .pool_connection_authorized(opctx) + .await + .unwrap(), + ) + .await + .unwrap(); + println!("{q:#?}"); + // no default let error = datastore.ip_pools_fetch_default(&opctx).await.unwrap_err(); assert_matches!(error, Error::ObjectNotFound { .. }); @@ -1525,7 +2033,14 @@ mod test { description: "".to_string(), }; let other_pool = datastore - .ip_pool_create(&opctx, IpPool::new(&identity, version)) + .ip_pool_create( + &opctx, + IpPool::new( + &identity, + version, + IpPoolReservationType::ExternalSilos, + ), + ) .await .expect("Failed to create IP pool"); assert_eq!(other_pool.ip_version, version); @@ -1562,117 +2077,43 @@ mod test { logctx.cleanup_successful(); } + // We're breaking out the utilization tests for IPv4 and IPv6 pools, since + // pools only contain one version now. + // + // See https://github.com/oxidecomputer/omicron/issues/8888. #[tokio::test] - async fn cannot_set_default_ip_pool_for_internal_silo() { - let logctx = - dev::test_setup_log("cannot_set_default_ip_pool_for_internal_silo"); + async fn test_ipv4_ip_pool_utilization() { + let logctx = dev::test_setup_log("test_ipv4_ip_pool_utilization"); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); - for ip_version in [IpVersion::V4, IpVersion::V6] { - // Make some new pool. - let params = IpPool { - identity: IpPoolIdentity::new( - Uuid::new_v4(), - IdentityMetadataCreateParams { - name: format!("test-pool-{}", ip_version) - .parse() - .unwrap(), - description: String::new(), - }, + let authz_silo = opctx.authn.silo_required().unwrap(); + let project = Project::new( + authz_silo.id(), + params::ProjectCreate { + identity: IdentityMetadataCreateParams { + name: "my-project".parse().unwrap(), + description: "".to_string(), + }, + }, + ); + let (.., project) = + datastore.project_create(&opctx, project).await.unwrap(); + + // create an IP pool for the silo, add a range to it, and link it to the silo + let identity = IdentityMetadataCreateParams { + name: "my-pool".parse().unwrap(), + description: "".to_string(), + }; + let pool = datastore + .ip_pool_create( + &opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::ExternalSilos, ), - ip_version, - rcgen: 0, - }; - let pool = datastore - .ip_pool_create(&opctx, params) - .await - .expect("Should be able to create pool"); - assert_eq!(pool.ip_version, ip_version); - let authz_pool = - nexus_db_lookup::LookupPath::new(&opctx, datastore) - .ip_pool_id(pool.id()) - .lookup_for(authz::Action::Read) - .await - .expect("Should be able to lookup new IP Pool") - .0; - - // Try to link it as the default. - let (authz_silo, ..) = - nexus_db_lookup::LookupPath::new(&opctx, datastore) - .silo_id(nexus_types::silo::INTERNAL_SILO_ID) - .lookup_for(authz::Action::Read) - .await - .expect("Should be able to lookup internal silo"); - let link = IpPoolResource { - ip_pool_id: authz_pool.id(), - resource_type: IpPoolResourceType::Silo, - resource_id: authz_silo.id(), - is_default: true, - }; - let Err(e) = datastore.ip_pool_link_silo(opctx, link).await else { - panic!( - "should have failed to link IP Pool to internal silo as a default" - ); - }; - let Error::InvalidRequest { message } = &e else { - panic!("should have received an invalid request, got: {:?}", e); - }; - assert_eq!(message.external_message(), INTERNAL_SILO_DEFAULT_ERROR); - - // We can link it if it's not the default. - let link = IpPoolResource { is_default: false, ..link }; - datastore.ip_pool_link_silo(opctx, link).await.expect( - "Should be able to link non-default pool to internal silo", - ); - - // Try to set it to the default, and ensure that this also fails. - let Err(e) = datastore - .ip_pool_set_default(opctx, &authz_pool, &authz_silo, true) - .await - else { - panic!("should have failed to set internal pool to default"); - }; - let Error::InvalidRequest { message } = &e else { - panic!("should have received an invalid request, got: {:?}", e); - }; - assert_eq!(message.external_message(), INTERNAL_SILO_DEFAULT_ERROR); - } - - db.terminate().await; - logctx.cleanup_successful(); - } - - // We're breaking out the utilization tests for IPv4 and IPv6 pools, since - // pools only contain one version now. - // - // See https://github.com/oxidecomputer/omicron/issues/8888. - #[tokio::test] - async fn test_ipv4_ip_pool_utilization() { - let logctx = dev::test_setup_log("test_ipv4_ip_pool_utilization"); - let db = TestDatabase::new_with_datastore(&logctx.log).await; - let (opctx, datastore) = (db.opctx(), db.datastore()); - - let authz_silo = opctx.authn.silo_required().unwrap(); - let project = Project::new( - authz_silo.id(), - params::ProjectCreate { - identity: IdentityMetadataCreateParams { - name: "my-project".parse().unwrap(), - description: "".to_string(), - }, - }, - ); - let (.., project) = - datastore.project_create(&opctx, project).await.unwrap(); - - // create an IP pool for the silo, add a range to it, and link it to the silo - let identity = IdentityMetadataCreateParams { - name: "my-pool".parse().unwrap(), - description: "".to_string(), - }; - let pool = datastore - .ip_pool_create(&opctx, IpPool::new(&identity, IpVersion::V4)) + ) .await .expect("Failed to create IP pool"); let authz_pool = authz::IpPool::new( @@ -1776,7 +2217,14 @@ mod test { description: "".to_string(), }; let pool = datastore - .ip_pool_create(&opctx, IpPool::new(&identity, IpVersion::V6)) + .ip_pool_create( + &opctx, + IpPool::new( + &identity, + IpVersion::V6, + IpPoolReservationType::ExternalSilos, + ), + ) .await .expect("Failed to create IP pool"); let authz_pool = authz::IpPool::new( @@ -1910,7 +2358,14 @@ mod test { description: "".to_string(), }; let pool = datastore - .ip_pool_create(&opctx, IpPool::new(&identity, version)) + .ip_pool_create( + &opctx, + IpPool::new( + &identity, + version, + IpPoolReservationType::ExternalSilos, + ), + ) .await .expect("Failed to create IP pool"); let authz_pool = authz::IpPool::new( @@ -1934,4 +2389,739 @@ mod test { db.terminate().await; logctx.cleanup_successful(); } + + #[tokio::test] + async fn paginate_ip_pools_by_delegation_type() { + let logctx = + dev::test_setup_log("paginate_ip_pools_by_delegation_type"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Insert a bunch of pools, not linked to any silo, and so reserved for + // customer use. + const N_POOLS: usize = 20; + let mut customer_pools = Vec::with_capacity(N_POOLS); + for i in 0..N_POOLS { + // Create the pool + let identity = IdentityMetadataCreateParams { + name: format!("ip-pool-{i}").parse().unwrap(), + description: "".to_string(), + }; + let pool = datastore + .ip_pool_create( + opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::ExternalSilos, + ), + ) + .await + .expect("Failed to create IP pool"); + customer_pools.push(pool); + } + customer_pools.sort_by_key(|pool| pool.id()); + + // Create a bunch which _are_ reserved for Oxide's usage. + let mut oxide_pools = Vec::with_capacity(N_POOLS); + for i in 0..N_POOLS { + // Create the pool + let identity = IdentityMetadataCreateParams { + name: format!("oxide-ip-pool-{i}").parse().unwrap(), + description: "".to_string(), + }; + let pool = datastore + .ip_pool_create( + opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::OxideInternal, + ), + ) + .await + .expect("Failed to create reserved IP pool"); + oxide_pools.push(pool); + } + assert_eq!(oxide_pools.len(), N_POOLS); + + let fetch_paginated = |reservation_type| async move { + let mut found = Vec::with_capacity(N_POOLS); + let mut paginator = Paginator::new( + NonZeroU32::new(5).unwrap(), + dropshot::PaginationOrder::Ascending, + ); + while let Some(page) = paginator.next() { + let batch = datastore + .ip_pools_list_paginated( + opctx, + reservation_type, + None, + &PaginatedBy::Id(page.current_pagparams()), + ) + .await + .expect("Should be able to list pools with pagination"); + paginator = page.found_batch(&batch, &|pool| pool.id()); + found.extend(batch.into_iter()); + } + found + }; + + // Paginate all the customer-reserved. + let customer_pools_found = + fetch_paginated(IpPoolReservationType::ExternalSilos).await; + assert_eq!(customer_pools.len(), customer_pools_found.len()); + assert_eq!(customer_pools, customer_pools_found); + + // Paginate all those reserved for Oxide. + // + // Note that we have 2 extra pools today, which are the builtin service + // pools. These will go away in the future, so we'll unfortunately need + // to update this test at that time. Until then, fetch those service + // pools explicitly and add them. + let oxide_reserved_found = + fetch_paginated(IpPoolReservationType::OxideInternal).await; + let pools = datastore + .ip_pools_service_lookup_both_versions(opctx) + .await + .unwrap(); + oxide_pools.push(pools.ipv4.db_pool); + oxide_pools.push(pools.ipv6.db_pool); + oxide_pools.sort_by_key(|pool| pool.id()); + assert_eq!(oxide_pools.len(), oxide_reserved_found.len()); + assert_eq!(oxide_pools, oxide_reserved_found); + + db.terminate().await; + logctx.cleanup_successful(); + } + + // Ensure we have the right query contents. + #[tokio::test] + async fn expectorate_insert_ip_pool_external_silo_link() { + let res = IpPoolResource { + ip_pool_id: uuid::uuid!("aaa84fbd-85a5-4fcb-b34f-23b7e56145c7"), + resource_type: IpPoolResourceType::Silo, + resource_id: INTERNAL_SILO_ID, + is_default: false, + }; + let query = link_ip_pool_to_external_silo_query(&res); + expectorate_query_contents( + &query, + "tests/output/ip_pool_external_silo_link.sql", + ) + .await; + } + + // Explain the SQL query inserting an IP Pool link to a customer silo + #[tokio::test] + async fn can_explain_link_ip_pool_to_silo_query() { + let logctx = + dev::test_setup_log("can_explain_link_ip_pool_to_silo_query"); + let db = TestDatabase::new_with_pool(&logctx.log).await; + let pool = db.pool(); + let conn = pool.claim().await.unwrap(); + + let res = IpPoolResource { + ip_pool_id: uuid::uuid!("aaa84fbd-85a5-4fcb-b34f-23b7e56145c7"), + resource_type: IpPoolResourceType::Silo, + resource_id: INTERNAL_SILO_ID, + is_default: false, + }; + + let query = link_ip_pool_to_external_silo_query(&res); + let _ = query + .explain_async(&conn) + .await + .expect("Failed to explain query - is it valid SQL?"); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn cannot_link_oxide_internal_pool_to_external_silo() { + let logctx = dev::test_setup_log( + "cannot_link_oxide_internal_pool_to_external_silo", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create the pool + let identity = IdentityMetadataCreateParams { + name: "internal-ip-pool".parse().unwrap(), + description: "".to_string(), + }; + let ip_pool = datastore + .ip_pool_create( + opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::OxideInternal, + ), + ) + .await + .expect("Failed to create IP pool"); + + // We should fail to link it to some other silo now. + let link = IpPoolResource { + ip_pool_id: ip_pool.id(), + resource_type: IpPoolResourceType::Silo, + resource_id: uuid::uuid!("cfb16a9d-764e-4c5d-8d0d-cf737885b84a"), + is_default: false, + }; + let res = datastore.ip_pool_link_silo(&opctx, link).await; + let Err(Error::InvalidRequest { message }) = &res else { + panic!( + "Expected to fail linking an internally-reserved \ + IP Pool to an external Silo, found: {res:#?}", + ); + }; + assert_eq!(message.external_message(), BAD_SILO_LINK_ERROR); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn cannot_reserve_externally_linked_pool_for_internal_use() { + let logctx = dev::test_setup_log( + "cannot_reserve_externally_linked_pool_for_internal_use", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create the pool, reserved for external silos. + let identity = IdentityMetadataCreateParams { + name: "external-ip-pool".parse().unwrap(), + description: "".to_string(), + }; + let ip_pool = datastore + .ip_pool_create( + opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::ExternalSilos, + ), + ) + .await + .expect("Failed to create IP pool"); + + // Link to an external silo. + let external_link = IpPoolResource { + ip_pool_id: ip_pool.id(), + resource_type: IpPoolResourceType::Silo, + resource_id: nexus_types::silo::DEFAULT_SILO_ID, + is_default: false, + }; + let _ = datastore + .ip_pool_link_silo(&opctx, external_link) + .await + .expect("Should be able to link unlinked pool to default silo"); + + // We should fail to reserve it for Oxide-internal use now. + let (authz_pool, db_pool) = LookupPath::new(opctx, datastore) + .ip_pool_id(ip_pool.id()) + .fetch_for(authz::Action::Modify) + .await + .unwrap(); + let res = datastore + .ip_pool_reserve( + opctx, + &authz_pool, + &db_pool, + IpPoolReservationType::OxideInternal, + ) + .await; + let Err(Error::InvalidRequest { message }) = &res else { + panic!( + "Expected to fail delegating an IP Pool \ + when it's already linked to an external silo, found {res:#?}" + ); + }; + assert_eq!(message.external_message(), BAD_SILO_LINK_ERROR); + + db.terminate().await; + logctx.cleanup_successful(); + } + + // Ensure that we fail to link to a silo that is deleted. + #[tokio::test] + async fn cannot_link_pool_to_deleted_silo() { + let logctx = dev::test_setup_log("cannot_link_pool_to_deleted_silo"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create the pool + let identity = IdentityMetadataCreateParams { + name: "external-ip-pool".parse().unwrap(), + description: "".to_string(), + }; + let ip_pool = datastore + .ip_pool_create( + opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::ExternalSilos, + ), + ) + .await + .expect("Failed to create IP pool"); + + // Delete the silo, directly to avoid a bunch of machinery. + use nexus_db_schema::schema::silo::dsl; + let c = + diesel::update(dsl::silo.find(nexus_types::silo::DEFAULT_SILO_ID)) + .set(dsl::time_deleted.eq(diesel::dsl::now)) + .execute_async( + &*datastore + .pool_connection_authorized(opctx) + .await + .unwrap(), + ) + .await + .expect("Should be able to soft-delete silo"); + assert_eq!(c, 1, "Should have deleted something"); + + // Now try link to it. + let external_link = IpPoolResource { + ip_pool_id: ip_pool.id(), + resource_type: IpPoolResourceType::Silo, + resource_id: nexus_types::silo::DEFAULT_SILO_ID, + is_default: false, + }; + let err = datastore + .ip_pool_link_silo(&opctx, external_link) + .await + .expect_err("Should have failed to link IP Pool to deleted Silo"); + assert_matches!(err, Error::ObjectNotFound { .. }); + + db.terminate().await; + logctx.cleanup_successful(); + } + + // Ensure that we fail to link a deleted pool to a silo. + #[tokio::test] + async fn cannot_link_silo_to_deleted_pool() { + let logctx = dev::test_setup_log("cannot_link_silo_to_deleted_pool"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create the pool + let identity = IdentityMetadataCreateParams { + name: "external-ip-pool".parse().unwrap(), + description: "".to_string(), + }; + let ip_pool = datastore + .ip_pool_create( + opctx, + IpPool::new( + &identity, + IpVersion::V4, + IpPoolReservationType::ExternalSilos, + ), + ) + .await + .expect("Failed to create IP pool"); + + // Delete the pool, directly to avoid a bunch of machinery. + use nexus_db_schema::schema::ip_pool::dsl; + let c = diesel::update(dsl::ip_pool.find(ip_pool.id())) + .set(dsl::time_deleted.eq(diesel::dsl::now)) + .execute_async( + &*datastore.pool_connection_authorized(opctx).await.unwrap(), + ) + .await + .expect("Should be able to soft-delete IP Pool"); + assert_eq!(c, 1, "Should have deleted something"); + + // Now try link to it. + let external_link = IpPoolResource { + ip_pool_id: ip_pool.id(), + resource_type: IpPoolResourceType::Silo, + resource_id: nexus_types::silo::DEFAULT_SILO_ID, + is_default: false, + }; + let err = datastore + .ip_pool_link_silo(&opctx, external_link) + .await + .expect_err("Should have failed to link deleted IP Pool to Silo"); + assert_matches!(err, Error::ObjectNotFound { .. }); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn can_explain_unlink_ip_pool_from_external_silo_query() { + let logctx = dev::test_setup_log( + "can_explain_unlink_ip_pool_from_external_silo_query", + ); + let db = TestDatabase::new_with_pool(&logctx.log).await; + let pool = db.pool(); + let conn = pool.claim().await.unwrap(); + + let ip_pool_id = uuid::uuid!("aaa84fbd-85a5-4fcb-b34f-23b7e56145c7"); + let silo_id = nexus_types::silo::DEFAULT_SILO_ID; + let query = + unlink_ip_pool_from_external_silo_query(ip_pool_id, silo_id); + let _ = query + .explain_async(&conn) + .await + .expect("Failed to explain query - is it valid SQL?"); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn cannot_delete_last_internally_reserved_ip_pool() { + let logctx = dev::test_setup_log( + "cannot_delete_last_internally_reserved_ip_pool", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Fetch the pools. + let pools = datastore + .ip_pools_service_lookup_both_versions(opctx) + .await + .unwrap(); + + // We should be able to delete one of these. + let _ = datastore + .ip_pool_delete(opctx, &pools.ipv4.authz_pool, &pools.ipv4.db_pool) + .await + .expect( + "Should be able to delete internally-reserved \ + IP Pool when at least one remains", + ); + + // Check there's only one left. + let pagparams = &PaginatedBy::Id(DataPageParams { + marker: None, + direction: dropshot::PaginationOrder::Ascending, + limit: 100.try_into().unwrap(), + }); + let l = datastore + .ip_pools_list_paginated( + opctx, + IpPoolReservationType::OxideInternal, + None, + &pagparams, + ) + .await + .unwrap(); + assert_eq!(l.len(), 1); + + // We should _not_ be able to delete the other now, because there's only + // one left. + let res = datastore + .ip_pool_delete(opctx, &pools.ipv6.authz_pool, &pools.ipv6.db_pool) + .await; + + let Err(Error::InvalidRequest { message }) = &res else { + panic!( + "Should not be able to delete internally-reserved \ + IP Pool when only one remains, found {res:#?}" + ); + }; + assert_eq!(message.external_message(), LAST_POOL_ERROR); + + let l = datastore + .ip_pools_list_paginated( + opctx, + IpPoolReservationType::OxideInternal, + None, + &pagparams, + ) + .await + .unwrap(); + assert_eq!(l.len(), 1); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn cannot_externally_reserve_last_internally_reserved_ip_pool() { + let logctx = dev::test_setup_log( + "cannot_externally_reserve_last_internally_reserved_ip_pool", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Fetch the pools. + let pools = datastore + .ip_pools_service_lookup_both_versions(opctx) + .await + .unwrap(); + + // We should be able to reserve one of these for external use. + let _ = datastore + .ip_pool_reserve( + opctx, + &pools.ipv4.authz_pool, + &pools.ipv4.db_pool, + IpPoolReservationType::ExternalSilos, + ) + .await + .expect( + "Should be able to externally reserve IP Pool \ + when at least one internally-reserved pool remains", + ); + + // Check there's only one left. + let pagparams = &PaginatedBy::Id(DataPageParams { + marker: None, + direction: dropshot::PaginationOrder::Ascending, + limit: 100.try_into().unwrap(), + }); + let l = datastore + .ip_pools_list_paginated( + opctx, + IpPoolReservationType::OxideInternal, + None, + &pagparams, + ) + .await + .unwrap(); + assert_eq!(l.len(), 1); + + // We should _not_ be able to reserve the other for external use now, + // because there's only one left for internal use. + let res = datastore + .ip_pool_reserve( + opctx, + &pools.ipv6.authz_pool, + &pools.ipv6.db_pool, + IpPoolReservationType::ExternalSilos, + ) + .await; + let Err(Error::InvalidRequest { message }) = &res else { + panic!( + "Should not be able to externally-reserve an \ + internally-reserved IP Pool when only one remains, \ + found {res:#?}" + ); + }; + assert_eq!(message.external_message(), LAST_POOL_ERROR); + + let l = datastore + .ip_pools_list_paginated( + opctx, + IpPoolReservationType::OxideInternal, + None, + &pagparams, + ) + .await + .unwrap(); + assert_eq!(l.len(), 1); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn cannot_externally_reserve_ip_pool_with_outstanding_external_ips() { + let logctx = dev::test_setup_log( + "cannot_externally_reserve_ip_pool_with_outstanding_external_ips", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Get pool, add a range, allocate an external IP. + let pools = datastore + .ip_pools_service_lookup_both_versions(opctx) + .await + .unwrap(); + let ip_range = IpRange::V4(Ipv4Range { + first: Ipv4Addr::new(1, 1, 1, 1), + last: Ipv4Addr::new(1, 1, 1, 10), + }); + datastore + .ip_pool_add_range( + opctx, + &pools.ipv4.authz_pool, + &pools.ipv4.db_pool, + &ip_range, + ) + .await + .unwrap(); + + // Create an IP for an Omicron zone. + let eip = datastore + .external_ip_allocate_omicron_zone( + opctx, + OmicronZoneUuid::from_untyped_uuid(uuid::uuid!( + "b7b641d6-f52c-4fd5-b5a5-66ac3918c8b4" + )), + ZoneKind::BoundaryNtp, + OmicronZoneExternalIp::Floating( + OmicronZoneExternalFloatingIp { + id: ExternalIpUuid::from_untyped_uuid(uuid::uuid!( + "4a7f86aa-5cab-42dd-afa5-7eee6304cb8c" + )), + ip: ip_range.first_address(), + }, + ), + ) + .await + .expect("Should be able to create zone external IP"); + + // Should not be able to externally-reserve the IPv4 pool now, since + // we've got an address in use. + let res = datastore + .ip_pool_reserve( + opctx, + &pools.ipv4.authz_pool, + &pools.ipv4.db_pool, + IpPoolReservationType::ExternalSilos, + ) + .await; + let Err(Error::InvalidRequest { message }) = &res else { + panic!( + "Should not be able to externally reserve internal \ + IP Pool when an address is in use, found {res:#?}" + ); + }; + assert_eq!(message.external_message(), POOL_HAS_IPS_ERROR); + + // Delete the address, and now we can reserve the pool for external use. + let _ = datastore + .deallocate_external_ip(opctx, eip.id) + .await + .expect("Should be able to delete external IP"); + let _ = datastore.ip_pool_reserve( + opctx, + &pools.ipv4.authz_pool, + &pools.ipv4.db_pool, + IpPoolReservationType::ExternalSilos, + ).await + .expect( + "Should be able to delete internal IP Pool when more than one remains, \ + after deleting external IP address" + ); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn can_explain_reserve_external_ip_pool_query() { + let logctx = + dev::test_setup_log("can_explain_reserve_external_ip_pool_query"); + let db = TestDatabase::new_with_pool(&logctx.log).await; + let pool = db.pool(); + let conn = pool.claim().await.unwrap(); + + let ip_pool = IpPool { + identity: IpPoolIdentity::new( + uuid::uuid!("93fea64d-5d0a-4cc6-8f94-7c527ee640a9"), + IdentityMetadataCreateParams { + name: "some-pool".parse().unwrap(), + description: String::new(), + }, + ), + ip_version: IpVersion::V4, + rcgen: 0, + reservation_type: IpPoolReservationType::ExternalSilos, + }; + let query = reserve_ip_pool_query( + &ip_pool, + IpPoolReservationType::OxideInternal, + ); + let _ = query + .explain_async(&conn) + .await + .expect("Failed to explain query - is it valid SQL?"); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn expectorate_reserve_external_ip_pool_query() { + let ip_pool = IpPool { + identity: IpPoolIdentity::new( + uuid::uuid!("93fea64d-5d0a-4cc6-8f94-7c527ee640a9"), + IdentityMetadataCreateParams { + name: "some-pool".parse().unwrap(), + description: String::new(), + }, + ), + ip_version: IpVersion::V4, + rcgen: 0, + reservation_type: IpPoolReservationType::ExternalSilos, + }; + let query = reserve_ip_pool_query( + &ip_pool, + IpPoolReservationType::OxideInternal, + ); + expectorate_query_contents( + &query, + "tests/output/reserve_external_ip_pool.sql", + ) + .await; + } + + #[tokio::test] + async fn can_explain_reserve_internal_ip_pool_query() { + let logctx = + dev::test_setup_log("can_explain_reserve_internal_ip_pool_query"); + let db = TestDatabase::new_with_pool(&logctx.log).await; + let pool = db.pool(); + let conn = pool.claim().await.unwrap(); + + let ip_pool = IpPool { + identity: IpPoolIdentity::new( + uuid::uuid!("93fea64d-5d0a-4cc6-8f94-7c527ee640a9"), + IdentityMetadataCreateParams { + name: "some-pool".parse().unwrap(), + description: String::new(), + }, + ), + ip_version: IpVersion::V4, + rcgen: 0, + reservation_type: IpPoolReservationType::OxideInternal, + }; + let query = reserve_ip_pool_query( + &ip_pool, + IpPoolReservationType::ExternalSilos, + ); + let _ = query + .explain_async(&conn) + .await + .expect("Failed to explain query - is it valid SQL?"); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn expectorate_reserve_internal_ip_pool_query() { + let ip_pool = IpPool { + identity: IpPoolIdentity::new( + uuid::uuid!("93fea64d-5d0a-4cc6-8f94-7c527ee640a9"), + IdentityMetadataCreateParams { + name: "some-pool".parse().unwrap(), + description: String::new(), + }, + ), + ip_version: IpVersion::V4, + rcgen: 0, + reservation_type: IpPoolReservationType::OxideInternal, + }; + let query = reserve_ip_pool_query( + &ip_pool, + IpPoolReservationType::ExternalSilos, + ); + expectorate_query_contents( + &query, + "tests/output/reserve_internal_ip_pool.sql", + ) + .await; + } } diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index d7f8e37e9ec..2bb72d4e9ad 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -58,7 +58,6 @@ use nexus_types::external_api::shared; use nexus_types::external_api::shared::IpRange; use nexus_types::external_api::shared::SiloRole; use nexus_types::identity::Resource; -use nexus_types::silo::INTERNAL_SILO_ID; use omicron_common::api::external::AllowedSourceIps; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Error; @@ -693,8 +692,12 @@ impl DataStore { opctx.authorize(authz::Action::CreateChild, &authz::FLEET).await?; - // We may need to populate external IP records for both IPv4 and IPv6 - // service pools, so fetch both now. + // The `RackInit` request will eventually be modified to include the + // full details of the IP Pool(s) delegated to Oxide at RSS time. For + // now, we still rely on the pre-populated IP Pools. There's one for + // IPv4 and one for IPv6. + // + // See https://github.com/oxidecomputer/omicron/issues/8946. let service_ip_pools = self.ip_pools_service_lookup_both_versions(&opctx).await?; @@ -1001,7 +1004,8 @@ impl DataStore { self.rack_insert(opctx, &db::model::Rack::new(rack_id)).await?; - // Insert and link the services IP Pool for both IP versions. + // Insert an IP Pool for both IP versions, reserved for Oxide internal + // use. for (version, name) in [ (IpVersion::V4, SERVICE_IPV4_POOL_NAME), (IpVersion::V6, SERVICE_IPV6_POOL_NAME), @@ -1014,29 +1018,11 @@ impl DataStore { ), }, version, + nexus_db_model::IpPoolReservationType::OxideInternal, ); - // Create the pool, and link it to the internal silo if needed. But - // we cannot set a default. - let internal_pool_id = internal_pool.id(); - let internal_created = self - .ip_pool_create(opctx, internal_pool) - .await - .map(|_| true) - .or_else(|e| match e { - Error::ObjectAlreadyExists { .. } => Ok(false), - _ => Err(e), - })?; - if internal_created { - self.ip_pool_link_silo( - opctx, - db::model::IpPoolResource { - ip_pool_id: internal_pool_id, - resource_type: db::model::IpPoolResourceType::Silo, - resource_id: INTERNAL_SILO_ID, - is_default: false, - }, - ) - .await?; + match self.ip_pool_create(opctx, internal_pool).await { + Ok(_) | Err(Error::ObjectAlreadyExists { .. }) => {} + Err(e) => return Err(e), } } diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index c7ae3743f3b..a045c726e01 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -879,6 +879,7 @@ mod tests { use nexus_db_model::ByteCount; use nexus_db_model::Instance; use nexus_db_model::InstanceCpuCount; + use nexus_db_model::IpPoolReservationType; use nexus_db_model::IpPoolResource; use nexus_db_model::IpPoolResourceType; use nexus_db_model::IpVersion; @@ -927,6 +928,7 @@ mod tests { description: format!("ip pool {}", name), }, IpVersion::V4, + IpPoolReservationType::ExternalSilos, ); self.db diff --git a/nexus/db-queries/src/db/queries/oximeter.rs b/nexus/db-queries/src/db/queries/oximeter.rs index cea319ccc5d..c04f28a5d13 100644 --- a/nexus/db-queries/src/db/queries/oximeter.rs +++ b/nexus/db-queries/src/db/queries/oximeter.rs @@ -5,8 +5,9 @@ //! Implementation of queries for Oximeter collectors and producers. use crate::db::column_walker::AllColumnsOf; -use crate::db::raw_query_builder::{QueryBuilder, TypedSqlQuery}; -use diesel::pg::Pg; +use crate::db::raw_query_builder::{ + QueryBuilder, SelectableSql, TypedSqlQuery, +}; use diesel::sql_types; use ipnetwork::IpNetwork; use nexus_db_model::{OximeterInfo, ProducerKind, SqlU16}; @@ -16,9 +17,6 @@ use uuid::Uuid; type AllColumnsOfOximeterInfo = AllColumnsOf; -type SelectableSql = < - >::SelectExpression as diesel::Expression ->::SqlType; /// Upsert a metric producer. /// diff --git a/nexus/db-queries/src/db/raw_query_builder.rs b/nexus/db-queries/src/db/raw_query_builder.rs index 2c42177aa13..f855dbd6ec9 100644 --- a/nexus/db-queries/src/db/raw_query_builder.rs +++ b/nexus/db-queries/src/db/raw_query_builder.rs @@ -195,3 +195,10 @@ pub async fn expectorate_query_contents>( expectorate::assert_contents(path, &s); } + +/// Type alias for the SQL type of a selectable expression. +/// +/// This is useful for the return type of a query built using [`QueryBuilder`]. +pub type SelectableSql = < + >::SelectExpression as diesel::Expression +>::SqlType; diff --git a/nexus/db-queries/tests/output/ip_pool_external_silo_link.sql b/nexus/db-queries/tests/output/ip_pool_external_silo_link.sql new file mode 100644 index 00000000000..8b1876dfe17 --- /dev/null +++ b/nexus/db-queries/tests/output/ip_pool_external_silo_link.sql @@ -0,0 +1,23 @@ +WITH + ip_pool + AS ( + SELECT + CAST(IF(reservation_type != $1, 'bad-link-type', $2) AS UUID) AS id + FROM + ip_pool + WHERE + id = $3 AND time_deleted IS NULL + ), + silo AS (SELECT id FROM silo WHERE id = $4 AND time_deleted IS NULL) +INSERT +INTO + ip_pool_resource (ip_pool_id, resource_type, resource_id, is_default) +SELECT + CAST(COALESCE(CAST(ip.id AS STRING), 'ip-pool-deleted') AS UUID), + $5, + CAST(COALESCE(CAST(s.id AS STRING), 'silo-deleted') AS UUID), + $6 +FROM + (SELECT 1) AS dummy LEFT JOIN ip_pool AS ip ON true LEFT JOIN silo AS s ON true +RETURNING + * diff --git a/nexus/db-queries/tests/output/reserve_external_ip_pool.sql b/nexus/db-queries/tests/output/reserve_external_ip_pool.sql new file mode 100644 index 00000000000..8dacb652b63 --- /dev/null +++ b/nexus/db-queries/tests/output/reserve_external_ip_pool.sql @@ -0,0 +1,16 @@ +UPDATE + ip_pool +SET + reservation_type = $1, time_modified = now() +WHERE + id = $2 + AND time_deleted IS NULL + AND reservation_type = $3 + AND CAST( + IF( + EXISTS(SELECT 1 FROM ip_pool_resource WHERE ip_pool_id = $4 LIMIT 1), + 'bad-link-type', + 'TRUE' + ) + AS BOOL + ) diff --git a/nexus/db-queries/tests/output/reserve_internal_ip_pool.sql b/nexus/db-queries/tests/output/reserve_internal_ip_pool.sql new file mode 100644 index 00000000000..4421c5612b2 --- /dev/null +++ b/nexus/db-queries/tests/output/reserve_internal_ip_pool.sql @@ -0,0 +1,38 @@ +UPDATE + ip_pool +SET + reservation_type = $1, time_modified = now() +WHERE + id = $2 + AND time_deleted IS NULL + AND reservation_type = $3 + AND ( + SELECT + CAST( + IF( + EXISTS( + SELECT + 1 + FROM + external_ip + WHERE + ip_pool_id = $4 AND external_ip.is_service AND time_deleted IS NULL + LIMIT + 1 + ), + 1 / 0, + 1 + ) + AS BOOL + ) + ) + AND CAST( + IF( + (SELECT count(1) FROM ip_pool WHERE time_deleted IS NULL AND reservation_type = $5 LIMIT 2) + >= 2, + '1', + $6 + ) + AS INT8 + ) + = 1 diff --git a/nexus/db-schema/src/enums.rs b/nexus/db-schema/src/enums.rs index 5ea98088306..9abb574eb86 100644 --- a/nexus/db-schema/src/enums.rs +++ b/nexus/db-schema/src/enums.rs @@ -57,6 +57,7 @@ define_enums! { InvZoneManifestSourceEnum => "inv_zone_manifest_source", IpAttachStateEnum => "ip_attach_state", IpKindEnum => "ip_kind", + IpPoolReservationTypeEnum => "ip_pool_reservation_type", IpPoolResourceTypeEnum => "ip_pool_resource_type", IpVersionEnum => "ip_version", MigrationStateEnum => "migration_state", diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 361ed15dc62..26130519728 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -630,6 +630,7 @@ table! { time_deleted -> Nullable, ip_version -> crate::enums::IpVersionEnum, rcgen -> Int8, + reservation_type -> crate::enums::IpPoolReservationTypeEnum, } } diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index f51bc8a5541..ecb5d2e9041 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -9,6 +9,7 @@ use crate::external_api::shared::IpRange; use ipnetwork::IpNetwork; use nexus_db_lookup::LookupPath; use nexus_db_lookup::lookup; +use nexus_db_model::IpPoolReservationType; use nexus_db_model::IpVersion; use nexus_db_queries::authz; use nexus_db_queries::authz::ApiResource; @@ -79,7 +80,11 @@ impl super::Nexus { "IPv6 pools are not yet supported", )); } - let pool = db::model::IpPool::new(&pool_params.identity, ip_version); + let pool = db::model::IpPool::new( + &pool_params.identity, + ip_version, + IpPoolReservationType::ExternalSilos, + ); self.db_datastore.ip_pool_create(opctx, pool).await } diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index fa6fa2839ec..19ce8bcf21c 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -6,6 +6,7 @@ use std::net::Ipv4Addr; +use crate::integration_tests::instances::create_project_and_pool; use crate::integration_tests::instances::instance_wait_for_state; use dropshot::HttpErrorResponseBody; use dropshot::ResultsPage; @@ -20,6 +21,7 @@ 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::resource_helpers::assert_ip_pool_utilization; +use nexus_test_utils::resource_helpers::create_floating_ip; use nexus_test_utils::resource_helpers::create_instance; use nexus_test_utils::resource_helpers::create_ip_pool; use nexus_test_utils::resource_helpers::create_local_user; @@ -584,6 +586,100 @@ async fn test_ip_pool_silo_link(cptestctx: &ControlPlaneTestContext) { object_delete(client, "/v1/system/ip-pools/p1").await; } +#[nexus_test] +async fn cannot_unlink_ip_pool_with_outstanding_instance_ips( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + let apictx = &cptestctx.server.server_context(); + let nexus = &apictx.nexus; + + // Create a project and add a default IP Pool. + const POOL_NAME: &str = "default"; + let proj = create_project_and_pool(client).await; + + // Create an instance, which allocates an IP address. + const INSTANCE_NAME: &str = "myinst"; + let instance = + create_instance(client, proj.name().as_str(), INSTANCE_NAME).await; + + // We cannot delete the link between the silo and pool until the instance is + // gone. + let url = format!( + "/v1/system/ip-pools/{}/silos/{}", + POOL_NAME, + DEFAULT_SILO.name().as_str() + ); + object_delete_error(client, &url, StatusCode::BAD_REQUEST).await; + + // Stop the instance and wait until it's actually stopped. + let instance_stop_url = format!( + "/v1/instances/{}/stop?project={}", + INSTANCE_NAME, + proj.name().as_str(), + ); + NexusRequest::new( + RequestBuilder::new(client, Method::POST, &instance_stop_url) + .body(None as Option<&serde_json::Value>) + .expect_status(Some(StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to stop instance"); + + // Simulate the transition, wait until it is in fact stopped. + let instance_id = InstanceUuid::from_untyped_uuid(instance.id()); + let info = nexus + .active_instance_info(&instance_id, None) + .await + .unwrap() + .expect("running instance should be on a sled"); + info.sled_client.vmm_finish_transition(info.propolis_id).await; + instance_wait_for_state(client, instance_id, InstanceState::Stopped).await; + + // Now that it's stopped, delete the instance, and then assert that we can + // actually unlink the IP Pool from the Silo. + object_delete(client, &format!("/v1/instances/{}", instance_id)).await; + object_delete(client, &url).await; +} + +#[nexus_test] +async fn cannot_unlink_ip_pool_with_outstanding_floating_ips( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + // Create a project and add a default IP Pool. + const POOL_NAME: &str = "default"; + let proj = create_project_and_pool(client).await; + + // Create a floating IP from the pool. + let fip = create_floating_ip( + client, + "fip", + proj.name().as_str(), + None, + Some(POOL_NAME), + ) + .await; + + // We cannot delete the link between the silo and pool until the floating IP + // has been deleted. + let url = format!( + "/v1/system/ip-pools/{}/silos/{}", + POOL_NAME, + DEFAULT_SILO.name().as_str() + ); + object_delete_error(client, &url, StatusCode::BAD_REQUEST).await; + + // Now delete the FIP. + object_delete(client, &format!("/v1/floating-ips/{}", fip.id())).await; + + // Now we can do the dew. + object_delete(client, &url).await; +} + /// Non-discoverable silos can be linked to a pool, but they do not show up /// in the list of silos for that pool, just as they do not show up in the /// top-level list of silos @@ -889,6 +985,7 @@ async fn test_ipv6_ip_pool_utilization_total( nexus_db_model::IpPool::new( &identity, nexus_db_model::IpVersion::V6, + nexus_db_model::IpPoolReservationType::ExternalSilos, ), ) .await diff --git a/schema/crdb/add-ip-pool-reservation-type-column/up01.sql b/schema/crdb/add-ip-pool-reservation-type-column/up01.sql new file mode 100644 index 00000000000..c3c30fc8148 --- /dev/null +++ b/schema/crdb/add-ip-pool-reservation-type-column/up01.sql @@ -0,0 +1,5 @@ +CREATE TYPE IF NOT EXISTS +omicron.public.ip_pool_reservation_type AS ENUM ( + 'external_silos', + 'oxide_internal' +); diff --git a/schema/crdb/add-ip-pool-reservation-type-column/up02.sql b/schema/crdb/add-ip-pool-reservation-type-column/up02.sql new file mode 100644 index 00000000000..c56b148d92a --- /dev/null +++ b/schema/crdb/add-ip-pool-reservation-type-column/up02.sql @@ -0,0 +1,3 @@ +ALTER TABLE omicron.public.ip_pool +ADD COLUMN IF NOT EXISTS reservation_type omicron.public.ip_pool_reservation_type NOT NULL +DEFAULT 'external_silos'; diff --git a/schema/crdb/add-ip-pool-reservation-type-column/up03.sql b/schema/crdb/add-ip-pool-reservation-type-column/up03.sql new file mode 100644 index 00000000000..f58953bd2e8 --- /dev/null +++ b/schema/crdb/add-ip-pool-reservation-type-column/up03.sql @@ -0,0 +1,4 @@ +SET LOCAL disallow_full_table_scans = 'off'; +UPDATE ip_pool +SET reservation_type = 'oxide_internal' +WHERE name = 'oxide-service-pool-v4' OR name = 'oxide-service-pool-v6'; diff --git a/schema/crdb/add-ip-pool-reservation-type-column/up04.sql b/schema/crdb/add-ip-pool-reservation-type-column/up04.sql new file mode 100644 index 00000000000..e2ec88a3969 --- /dev/null +++ b/schema/crdb/add-ip-pool-reservation-type-column/up04.sql @@ -0,0 +1,3 @@ +ALTER TABLE omicron.public.ip_pool +ALTER COLUMN reservation_type +DROP DEFAULT; diff --git a/schema/crdb/add-ip-pool-reservation-type-column/up05.sql b/schema/crdb/add-ip-pool-reservation-type-column/up05.sql new file mode 100644 index 00000000000..a291be977e1 --- /dev/null +++ b/schema/crdb/add-ip-pool-reservation-type-column/up05.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.ip_pool_resource +DROP CONSTRAINT IF EXISTS internal_silo_has_no_default_pool; diff --git a/schema/crdb/add-ip-pool-reservation-type-column/up06.sql b/schema/crdb/add-ip-pool-reservation-type-column/up06.sql new file mode 100644 index 00000000000..7fb76218367 --- /dev/null +++ b/schema/crdb/add-ip-pool-reservation-type-column/up06.sql @@ -0,0 +1,4 @@ +SET LOCAL disallow_full_table_scans = 'off'; +DELETE FROM +ip_pool_resource +WHERE resource_type = 'silo' AND resource_id = '001de000-5110-4000-8000-000000000001'; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 23b99aa0070..92236b030e2 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -2126,6 +2126,12 @@ CREATE TYPE IF NOT EXISTS omicron.public.ip_version AS ENUM ( 'v6' ); +/* Indicates what an IP Pool is reserved for. */ +CREATE TYPE IF NOT EXISTS omicron.public.ip_pool_reservation_type AS ENUM ( + 'external_silos', + 'oxide_internal' +); + /* * An IP Pool, a collection of zero or more IP ranges for external IPs. */ @@ -2142,7 +2148,10 @@ CREATE TABLE IF NOT EXISTS omicron.public.ip_pool ( rcgen INT8 NOT NULL, /* The IP version of the ranges contained in this pool. */ - ip_version omicron.public.ip_version NOT NULL + ip_version omicron.public.ip_version NOT NULL, + + /* Indicates what the IP Pool is reserved for. */ + reservation_type omicron.public.ip_pool_reservation_type NOT NULL ); /* @@ -2171,17 +2180,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.ip_pool_resource ( -- resource_type is redundant because resource IDs are globally unique, but -- logically it belongs here - PRIMARY KEY (ip_pool_id, resource_type, resource_id), - - -- Check that there are no default pools for the internal silo - CONSTRAINT internal_silo_has_no_default_pool CHECK ( - NOT ( - resource_type = 'silo' AND - resource_id = '001de000-5110-4000-8000-000000000001' AND - is_default - ) - ) - + PRIMARY KEY (ip_pool_id, resource_type, resource_id) ); -- a given resource can only have one default ip pool @@ -6735,7 +6734,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '196.0.0', NULL) + (TRUE, NOW(), NOW(), '197.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT;