Skip to content

Commit 3b27679

Browse files
[floating-ip,fix,api] Fix floating IP creation API to distinguish explicit IP from pool selection (#9687)
Fixes #9680. This work clears up ambiguity. `AddressAllocator::Explicit` now requires only an IP field, with the pool inferred from the address (since IP pools cannot have overlapping ranges). Pool-based allocation moves to `AddressAllocator::Auto` with `PoolSelector::Explicit { pool }`. Includes: - Simplifies `AddressAllocator::Explicit` to only require `ip` as a field (pool inferred) - Moves explicit pool selection to `Auto { pool_selector: PoolSelector::Explicit { pool } }` - Adds API version 2026012200 (`FLOATING_IP_ALLOCATOR_UPDATE`) - Adds `ip_pool_fetch_containing_address()` to resolve pool from explicit IP address correctly * Consolidates multicast pool resolution to use `ip_pool_fetch_containing_address()` too - Adds versioned types in v2026011600.rs and updates v2026011501.rs delegation chain - Adds unit tests for wire format compatibility and version conversions - Adds FloatingIpAllocation enum at datastore layer (type-safe, mirrors multicast pattern) - Adds Display impl for consistent pool selection logging (inferred/explicit/default/auto) - Converts `NotFound` to `InvalidRequest` for an IP not in any configured pool - Adds an index on `ip_pool_range.ip_pool_id` (schema version 223) for optimizing `ip_pool_fetch_containing_address()` and other queries Current params JSON: - Explicit IP (pool inferred from address): ```json {"type": "explicit", "ip": "10.0.0.1"} ``` - Auto with explicit pool: ```json {"type": "auto", "pool_selector": {"type": "explicit", "pool": "my-pool"}} ``` - Auto with default pool (specific IP version) ```json {"type": "auto", "pool_selector": {"type": "auto", "ip_version": "v4"}} ``` - Auto with default pool (silo default, works only if there's a single default pool) ```json {"type": "auto", "pool_selector": {"type": "auto"}} ``` - Auto with all defaults (works only if there's a single default pool) ```json {"type": "auto"} ``` Example `FloatingIpCreate` request body: ```json { "name": "my-floating-ip", "description": "Example floating IP", "address_allocator": {"type": "explicit", "ip": "10.0.0.1"} } ``` Or with pool selection: ```json { "name": "my-floating-ip", "description": "Example floating IP", "address_allocator": { "type": "auto", "pool_selector": {"type": "explicit", "pool": "my-pool"} } } ``` Note: There's some multicast cleanup here to make sure of the same functionality, which just removes unnecessary code.
1 parent 6cef874 commit 3b27679

File tree

30 files changed

+32567
-621
lines changed

30 files changed

+32567
-621
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

nexus/db-model/src/schema_versions.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
1616
///
1717
/// This must be updated when you change the database schema. Refer to
1818
/// schema/crdb/README.adoc in the root of this repository for details.
19-
pub const SCHEMA_VERSION: Version = Version::new(222, 0, 0);
19+
pub const SCHEMA_VERSION: Version = Version::new(223, 0, 0);
2020

2121
/// List of all past database schema versions, in *reverse* order
2222
///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
2828
// | leaving the first copy as an example for the next person.
2929
// v
3030
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
31+
KnownVersion::new(223, "ip-pool-range-by-pool-id-index"),
3132
KnownVersion::new(222, "audit-log-credential-id"),
3233
KnownVersion::new(221, "audit-log-auth-method-enum"),
3334
KnownVersion::new(220, "multicast-implicit-lifecycle"),

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

Lines changed: 76 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,40 @@ use omicron_uuid_kinds::InstanceUuid;
5858
use omicron_uuid_kinds::OmicronZoneUuid;
5959
use ref_cast::RefCast;
6060
use sled_agent_types::inventory::ZoneKind;
61+
use slog::debug;
6162
use std::net::IpAddr;
6263
use uuid::Uuid;
6364

65+
/// Floating IP allocation method.
66+
///
67+
/// Separate from `params::AddressAllocator` because pool resolution requires
68+
/// async authz lookup. The app layer converts API params to this type.
69+
#[derive(Debug, Clone)]
70+
pub enum FloatingIpAllocation {
71+
/// Use a specific IP address. Pool is inferred from the address since
72+
/// IP pool ranges cannot overlap.
73+
Explicit { ip: IpAddr },
74+
/// Auto-allocate from a pool.
75+
Auto {
76+
/// Explicit pool to allocate from. If None, uses the silo's default.
77+
pool: Option<authz::IpPool>,
78+
/// IP version for default pool selection.
79+
/// Required if both IPv4 and IPv6 default pools exist and no explicit
80+
/// pool is specified.
81+
ip_version: Option<IpVersion>,
82+
},
83+
}
84+
85+
impl std::fmt::Display for FloatingIpAllocation {
86+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
87+
match self {
88+
Self::Explicit { .. } => write!(f, "inferred"),
89+
Self::Auto { pool: Some(_), .. } => write!(f, "explicit"),
90+
Self::Auto { pool: None, .. } => write!(f, "default"),
91+
}
92+
}
93+
}
94+
6495
// NOTE: This number includes the automatically-created SNAT IP for every
6596
// instance. We need to change that allocation so that it's only when necessary,
6697
// tracked by https://github.com/oxidecomputer/omicron/issues/9003.
@@ -220,31 +251,56 @@ impl DataStore {
220251
}
221252

