Skip to content

Commit

Permalink
clean up the API names given only silo associations
Browse files Browse the repository at this point in the history
  • Loading branch information
david-crespo committed Dec 5, 2023
1 parent bec0581 commit e14dbe5
Show file tree
Hide file tree
Showing 13 changed files with 194 additions and 293 deletions.
15 changes: 3 additions & 12 deletions nexus/db-model/src/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use db_macros::Resource;
use diesel::Selectable;
use ipnetwork::IpNetwork;
use nexus_types::external_api::params;
use nexus_types::external_api::shared::{self, IpRange};
use nexus_types::external_api::shared::IpRange;
use nexus_types::external_api::views;
use nexus_types::identity::Resource;
use omicron_common::api::external;
Expand Down Expand Up @@ -97,14 +97,6 @@ pub struct IpPoolResource {
pub is_default: bool,
}

impl From<IpPoolResourceType> for shared::IpPoolResourceType {
fn from(typ: IpPoolResourceType) -> Self {
match typ {
IpPoolResourceType::Silo => Self::Silo,
}
}
}

/// Information required to delete an IP pool association. Comes from request
/// params -- silo is a NameOrId and must be resolved to ID.
pub struct IpPoolResourceDelete {
Expand All @@ -113,12 +105,11 @@ pub struct IpPoolResourceDelete {
pub resource_id: Uuid,
}

impl From<IpPoolResource> for views::IpPoolResource {
impl From<IpPoolResource> for views::IpPoolSilo {
fn from(assoc: IpPoolResource) -> Self {
Self {
ip_pool_id: assoc.ip_pool_id,
resource_type: assoc.resource_type.into(),
resource_id: assoc.resource_id,
silo_id: assoc.resource_id,
is_default: assoc.is_default,
}
}
Expand Down
46 changes: 23 additions & 23 deletions nexus/db-queries/src/db/datastore/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,29 +681,29 @@ mod test {

assert_eq!(fleet_default_pool.identity.name.as_str(), "default");

// unique index prevents second fleet-level default
let identity = IdentityMetadataCreateParams {
name: "another-fleet-default".parse().unwrap(),
description: "".to_string(),
};
let second_default = datastore
.ip_pool_create(&opctx, IpPool::new(&identity))
.await
.expect("Failed to create pool");
let err = datastore
.ip_pool_associate_resource(
&opctx,
IpPoolResource {
ip_pool_id: second_default.id(),
resource_type: IpPoolResourceType::Fleet,
resource_id: *FLEET_ID,
is_default: true,
},
)
.await
.expect_err("Failed to fail to make IP pool fleet default");

assert_matches!(err, Error::ObjectAlreadyExists { .. });
// // unique index prevents second fleet-level default
// let identity = IdentityMetadataCreateParams {
// name: "another-fleet-default".parse().unwrap(),
// description: "".to_string(),
// };
// let second_default = datastore
// .ip_pool_create(&opctx, IpPool::new(&identity))
// .await
// .expect("Failed to create pool");
// let err = datastore
// .ip_pool_associate_resource(
// &opctx,
// IpPoolResource {
// ip_pool_id: second_default.id(),
// resource_type: IpPoolResourceType::Fleet,
// resource_id: *FLEET_ID,
// is_default: true,
// },
// )
// .await
// .expect_err("Failed to fail to make IP pool fleet default");

// assert_matches!(err, Error::ObjectAlreadyExists { .. });

// now test logic preferring most specific available default

Expand Down
25 changes: 8 additions & 17 deletions nexus/src/app/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,31 +91,22 @@ impl super::Nexus {
&self,
opctx: &OpContext,
pool_lookup: &lookup::IpPool<'_>,
assoc_create: &params::IpPoolAssociationCreate,
silo_link: &params::IpPoolSiloLink,
) -> CreateResult<db::model::IpPoolResource> {
let (.., authz_pool) =
pool_lookup.lookup_for(authz::Action::Modify).await?;
let (resource_type, resource_id, is_default) = match assoc_create {
params::IpPoolAssociationCreate::Silo(assoc_silo) => {
let (silo,) = self
.silo_lookup(&opctx, assoc_silo.silo.clone())?
.lookup_for(authz::Action::Read)
.await?;
(
db::model::IpPoolResourceType::Silo,
silo.id(),
assoc_silo.is_default,
)
}
};
let (silo,) = self
.silo_lookup(&opctx, silo_link.silo.clone())?
.lookup_for(authz::Action::Read)
.await?;
self.db_datastore
.ip_pool_associate_resource(
opctx,
db::model::IpPoolResource {
ip_pool_id: authz_pool.id(),
resource_type,
resource_id,
is_default,
resource_type: db::model::IpPoolResourceType::Silo,
resource_id: silo.id(),
is_default: silo_link.is_default,
},
)
.await
Expand Down
26 changes: 13 additions & 13 deletions nexus/src/external_api/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ pub(crate) fn external_api() -> NexusApiDescription {
// Operator-Accessible IP Pools API
api.register(ip_pool_list)?;
api.register(ip_pool_create)?;
api.register(ip_pool_association_list)?;
api.register(ip_pool_association_create)?;
api.register(ip_pool_association_delete)?;
api.register(ip_pool_silos_list)?;
api.register(ip_pool_silo_link)?;
api.register(ip_pool_silo_unlink)?;
api.register(ip_pool_view)?;
api.register(ip_pool_delete)?;
api.register(ip_pool_update)?;
Expand Down Expand Up @@ -1323,20 +1323,20 @@ async fn ip_pool_update(
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
}

/// List an IP pool's associated silo configuration
/// List an IP pool's linked silos
#[endpoint {
method = GET,
path = "/v1/system/ip-pools/{pool}/silos",
tags = ["system/networking"],
}]
async fn ip_pool_association_list(
async fn ip_pool_silos_list(
rqctx: RequestContext<Arc<ServerContext>>,
path_params: Path<params::IpPoolPath>,
// paginating by resource_id because they're unique per pool. most robust
// option would be to paginate by a composite key representing the (pool,
// resource_type, resource)
query_params: Query<PaginatedById>,
) -> Result<HttpResponseOk<ResultsPage<views::IpPoolResource>>, HttpError> {
) -> Result<HttpResponseOk<ResultsPage<views::IpPoolSilo>>, HttpError> {
let apictx = rqctx.context();
let handler = async {
let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
Expand All @@ -1358,23 +1358,23 @@ async fn ip_pool_association_list(
Ok(HttpResponseOk(ScanById::results_page(
&query,
assocs,
&|_, x: &views::IpPoolResource| x.resource_id,
&|_, x: &views::IpPoolSilo| x.silo_id,
)?))
};
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
}

/// Associate an IP Pool with a silo
/// Make an IP pool available within a silo
#[endpoint {
method = POST,
path = "/v1/system/ip-pools/{pool}/silos",
tags = ["system/networking"],
}]
async fn ip_pool_association_create(
async fn ip_pool_silo_link(
rqctx: RequestContext<Arc<ServerContext>>,
path_params: Path<params::IpPoolPath>,
resource_assoc: TypedBody<params::IpPoolAssociationCreate>,
) -> Result<HttpResponseCreated<views::IpPoolResource>, HttpError> {
resource_assoc: TypedBody<params::IpPoolSiloLink>,
) -> Result<HttpResponseCreated<views::IpPoolSilo>, HttpError> {
let apictx = rqctx.context();
let handler = async {
let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
Expand All @@ -1396,10 +1396,10 @@ async fn ip_pool_association_create(
path = "/v1/system/ip-pools/{pool}/silos",
tags = ["system/networking"],
}]
async fn ip_pool_association_delete(
async fn ip_pool_silo_unlink(
rqctx: RequestContext<Arc<ServerContext>>,
path_params: Path<params::IpPoolPath>,
query_params: Query<params::IpPoolAssociationDelete>,
query_params: Query<params::IpPoolSiloUnlink>,
) -> Result<HttpResponseUpdatedNoContent, HttpError> {
let apictx = rqctx.context();
let handler = async {
Expand Down
16 changes: 8 additions & 8 deletions nexus/test-utils/src/resource_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,14 @@ pub async fn create_ip_pool(
.await;

// make pool available for use anywhere in fleet
let _assoc: views::IpPoolResource = object_create(
client,
&format!("/v1/system/ip-pools/{pool_name}/associations"),
&params::IpPoolAssociationCreate::Fleet(params::IpPoolAssociateFleet {
is_default: false,
}),
)
.await;
// let _assoc: views::IpPoolSilo = object_create(
// client,
// &format!("/v1/system/ip-pools/{pool_name}/associations"),
// &params::IpPoolAssociationCreate::Fleet(params::IpPoolAssociateFleet {
// is_default: false,
// }),
// )
// .await;

// TODO: associate with fleet as a non-default like before?
let range = populate_ip_pool(client, pool_name, ip_range).await;
Expand Down
8 changes: 4 additions & 4 deletions nexus/tests/integration_tests/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,12 +522,12 @@ lazy_static! {
description: Some(String::from("a new IP pool")),
},
};
pub static ref DEMO_IP_POOL_ASSOC_URL: String = format!("{}/associations", *DEMO_IP_POOL_URL);
pub static ref DEMO_IP_POOL_ASSOC_BODY: params::IpPoolAssociationCreate =
params::IpPoolAssociationCreate::Silo(params::IpPoolAssociateSilo {
pub static ref DEMO_IP_POOL_ASSOC_URL: String = format!("{}/silos", *DEMO_IP_POOL_URL);
pub static ref DEMO_IP_POOL_ASSOC_BODY: params::IpPoolSiloLink =
params::IpPoolSiloLink {
silo: NameOrId::Id(DEFAULT_SILO.identity().id),
is_default: false,
});
};
pub static ref DEMO_IP_POOL_RANGE: IpRange = IpRange::V4(Ipv4Range::new(
std::net::Ipv4Addr::new(10, 0, 0, 0),
std::net::Ipv4Addr::new(10, 0, 0, 255),
Expand Down
11 changes: 5 additions & 6 deletions nexus/tests/integration_tests/instances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3613,12 +3613,11 @@ async fn test_instance_ephemeral_ip_from_correct_pool(
},
)
.await;
let params =
params::IpPoolAssociationCreate::Silo(params::IpPoolAssociateSilo {
silo: NameOrId::Id(DEFAULT_SILO.id()),
is_default: true,
});
let assoc_url = format!("/v1/system/ip-pools/{pool_name}/associations");
let params = params::IpPoolSiloLink {
silo: NameOrId::Id(DEFAULT_SILO.id()),
is_default: true,
};
let assoc_url = format!("/v1/system/ip-pools/{pool_name}/silo");
let _ = NexusRequest::objects_post(client, &assoc_url, &params)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
Expand Down
Loading

0 comments on commit e14dbe5

Please sign in to comment.