Skip to content

Commit d9b2fc6

Browse files
authored
Expose SNAT IPs in the public API (#8685)
Fixes #8163
1 parent 609bd77 commit d9b2fc6

File tree

9 files changed

+208
-41
lines changed

9 files changed

+208
-41
lines changed

common/src/api/external/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2058,7 +2058,6 @@ impl FromStr for L4PortRange {
20582058
.parse::<NonZeroU16>()
20592059
.map_err(|e| L4PortRangeError::Value(right.into(), e))?
20602060
.into();
2061-
20622061
Ok(L4PortRange { first, last })
20632062
}
20642063
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ async fn instance_launch() -> Result<()> {
9191
.send()
9292
.await?
9393
.items
94-
.first()
94+
.iter()
95+
.find(|eip| matches!(eip, ExternalIp::Ephemeral { .. }))
9596
.context("no external IPs")?
9697
.clone();
9798

nexus/db-model/src/external_ip.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -533,9 +533,12 @@ impl TryFrom<ExternalIp> for views::ExternalIp {
533533
ip: ip.ip.ip(),
534534
ip_pool_id: ip.ip_pool_id,
535535
}),
536-
IpKind::SNat => Err(Error::internal_error(
537-
"SNAT IP addresses should not be exposed in the API",
538-
)),
536+
IpKind::SNat => Ok(views::ExternalIp::SNat(views::SNatIp {
537+
ip: ip.ip.ip(),
538+
first_port: u16::from(ip.first_port),
539+
last_port: u16::from(ip.last_port),
540+
ip_pool_id: ip.ip_pool_id,
541+
})),
539542
}
540543
}
541544
}