222253
/// Allocates a floating IP address for instance usage.
223-
///
224-
/// If `ip_version` is provided and no pool is specified, the default pool
225-
/// lookup will be filtered to that version. If both IPv4 and IPv6 default
226-
/// pools exist and `ip_version` is `None`, an error is returned.
227254
pub async fn allocate_floating_ip(
228255
&self,
229256
opctx: &OpContext,
230257
project_id: Uuid,
231258
identity: IdentityMetadataCreateParams,
232-
ip: Option<IpAddr>,
233-
pool: Option<authz::IpPool>,
234-
ip_version: Option<IpVersion>,
259+
allocation: FloatingIpAllocation,
235260
) -> CreateResult<ExternalIp> {
236261
let ip_id = Uuid::new_v4();
237262

238-
let authz_pool = self
239-
.resolve_pool_for_allocation(
240-
opctx,
241-
pool,
242-
IpPoolType::Unicast,
243-
ip_version,
244-
)
245-
.await?;
263+
let (authz_pool, explicit_ip) = match &allocation {
264+
FloatingIpAllocation::Explicit { ip } => {
265+
let pool = self
266+
.ip_pool_fetch_containing_address(
267+
opctx,
268+
*ip,
269+
IpPoolType::Unicast,
270+
)
271+
.await
272+
.map_err(|e| match e {
273+
Error::ObjectNotFound { .. } => {
274+
Error::invalid_request(format!(
275+
"IP address {ip} is not in any configured pool"
276+
))
277+
}
278+
other => other,
279+
})?;
280+
(pool, Some(*ip))
281+
}
282+
FloatingIpAllocation::Auto { pool, ip_version } => {
283+
let pool = self
284+
.resolve_pool_for_allocation(
285+
opctx,
286+
pool.clone(),
287+
IpPoolType::Unicast,
288+
*ip_version,
289+
)
290+
.await?;
291+
(pool, None)
292+
}
293+
};
294+
295+
debug!(
296+
opctx.log,
297+
"floating IP allocation";
298+
"pool_selection" => %allocation,
299+
"pool_id" => %authz_pool.id(),
300+
"project_id" => %project_id,
301+
);
246302

247-
let data = if let Some(ip) = ip {
303+
let data = if let Some(ip) = explicit_ip {
248304
IncompleteExternalIp::for_floating_explicit(
249305
ip_id,
250306
&Name(identity.name),
@@ -1699,7 +1755,7 @@ mod tests {
16991755
.await
17001756
.expect("Should link multicast pool");
17011757

1702-
// Try to allocate floating IP
1758+
// Try to allocate floating IP from default pool (no pool, no IP version)
17031759
let res = datastore
17041760
.allocate_floating_ip(
17051761
&opctx,
@@ -1708,9 +1764,7 @@ mod tests {
17081764
name: "my-floating-ip".parse().unwrap(),
17091765
description: "A floating IP".to_string(),
17101766
},
1711-
None, // No specific IP
1712-
None, // No specific pool - use default
1713-
None, // No specific IP version
1767+
FloatingIpAllocation::Auto { pool: None, ip_version: None },
17141768
)
17151769
.await;
17161770

@@ -1773,7 +1827,7 @@ mod tests {
17731827
.await
17741828
.expect("Should link unicast pool");
17751829

1776-
// Now floating IP allocation should succeed
1830+
// Now floating IP allocation from default pool should succeed
17771831
let floating_ip = datastore
17781832
.allocate_floating_ip(
17791833
&opctx,
@@ -1782,9 +1836,7 @@ mod tests {
17821836
name: "my-floating-ip".parse().unwrap(),
17831837
description: "A floating IP".to_string(),
17841838
},
1785-
None, // No specific IP
1786-
None, // No specific pool - use default
1787-
None, // No specific IP version, but just 1 default pool
1839+
FloatingIpAllocation::Auto { pool: None, ip_version: None },
17881840
)
17891841
.await
17901842
.expect("Floating IP allocation should succeed with unicast default pool");

0 commit comments

Comments
 (0)