From f256c3fbc82e58e338c239e315390b7e4833c04a Mon Sep 17 00:00:00 2001
From: Benjamin Naecker <ben@oxide.computer>
Date: Fri, 26 Sep 2025 21:26:10 +0000
Subject: [PATCH] Only allocate an SNAT IP for instances if needed

- Fix #4317
- Only allocate an SNAT IP on instance create if there are no other
  external IPs specified for the instance.
---
 nexus/src/app/sagas/instance_create.rs        | 27 +++++---
 nexus/tests/integration_tests/external_ips.rs | 61 +++++++++++++++++++
 2 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs
index 89f8ccaf887..040b7960742 100644
--- a/nexus/src/app/sagas/instance_create.rs
+++ b/nexus/src/app/sagas/instance_create.rs
@@ -269,13 +269,16 @@ impl NexusSaga for SagaInstanceCreate {
             )?;
         }
 
-        // Allocate an external IP address for the default outbound connectivity
-        builder.append(Node::action(
-            "snat_ip_id",
-            "CreateSnatIpId",
-            ACTION_GENERATE_ID.as_ref(),
-        ));
-        builder.append(create_snat_ip_action());
+        // Allocate an external IP address for the default outbound
+        // connectivity, if there are no explicit IP addresses.
+        if need_snat_ip(params) {
+            builder.append(Node::action(
+                "snat_ip_id",
+                "CreateSnatIpId",
+                ACTION_GENERATE_ID.as_ref(),
+            ));
+            builder.append(create_snat_ip_action());
+        }
 
         // See the comment above where we add nodes for creating NICs.  We use
         // the same pattern here.
@@ -734,7 +737,17 @@ async fn create_default_primary_network_interface(
     Ok(())
 }
 
+// Return `true` if we need to allocate an SNAT IP for this instance.
+//
+// This is currently done only if there is no other external IP address for the
+// instance.
+fn need_snat_ip(params: &Params) -> bool {
+    params.create_params.external_ips.is_empty()
+}
+
 /// Create an external IP address for instance source NAT.
+///
+/// Note that we only do this if there is no other outbound connectivity.
 async fn sic_allocate_instance_snat_ip(
     sagactx: NexusActionContext,
 ) -> Result<(), ActionError> {
diff --git a/nexus/tests/integration_tests/external_ips.rs b/nexus/tests/integration_tests/external_ips.rs
index b8183eb9ad9..fc65ba44f7b 100644
--- a/nexus/tests/integration_tests/external_ips.rs
+++ b/nexus/tests/integration_tests/external_ips.rs
@@ -1326,6 +1326,67 @@ async fn can_list_instance_snat_ip(cptestctx: &ControlPlaneTestContext) {
     assert_eq!(*last_port, NUM_SOURCE_NAT_PORTS - 1);
 }
 
+#[nexus_test]
+async fn do_not_create_snat_ip_with_ephemeral(
+    cptestctx: &ControlPlaneTestContext,
+) {
+    let client = &cptestctx.external_client;
+
+    let pool = create_default_ip_pool(&client).await;
+    let _project = create_project(client, PROJECT_NAME).await;
+
+    // Get the first address in the pool.
+    let range = NexusRequest::object_get(
+        client,
+        &format!("/v1/system/ip-pools/{}/ranges", pool.identity.id),
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute()
+    .await
+    .unwrap_or_else(|e| panic!("failed to get IP pool range: {e}"))
+    .parsed_body::<IpPoolRangeResultsPage>()
+    .unwrap_or_else(|e| panic!("failed to parse IP pool range: {e}"));
+    assert_eq!(range.items.len(), 1, "Should have 1 range in the pool");
+    let oxide_client::types::IpRange::V4(oxide_client::types::Ipv4Range {
+        first,
+        ..
+    }) = &range.items[0].range
+    else {
+        panic!("Expected IPv4 range, found {:?}", &range.items[0]);
+    };
+    let expected_ip = IpAddr::V4(*first);
+
+    // Create a running instance with an Ephemeral IP, which means the instance
+    // should not have an SNAT IP as well.
+    let instance_name = INSTANCE_NAMES[0];
+    let instance =
+        instance_for_external_ips(client, instance_name, true, true, &[]).await;
+    let url = format!("/v1/instances/{}/external-ips", instance.identity.id);
+    let page = NexusRequest::object_get(client, &url)
+        .authn_as(AuthnMode::PrivilegedUser)
+        .execute()
+        .await
+        .unwrap_or_else(|e| {
+            panic!("failed to make \"get\" request to {url}: {e}")
+        })
+        .parsed_body::<ExternalIpResultsPage>()
+        .unwrap_or_else(|e| {
+            panic!("failed to make \"get\" request to {url}: {e}")
+        });
+    let ips = page.items;
+    assert_eq!(
+        ips.len(),
+        1,
+        "Instance should have been created with exactly 1 IP"
+    );
+    let oxide_client::types::ExternalIp::Ephemeral { ip, ip_pool_id } = &ips[0]
+    else {
+        panic!("Expected an SNAT external IP, found {:?}", &ips[0]);
+    };
+    assert_eq!(ip_pool_id, &pool.identity.id);
+    assert_eq!(ip, &expected_ip);
+}
+
 pub async fn floating_ip_get(
     client: &ClientTestContext,
     fip_url: &str,