Skip to content

Commit eed8606

Browse files
committed
Add operator APIs for manipulating system IP Pools
- Add Nexus endpoints for reserving IP Pools for Oxide use or for external customer Silos - Remove check used to hide internal IP Pools, since they're all visible to an operator now - Rename operator IP Pool endpoints by prefixing `system_*`, to avoid conflicts with previous "silo-scoped" endpoints. - Handle possibility of any number of Oxide-internal IP Pools in various places where we previously assumed exactly 2 of them. - Fixes #8947 - Fixes #8226
1 parent 98de948 commit eed8606

File tree

30 files changed

+1038
-1404
lines changed

30 files changed

+1038
-1404
lines changed

end-to-end-tests/src/bin/bootstrap.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use end_to_end_tests::helpers::{
66
use omicron_test_utils::dev::poll::{CondCheckError, wait_for_condition};
77
use oxide_client::types::{
88
ByteCount, DeviceAccessTokenRequest, DeviceAuthRequest, DeviceAuthVerify,
9-
DiskCreate, DiskSource, IpPoolCreate, IpPoolLinkSilo, IpPoolType,
10-
IpVersion, NameOrId, SiloQuotasUpdate,
9+
DiskCreate, DiskSource, IpPoolCreate, IpPoolLinkSilo,
10+
IpPoolReservationType, IpPoolType, IpVersion, NameOrId, SiloQuotasUpdate,
1111
};
1212
use oxide_client::{
1313
ClientConsoleAuthExt, ClientDisksExt, ClientProjectsExt,
@@ -48,17 +48,18 @@ async fn run_test() -> Result<()> {
4848
eprintln!("creating IP{} IP pool... {:?} - {:?}", ip_version, first, last);
4949
let pool_name = "default";
5050
client
51-
.ip_pool_create()
51+
.system_ip_pool_create()
5252
.body(IpPoolCreate {
5353
name: pool_name.parse().unwrap(),
5454
description: "Default IP pool".to_string(),
5555
ip_version,
5656
pool_type: IpPoolType::Unicast,
57+
reservation_type: IpPoolReservationType::ExternalSilos,
5758
})
5859
.send()
5960
.await?;
6061
client
61-
.ip_pool_silo_link()
62+
.system_ip_pool_silo_link()
6263
.pool(pool_name)
6364
.body(IpPoolLinkSilo {
6465
silo: NameOrId::Name(params.silo_name().parse().unwrap()),
@@ -67,7 +68,7 @@ async fn run_test() -> Result<()> {
6768
.send()
6869
.await?;
6970
client
70-
.ip_pool_range_add()
71+
.system_ip_pool_range_add()
7172
.pool(pool_name)
7273
.body(try_create_ip_range(first, last)?)
7374
.send()

end-to-end-tests/src/bin/commtest.rs

Lines changed: 41 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ use oxide_client::{
77
ClientSystemHardwareExt, ClientSystemIpPoolsExt, ClientSystemStatusExt,
88
ClientVpcsExt,
99
types::{
10-
IpPoolCreate, IpPoolLinkSilo, IpPoolType, IpRange, IpVersion, Name,
11-
NameOrId, PingStatus, ProbeCreate, ProbeInfo, ProjectCreate,
12-
UsernamePasswordCredentials,
10+
IpPoolCreate, IpPoolLinkSilo, IpPoolReservationType, IpPoolType,
11+
IpRange, IpVersion, Name, NameOrId, PingStatus, ProbeCreate, ProbeInfo,
12+
ProjectCreate, UsernamePasswordCredentials,
1313
},
1414
};
1515
use std::{
@@ -280,48 +280,49 @@ async fn rack_prepare(
280280
})?;
281281

282282
let pool_name = "default";
283-
api_retry!(
284-
if let Err(e) = oxide.ip_pool_view().pool("default").send().await {
285-
if let Some(reqwest::StatusCode::NOT_FOUND) = e.status() {
286-
print!("default ip pool does not exist, creating ...");
287-
let ip_version = if args.ip_pool_begin.is_ipv4() {
288-
IpVersion::V4
289-
} else {
290-
IpVersion::V6
291-
};
292-
oxide
293-
.ip_pool_create()
294-
.body(IpPoolCreate {
295-
name: pool_name.parse().unwrap(),
296-
description: "Default IP pool".to_string(),
297-
ip_version,
298-
pool_type: IpPoolType::Unicast,
299-
})
300-
.send()
301-
.await?;
302-
oxide
303-
.ip_pool_silo_link()
304-
.pool(pool_name)
305-
.body(IpPoolLinkSilo {
306-
silo: NameOrId::Name("recovery".parse().unwrap()),
307-
is_default: true,
308-
})
309-
.send()
310-
.await?;
311-
println!("done");
312-
Ok(())
283+
api_retry!(if let Err(e) =
284+
oxide.system_ip_pool_view().pool("default").send().await
285+
{
286+
if let Some(reqwest::StatusCode::NOT_FOUND) = e.status() {
287+
print!("default ip pool does not exist, creating ...");
288+
let ip_version = if args.ip_pool_begin.is_ipv4() {
289+
IpVersion::V4
313290
} else {
314-
Err(e)
315-
}
316-
} else {
317-
println!("default ip pool already exists");
291+
IpVersion::V6
292+
};
293+
oxide
294+
.system_ip_pool_create()
295+
.body(IpPoolCreate {
296+
name: pool_name.parse().unwrap(),
297+
description: "Default IP pool".to_string(),
298+
ip_version,
299+
pool_type: IpPoolType::Unicast,
300+
reservation_type: IpPoolReservationType::ExternalSilos,
301+
})
302+
.send()
303+
.await?;
304+
oxide
305+
.system_ip_pool_silo_link()
306+
.pool(pool_name)
307+
.body(IpPoolLinkSilo {
308+
silo: NameOrId::Name("recovery".parse().unwrap()),
309+
is_default: true,
310+
})
311+
.send()
312+
.await?;
313+
println!("done");
318314
Ok(())
315+
} else {
316+
Err(e)
319317
}
320-
)?;
318+
} else {
319+
println!("default ip pool already exists");
320+
Ok(())
321+
})?;
321322

322323
let pool = api_retry!(
323324
oxide
324-
.ip_pool_range_list()
325+
.system_ip_pool_range_list()
325326
.limit(u32::MAX)
326327
.pool(Name::try_from("default").unwrap())
327328
.send()
@@ -346,7 +347,7 @@ async fn rack_prepare(
346347
print!("ip range does not exist, creating ... ");
347348
api_retry!(
348349
oxide
349-
.ip_pool_range_add()
350+
.system_ip_pool_range_add()
350351
.pool(Name::try_from("default").unwrap())
351352
.body(range.clone())
352353
.send()

nexus/db-model/src/ip_pool.rs

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,24 @@ impl ::std::fmt::Display for IpPoolReservationType {
8080
}
8181
}
8282

83+
impl From<shared::IpPoolReservationType> for IpPoolReservationType {
84+
fn from(value: shared::IpPoolReservationType) -> Self {
85+
match value {
86+
shared::IpPoolReservationType::ExternalSilos => Self::ExternalSilos,
87+
shared::IpPoolReservationType::OxideInternal => Self::OxideInternal,
88+
}
89+
}
90+
}
91+
92+
impl From<IpPoolReservationType> for shared::IpPoolReservationType {
93+
fn from(value: IpPoolReservationType) -> Self {
94+
match value {
95+
IpPoolReservationType::ExternalSilos => Self::ExternalSilos,
96+
IpPoolReservationType::OxideInternal => Self::OxideInternal,
97+
}
98+
}
99+
}
100+
83101
impl_enum_type!(
84102
IpVersionEnum:
85103

@@ -226,15 +244,13 @@ impl IpPool {
226244
}
227245
}
228246

229-
impl From<IpPool> for views::IpPool {
247+
impl From<IpPool> for views::SystemIpPool {
230248
fn from(pool: IpPool) -> Self {
231-
let identity = pool.identity();
232-
let pool_type = pool.pool_type;
233-
234249
Self {
235-
identity,
236-
pool_type: pool_type.into(),
250+
identity: pool.identity(),
251+
pool_type: pool.pool_type.into(),
237252
ip_version: pool.ip_version.into(),
253+
reservation_type: pool.reservation_type.into(),
238254
}
239255
}
240256
}

nexus/db-queries/src/db/datastore/deployment.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4310,9 +4310,13 @@ mod tests {
43104310
))
43114311
.unwrap();
43124312
let (service_authz_ip_pool, service_ip_pool) = datastore
4313-
.ip_pools_service_lookup(&opctx, IpVersion::V4)
4313+
.fetch_first_oxide_internal_ip_pool(
4314+
&opctx,
4315+
authz::Action::CreateChild,
4316+
Some(IpVersion::V4),
4317+
)
43144318
.await
4315-
.expect("lookup service ip pool");
4319+
.expect("Failed authz check on delegated IP Pool");
43164320
datastore
43174321
.ip_pool_add_range(
43184322
&opctx,
@@ -4449,9 +4453,13 @@ mod tests {
44494453
})
44504454
.expect("found external IP");
44514455
let (service_authz_ip_pool, service_ip_pool) = datastore
4452-
.ip_pools_service_lookup(&opctx, IpVersion::V4)
4456+
.fetch_first_oxide_internal_ip_pool(
4457+
&opctx,
4458+
authz::Action::CreateChild,
4459+
Some(IpVersion::V4),
4460+
)
44534461
.await
4454-
.expect("lookup service ip pool");
4462+
.expect("Failed authz check for delegated IP Pool");
44554463
datastore
44564464
.ip_pool_add_range(
44574465
&opctx,

nexus/db-queries/src/db/datastore/deployment/external_networking.rs

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,15 @@ impl DataStore {
4040
// Looking up the service pool IDs requires an opctx; we'll do this at
4141
// most once inside the loop below, when we first encounter an address
4242
// of the same IP version.
43+
//
44+
// TODO-correctness: We really need to know which delegated IP Pool to
45+
// select an address from. It's not clear how we do that today, but we
46+
// could make the IpPoolReservationType more fine-grained.
47+
//
48+
// In the meantime, select the first one for a specific IP version,
49+
// which is no worse than today.
50+
//
51+
// See: https://github.com/oxidecomputer/omicron/issues/8949.
4352
let mut v4_pool = None;
4453
let mut v6_pool = None;
4554

@@ -66,10 +75,13 @@ impl DataStore {
6675
let pool = match pool_ref {
6776
Some(p) => p,
6877
None => {
69-
let new = self
70-
.ip_pools_service_lookup(opctx, version.into())
71-
.await?
72-
.1;
78+
let (_, new) = self
79+
.fetch_first_oxide_internal_ip_pool(
80+
opctx,
81+
nexus_auth::authz::Action::CreateChild,
82+
Some(version.into()),
83+
)
84+
.await?;
7385
*pool_ref = Some(new);
7486
pool_ref.as_ref().unwrap()
7587
}
@@ -615,9 +627,13 @@ mod tests {
615627
datastore: &DataStore,
616628
) {
617629
let (ip_pool, db_pool) = datastore
618-
.ip_pools_service_lookup(&opctx, IpVersion::V4.into())
630+
.fetch_first_oxide_internal_ip_pool(
631+
&opctx,
632+
nexus_auth::authz::Action::CreateChild,
633+
Some(IpVersion::V4.into()),
634+
)
619635
.await
620-
.expect("failed to find service IP pool");
636+
.expect("Failed authz check on delegated IP Pool");
621637
datastore
622638
.ip_pool_add_range(
623639
&opctx,

nexus/db-queries/src/db/datastore/external_ip.rs

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,19 @@ impl DataStore {
299299
external_ip: OmicronZoneExternalIp,
300300
) -> CreateResult<ExternalIp> {
301301
let version = IpVersion::from(external_ip.ip_version());
302-
let (authz_pool, pool) =
303-
self.ip_pools_service_lookup(opctx, version).await?;
304-
opctx.authorize(authz::Action::CreateChild, &authz_pool).await?;
302+
303+
// TODO-correctness: We need to figure out which IP Pool this address
304+
// actually belongs in, if there are more than one of the same address
305+
// version.
306+
//
307+
// See: https://github.com/oxidecomputer/omicron/issues/8949.
308+
let (.., pool) = self
309+
.fetch_first_oxide_internal_ip_pool(
310+
opctx,
311+
authz::Action::CreateChild,
312+
Some(version),
313+
)
314+
.await?;
305315
let data = IncompleteExternalIp::for_omicron_zone(
306316
pool.id(),
307317
external_ip,
@@ -341,11 +351,14 @@ impl DataStore {
341351
// Note the IP version used here isn't important. It's just for the
342352
// authz check to list children, and not used for the actual database
343353
// query below, which filters on is_service to get external IPs from
344-
// either pool.
345-
let (authz_pool, _pool) =
346-
self.ip_pools_service_lookup(opctx, IpVersion::V4).await?;
347-
opctx.authorize(authz::Action::ListChildren, &authz_pool).await?;
348-
354+
// any pool.
355+
let _ = self
356+
.fetch_first_oxide_internal_ip_pool(
357+
opctx,
358+
authz::Action::ListChildren,
359+
None,
360+
)
361+
.await?;
349362
paginated(dsl::external_ip, dsl::id, pagparams)
350363
.filter(dsl::is_service)
351364
.filter(dsl::time_deleted.is_null())
@@ -1171,9 +1184,13 @@ mod tests {
11711184
))
11721185
.unwrap();
11731186
let (service_ip_pool, db_pool) = datastore
1174-
.ip_pools_service_lookup(opctx, IpVersion::V4)
1187+
.fetch_first_oxide_internal_ip_pool(
1188+
opctx,
1189+
authz::Action::CreateChild,
1190+
Some(IpVersion::V4),
1191+
)
11751192
.await
1176-
.expect("lookup service ip pool");
1193+
.expect("Failed authz check on delegated IP Pool");
11771194
datastore
11781195
.ip_pool_add_range(opctx, &service_ip_pool, &db_pool, &ip_range)
11791196
.await

0 commit comments

Comments
 (0)