Skip to content

Commit b3fa1b5

Browse files
[ip_version/pool allocation] Refactor IP pool selection to use tagged enums (type-safe API) (#9598)
This PR refactors `FloatingIpCreate`, `EphemeralIpCreate`, `ExternalIpCreate::Ephemeral`, and `ProbeCreate` to use tagged enums for pool selection, making invalid states unrepresentable at the type level. Previously, these types used flat Option fields (`pool`, `ip_version`, `ip`) that required runtime validation to catch invalid combinations like `ip_version + pool` or `ip + ip_version`. The new design uses: - `PoolSelector` enum with `Explicit { pool }` and `Auto { ip_version }` variants, used by `EphemeralIpCreate`, `ExternalIpCreate::Ephemeral`, and `ProbeCreate` - `AddressSelector` enum with `Explicit { ip, pool }` and `Auto { pool_selector }` variants for `FloatingIpCreate` This all ensures at the type level that... - `ip_version` can only be specified when auto-selecting from a pool - `ip_version` cannot be combined with an explicit IP address - Explicit pools cannot accept `ip_version` (the pool determines available IPs) Includes: - The `Explicit { ip, pool }` combination remains valid for reserving a specific IP, validating that it exists in the given pool (pool is a constraint, not a selection method). - When using `Auto` mode (or when the pool/address selector is omitted), the silo's default pool is used. If the silo has default pools for both IPv4 and IPv6, the request will fail unless `ip_version` is specified to disambiguate. - API versioning: Added `v2026010300.rs` with the old flat types and `TryFrom` implementations that validate and convert to the new enum-based types for delegation. Older API versions continue to work via the existing conversion chain. - `ProbeCreate` in the experimental API previously lacked `ip_version` support; this is now addressed using the `PoolSelector` enumerator.
1 parent 2279d23 commit b3fa1b5

File tree

24 files changed

+30715
-207
lines changed

24 files changed

+30715
-207
lines changed

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use oxide_client::{
88
ClientVpcsExt,
99
types::{
1010
IpPoolCreate, IpPoolLinkSilo, IpPoolType, IpRange, IpVersion, Name,
11-
NameOrId, PingStatus, ProbeCreate, ProbeInfo, ProjectCreate,
12-
UsernamePasswordCredentials,
11+
NameOrId, PingStatus, PoolSelector, ProbeCreate, ProbeInfo,
12+
ProjectCreate, UsernamePasswordCredentials,
1313
},
1414
};
1515
use std::{
@@ -389,7 +389,9 @@ async fn launch_probes(
389389
.project(Name::try_from("classone").unwrap())
390390
.body(ProbeCreate {
391391
description: format!("probe {i}"),
392-
ip_pool: Some("default".parse().unwrap()),
392+
pool_selector: PoolSelector::Explicit {
393+
pool: "default".parse().unwrap(),
394+
},
393395
name: format!("probe{i}").parse().unwrap(),
394396
sled,
395397
})

end-to-end-tests/src/instance_launch.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ use omicron_test_utils::dev::poll::{CondCheckError, wait_for_condition};
77
use oxide_client::types::{
88
ByteCount, DiskBackend, DiskCreate, DiskSource, ExternalIp,
99
ExternalIpCreate, InstanceCpuCount, InstanceCreate, InstanceDiskAttachment,
10-
InstanceNetworkInterfaceAttachment, InstanceState, IpVersion, SshKeyCreate,
10+
InstanceNetworkInterfaceAttachment, InstanceState, IpVersion, PoolSelector,
11+
SshKeyCreate,
1112
};
1213
use oxide_client::{ClientCurrentUserExt, ClientDisksExt, ClientInstancesExt};
1314
use russh::{ChannelMsg, Disconnect};
@@ -73,8 +74,9 @@ async fn instance_launch() -> Result<()> {
7374
network_interfaces:
7475
InstanceNetworkInterfaceAttachment::DefaultDualStack,
7576
external_ips: vec![ExternalIpCreate::Ephemeral {
76-
pool: None,
77-
ip_version: Some(IpVersion::V4),
77+
pool_selector: PoolSelector::Auto {
78+
ip_version: Some(IpVersion::V4),
79+
},
7880
}],
7981
user_data: String::new(),
8082
ssh_public_keys: Some(vec![oxide_client::types::NameOrId::Name(

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,15 @@ impl DataStore {
9898
ip_id: Uuid,
9999
probe_id: Uuid,
100100
pool: Option<authz::IpPool>,
101+
ip_version: Option<IpVersion>,
101102
) -> CreateResult<ExternalIp> {
102103
let authz_pool = self
103-
.resolve_pool_for_allocation(opctx, pool, IpPoolType::Unicast, None)
104+
.resolve_pool_for_allocation(
105+
opctx,
106+
pool,
107+
IpPoolType::Unicast,
108+
ip_version,
109+
)
104110
.await?;
105111
let data = IncompleteExternalIp::for_ephemeral_probe(
106112
ip_id,

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use nexus_db_errors::ErrorHandler;
1919
use nexus_db_errors::public_error_from_diesel;
2020
use nexus_db_lookup::LookupPath;
2121
use nexus_db_model::IncompleteNetworkInterface;
22+
use nexus_db_model::IpVersion;
2223
use nexus_db_model::Probe;
2324
use nexus_db_model::VpcSubnet;
2425
use nexus_types::external_api::params::PrivateIpStackCreate;
@@ -219,6 +220,7 @@ impl super::DataStore {
219220
authz_project: &authz::Project,
220221
probe: &Probe,
221222
ip_pool: Option<authz::IpPool>,
223+
ip_version: Option<IpVersion>,
222224
) -> CreateResult<Probe> {
223225
// TODO-correctness: These need to be in a transaction.
224226
// See https://github.com/oxidecomputer/omicron/issues/9340.
@@ -231,6 +233,7 @@ impl super::DataStore {
231233
Uuid::new_v4(),
232234
probe.id(),
233235
ip_pool,
236+
ip_version,
234237
)
235238
.await?;
236239

nexus/external-api/src/lib.rs

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ mod v2025120300;
3939
mod v2025121200;
4040
mod v2025122300;
4141
mod v2026010100;
42+
mod v2026010300;
4243

4344
api_versions!([
4445
// API versions are in the format YYYYMMDDNN.0.0, defined below as
@@ -68,6 +69,7 @@ api_versions!([
6869
// | date-based version should be at the top of the list.
6970
// v
7071
// (next_yyyymmddnn, IDENT),
72+
(2026010500, POOL_SELECTION_ENUMS),
7173
(2026010300, DUAL_STACK_NICS),
7274
(2026010100, SILO_PROJECT_IP_VERSION_AND_POOL_TYPE),
7375
(2025122300, IP_VERSION_AND_MULTIPLE_DEFAULT_POOLS),
@@ -1298,12 +1300,30 @@ pub trait NexusExternalApi {
12981300
.await
12991301
}
13001302

1303+
/// Create floating IP
1304+
#[endpoint {
1305+
operation_id = "floating_ip_create",
1306+
method = POST,
1307+
path = "/v1/floating-ips",
1308+
tags = ["floating-ips"],
1309+
versions = VERSION_IP_VERSION_AND_MULTIPLE_DEFAULT_POOLS
1310+
..VERSION_POOL_SELECTION_ENUMS,
1311+
}]
1312+
async fn v2026010300_floating_ip_create(
1313+
rqctx: RequestContext<Self::Context>,
1314+
query_params: Query<params::ProjectSelector>,
1315+
floating_params: TypedBody<v2026010300::FloatingIpCreate>,
1316+
) -> Result<HttpResponseCreated<views::FloatingIp>, HttpError> {
1317+
let floating_params = floating_params.try_map(TryInto::try_into)?;
1318+
Self::floating_ip_create(rqctx, query_params, floating_params).await
1319+
}
1320+
13011321
/// Create floating IP
13021322
#[endpoint {
13031323
method = POST,
13041324
path = "/v1/floating-ips",
13051325
tags = ["floating-ips"],
1306-
versions = VERSION_IP_VERSION_AND_MULTIPLE_DEFAULT_POOLS..,
1326+
versions = VERSION_POOL_SELECTION_ENUMS..,
13071327
}]
13081328
async fn floating_ip_create(
13091329
rqctx: RequestContext<Self::Context>,
@@ -1756,10 +1776,27 @@ pub trait NexusExternalApi {
17561776

17571777
/// Create instance
17581778
#[endpoint {
1779+
operation_id = "instance_create",
17591780
method = POST,
17601781
path = "/v1/instances",
17611782
tags = ["instances"],
1762-
versions = VERSION_DUAL_STACK_NICS..,
1783+
versions = VERSION_DUAL_STACK_NICS..VERSION_POOL_SELECTION_ENUMS,
1784+
}]
1785+
async fn v2026010300_instance_create(
1786+
rqctx: RequestContext<Self::Context>,
1787+
query_params: Query<params::ProjectSelector>,
1788+
new_instance: TypedBody<v2026010300::InstanceCreate>,
1789+
) -> Result<HttpResponseCreated<Instance>, HttpError> {
1790+
let new_instance = new_instance.try_map(TryInto::try_into)?;
1791+
Self::instance_create(rqctx, query_params, new_instance).await
1792+
}
1793+
1794+
/// Create instance
1795+
#[endpoint {
1796+
method = POST,
1797+
path = "/v1/instances",
1798+
tags = ["instances"],
1799+
versions = VERSION_POOL_SELECTION_ENUMS..,
17631800
}]
17641801
async fn instance_create(
17651802
rqctx: RequestContext<Self::Context>,
@@ -3097,12 +3134,37 @@ pub trait NexusExternalApi {
30973134
.await
30983135
}
30993136

3137+
/// Allocate and attach ephemeral IP to instance
3138+
#[endpoint {
3139+
operation_id = "instance_ephemeral_ip_attach",
3140+
method = POST,
3141+
path = "/v1/instances/{instance}/external-ips/ephemeral",
3142+
tags = ["instances"],
3143+
versions = VERSION_IP_VERSION_AND_MULTIPLE_DEFAULT_POOLS
3144+
..VERSION_POOL_SELECTION_ENUMS,
3145+
}]
3146+
async fn v2026010300_instance_ephemeral_ip_attach(
3147+
rqctx: RequestContext<Self::Context>,
3148+
path_params: Path<params::InstancePath>,
3149+
query_params: Query<params::OptionalProjectSelector>,
3150+
ip_to_create: TypedBody<v2026010300::EphemeralIpCreate>,
3151+
) -> Result<HttpResponseAccepted<views::ExternalIp>, HttpError> {
3152+
let ip_to_create = ip_to_create.try_map(TryInto::try_into)?;
3153+
Self::instance_ephemeral_ip_attach(
3154+
rqctx,
3155+
path_params,
3156+
query_params,
3157+
ip_to_create,
3158+
)
3159+
.await
3160+
}
3161+
31003162
/// Allocate and attach ephemeral IP to instance
31013163
#[endpoint {
31023164
method = POST,
31033165
path = "/v1/instances/{instance}/external-ips/ephemeral",
31043166
tags = ["instances"],
3105-
versions = VERSION_IP_VERSION_AND_MULTIPLE_DEFAULT_POOLS..,
3167+
versions = VERSION_POOL_SELECTION_ENUMS..,
31063168
}]
31073169
async fn instance_ephemeral_ip_attach(
31083170
rqctx: RequestContext<Self::Context>,
@@ -4466,11 +4528,28 @@ pub trait NexusExternalApi {
44664528
query_params: Query<params::ProjectSelector>,
44674529
) -> Result<HttpResponseOk<shared::ProbeInfo>, HttpError>;
44684530

4531+
/// Create instrumentation probe
4532+
#[endpoint {
4533+
operation_id = "probe_create",
4534+
method = POST,
4535+
path = "/experimental/v1/probes",
4536+
tags = ["experimental"], // system/probes: only one tag is allowed
4537+
versions = ..VERSION_POOL_SELECTION_ENUMS,
4538+
}]
4539+
async fn v2026010300_probe_create(
4540+
rqctx: RequestContext<Self::Context>,
4541+
query_params: Query<params::ProjectSelector>,
4542+
new_probe: TypedBody<v2026010300::ProbeCreate>,
4543+
) -> Result<HttpResponseCreated<Probe>, HttpError> {
4544+
Self::probe_create(rqctx, query_params, new_probe.map(Into::into)).await
4545+
}
4546+
44694547
/// Create instrumentation probe
44704548
#[endpoint {
44714549
method = POST,
44724550
path = "/experimental/v1/probes",
44734551
tags = ["experimental"], // system/probes: only one tag is allowed
4552+
versions = VERSION_POOL_SELECTION_ENUMS..,
44744553
}]
44754554
async fn probe_create(
44764555
rqctx: RequestContext<Self::Context>,

nexus/external-api/src/v2025112000.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ pub struct InstanceCreate {
220220
/// By default, all instances have outbound connectivity, but no inbound
221221
/// connectivity. These external addresses can be used to provide a fixed,
222222
/// known IP address for making inbound connections to the instance.
223+
// Delegates through v2025121200 → params::ExternalIpCreate
223224
#[serde(default)]
224225
pub external_ips: Vec<v2025121200::ExternalIpCreate>,
225226

nexus/external-api/src/v2025121200.rs

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

5-
//! Nexus external types that changed from 2025121200 to 2025601010.
5+
//! Nexus external types that changed from 2025121200 to 2026010100.
66
//!
77
//! Version 2025121200 types (before `ip_version` preference was added for
88
//! default IP pool selection).
@@ -40,7 +40,11 @@ pub struct EphemeralIpCreate {
4040

4141
impl From<EphemeralIpCreate> for params::EphemeralIpCreate {
4242
fn from(old: EphemeralIpCreate) -> params::EphemeralIpCreate {
43-
params::EphemeralIpCreate { pool: old.pool, ip_version: None }
43+
let pool_selector = match old.pool {
44+
Some(pool) => params::PoolSelector::Explicit { pool },
45+
None => params::PoolSelector::Auto { ip_version: None },
46+
};
47+
params::EphemeralIpCreate { pool_selector }
4448
}
4549
}
4650

@@ -67,7 +71,11 @@ impl From<ExternalIpCreate> for params::ExternalIpCreate {
6771
fn from(old: ExternalIpCreate) -> params::ExternalIpCreate {
6872
match old {
6973
ExternalIpCreate::Ephemeral { pool } => {
70-
params::ExternalIpCreate::Ephemeral { pool, ip_version: None }
74+
let pool_selector = match pool {
75+
Some(pool) => params::PoolSelector::Explicit { pool },
76+
None => params::PoolSelector::Auto { ip_version: None },
77+
};
78+
params::ExternalIpCreate::Ephemeral { pool_selector }
7179
}
7280
ExternalIpCreate::Floating { floating_ip } => {
7381
params::ExternalIpCreate::Floating { floating_ip }
@@ -93,12 +101,16 @@ pub struct FloatingIpCreate {
93101

94102
impl From<FloatingIpCreate> for params::FloatingIpCreate {
95103
fn from(old: FloatingIpCreate) -> params::FloatingIpCreate {
96-
params::FloatingIpCreate {
97-
identity: old.identity,
98-
ip: old.ip,
99-
pool: old.pool,
100-
ip_version: None,
101-
}
104+
let address_selector = match (old.ip, old.pool) {
105+
(Some(ip), pool) => params::AddressSelector::Explicit { ip, pool },
106+
(None, Some(pool)) => params::AddressSelector::Auto {
107+
pool_selector: params::PoolSelector::Explicit { pool },
108+
},
109+
(None, None) => params::AddressSelector::Auto {
110+
pool_selector: params::PoolSelector::Auto { ip_version: None },
111+
},
112+
};
113+
params::FloatingIpCreate { identity: old.identity, address_selector }
102114
}
103115
}
104116

@@ -120,6 +132,8 @@ pub struct InstanceCreate {
120132
#[serde(default)]
121133
pub network_interfaces: v2026010100::InstanceNetworkInterfaceAttachment,
122134
/// The external IP addresses provided to this instance.
135+
// Uses local ExternalIpCreate (no ip_version field) → params::ExternalIpCreate
136+
// (defaults ip_version to None in From impl)
123137
#[serde(default)]
124138
pub external_ips: Vec<ExternalIpCreate>,
125139
/// The multicast groups this instance should join.

nexus/external-api/src/v2026010100.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ use serde::Serialize;
3131
use std::net::IpAddr;
3232
use uuid::Uuid;
3333

34+
use crate::v2026010300;
35+
3436
/// Describes an attachment of an `InstanceNetworkInterface` to an `Instance`,
3537
/// at the time the instance is created.
3638
// NOTE: VPC's are an organizing concept for networking resources, not for
@@ -81,7 +83,7 @@ impl TryFrom<InstanceNetworkInterfaceAttachment>
8183
.collect::<Result<_, _>>()
8284
.map(Self::Create),
8385
InstanceNetworkInterfaceAttachment::Default => {
84-
Ok(Self::DefaultDualStack)
86+
Ok(Self::DefaultIpv4)
8587
}
8688
InstanceNetworkInterfaceAttachment::None => Ok(Self::None),
8789
}
@@ -255,7 +257,7 @@ impl TryFrom<InstanceNetworkInterfaceCreate>
255257
transit_ips: ipv6_transit_ips,
256258
})
257259
} else {
258-
PrivateIpStackCreate::auto_dual_stack()
260+
PrivateIpStackCreate::auto_ipv4()
259261
}
260262
}
261263
Some(IpAddr::V4(ipv4)) => {
@@ -325,8 +327,9 @@ pub struct InstanceCreate {
325327
/// By default, all instances have outbound connectivity, but no inbound
326328
/// connectivity. These external addresses can be used to provide a fixed,
327329
/// known IP address for making inbound connections to the instance.
330+
// Delegates through v2026010300 → params::ExternalIpCreate
328331
#[serde(default)]
329-
pub external_ips: Vec<params::ExternalIpCreate>,
332+
pub external_ips: Vec<v2026010300::ExternalIpCreate>,
330333

331334
/// The multicast groups this instance should join.
332335
///
@@ -418,7 +421,11 @@ impl TryFrom<InstanceCreate> for params::InstanceCreate {
418421
hostname: value.hostname,
419422
user_data: value.user_data,
420423
network_interfaces,
421-
external_ips: value.external_ips,
424+
external_ips: value
425+
.external_ips
426+
.into_iter()
427+
.map(TryInto::try_into)
428+
.collect::<Result<Vec<_>, _>>()?,
422429
multicast_groups: value.multicast_groups,
423430
disks: value.disks,
424431
boot_disk: value.boot_disk,

0 commit comments

Comments
 (0)