From 6659313bfe6d0c0f16cda8a0455e02edccfdb590 Mon Sep 17 00:00:00 2001
From: Sean Klein <sean@oxidecomputer.com>
Date: Fri, 30 Jan 2026 16:06:14 -0800
Subject: [PATCH] Remove redundant plumb_service_firewall_rules call from
 rack_initialize

This call was added in March 2024 (05ed7903e5) to ensure firewall rules
are plumbed during initial rack setup. However, in May 2024 (f2602b5865),
await_ip_allowlist_plumbing() was added to Server::start() to ensure
the allowlist is enforced before the external HTTP server starts.

This created redundancy: in the test/RSS path, rack_initialize() plumbs
the rules, then Server::start() immediately re-plumbs them via
await_ip_allowlist_plumbing().

Removing the call from rack_initialize() is safe because:

1. await_ip_allowlist_plumbing() handles both fresh rack and restart:
   - Fresh rack: rules plumbed before external server starts
   - Restart: rules re-plumbed before external server starts

2. The call in rack_initialize() has no unique purpose - the external
   server doesn't start until AFTER await_ip_allowlist_plumbing() anyway

3. Order is preserved:
   - Allowlist written to DB in rack_initialize() (unchanged)
   - Rules plumbed to sleds in await_ip_allowlist_plumbing() (unchanged)
   - External server starts AFTER plumbing completes (unchanged)

4. Background task (ServiceRulePropagator) provides additional safety,
   periodically re-plumbing rules every 5 minutes

Also removes the now-unused Nexus::plumb_service_firewall_rules wrapper
method from sled.rs, since all callers use nexus_networking:: directly.

Saves ~400ms in nexus test setup time.
---
 nexus/src/app/rack.rs |  6 ++++--
 nexus/src/app/sled.rs | 16 ----------------
 2 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs
index 3a442d975d0..e4dfd88fd73 100644
--- a/nexus/src/app/rack.rs
+++ b/nexus/src/app/rack.rs
@@ -740,8 +740,10 @@ impl super::Nexus {
             )
             .await?;
 
-        // Plumb the firewall rules for the built-in services
-        self.plumb_service_firewall_rules(opctx, &[]).await?;
+        // Note: Service firewall rules are plumbed in Server::start() via
+        // await_ip_allowlist_plumbing(), which runs before the external HTTP
+        // server starts. This ensures rules are in place for both fresh rack
+        // initialization and Nexus restart scenarios.
 
         // We've potentially updated the list of DNS servers and the DNS
         // configuration for both internal and external DNS, plus the Silo
diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs
index d744e83d5ce..28c1a5dafb9 100644
--- a/nexus/src/app/sled.rs
+++ b/nexus/src/app/sled.rs
@@ -363,20 +363,4 @@ impl super::Nexus {
         self.db_datastore.crucible_dataset_upsert(dataset).await?;
         Ok(())
     }
-
-    /// Ensure firewall rules for internal services get reflected on all the relevant sleds.
-    pub(crate) async fn plumb_service_firewall_rules(
-        &self,
-        opctx: &OpContext,
-        sleds_filter: &[SledUuid],
-    ) -> Result<(), Error> {
-        nexus_networking::plumb_service_firewall_rules(
-            &self.db_datastore,
-            opctx,
-            sleds_filter,
-            &self.opctx_alloc,
-            &self.log,
-        )
-        .await
-    }
 }