Skip to content

Commit

Permalink
remove concept of fleet association
Browse files Browse the repository at this point in the history
  • Loading branch information
@zephraph @david-crespo
zephraph authored and david-crespo committed Dec 5, 2023
1 parent b871acf commit bec0581
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 186 deletions.
2 changes: 0 additions & 2 deletions nexus/db-model/src/ip_pool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ impl_enum_type!(
#[diesel(sql_type = IpPoolResourceTypeEnum)]
pub enum IpPoolResourceType;

Fleet => b"fleet"
Silo => b"silo"
);

Expand All @@ -101,7 +100,6 @@ pub struct IpPoolResource {
impl From<IpPoolResourceType> for shared::IpPoolResourceType {
fn from(typ: IpPoolResourceType) -> Self {
match typ {
IpPoolResourceType::Fleet => Self::Fleet,
IpPoolResourceType::Silo => Self::Silo,
}
}
Expand Down
89 changes: 31 additions & 58 deletions nexus/db-queries/src/db/datastore/ip_pool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,9 @@ impl DataStore {
ip_pool::table
.inner_join(ip_pool_resource::table)
.filter(
(ip_pool_resource::resource_type
ip_pool_resource::resource_type
.eq(IpPoolResourceType::Silo)
.and(ip_pool_resource::resource_id.eq(authz_silo.id())))
.or(ip_pool_resource::resource_type
.eq(IpPoolResourceType::Fleet)),
.and(ip_pool_resource::resource_id.eq(authz_silo.id())),
)
.filter(ip_pool::id.eq(authz_pool.id()))
.filter(ip_pool::time_deleted.is_null())
Expand Down Expand Up @@ -139,11 +137,9 @@ impl DataStore {
ip_pool::table
.inner_join(ip_pool_resource::table)
.filter(
(ip_pool_resource::resource_type
ip_pool_resource::resource_type
.eq(IpPoolResourceType::Silo)
.and(ip_pool_resource::resource_id.eq(authz_silo_id)))
.or(ip_pool_resource::resource_type
.eq(IpPoolResourceType::Fleet)),
.and(ip_pool_resource::resource_id.eq(authz_silo_id)),
)
.filter(ip_pool_resource::is_default.eq(true))
.filter(ip_pool::time_deleted.is_null())
Expand Down Expand Up @@ -429,66 +425,43 @@ impl DataStore {
// We can only delete the association if there are no IPs allocated
// from this pool in the associated resource.

// most of the query is the same between silo and fleet
let base_query = |table: external_ip::table| {
table
.inner_join(
instance::table
.on(external_ip::parent_id.eq(instance::id.nullable())),
)
.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(association.ip_pool_id))
.filter(instance::time_deleted.is_not_null())
.select(ExternalIp::as_select())
.limit(1)
};

let existing_ips = match association.resource_type {
IpPoolResourceType::Silo => {

// if it's a silo association, we also have to join through IPs to instances
// to projects to get the silo ID
base_query(external_ip::table)
.inner_join(
project::table.on(instance::project_id.eq(project::id)),
)
.filter(project::silo_id.eq(association.resource_id))
.load_async::<ExternalIp>(
&*self.pool_connection_authorized(opctx).await?,
)
.await
},
IpPoolResourceType::Fleet => {
// If it's a fleet association, we can't delete it if there are any IPs
// allocated from the pool anywhere
base_query(external_ip::table)
.load_async::<ExternalIp>(
&*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
))
})?;
let existing_ips = external_ip::table
.inner_join(
instance::table
.on(external_ip::parent_id.eq(instance::id.nullable())),
)
.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(association.ip_pool_id))
.filter(instance::time_deleted.is_not_null())
.select(ExternalIp::as_select())
.limit(1)
// we have to join through IPs to instances to projects to get the silo ID
.inner_join(project::table.on(instance::project_id.eq(project::id)))
.filter(project::silo_id.eq(association.resource_id))
.load_async::<ExternalIp>(
&*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::InvalidRequest {
message: "IP addresses from this pool are in use in the associated silo/fleet".to_string()
message: "IP addresses from this pool are in use in the associated silo".to_string()
});
}

Ok(())
}