nexus/src/app/external_ip.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use nexus_db_lookup::lookup;
1313
use nexus_db_model::IpAttachState;
1414
use nexus_db_queries::authz;
1515
use nexus_db_queries::context::OpContext;
16-
use nexus_db_queries::db::model::IpKind;
1716
use nexus_types::external_api::params;
1817
use nexus_types::external_api::views;
1918
use omicron_common::api::external::CreateResult;
@@ -44,9 +43,7 @@ impl super::Nexus {
4443
.await?
4544
.into_iter()
4645
.filter_map(|ip| {
47-
if ip.kind == IpKind::SNat
48-
|| ip.state != IpAttachState::Attached
49-
{
46+
if ip.state != IpAttachState::Attached {
5047
None
5148
} else {
5249
Some(ip.try_into().unwrap())

nexus/tests/integration_tests/external_ips.rs

Lines changed: 115 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ use nexus_types::external_api::views::FloatingIp;
4444
use nexus_types::identity::Resource;
4545
use omicron_common::address::IpRange;
4646
use omicron_common::address::Ipv4Range;
47+
use omicron_common::address::NUM_SOURCE_NAT_PORTS;
4748
use omicron_common::api::external::ByteCount;
4849
use omicron_common::api::external::IdentityMetadataCreateParams;
4950
use omicron_common::api::external::IdentityMetadataUpdateParams;
@@ -54,6 +55,8 @@ use omicron_common::api::external::Name;
5455
use omicron_common::api::external::NameOrId;
5556
use omicron_uuid_kinds::GenericUuid;
5657
use omicron_uuid_kinds::InstanceUuid;
58+
use oxide_client::types::ExternalIpResultsPage;
59+
use oxide_client::types::IpPoolRangeResultsPage;
5760
use uuid::Uuid;
5861

5962
type ControlPlaneTestContext =
@@ -778,18 +781,19 @@ async fn test_external_ip_live_attach_detach(
778781
instance_simulate(nexus, &instance_id).await;
779782
}
780783

781-
// Verify that each instance has no external IPs.
784+
// Verify that each instance has only an SNAT external IP.
785+
let eips = fetch_instance_external_ips(
786+
client,
787+
INSTANCE_NAMES[i],
788+
PROJECT_NAME,
789+
)
790+
.await;
791+
assert_eq!(eips.len(), 1, "Expected exactly 1 SNAT external IP");
782792
assert_eq!(
783-
fetch_instance_external_ips(
784-
client,
785-
INSTANCE_NAMES[i],
786-
PROJECT_NAME
787-
)
788-
.await
789-
.len(),
790-
0
793+
eips[0].kind(),
794+
shared::IpKind::SNat,
795+
"Expected exactly 1 SNAT external IP"
791796
);
792-
793797
instances.push(instance);
794798
}
795799

@@ -816,7 +820,7 @@ async fn test_external_ip_live_attach_detach(
816820
fetch_instance_external_ips(client, instance_name, PROJECT_NAME)
817821
.await;
818822

819-
assert_eq!(eip_list.len(), 2);
823+
assert_eq!(eip_list.len(), 3);
820824
assert!(eip_list.contains(&eph_resp));
821825
assert!(
822826
eip_list
@@ -857,7 +861,8 @@ async fn test_external_ip_live_attach_detach(
857861
fetch_instance_external_ips(client, instance_name, PROJECT_NAME)
858862
.await;
859863

860-
assert_eq!(eip_list.len(), 0);
864+
// We still have 1 SNAT IP
865+
assert_eq!(eip_list.len(), 1);
861866
assert_eq!(fip.ip, fip_resp.ip);
862867

863868
// Check for idempotency: repeat requests should return same values for FIP,
@@ -907,8 +912,21 @@ async fn test_external_ip_live_attach_detach(
907912
let instance_name = instances[0].identity.name.as_str();
908913
let eip_list =
909914
fetch_instance_external_ips(client, instance_name, PROJECT_NAME).await;
910-
assert_eq!(eip_list.len(), 1);
911-
assert_eq!(eip_list[0].ip(), fips[0].ip);
915+
assert_eq!(eip_list.len(), 2, "Expected a Floating and SNAT IP");
916+
assert_eq!(
917+
eip_list
918+
.iter()
919+
.find_map(|eip| {
920+
if eip.kind() == shared::IpKind::Floating {
921+
Some(eip.ip())
922+
} else {
923+
None
924+
}
925+
})
926+
.expect("Should have a Floating IP"),
927+
fips[0].ip,
928+
"Floating IP object's IP address is not correct",
929+
);
912930

913931
// now the other way: floating IP by ID and instance by name
914932
let floating_ip_id = fips[1].identity.id;
@@ -934,8 +952,21 @@ async fn test_external_ip_live_attach_detach(
934952

935953
let eip_list =
936954
fetch_instance_external_ips(client, instance_name, PROJECT_NAME).await;
937-
assert_eq!(eip_list.len(), 1);
938-
assert_eq!(eip_list[0].ip(), fips[1].ip);
955+
assert_eq!(eip_list.len(), 2, "Expect a Floating and SNAT IP");
956+
assert_eq!(
957+
eip_list
958+
.iter()
959+
.find_map(|eip| {
960+
if eip.kind() == shared::IpKind::Floating {
961+
Some(eip.ip())
962+
} else {
963+
None
964+
}
965+
})
966+
.expect("Should have a Floating IP"),
967+
fips[1].ip,
968+
"Floating IP object's IP address is not correct",
969+
);
939970

940971
// none of that changed the number of allocated IPs
941972
assert_ip_pool_utilization(client, "default", 3, 65536, 0, 0).await;
@@ -1223,6 +1254,74 @@ async fn test_external_ip_attach_ephemeral_at_pool_exhaustion(
12231254
assert_eq!(eph_resp_2, eph_resp);
12241255
}
12251256

1257+
#[nexus_test]
1258+
async fn can_list_instance_snat_ip(cptestctx: &ControlPlaneTestContext) {
1259+
let client = &cptestctx.external_client;
1260+
1261+
let pool = create_default_ip_pool(&client).await;
1262+
let _project = create_project(client, PROJECT_NAME).await;
1263+
1264+
// Get the first address in the pool.
1265+
let range = NexusRequest::object_get(
1266+
client,
1267+
&format!("/v1/system/ip-pools/{}/ranges", pool.identity.id),
1268+
)
1269+
.authn_as(AuthnMode::PrivilegedUser)
1270+
.execute()
1271+
.await
1272+
.unwrap_or_else(|e| panic!("failed to get IP pool range: {e}"))
1273+
.parsed_body::<IpPoolRangeResultsPage>()
1274+
.unwrap_or_else(|e| panic!("failed to parse IP pool range: {e}"));
1275+
assert_eq!(range.items.len(), 1, "Should have 1 range in the pool");
1276+
let oxide_client::types::IpRange::V4(oxide_client::types::Ipv4Range {
1277+
first,
1278+
..
1279+
}) = &range.items[0].range
1280+
else {
1281+
panic!("Expected IPv4 range, found {:?}", &range.items[0]);
1282+
};
1283+
let expected_ip = IpAddr::V4(*first);
1284+
1285+
// Create a running instance with only an SNAT IP address.
1286+
let instance_name = INSTANCE_NAMES[0];
1287+
let instance =
1288+
instance_for_external_ips(client, instance_name, true, false, &[])
1289+
.await;
1290+
let url = format!("/v1/instances/{}/external-ips", instance.identity.id);
1291+
let page = NexusRequest::object_get(client, &url)
1292+
.authn_as(AuthnMode::PrivilegedUser)
1293+
.execute()
1294+
.await
1295+
.unwrap_or_else(|e| {
1296+
panic!("failed to make \"get\" request to {url}: {e}")
1297+
})
1298+
.parsed_body::<ExternalIpResultsPage>()
1299+
.unwrap_or_else(|e| {
1300+
panic!("failed to make \"get\" request to {url}: {e}")
1301+
});
1302+
let ips = page.items;
1303+
assert_eq!(
1304+
ips.len(),
1305+
1,
1306+
"Instance should have been created with exactly 1 IP"
1307+
);
1308+
let oxide_client::types::ExternalIp::Snat {
1309+
ip,
1310+
ip_pool_id,
1311+
first_port,
1312+
last_port,
1313+
} = &ips[0]
1314+
else {
1315+
panic!("Expected an SNAT external IP, found {:?}", &ips[0]);
1316+
};
1317+
assert_eq!(ip_pool_id, &pool.identity.id);
1318+
assert_eq!(ip, &expected_ip);
1319+
1320+
// Port ranges are half-open on the right, e.g., [0, 16384).
1321+
assert_eq!(*first_port, 0);
1322+
assert_eq!(*last_port, NUM_SOURCE_NAT_PORTS - 1);
1323+
}
1324+
12261325
pub async fn floating_ip_get(
12271326
client: &ClientTestContext,
12281327
fip_url: &str,

nexus/tests/integration_tests/instances.rs

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6451,29 +6451,31 @@ async fn test_instance_attach_several_external_ips(
64516451
.await;
64526452

64536453
// 1 ephemeral + 7 floating + 1 SNAT
6454-
assert_ip_pool_utilization(client, "default", 9, 10, 0, 0).await;
6454+
const N_EXPECTED_IPS: u32 = 9;
6455+
assert_ip_pool_utilization(client, "default", N_EXPECTED_IPS, 10, 0, 0)
6456+
.await;
64556457

64566458
// Verify that all external IPs are visible on the instance and have
64576459
// been allocated in order.
64586460
let external_ips =
64596461
fetch_instance_external_ips(&client, instance_name, PROJECT_NAME).await;
6460-
assert_eq!(external_ips.len(), 8);
6461-
eprintln!("{external_ips:?}");
6462+
eprintln!("{external_ips:#?}");
6463+
assert_eq!(external_ips.len(), N_EXPECTED_IPS as usize);
6464+
6465+
// We've created all the FIPs first, before the instance. The instance gets
6466+
// an SNAT IP automatically, and then _also_ an Ephemeral IP.
64626467
for (i, eip) in external_ips
64636468
.iter()
64646469
.sorted_unstable_by(|a, b| a.ip().cmp(&b.ip()))
64656470
.enumerate()
64666471
{
6467-
let last_octet = i + if i != external_ips.len() - 1 {
6468-
assert_eq!(eip.kind(), IpKind::Floating);
6469-
1
6470-
} else {
6471-
// SNAT will occupy 1.0.0.8 here, since it it alloc'd before
6472-
// the ephemeral.
6473-
assert_eq!(eip.kind(), IpKind::Ephemeral);
6474-
2
6472+
match i {
6473+
0..7 => assert_eq!(eip.kind(), IpKind::Floating),
6474+
7 => assert_eq!(eip.kind(), IpKind::SNat),
6475+
8 => assert_eq!(eip.kind(), IpKind::Ephemeral),
6476+
_ => unreachable!(),
64756477
};
6476-
assert_eq!(eip.ip(), Ipv4Addr::new(10, 0, 0, last_octet as u8));
6478+
assert_eq!(eip.ip(), Ipv4Addr::new(10, 0, 0, (i + 1) as u8));
64776479
}
64786480

64796481
// Verify that all floating IPs are bound to their parent instance.

nexus/types/src/external_api/shared.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ pub enum ServiceUsingCertificate {
237237
)]
238238
#[serde(rename_all = "snake_case")]
239239
pub enum IpKind {
240+
SNat,
240241
Ephemeral,
241242
Floating,
242243
}

nexus/types/src/external_api/views.rs

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -502,26 +502,48 @@ pub struct IpPoolRange {
502502
#[derive(Debug, Clone, Deserialize, PartialEq, Serialize, JsonSchema)]
503503
#[serde(tag = "kind", rename_all = "snake_case")]
504504
pub enum ExternalIp {
505-
Ephemeral { ip: IpAddr, ip_pool_id: Uuid },
505+
#[serde(rename = "snat")]
506+
SNat(SNatIp),
507+
Ephemeral {
508+
ip: IpAddr,
509+
ip_pool_id: Uuid,
510+
},
506511
Floating(FloatingIp),
507512
}
508513

509514
impl ExternalIp {
510515
pub fn ip(&self) -> IpAddr {
511516
match self {
517+
Self::SNat(snat) => snat.ip,
512518
Self::Ephemeral { ip, .. } => *ip,
513519
Self::Floating(float) => float.ip,
514520
}
515521
}
516522

517523
pub fn kind(&self) -> IpKind {
518524
match self {
525+
Self::SNat(_) => IpKind::SNat,
519526
Self::Ephemeral { .. } => IpKind::Ephemeral,
520527
Self::Floating(_) => IpKind::Floating,
521528
}
522529
}
523530
}
524531

532+
/// A source NAT IP address.
533+
///
534+
/// SNAT addresses are ephemeral addresses used only for outbound connectivity.
535+
#[derive(Debug, Clone, Deserialize, PartialEq, Serialize, JsonSchema)]
536+
pub struct SNatIp {
537+
/// The IP address.
538+
pub ip: IpAddr,
539+
/// The first usable port within the IP address.
540+
pub first_port: u16,
541+
/// The last usable port within the IP address.
542+
pub last_port: u16,
543+
/// ID of the IP Pool from which the address is taken.
544+
pub ip_pool_id: Uuid,
545+
}
546+
525547
/// A Floating IP is a well-known IP address which can be attached
526548
/// and detached from instances.
527549
#[derive(
@@ -553,9 +575,11 @@ impl TryFrom<ExternalIp> for FloatingIp {
553575

554576
fn try_from(value: ExternalIp) -> Result<Self, Self::Error> {
555577
match value {
556-
ExternalIp::Ephemeral { .. } => Err(Error::internal_error(
557-
"tried to convert an ephemeral IP into a floating IP",
558-
)),
578+
ExternalIp::SNat(_) | ExternalIp::Ephemeral { .. } => {
579+
Err(Error::internal_error(
580+
"tried to convert an SNAT or ephemeral IP into a floating IP",
581+
))
582+
}
559583
ExternalIp::Floating(v) => Ok(v),
560584
}
561585
}

0 commit comments

Comments
 (0)