/// Delete IP pool assocation with resource unless there are outstanding
/// IPs allocated from the pool in the associated silo (or the fleet, if
/// it's a fleet association).
/// IPs allocated from the pool in the associated silo
pub async fn ip_pool_dissociate_resource(
&self,
opctx: &OpContext,
Expand Down
6 changes: 2 additions & 4 deletions nexus/db-queries/src/db/datastore/project.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,13 +353,11 @@ impl DataStore {
&pagparams.map_name(|n| Name::ref_cast(n)),
),
}
// TODO: make sure this join is compatible with pagination logic
.inner_join(ip_pool_resource::table)
.filter(
(ip_pool_resource::resource_type
ip_pool_resource::resource_type
.eq(IpPoolResourceType::Silo)
.and(ip_pool_resource::resource_id.eq(silo_id)))
.or(ip_pool_resource::resource_type.eq(IpPoolResourceType::Fleet)),
.and(ip_pool_resource::resource_id.eq(silo_id)),
)
.filter(ip_pool::time_deleted.is_null())
.select(db::model::IpPool::as_select())
Expand Down
33 changes: 0 additions & 33 deletions nexus/db-queries/src/db/datastore/rack.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use crate::db::fixed_data::silo::INTERNAL_SILO_ID;
use crate::db::fixed_data::vpc_subnet::DNS_VPC_SUBNET;
use crate::db::fixed_data::vpc_subnet::NEXUS_VPC_SUBNET;
use crate::db::fixed_data::vpc_subnet::NTP_VPC_SUBNET;
use crate::db::fixed_data::FLEET_ID;
use crate::db::identity::Asset;
use crate::db::model::Dataset;
use crate::db::model::IncompleteExternalIp;
Expand Down Expand Up @@ -736,38 +735,6 @@ impl DataStore {
.await?;
}

let default_pool =
db::model::IpPool::new(&IdentityMetadataCreateParams {
name: "default".parse::<Name>().unwrap(),
description: String::from("default IP pool"),
});

let default_pool_id = default_pool.id();

let default_created = self
.ip_pool_create(opctx, default_pool)
.await
.map(|_| true)
.or_else(|e| match e {
Error::ObjectAlreadyExists { .. } => Ok(false),
_ => Err(e),
})?;

// make pool default for fleet. only need to do this if the create went
// through, i.e., if it wasn't already there
if default_created {
self.ip_pool_associate_resource(
opctx,
db::model::IpPoolResource {
ip_pool_id: default_pool_id,
resource_type: db::model::IpPoolResourceType::Fleet,
resource_id: *FLEET_ID,
is_default: true,
},
)
.await?;
}

Ok(())
}

Expand Down
32 changes: 5 additions & 27 deletions nexus/src/app/ip_pool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ use nexus_db_model::IpPoolResourceType;
use nexus_db_queries::authz;
use nexus_db_queries::context::OpContext;
use nexus_db_queries::db;
use nexus_db_queries::db::fixed_data::FLEET_ID;
use nexus_db_queries::db::lookup;
use nexus_db_queries::db::lookup::LookupPath;
use nexus_db_queries::db::model::Name;
use nexus_types::identity::Resource;
use omicron_common::api::external::http_pagination::PaginatedBy;
use omicron_common::api::external::CreateResult;
use omicron_common::api::external::DataPageParams;
Expand Down Expand Up @@ -95,7 +93,6 @@ impl super::Nexus {
pool_lookup: &lookup::IpPool<'_>,
assoc_create: &params::IpPoolAssociationCreate,
) -> CreateResult<db::model::IpPoolResource> {
// TODO: check for perms on specified resource? or unnecessary because this is an operator action?
let (.., authz_pool) =
pool_lookup.lookup_for(authz::Action::Modify).await?;
let (resource_type, resource_id, is_default) = match assoc_create {
Expand All @@ -110,14 +107,6 @@ impl super::Nexus {
assoc_silo.is_default,
)
}
params::IpPoolAssociationCreate::Fleet(assoc_fleet) => {
// we don't need to be assured of the fleet's existence
(
db::model::IpPoolResourceType::Fleet,
*FLEET_ID,
assoc_fleet.is_default,
)
}
};
self.db_datastore
.ip_pool_associate_resource(
Expand All @@ -136,31 +125,20 @@ impl super::Nexus {
&self,
opctx: &OpContext,
pool_lookup: &lookup::IpPool<'_>,
ip_pool_dissoc: &params::IpPoolAssociationDeleteValidated,
silo_lookup: &lookup::Silo<'_>,
) -> DeleteResult {
let (.., authz_pool) =
pool_lookup.lookup_for(authz::Action::Modify).await?;

let (resource_type, resource_id) = match ip_pool_dissoc {
params::IpPoolAssociationDeleteValidated::Silo(assoc) => {
let (.., silo) = self
.silo_lookup(opctx, assoc.silo.clone())?
.fetch()
.await?;
(IpPoolResourceType::Silo, silo.id())
}
params::IpPoolAssociationDeleteValidated::Fleet => {
(IpPoolResourceType::Fleet, *FLEET_ID)
}
};
let (.., authz_silo) =
silo_lookup.lookup_for(authz::Action::Modify).await?;

self.db_datastore
.ip_pool_dissociate_resource(
opctx,
&IpPoolResourceDelete {
ip_pool_id: authz_pool.id(),
resource_id,
resource_type,
resource_id: authz_silo.id(),
resource_type: IpPoolResourceType::Silo,
},
)
.await
Expand Down
25 changes: 7 additions & 18 deletions nexus/src/external_api/http_entrypoints.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1323,14 +1323,10 @@ async fn ip_pool_update(
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
}

// TODO: associate just seems like the wrong word and I'd like to change it
// across the board. What I really mean is "make available to" or "make availale
// for use in"

/// List IP pool resource associations
/// List an IP pool's associated silo configuration
#[endpoint {
method = GET,
path = "/v1/system/ip-pools/{pool}/associations",
path = "/v1/system/ip-pools/{pool}/silos",
tags = ["system/networking"],
}]
async fn ip_pool_association_list(
Expand Down Expand Up @@ -1368,10 +1364,10 @@ async fn ip_pool_association_list(
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
}

/// Associate an IP Pool with a silo or the fleet
/// Associate an IP Pool with a silo
#[endpoint {
method = POST,
path = "/v1/system/ip-pools/{pool}/associations",
path = "/v1/system/ip-pools/{pool}/silos",
tags = ["system/networking"],
}]
async fn ip_pool_association_create(
Expand All @@ -1397,7 +1393,7 @@ async fn ip_pool_association_create(
/// Remove an IP pool's association with a silo or project
#[endpoint {
method = DELETE,
path = "/v1/system/ip-pools/{pool}/associations",
path = "/v1/system/ip-pools/{pool}/silos",
tags = ["system/networking"],
}]
async fn ip_pool_association_delete(
Expand All @@ -1412,17 +1408,10 @@ async fn ip_pool_association_delete(
let path = path_params.into_inner();
let query = query_params.into_inner();

let validated_params =
params::IpPoolAssociationDeleteValidated::try_from(query)
.map_err(|e| HttpError::for_bad_request(None, e))?;

let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?;
let silo_lookup = nexus.silo_lookup(&opctx, query.silo)?;
nexus
.ip_pool_dissociate_resource(
&opctx,
&pool_lookup,
&validated_params,
)
.ip_pool_dissociate_resource(&opctx, &pool_lookup, &silo_lookup)
.await?;
Ok(HttpResponseUpdatedNoContent())
};
Expand Down
41 changes: 2 additions & 39 deletions nexus/types/src/external_api/params.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -763,27 +763,16 @@ pub struct IpPoolAssociateSilo {
pub is_default: bool,
}

#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
pub struct IpPoolAssociateFleet {
pub is_default: bool,
}

/// Parameters for associating an IP pool with a resource (fleet, silo)
/// Parameters for associating an IP pool with a silo
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
#[serde(tag = "resource_type", rename_all = "snake_case")]
pub enum IpPoolAssociationCreate {
Silo(IpPoolAssociateSilo),
Fleet(IpPoolAssociateFleet),
}

// It would be cool if this was an enum like Silo(NameOrId) | Fleet, but
// OpenAPI doesn't support fancy schemas on query params. So instead we
// use this flat struct for the query params directly, and manually parse
// that into the good struct below.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
pub struct IpPoolAssociationDelete {
pub silo: Option<NameOrId>,
pub resource_type: shared::IpPoolResourceType,
pub silo: NameOrId,
}

// technically these are not params, but they are used with params
Expand All @@ -792,32 +781,6 @@ pub struct IpPoolSiloAssociationDelete {
pub silo: NameOrId,
}

#[derive(Clone, Debug)]
pub enum IpPoolAssociationDeleteValidated {
Silo(IpPoolSiloAssociationDelete),
Fleet,
}

impl TryFrom<IpPoolAssociationDelete> for IpPoolAssociationDeleteValidated {
type Error = String;

fn try_from(value: IpPoolAssociationDelete) -> Result<Self, Self::Error> {
match (value.silo, value.resource_type) {
(Some(silo), shared::IpPoolResourceType::Silo) => {
Ok(Self::Silo(IpPoolSiloAssociationDelete { silo }))
}
(None, shared::IpPoolResourceType::Fleet) => Ok(Self::Fleet),
(Some(_), shared::IpPoolResourceType::Fleet) => {
Err("Silo must be null if resource_type is fleet".to_string())
}
(None, shared::IpPoolResourceType::Silo) => {
Err("Silo must be specified if resource_type is silo"
.to_string())
}
}
}
}

// INSTANCES

/// Describes an attachment of an `InstanceNetworkInterface` to an `Instance`,
Expand Down
Loading

0 comments on commit bec0581

Please sign in to comment.