From 9c66fa540e58763bdc6ee5c246de0ca186ed1d6f Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Tue, 25 Aug 2026 13:34:22 -0700 Subject: [PATCH] Support multiple external IPs in blueprint zone type - Add support for multiple EIPs to the blueprint zone types for Nexus, External DNS, and Boundary NTP zones. - Add some newtypes and wrappers to support lists of these or up to 2 of them for the SNAT case of Boundary NTP. - Add a test that the full blueprint with multiple addresses round-trips through the database. - Planner still emits exactly one address in all these cases, this is only the structural change to support multiple addresses. - Update lockstep OpenAPI docs - Closes #9288 --- clients/nexus-lockstep-client/src/lib.rs | 13 +- dev-tools/reconfigurator-cli/src/lib.rs | 10 +- live-tests/tests/test_nexus_handoff.rs | 8 +- nexus/db-model/src/deployment.rs | 222 ++++--- .../db-queries/src/db/datastore/deployment.rs | 112 ++-- .../deployment/external_networking.rs | 153 +++-- nexus/db-queries/src/db/datastore/rack.rs | 108 ++-- nexus/reconfigurator/blippy/src/checks.rs | 83 ++- .../reconfigurator/execution/src/database.rs | 12 +- nexus/reconfigurator/execution/src/dns.rs | 34 +- nexus/reconfigurator/execution/src/sagas.rs | 15 +- .../planning/src/blueprint_builder/builder.rs | 46 +- .../allocators/external_networking.rs | 52 +- nexus/reconfigurator/planning/src/example.rs | 13 +- .../tests/integration_tests/planner.rs | 37 +- .../output/planner_nonprovisionable_2_2a.txt | 12 +- nexus/src/lib.rs | 45 +- nexus/test-utils/src/starter.rs | 54 +- nexus/types/src/deployment.rs | 92 ++- nexus/types/src/deployment/execution/utils.rs | 21 +- .../types/src/deployment/network_resources.rs | 607 +++++++++++++++++- nexus/types/src/deployment/zone_type.rs | 74 ++- openapi/nexus-lockstep.json | 188 ++++-- sled-agent/rack-setup/src/plan/service.rs | 24 +- sled-agent/src/services.rs | 120 ++-- sled-agent/src/sim/server.rs | 19 +- .../types/versions/src/impls/inventory.rs | 32 +- sled-agent/types/versions/src/latest.rs | 3 + .../multiple_zone_external_ips/inventory.rs | 12 +- 29 files changed, 1604 insertions(+), 617 deletions(-) diff --git a/clients/nexus-lockstep-client/src/lib.rs b/clients/nexus-lockstep-client/src/lib.rs index a5d0741d2cc..87e51491857 100644 --- a/clients/nexus-lockstep-client/src/lib.rs +++ b/clients/nexus-lockstep-client/src/lib.rs @@ -66,6 +66,10 @@ progenitor::generate_api!( NetworkInterface = sled_agent_types::inventory::NetworkInterface, NetworkInterfaceKind = sled_agent_types::inventory::NetworkInterfaceKind, NewPasswordHash = omicron_passwords::NewPasswordHash, + OmicronZoneExternalFloatingAddr = + nexus_types::deployment::OmicronZoneExternalFloatingAddr, + OmicronZoneExternalFloatingIp = + nexus_types::deployment::OmicronZoneExternalFloatingIp, OximeterReadMode = nexus_types::deployment::OximeterReadMode, OximeterReadPolicy = nexus_types::deployment::OximeterReadPolicy, PendingMgsUpdate = nexus_types::deployment::PendingMgsUpdate, @@ -151,15 +155,6 @@ impl From for types::Ipv6Range { } } -impl From<&sled_agent_types::inventory::SourceNatConfigGeneric> - for types::SourceNatConfigGeneric -{ - fn from(r: &sled_agent_types::inventory::SourceNatConfigGeneric) -> Self { - let (first_port, last_port) = r.port_range_raw(); - Self { ip: r.ip, first_port, last_port } - } -} - impl From<&omicron_common::api::external::AllowedSourceIps> for types::AllowedSourceIps { diff --git a/dev-tools/reconfigurator-cli/src/lib.rs b/dev-tools/reconfigurator-cli/src/lib.rs index ee3004dea2b..f796942e81e 100644 --- a/dev-tools/reconfigurator-cli/src/lib.rs +++ b/dev-tools/reconfigurator-cli/src/lib.rs @@ -157,12 +157,14 @@ impl ReconfiguratorSim { // Handle zone networking setup first for (_, zone) in parent_blueprint.in_service_zones() { - if let Some((external_ip, nic)) = + if let Some((external_ips, nic)) = zone.zone_type.external_networking() { - builder - .add_omicron_zone_external_ip(zone.id, external_ip) - .context("adding omicron zone external IP")?; + for external_ip in external_ips { + builder + .add_omicron_zone_external_ip(zone.id, external_ip) + .context("adding omicron zone external IP")?; + } let nic = OmicronZoneNic { // TODO-cleanup use `TypedUuid` everywhere id: VnicUuid::from_untyped_uuid(nic.id), diff --git a/live-tests/tests/test_nexus_handoff.rs b/live-tests/tests/test_nexus_handoff.rs index 85ef44e8aea..745d588976e 100644 --- a/live-tests/tests/test_nexus_handoff.rs +++ b/live-tests/tests/test_nexus_handoff.rs @@ -565,9 +565,11 @@ async fn check_external_dns( // what's in-service in the blueprint. let expected_nexus_addrs = blueprint .in_service_nexus_zones() - .filter_map(|(_sled_id, _zone_cfg, nexus_config)| { - (nexus_config.nexus_generation == active_generation) - .then_some(nexus_config.external_ip.ip) + .filter(|(_sled_id, _zone_cfg, nexus_config)| { + nexus_config.nexus_generation == active_generation + }) + .flat_map(|(_sled_id, _zone_cfg, nexus_config)| { + nexus_config.external_ips.iter().map(|e| e.ip) }) .collect::>(); diff --git a/nexus/db-model/src/deployment.rs b/nexus/db-model/src/deployment.rs index 19e06626b2f..a392d1142d4 100644 --- a/nexus/db-model/src/deployment.rs +++ b/nexus/db-model/src/deployment.rs @@ -60,8 +60,9 @@ use nexus_types::deployment::{ use nexus_types::deployment::{BlueprintPhysicalDiskConfig, BlueprintSource}; use nexus_types::deployment::{BlueprintZoneImageSource, blueprint_zone_type}; use nexus_types::deployment::{ - OmicronZoneExternalFloatingAddr, OmicronZoneExternalFloatingIp, - OmicronZoneExternalSnatIp, + OmicronZoneExternalFloatingAddr, OmicronZoneExternalFloatingAddrs, + OmicronZoneExternalFloatingIp, OmicronZoneExternalFloatingIps, + OmicronZoneExternalSnat, OmicronZoneExternalSnatIp, }; use omicron_common::address::Ipv6Subnet; use omicron_common::address::SLED_PREFIX_LENGTH; @@ -855,7 +856,7 @@ impl BpOmicronZone { http_address, // The external DNS address is stored in the // `bp_omicron_zone_external_ip` table, not here. - dns_address: _, + dns_addresses: _, nic, }, ) => { @@ -901,7 +902,7 @@ impl BpOmicronZone { lockstep_port, // The external IP is stored in the // `bp_omicron_zone_external_ip` table, not here. - external_ip: _, + external_ips: _, nic, external_tls, external_dns_servers, @@ -978,18 +979,7 @@ impl BpOmicronZone { nic_row.map(Into::into), )?; - // The external IP(s) for this zone live in the - // `bp_omicron_zone_external_ip` table. Until `BlueprintZoneType` can - // handle multiple IPs (#9288), we need zero or exactly one row here, - // for the zone types that have external networking. - // - // NOTE: This returns an error if `external_ip_rows` is empty. That's - // fine if the zone doesn't need external networking, so we only - // ?-propagate this inside the zone-specific code below. - let external_ip = BpOmicronZoneExternalIp::into_single( - external_ip_rows, - self.id.into(), - ); + let zone_id = self.id.into(); // NOTE: this is the *internal* DNS underlay address, held in // `second_service_ip` / `second_service_port`. External DNS's external @@ -1011,8 +1001,11 @@ impl BpOmicronZone { let zone_type = match self.zone_type { ZoneType::BoundaryNtp => { - let external_ip = external_ip?; - let snat_cfg = external_ip.to_snat_config()?; + let external_ip = + BpOmicronZoneExternalIp::into_boundary_ntp_snat( + external_ip_rows, + zone_id, + )?; BlueprintZoneType::BoundaryNtp( blueprint_zone_type::BoundaryNtp { address: primary_address, @@ -1020,10 +1013,7 @@ impl BpOmicronZone { dns_servers: ntp_dns_servers?, domain: self.ntp_domain, nic: nic?, - external_ip: OmicronZoneExternalSnatIp { - id: external_ip.external_ip_id.into(), - snat_cfg, - }, + external_ip, }, ) } @@ -1064,15 +1054,16 @@ impl BpOmicronZone { }, ), ZoneType::ExternalDns => { - let external_ip = external_ip?; + let dns_addresses = + BpOmicronZoneExternalIp::into_external_dns_addrs( + external_ip_rows, + zone_id, + )?; BlueprintZoneType::ExternalDns( blueprint_zone_type::ExternalDns { dataset: dataset?, http_address: primary_address, - dns_address: OmicronZoneExternalFloatingAddr { - id: external_ip.external_ip_id.into(), - addr: external_ip.to_floating_addr()?, - }, + dns_addresses, nic: nic?, }, ) @@ -1099,16 +1090,17 @@ impl BpOmicronZone { blueprint_zone_type::InternalNtp { address: primary_address }, ), ZoneType::Nexus => { - let external_ip = external_ip?; + let external_ips = + BpOmicronZoneExternalIp::into_nexus_external_ips( + external_ip_rows, + zone_id, + )?; BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { internal_address: primary_address, lockstep_port: *self.nexus_lockstep_port.ok_or_else( || anyhow!("expected 'nexus_lockstep_port'"), )?, - external_ip: OmicronZoneExternalFloatingIp { - id: external_ip.external_ip_id.into(), - ip: external_ip.ip.ip(), - }, + external_ips, nic: nic?, external_tls: self .nexus_external_tls @@ -1186,10 +1178,9 @@ pub struct BpOmicronZoneExternalIp { impl BpOmicronZoneExternalIp { /// Build the external IP child rows for a blueprint zone. /// - /// Returns one row per external IP. Today the in-memory `BlueprintZoneType` - /// only ever has at most one external IP per zone, so this returns at most - /// one row. In general though, the `bp_omicron_zone_external_ip` table can - /// store any number of rows per zone, so we're returning an array. + /// Returns one row per external IP: Nexus and external DNS may each have + /// several, and boundary NTP may have a source-NAT address per IP family + /// (with at least one address). pub fn for_zone( blueprint_id: BlueprintUuid, blueprint_zone: &BlueprintZoneConfig, @@ -1198,43 +1189,52 @@ impl BpOmicronZoneExternalIp { let zone_id = blueprint_zone.id.into(); match &blueprint_zone.zone_type { BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { - external_ip, + external_ips, .. - }) => vec![Self { - blueprint_id, - zone_id, - external_ip_id: external_ip.id.into(), - ip: IpNetwork::from(external_ip.ip), - port: None, - snat_first_port: None, - snat_last_port: None, - }], - BlueprintZoneType::ExternalDns( - blueprint_zone_type::ExternalDns { dns_address, .. }, - ) => vec![Self { - blueprint_id, - zone_id, - external_ip_id: dns_address.id.into(), - ip: IpNetwork::from(dns_address.addr.ip()), - port: Some(SqlU16::from(dns_address.addr.port())), - snat_first_port: None, - snat_last_port: None, - }], - BlueprintZoneType::BoundaryNtp( - blueprint_zone_type::BoundaryNtp { external_ip, .. }, - ) => { - let (first_port, last_port) = - external_ip.snat_cfg.port_range_raw(); - vec![Self { + }) => external_ips + .iter() + .map(|external_ip| Self { blueprint_id, zone_id, external_ip_id: external_ip.id.into(), - ip: IpNetwork::from(external_ip.snat_cfg.ip), + ip: IpNetwork::from(external_ip.ip), port: None, - snat_first_port: Some(SqlU16::from(first_port)), - snat_last_port: Some(SqlU16::from(last_port)), - }] - } + snat_first_port: None, + snat_last_port: None, + }) + .collect(), + BlueprintZoneType::ExternalDns( + blueprint_zone_type::ExternalDns { dns_addresses, .. }, + ) => dns_addresses + .iter() + .map(|dns_address| Self { + blueprint_id, + zone_id, + external_ip_id: dns_address.id.into(), + ip: IpNetwork::from(dns_address.addr.ip()), + port: Some(SqlU16::from(dns_address.addr.port())), + snat_first_port: None, + snat_last_port: None, + }) + .collect(), + BlueprintZoneType::BoundaryNtp( + blueprint_zone_type::BoundaryNtp { external_ip, .. }, + ) => external_ip + .iter() + .map(|snat| { + let (first_port, last_port) = + snat.snat_cfg.port_range_raw(); + Self { + blueprint_id, + zone_id, + external_ip_id: snat.id.into(), + ip: IpNetwork::from(snat.snat_cfg.ip), + port: None, + snat_first_port: Some(SqlU16::from(first_port)), + snat_last_port: Some(SqlU16::from(last_port)), + } + }) + .collect(), BlueprintZoneType::Clickhouse(_) | BlueprintZoneType::ClickhouseKeeper(_) | BlueprintZoneType::ClickhouseServer(_) @@ -1247,28 +1247,72 @@ impl BpOmicronZoneExternalIp { } } - /// Collapse the external IP rows for a zone into exactly one. - /// - /// NOTE: This is a temporary method until the `BlueprintZoneType` variants - /// with external addresses can handle more than one EIP. Until then, these - /// zones must have exactly one external IP. This returns that single - /// address, or an error if there is any other number of rows. - fn into_single( - mut rows: Vec, + /// Reconstruct a Nexus zone's external IPs from its child rows. + fn into_nexus_external_ips( + rows: Vec, zone_id: OmicronZoneUuid, - ) -> anyhow::Result { - match rows.len() { - 1 => Ok(rows.pop().expect("length checked to be 1")), - 0 => bail!( - "zone {zone_id} has no external IP, \ - but its type requires one" - ), - n => bail!( - "zone {zone_id} has {n} external IPs, but only one is \ - supported until the in-memory blueprint type is widened \ - (#9288)" - ), - } + ) -> anyhow::Result { + let ips = + iddqd::IdOrdMap::from_iter_unique(rows.into_iter().map(|row| { + OmicronZoneExternalFloatingIp { + id: row.external_ip_id.into(), + ip: row.ip.ip(), + } + })) + .map_err(|dup| { + anyhow!( + "zone {zone_id} has a duplicate external IP: {}", + dup.new_item().ip + ) + })?; + OmicronZoneExternalFloatingIps::new(ips).with_context(|| { + format!("zone {zone_id} has invalid Nexus external IPs") + }) + } + + /// Reconstruct an external DNS zone's addresses from its child rows. + fn into_external_dns_addrs( + rows: Vec, + zone_id: OmicronZoneUuid, + ) -> anyhow::Result { + let addrs = rows + .into_iter() + .map(|row| { + Ok(OmicronZoneExternalFloatingAddr { + id: row.external_ip_id.into(), + addr: row.to_floating_addr()?, + }) + }) + .collect::>>()?; + let addrs = + iddqd::IdOrdMap::from_iter_unique(addrs).map_err(|dup| { + anyhow!( + "zone {zone_id} has a duplicate external DNS IP: {}", + dup.new_item().addr.ip() + ) + })?; + OmicronZoneExternalFloatingAddrs::new(addrs).with_context(|| { + format!("zone {zone_id} has invalid external DNS addresses") + }) + } + + /// Reconstruct a boundary NTP zone's SNAT configuration from its child rows. + fn into_boundary_ntp_snat( + rows: Vec, + zone_id: OmicronZoneUuid, + ) -> anyhow::Result { + let snat_ips = rows + .into_iter() + .map(|row| { + Ok(OmicronZoneExternalSnatIp { + id: row.external_ip_id.into(), + snat_cfg: row.to_snat_config()?, + }) + }) + .collect::>>()?; + OmicronZoneExternalSnat::from_ips(snat_ips).with_context(|| { + format!("zone {zone_id} has invalid boundary NTP SNAT config") + }) } /// Interpret this row as a boundary NTP source-NAT configuration. diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 2ceb0407477..53eb931b239 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -3518,6 +3518,7 @@ mod tests { use nexus_types::deployment::BlueprintZoneType; use nexus_types::deployment::ExpectedActiveRotSlot; use nexus_types::deployment::OmicronZoneExternalFloatingIp; + use nexus_types::deployment::OmicronZoneExternalFloatingIps; use nexus_types::deployment::PendingMgsUpdate; use nexus_types::deployment::PlanningInput; use nexus_types::deployment::ReconfiguratorDisruptionPolicy; @@ -3720,72 +3721,70 @@ mod tests { logctx.cleanup_successful(); } - // A zone with more than one external IP is not yet representable in the - // in-memory `BlueprintZoneType` (#9288). But the tables for its EIPs can - // store multiple rows per zone, so we have to check that we fail when - // reading a blueprint with more than one row. We'll adjust this to ensure - // we _can_ read such a blueprint when the zone-type enum is expanded. #[tokio::test] - async fn test_blueprint_zone_requires_exactly_one_external_ip() { - let logctx = dev::test_setup_log( - "test_blueprint_zone_requires_exactly_one_external_ip", - ); + async fn test_blueprint_zone_multiple_external_ips_round_trip() { + const TEST_NAME: &str = + "test_blueprint_zone_multiple_external_ips_round_trip"; + let logctx = dev::test_setup_log(TEST_NAME); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); - let (_collection, _planning_input, blueprint) = representative( - &logctx.log, - "test_blueprint_zone_requires_exactly_one_external_ip", - ); + let (_collection, _planning_input, mut blueprint) = + representative(&logctx.log, TEST_NAME); let authz_blueprint = authz_blueprint_from_id(blueprint.id); - // Find a Nexus zone, and ensure we can write / read it with one IP. - let nexus_zone_id = blueprint - .sleds - .values() - .flat_map(|sled| sled.zones.iter()) - .find(|zone| zone.zone_type.is_nexus()) - .expect("representative blueprint has a Nexus zone") - .id; + // Give a Nexus zone a second external IP by hand. + let second_ip = "192.0.2.222".parse::().unwrap(); + let second_id = ExternalIpUuid::new_v4(); + let mut nexus_zone_id = None; + 'outer: for sled in blueprint.sleds.values_mut() { + for mut zone in sled.zones.iter_mut() { + if let BlueprintZoneType::Nexus(nexus) = &mut zone.zone_type { + let mut ips: Vec<_> = + nexus.external_ips.iter().copied().collect(); + ips.push(OmicronZoneExternalFloatingIp { + id: second_id, + ip: second_ip, + }); + let ips = iddqd::IdOrdMap::from_iter_unique(ips) + .expect("external IPs are distinct"); + nexus.external_ips = + OmicronZoneExternalFloatingIps::new(ips) + .expect("two external IPs is valid"); + nexus_zone_id = Some(zone.id); + break 'outer; + } + } + } + let nexus_zone_id = + nexus_zone_id.expect("representative blueprint has a Nexus zone"); datastore .blueprint_insert(&opctx, &blueprint) .await .expect("failed to insert blueprint"); - - // With exactly one external IP per zone, the blueprint round-trips. let blueprint_read = datastore .blueprint_read(&opctx, &authz_blueprint) .await - .expect("failed to read blueprint back"); + .expect("blueprint with multiple external IPs reads back"); assert_eq!(blueprint, blueprint_read); - // Inject a second external IP row for the Nexus zone. We should fail - // when reading this, because we can't represent it in the zone-type - // enum. - const SECOND_EXTERNAL_IP_ID: &str = - "b3f7d6c2-9a1e-4c8b-8d2f-0a1b2c3d4e5f"; - let conn = datastore.pool_connection_for_tests().await.unwrap(); - let sql = format!( - "INSERT INTO omicron.public.bp_omicron_zone_external_ip \ - (blueprint_id, zone_id, external_ip_id, ip, port, \ - snat_first_port, snat_last_port) \ - VALUES ('{}', '{}', '{SECOND_EXTERNAL_IP_ID}', \ - '192.0.2.222', NULL, NULL, NULL)", - blueprint.id, nexus_zone_id, - ); - conn.batch_execute_async(&sql) - .await - .expect("injected a second external IP row"); - let err = datastore - .blueprint_read(&opctx, &authz_blueprint) - .await - .expect_err("a zone with two external IPs must fail to read back"); - let msg = InlineErrorChain::new(&err).to_string(); + // Re-read the Nexus zone and check it still has both external IPs. + let zone = blueprint_read + .sleds + .values() + .flat_map(|sled| sled.zones.iter()) + .find(|zone| zone.id == nexus_zone_id) + .expect("the modified Nexus zone"); + let BlueprintZoneType::Nexus(nexus) = &zone.zone_type else { + panic!("expected a Nexus zone"); + }; + let ips: Vec<_> = nexus.external_ips.iter().map(|e| e.ip).collect(); assert!( - msg.contains("external IPs"), - "expected an exactly-one-external-IP error, got: {msg}", + ips.contains(&second_ip), + "second external IP survived the round trip: {ips:?}", ); + assert_eq!(ips.len(), 2, "expected two external IPs, got {ips:?}"); db.terminate().await; logctx.cleanup_successful(); @@ -5551,10 +5550,9 @@ mod tests { let nexus_ip = blueprint1 .in_service_zones() .find_map(|(_, zone_config)| { - zone_config - .zone_type - .external_networking() - .map(|(ip, _nic)| ip.ip()) + zone_config.zone_type.external_networking().and_then( + |(ips, _nic)| ips.into_iter().next().map(|ip| ip.ip()), + ) }) .expect("found external IP"); let service_pool = create_service_ip_pool( @@ -6241,10 +6239,12 @@ mod tests { zone_type: BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { internal_address: "[::1]:12345".parse().unwrap(), lockstep_port: 12346, - external_ip: OmicronZoneExternalFloatingIp { - id: ExternalIpUuid::new_v4(), - ip: "192.0.2.1".parse().unwrap(), - }, + external_ips: OmicronZoneExternalFloatingIps::from_single( + OmicronZoneExternalFloatingIp { + id: ExternalIpUuid::new_v4(), + ip: "192.0.2.1".parse().unwrap(), + }, + ), nic: nic.clone(), external_tls: false, external_dns_servers: Vec::new(), diff --git a/nexus/db-queries/src/db/datastore/deployment/external_networking.rs b/nexus/db-queries/src/db/datastore/deployment/external_networking.rs index 4a068347204..6a7a4aebcf9 100644 --- a/nexus/db-queries/src/db/datastore/deployment/external_networking.rs +++ b/nexus/db-queries/src/db/datastore/deployment/external_networking.rs @@ -200,45 +200,48 @@ impl DataStore { zones_to_allocate: impl Iterator, ) -> Result<(), TransactionError> { for z in zones_to_allocate { - let Some((external_ip, nic)) = z.zone_type.external_networking() + let Some((external_ips, nic)) = z.zone_type.external_networking() else { continue; }; + let kind = z.zone_type.kind(); let log = opctx.log.new(slog::o!( "action" => "allocate-external-networking", - "zone_kind" => z.zone_type.kind().report_str(), + "zone_kind" => kind.report_str(), "zone_id" => z.id.to_string(), - "ip" => format!("{external_ip:?}"), "nic" => format!("{nic:?}"), )); - // Look up the system-service pool containing this address, if any. - let (_authz_pool, db_pool) = self - .ip_pool_fetch_containing_address_for_services_on_connection( - opctx, - conn, - external_ip.ip(), - ) - .await - .map_err(|e| { - Self::map_external_ip_not_found_for_zone_error( - e, + // Ensure each external IP of the zone. + for external_ip in external_ips { + // Look up the system-service pool containing this address, if + // any. + let (_authz_pool, db_pool) = self + .ip_pool_fetch_containing_address_for_services_on_connection( + opctx, + conn, external_ip.ip(), ) - })?; + .await + .map_err(|e| { + Self::map_external_ip_not_found_for_zone_error( + e, + external_ip.ip(), + ) + })?; - // Actually ensure the IP address. - let kind = z.zone_type.kind(); - self.ensure_external_service_ip( - conn, - &db_pool, - kind, - z.id, - external_ip, - &log, - ) - .await?; + // Actually ensure the IP address. + self.ensure_external_service_ip( + conn, + &db_pool, + kind, + z.id, + external_ip, + &log, + ) + .await?; + } self.ensure_service_nic(conn, kind, z.id, nic, &log).await?; } @@ -252,7 +255,7 @@ impl DataStore { zones_to_deallocate: impl Iterator, ) -> Result<(), TransactionError> { for z in zones_to_deallocate { - let Some((external_ip, nic)) = z.zone_type.external_networking() + let Some((external_ips, nic)) = z.zone_type.external_networking() else { continue; }; @@ -262,29 +265,39 @@ impl DataStore { "action" => "deallocate-external-networking", "zone_kind" => kind.report_str(), "zone_id" => z.id.to_string(), - "ip" => format!("{external_ip:?}"), "nic" => format!("{nic:?}"), )); - let deleted_ip = self - .deallocate_external_ip_on_connection( - conn, - external_ip.id().into_untyped_uuid(), - ) - .await?; - match deleted_ip { - SoftDeleteResult::SoftDeleteApplied => { - info!(log, "successfully deleted Omicron zone external IP"); - } - SoftDeleteResult::AlreadySoftDeleted => { - debug!(log, "Omicron zone external IP already deleted"); - } - SoftDeleteResult::NotFound => { - debug!( - log, - "Skipped soft-deletion of Omicron zone external IP \ - (external IP does not exist)" - ); + for external_ip in external_ips { + let deleted_ip = self + .deallocate_external_ip_on_connection( + conn, + external_ip.id().into_untyped_uuid(), + ) + .await?; + match deleted_ip { + SoftDeleteResult::SoftDeleteApplied => { + info!( + log, + "successfully deleted Omicron zone external IP"; + "ip" => ?external_ip, + ); + } + SoftDeleteResult::AlreadySoftDeleted => { + debug!( + log, + "Omicron zone external IP already deleted"; + "ip" => ?external_ip, + ); + } + SoftDeleteResult::NotFound => { + debug!( + log, + "Skipped soft-deletion of Omicron zone external \ + IP (external IP does not exist)"; + "ip" => ?external_ip, + ); + } } } @@ -502,6 +515,7 @@ impl DataStore { { return Ok(()); } + let eip = external_ip.ip(); self.external_ip_allocate_omicron_zone_on_connection( conn, pool, @@ -511,7 +525,7 @@ impl DataStore { ) .await?; - info!(log, "successfully allocated external IP"); + info!(log, "successfully allocated external IP"; "ip" => %eip); Ok(()) } @@ -665,7 +679,10 @@ mod tests { use nexus_types::deployment::BlueprintZoneImageSource; use nexus_types::deployment::BlueprintZoneType; use nexus_types::deployment::OmicronZoneExternalFloatingAddr; + use nexus_types::deployment::OmicronZoneExternalFloatingAddrs; use nexus_types::deployment::OmicronZoneExternalFloatingIp; + use nexus_types::deployment::OmicronZoneExternalFloatingIps; + use nexus_types::deployment::OmicronZoneExternalSnat; use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::blueprint_zone_type; use nexus_types::identity::Resource; @@ -891,7 +908,10 @@ mod tests { blueprint_zone_type::Nexus { internal_address: "[::1]:0".parse().unwrap(), lockstep_port: 0, - external_ip: self.nexus_external_ip, + external_ips: + OmicronZoneExternalFloatingIps::from_single( + self.nexus_external_ip, + ), nic: self.nexus_nic.clone(), external_tls: false, external_dns_servers: Vec::new(), @@ -914,7 +934,10 @@ mod tests { .expect("bad name"), }, http_address: "[::1]:0".parse().unwrap(), - dns_address: self.dns_external_addr, + dns_addresses: + OmicronZoneExternalFloatingAddrs::from_single( + self.dns_external_addr, + ), nic: self.dns_nic.clone(), }, ), @@ -933,7 +956,9 @@ mod tests { dns_servers: Vec::new(), domain: None, nic: self.ntp_nic.clone(), - external_ip: self.ntp_external_ip, + external_ip: OmicronZoneExternalSnat::from_single( + self.ntp_external_ip, + ), }, ), image_source: BlueprintZoneImageSource::InstallDataset, @@ -1251,10 +1276,14 @@ mod tests { (&|zones: &mut [BlueprintZoneConfig]| { for zone in zones { if let BlueprintZoneType::Nexus( - blueprint_zone_type::Nexus { external_ip, .. }, + blueprint_zone_type::Nexus { external_ips, .. }, ) = &mut zone.zone_type { - external_ip.ip = bogus_ip; + let mut ip = + *external_ips.iter().next().expect("has one IP"); + ip.ip = bogus_ip; + *external_ips = + OmicronZoneExternalFloatingIps::from_single(ip); return format!( "zone {} has a different IP allocated", zone.id @@ -1269,11 +1298,15 @@ mod tests { for zone in zones { if let BlueprintZoneType::ExternalDns( blueprint_zone_type::ExternalDns { - dns_address, .. + dns_addresses, .. }, ) = &mut zone.zone_type { - dns_address.addr.set_ip(bogus_ip); + let mut addr = + *dns_addresses.iter().next().expect("has one addr"); + addr.addr.set_ip(bogus_ip); + *dns_addresses = + OmicronZoneExternalFloatingAddrs::from_single(addr); return format!( "zone {} has a different IP allocated", zone.id @@ -1291,16 +1324,20 @@ mod tests { }, ) = &mut zone.zone_type { + let mut snat = + external_ip.iter().next().expect("has one SNAT IP"); let (mut first, mut last) = - external_ip.snat_cfg.port_range_raw(); + snat.snat_cfg.port_range_raw(); first += NUM_SOURCE_NAT_PORTS; last += NUM_SOURCE_NAT_PORTS; - external_ip.snat_cfg = SourceNatConfigGeneric::new( - external_ip.snat_cfg.ip, + snat.snat_cfg = SourceNatConfigGeneric::new( + snat.snat_cfg.ip, first, last, ) .unwrap(); + *external_ip = + OmicronZoneExternalSnat::from_single(snat); return format!( "zone {} has a different IP allocated", zone.id diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index d5a73a10b0f..ee2f0c195d7 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -554,10 +554,13 @@ impl DataStore { let service_ip_nic = match zone_type { BlueprintZoneType::ExternalDns( - blueprint_zone_type::ExternalDns { nic, dns_address, .. }, + blueprint_zone_type::ExternalDns { nic, dns_addresses, .. }, ) => { - let external_ip = - OmicronZoneExternalIp::Floating(dns_address.into_ip()); + let external_ips = dns_addresses + .iter() + .copied() + .map(|a| OmicronZoneExternalIp::Floating(a.into_ip())) + .collect::>(); let ip_config = extract_ip_config(nic); let db_nic = IncompleteNetworkInterface::new_service( nic.id, @@ -575,14 +578,18 @@ impl DataStore { nic.slot, ) .map_err(|e| RackInitError::AddingNic(e))?; - Some((external_ip, db_nic)) + Some((external_ips, db_nic)) } BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { nic, - external_ip, + external_ips, .. }) => { - let external_ip = OmicronZoneExternalIp::Floating(*external_ip); + let external_ips = external_ips + .iter() + .copied() + .map(OmicronZoneExternalIp::Floating) + .collect::>(); let ip_config = extract_ip_config(nic); let db_nic = IncompleteNetworkInterface::new_service( nic.id, @@ -600,12 +607,15 @@ impl DataStore { nic.slot, ) .map_err(|e| RackInitError::AddingNic(e))?; - Some((external_ip, db_nic)) + Some((external_ips, db_nic)) } BlueprintZoneType::BoundaryNtp( blueprint_zone_type::BoundaryNtp { external_ip, nic, .. }, ) => { - let external_ip = OmicronZoneExternalIp::Snat(*external_ip); + let external_ips = external_ip + .iter() + .map(OmicronZoneExternalIp::Snat) + .collect::>(); let ip_config = extract_ip_config(nic); let db_nic = IncompleteNetworkInterface::new_service( nic.id, @@ -623,7 +633,7 @@ impl DataStore { nic.slot, ) .map_err(|e| RackInitError::AddingNic(e))?; - Some((external_ip, db_nic)) + Some((external_ips, db_nic)) } BlueprintZoneType::InternalNtp(_) | BlueprintZoneType::Clickhouse(_) @@ -635,49 +645,51 @@ impl DataStore { | BlueprintZoneType::InternalDns(_) | BlueprintZoneType::Oximeter(_) => None, }; - let Some((external_ip, db_nic)) = service_ip_nic else { + let Some((external_ips, db_nic)) = service_ip_nic else { info!( log, "No networking records needed for {} service", zone_report_str, ); return Ok(()); }; - let (_authz_pool, db_pool) = self - .ip_pool_fetch_containing_address_for_services_on_connection( - opctx, + for external_ip in external_ips { + let (_authz_pool, db_pool) = self + .ip_pool_fetch_containing_address_for_services_on_connection( + opctx, + conn, + external_ip.ip(), + ) + .await + .map_err(|e| { + RackInitError::AddingIp(Error::internal_error(&format!( + "no system services pool for external IP '{}': {}", + external_ip.ip(), + e, + ))) + })?; + let db_ip = IncompleteExternalIp::for_omicron_zone( + db_pool.id(), + external_ip, + zone_config.id, + zone_config.zone_type.kind(), + ); + Self::allocate_external_ip_on_connection( conn, - external_ip.ip(), + db_ip, + LookupType::ById(db_pool.id()), ) .await - .map_err(|e| { - RackInitError::AddingIp(Error::internal_error(&format!( - "no system services pool for external IP '{}': {}", - external_ip.ip(), - e, - ))) + .map_err(|err| { + error!( + log, + "Initializing Rack: Failed to allocate \ + IP address for {}", + zone_report_str; + "err" => %err, + ); + RackInitError::AddingIp(err.into_public_ignore_retries()) })?; - let db_ip = IncompleteExternalIp::for_omicron_zone( - db_pool.id(), - external_ip, - zone_config.id, - zone_config.zone_type.kind(), - ); - Self::allocate_external_ip_on_connection( - conn, - db_ip, - LookupType::ById(db_pool.id()), - ) - .await - .map_err(|err| { - error!( - log, - "Initializing Rack: Failed to allocate \ - IP address for {}", - zone_report_str; - "err" => %err, - ); - RackInitError::AddingIp(err.into_public_ignore_retries()) - })?; + } self.create_network_interface_raw_conn(conn, db_nic) .await @@ -1832,11 +1844,11 @@ mod test { .ip .ip(), if let BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { - external_ip, + external_ips, .. }) = &blueprint.in_service_zones().next().unwrap().1.zone_type { - external_ip.ip + external_ips.iter().next().unwrap().ip } else { panic!("Unexpected zone type") } @@ -1846,11 +1858,11 @@ mod test { .ip .ip(), if let BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { - external_ip, + external_ips, .. }) = &blueprint.in_service_zones().nth(1).unwrap().1.zone_type { - external_ip.ip + external_ips.iter().next().unwrap().ip } else { panic!("Unexpected service kind") } @@ -2162,11 +2174,11 @@ mod test { assert_eq!( actual_ip, if let BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { - external_ip, + external_ips, .. }) = &blueprint.in_service_zones().next().unwrap().1.zone_type { - external_ip.ip + external_ips.iter().next().unwrap().ip } else { panic!("Unexpected zone type") } diff --git a/nexus/reconfigurator/blippy/src/checks.rs b/nexus/reconfigurator/blippy/src/checks.rs index 4b767758bd0..4b920c258a0 100644 --- a/nexus/reconfigurator/blippy/src/checks.rs +++ b/nexus/reconfigurator/blippy/src/checks.rs @@ -162,36 +162,40 @@ fn check_external_networking(blippy: &mut Blippy<'_>) { let mut used_nic_ips = BTreeMap::new(); let mut used_nic_macs = BTreeMap::new(); - for (sled_id, zone, external_ip, nic) in + for (sled_id, zone, external_ips, nic) in blippy.blueprint().in_service_zones().filter_map(|(sled_id, zone)| { zone.zone_type .external_networking() - .map(|(external_ip, nic)| (sled_id, zone, external_ip, nic)) + .map(|(external_ips, nic)| (sled_id, zone, external_ips, nic)) }) { - // There should be no duplicate external IPs. - if let Some(prev_zone) = used_external_ips.insert(external_ip, zone) { - blippy.push_sled_note( - sled_id, - Severity::Fatal, - SledKind::DuplicateExternalIp { - zone1: prev_zone.clone(), - zone2: zone.clone(), - ip: external_ip.ip(), - }, - ); - } - - // See the loop below; we build up separate maps to check for - // Floating/SNAT overlap that wouldn't be caught by the exact - // `used_external_ips` map above. - match external_ip { - OmicronZoneExternalIp::Floating(floating) => { - used_external_floating_ips.insert(floating.ip, zone); + // A zone may have more than one external IP; check each of them. + for external_ip in external_ips { + // There should be no duplicate external IPs. + if let Some(prev_zone) = used_external_ips.insert(external_ip, zone) + { + blippy.push_sled_note( + sled_id, + Severity::Fatal, + SledKind::DuplicateExternalIp { + zone1: prev_zone.clone(), + zone2: zone.clone(), + ip: external_ip.ip(), + }, + ); } - OmicronZoneExternalIp::Snat(snat) => { - used_external_snat_ips - .insert(snat.snat_cfg.ip, (sled_id, zone)); + + // See the loop below; we build up separate maps to check for + // Floating/SNAT overlap that wouldn't be caught by the exact + // `used_external_ips` map above. + match external_ip { + OmicronZoneExternalIp::Floating(floating) => { + used_external_floating_ips.insert(floating.ip, zone); + } + OmicronZoneExternalIp::Snat(snat) => { + used_external_snat_ips + .insert(snat.snat_cfg.ip, (sled_id, zone)); + } } } @@ -776,6 +780,7 @@ mod tests { use nexus_reconfigurator_planning::example::example; use nexus_types::deployment::BlueprintArtifactVersion; use nexus_types::deployment::BlueprintZoneType; + use nexus_types::deployment::OmicronZoneExternalFloatingIps; use nexus_types::deployment::blueprint_zone_type; use omicron_test_utils::dev::test_setup_log; use omicron_uuid_kinds::MupdateOverrideUuid; @@ -1089,6 +1094,9 @@ mod tests { .external_networking() .expect("Nexus has external networking") .0 + .into_iter() + .next() + .expect("Nexus has an external IP") { OmicronZoneExternalIp::Floating(ip) => ip, OmicronZoneExternalIp::Snat(_) => { @@ -1097,10 +1105,11 @@ mod tests { }; match &mut nexus1.zone_type { BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { - external_ip, + external_ips, .. }) => { - *external_ip = dup_ip; + *external_ips = + OmicronZoneExternalFloatingIps::from_single(dup_ip); } _ => unreachable!("this is a Nexus zone"), }; @@ -2267,10 +2276,12 @@ fn check_planning_input_network_records_appear_in_blueprint( _ => (), } - if let Some((external_ip, nic)) = zone_type.external_networking() { - // Ignore localhost (used by the test suite). - if !external_ip.ip().is_loopback() { - all_external_ips.insert(external_ip); + if let Some((external_ips, nic)) = zone_type.external_networking() { + for external_ip in external_ips { + // Ignore localhost (used by the test suite). + if !external_ip.ip().is_loopback() { + all_external_ips.insert(external_ip); + } } all_macs.insert(nic.mac); } @@ -2359,9 +2370,15 @@ fn check_external_networking_generation( )> { blueprint .in_service_zones() - .filter_map(|(sled_id, zone_config)| { - let (ip, nic) = zone_config.zone_type.external_networking()?; - Some((sled_id, zone_config.id, ip, nic)) + .flat_map(|(sled_id, zone_config)| { + zone_config + .zone_type + .external_networking() + .into_iter() + .flat_map(move |(ips, nic)| { + ips.into_iter() + .map(move |ip| (sled_id, zone_config.id, ip, nic)) + }) }) .collect() } diff --git a/nexus/reconfigurator/execution/src/database.rs b/nexus/reconfigurator/execution/src/database.rs index 05acb331176..2f7935306b4 100644 --- a/nexus/reconfigurator/execution/src/database.rs +++ b/nexus/reconfigurator/execution/src/database.rs @@ -82,6 +82,7 @@ mod test { use nexus_types::deployment::CockroachDbPreserveDowngrade; use nexus_types::deployment::LastAllocatedSubnetIpOffset; use nexus_types::deployment::OmicronZoneExternalFloatingIp; + use nexus_types::deployment::OmicronZoneExternalFloatingIps; use nexus_types::deployment::OximeterReadMode; use nexus_types::deployment::PendingMgsUpdates; use nexus_types::deployment::blueprint_zone_type; @@ -139,10 +140,13 @@ mod test { internal_address: "[::1]:0".parse().unwrap(), lockstep_port: 0, external_dns_servers: Vec::new(), - external_ip: OmicronZoneExternalFloatingIp { - id: ExternalIpUuid::new_v4(), - ip: IpAddr::V6(Ipv6Addr::LOCALHOST), - }, + external_ips: + OmicronZoneExternalFloatingIps::from_single( + OmicronZoneExternalFloatingIp { + id: ExternalIpUuid::new_v4(), + ip: IpAddr::V6(Ipv6Addr::LOCALHOST), + }, + ), external_tls: true, nic: NetworkInterface { id: uuid::Uuid::new_v4(), diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index e72cfbfd86c..6c6e93fdfab 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -332,7 +332,10 @@ mod test { use nexus_types::deployment::ExternalIpPolicy; use nexus_types::deployment::LastAllocatedSubnetIpOffset; pub use nexus_types::deployment::OmicronZoneExternalFloatingAddr; + pub use nexus_types::deployment::OmicronZoneExternalFloatingAddrs; pub use nexus_types::deployment::OmicronZoneExternalFloatingIp; + pub use nexus_types::deployment::OmicronZoneExternalFloatingIps; + pub use nexus_types::deployment::OmicronZoneExternalSnat; pub use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::OximeterReadMode; use nexus_types::deployment::PendingMgsUpdates; @@ -489,10 +492,12 @@ mod test { dns_servers, domain, nic, - external_ip: OmicronZoneExternalSnatIp { - id: external_ip_id, - snat_cfg, - }, + external_ip: OmicronZoneExternalSnat::from_single( + OmicronZoneExternalSnatIp { + id: external_ip_id, + snat_cfg, + }, + ), }, ) } @@ -544,10 +549,13 @@ mod test { blueprint_zone_type::ExternalDns { dataset, http_address, - dns_address: OmicronZoneExternalFloatingAddr { - id: external_ip_id, - addr, - }, + dns_addresses: + OmicronZoneExternalFloatingAddrs::from_single( + OmicronZoneExternalFloatingAddr { + id: external_ip_id, + addr, + }, + ), nic, }, ) @@ -589,10 +597,12 @@ mod test { BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { internal_address, lockstep_port, - external_ip: OmicronZoneExternalFloatingIp { - id: external_ip_id, - ip, - }, + external_ips: OmicronZoneExternalFloatingIps::from_single( + OmicronZoneExternalFloatingIp { + id: external_ip_id, + ip, + }, + ), nic, external_tls, external_dns_servers, diff --git a/nexus/reconfigurator/execution/src/sagas.rs b/nexus/reconfigurator/execution/src/sagas.rs index 3064960045d..7d52af7ca56 100644 --- a/nexus/reconfigurator/execution/src/sagas.rs +++ b/nexus/reconfigurator/execution/src/sagas.rs @@ -222,8 +222,8 @@ mod test { BlueprintZoneConfig, BlueprintZoneDisposition, BlueprintZoneImageSource, BlueprintZoneType, CockroachDbPreserveDowngrade, LastAllocatedSubnetIpOffset, - OmicronZoneExternalFloatingIp, OximeterReadMode, PendingMgsUpdates, - blueprint_zone_type, + OmicronZoneExternalFloatingIp, OmicronZoneExternalFloatingIps, + OximeterReadMode, PendingMgsUpdates, blueprint_zone_type, }; use nexus_types::external_api::sled::SledState; use omicron_common::address::Ipv6Subnet; @@ -281,10 +281,13 @@ mod test { internal_address: "[::1]:0".parse().unwrap(), lockstep_port: 0, external_dns_servers: Vec::new(), - external_ip: OmicronZoneExternalFloatingIp { - id: ExternalIpUuid::new_v4(), - ip: IpAddr::V6(Ipv6Addr::LOCALHOST), - }, + external_ips: + OmicronZoneExternalFloatingIps::from_single( + OmicronZoneExternalFloatingIp { + id: ExternalIpUuid::new_v4(), + ip: IpAddr::V6(Ipv6Addr::LOCALHOST), + }, + ), external_tls: true, nic: NetworkInterface { id: uuid::Uuid::new_v4(), diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs index 889843cac88..52c7be1e3af 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs @@ -43,8 +43,11 @@ use nexus_types::deployment::ClickhouseClusterConfig; use nexus_types::deployment::CockroachDbPreserveDowngrade; use nexus_types::deployment::DiskFilter; use nexus_types::deployment::OmicronZoneExternalFloatingAddr; +use nexus_types::deployment::OmicronZoneExternalFloatingAddrs; use nexus_types::deployment::OmicronZoneExternalFloatingIp; +use nexus_types::deployment::OmicronZoneExternalFloatingIps; use nexus_types::deployment::OmicronZoneExternalIp; +use nexus_types::deployment::OmicronZoneExternalSnat; use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::OperatorNexusConfig; use nexus_types::deployment::OximeterReadMode; @@ -1587,7 +1590,9 @@ impl<'a> BlueprintBuilder<'a> { BlueprintZoneType::ExternalDns(blueprint_zone_type::ExternalDns { dataset: OmicronZoneDataset { pool_name }, http_address, - dns_address, + dns_addresses: OmicronZoneExternalFloatingAddrs::from_single( + dns_address, + ), nic, }); @@ -1740,7 +1745,9 @@ impl<'a> BlueprintBuilder<'a> { let zone_type = BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { internal_address, lockstep_port: omicron_common::address::NEXUS_LOCKSTEP_PORT, - external_ip, + external_ips: OmicronZoneExternalFloatingIps::from_single( + external_ip, + ), nic, external_tls: config.external_tls, external_dns_servers: config.external_dns_servers.to_vec(), @@ -1965,10 +1972,11 @@ impl<'a> BlueprintBuilder<'a> { let new_zone_id = self.rng.sled_rng(sled_id).next_zone(); let ExternalSnatNetworkingChoice { snat_cfg, nic_ip_config, nic_mac } = external_ip; - let external_ip = OmicronZoneExternalSnatIp { - id: self.rng.sled_rng(sled_id).next_external_ip(), - snat_cfg, - }; + let external_ip = + OmicronZoneExternalSnat::from_single(OmicronZoneExternalSnatIp { + id: self.rng.sled_rng(sled_id).next_external_ip(), + snat_cfg, + }); let nic = NetworkInterface { id: self.rng.sled_rng(sled_id).next_network_interface(), kind: NetworkInterfaceKind::Service { @@ -2760,9 +2768,15 @@ fn is_external_networking_config_different( )> { blueprint .in_service_zones() - .filter_map(|(sled_id, zone_config)| { - let (ip, nic) = zone_config.zone_type.external_networking()?; - Some((sled_id, zone_config.id, ip, nic)) + .flat_map(|(sled_id, zone_config)| { + zone_config + .zone_type + .external_networking() + .into_iter() + .flat_map(move |(ips, nic)| { + ips.into_iter() + .map(move |ip| (sled_id, zone_config.id, ip, nic)) + }) }) .collect() } @@ -3423,9 +3437,15 @@ pub mod test { let mut new_network_resources = OmicronZoneNetworkResources::new(); let old_network_resources = builder.network_resources_mut(); + let removed_id = removed_nexus + .external_ips + .iter() + .next() + .expect("Nexus has an external IP") + .id; for ip in old_network_resources.omicron_zone_external_ips() { - if ip.ip.id() != removed_nexus.external_ip.id { + if ip.ip.id() != removed_id { new_network_resources .add_external_ip(ip.zone_id, ip.ip) .expect("copied IP to new input"); @@ -3525,10 +3545,12 @@ pub mod test { // Nexus with no remaining external IPs should fail. let mut used_ip_ranges = Vec::new(); for (_, z) in parent.in_service_zones() { - if let Some((external_ip, _)) = + if let Some((external_ips, _)) = z.zone_type.external_networking() { - used_ip_ranges.push(IpRange::from(external_ip.ip())); + for external_ip in external_ips { + used_ip_ranges.push(IpRange::from(external_ip.ip())); + } } } assert!(!used_ip_ranges.is_empty()); diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/allocators/external_networking.rs b/nexus/reconfigurator/planning/src/blueprint_editor/allocators/external_networking.rs index 3f4e67f1618..c190a779db3 100644 --- a/nexus/reconfigurator/planning/src/blueprint_editor/allocators/external_networking.rs +++ b/nexus/reconfigurator/planning/src/blueprint_editor/allocators/external_networking.rs @@ -179,12 +179,14 @@ impl ExternalNetworkingAllocator { } } BlueprintZoneType::ExternalDns(dns) => { - if !used_external_dns_ips.insert(dns.dns_address.addr.ip()) - { - bail!( - "duplicate external DNS external IP: {}", - dns.dns_address.addr - ); + for dns_address in dns.dns_addresses.iter() { + if !used_external_dns_ips.insert(dns_address.addr.ip()) + { + bail!( + "duplicate external DNS external IP: {}", + dns_address.addr + ); + } } if let Some(ip) = dns.nic.ip_config.ipv4_addr() { if !existing_external_dns_v4_ips.insert(*ip) { @@ -200,12 +202,14 @@ impl ExternalNetworkingAllocator { _ => (), } - if let Some((external_ip, nic)) = zone_type.external_networking() { - // For the test suite, ignore localhost. It gets reused many - // times and that's okay. We don't expect to see localhost - // outside the test suite. - if !external_ip.ip().is_loopback() { - external_ip_alloc.mark_ip_used(&external_ip)?; + if let Some((external_ips, nic)) = zone_type.external_networking() { + for external_ip in external_ips { + // For the test suite, ignore localhost. It gets reused + // many times and that's okay. We don't expect to see + // localhost outside the test suite. + if !external_ip.ip().is_loopback() { + external_ip_alloc.mark_ip_used(&external_ip)?; + } } if !used_macs.insert(nic.mac) { @@ -683,6 +687,7 @@ pub mod test { use nexus_types::deployment::BlueprintZoneDisposition; use nexus_types::deployment::BlueprintZoneImageSource; use nexus_types::deployment::OmicronZoneExternalFloatingAddr; + use nexus_types::deployment::OmicronZoneExternalFloatingAddrs; use nexus_types::deployment::OmicronZoneExternalFloatingIp; use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::blueprint_zone_type; @@ -921,17 +926,20 @@ pub mod test { blueprint_zone_type::ExternalDns { dataset: OmicronZoneDataset { pool_name }, http_address: "[::1]:0".parse().unwrap(), - dns_address: OmicronZoneExternalFloatingAddr { - id: ExternalIpUuid::new_v4(), - addr: SocketAddr::new( - service_ip_pool - .iter() - .nth(index) - .unwrap() - .into(), - 0, + dns_addresses: + OmicronZoneExternalFloatingAddrs::from_single( + OmicronZoneExternalFloatingAddr { + id: ExternalIpUuid::new_v4(), + addr: SocketAddr::new( + service_ip_pool + .iter() + .nth(index) + .unwrap() + .into(), + 0, + ), + }, ), - }, nic: NetworkInterface { id: Uuid::new_v4(), kind: NetworkInterfaceKind::Service { diff --git a/nexus/reconfigurator/planning/src/example.rs b/nexus/reconfigurator/planning/src/example.rs index 729e3faf914..5d533c4af03 100644 --- a/nexus/reconfigurator/planning/src/example.rs +++ b/nexus/reconfigurator/planning/src/example.rs @@ -995,12 +995,17 @@ impl ExampleSystemBuilder { for sled_cfg in blueprint.sleds.values() { for zone in sled_cfg.zones.iter() { let service_id = zone.id; - if let Some((external_ip, nic)) = + if let Some((external_ips, nic)) = zone.zone_type.external_networking() { - input_builder - .add_omicron_zone_external_ip(service_id, external_ip) - .expect("failed to add Omicron zone external IP"); + for external_ip in external_ips { + input_builder + .add_omicron_zone_external_ip( + service_id, + external_ip, + ) + .expect("failed to add Omicron zone external IP"); + } input_builder .add_omicron_zone_nic( service_id, diff --git a/nexus/reconfigurator/planning/tests/integration_tests/planner.rs b/nexus/reconfigurator/planning/tests/integration_tests/planner.rs index 65d40066836..bf1f388b78a 100644 --- a/nexus/reconfigurator/planning/tests/integration_tests/planner.rs +++ b/nexus/reconfigurator/planning/tests/integration_tests/planner.rs @@ -34,7 +34,8 @@ use nexus_types::deployment::ClickhousePolicy; use nexus_types::deployment::CockroachDbClusterVersion; use nexus_types::deployment::CockroachDbPreserveDowngrade; use nexus_types::deployment::CockroachDbSettings; -use nexus_types::deployment::OmicronZoneExternalSnatIp; +use nexus_types::deployment::OmicronZoneExternalSnat; +use nexus_types::deployment::OmicronZoneExternalSnatIpV6; use nexus_types::deployment::PendingMgsUpdateDetails; use nexus_types::deployment::PendingMgsUpdates; use nexus_types::deployment::SledDisk; @@ -85,7 +86,7 @@ use sled_agent_types::inventory::ConfigReconcilerInventoryResult; use sled_agent_types::inventory::NetworkInterface; use sled_agent_types::inventory::NetworkInterfaceKind; use sled_agent_types::inventory::OmicronZoneType; -use sled_agent_types::inventory::SourceNatConfigGeneric; +use sled_agent_types::inventory::SourceNatConfigV6; use sled_agent_types::inventory::ZoneKind; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; @@ -685,7 +686,7 @@ fn test_reuse_external_ips_from_expunged_zones() { println!("2 -> 3 (maximum Nexus):\n{}", diff.display()); // Planning succeeded, but let's prove that we reused the IP address! - let expunged_ip = zone.zone_type.external_networking().unwrap().0.ip(); + let expunged_ip = zone.zone_type.external_networking().unwrap().0[0].ip(); let new_zone = blueprint3 .sleds .values() @@ -695,7 +696,9 @@ fn test_reuse_external_ips_from_expunged_zones() { && zone .zone_type .external_networking() - .map_or(false, |(ip, _)| expunged_ip == ip.ip()) + .map_or(false, |(ips, _)| { + ips.iter().any(|ip| expunged_ip == ip.ip()) + }) }) .expect("couldn't find that the external IP was reused"); println!( @@ -887,9 +890,9 @@ fn test_reuse_external_dns_ips_from_expunged_zones() { let mut ips = blueprint3 .in_service_zones() .filter_map(|(_id, zone)| { - zone.zone_type - .is_external_dns() - .then(|| zone.zone_type.external_networking().unwrap().0.ip()) + zone.zone_type.is_external_dns().then(|| { + zone.zone_type.external_networking().unwrap().0[0].ip() + }) }) .collect::>(); ips.sort(); @@ -4111,15 +4114,17 @@ fn test_update_boundary_ntp() { primary: true, slot: 0, }, - external_ip: OmicronZoneExternalSnatIp { - id: ExternalIpUuid::new_v4(), - snat_cfg: SourceNatConfigGeneric::new( - IpAddr::V6(Ipv6Addr::LOCALHOST), - 0, - 0x4000 - 1, - ) - .unwrap(), - }, + external_ip: OmicronZoneExternalSnat::Ipv6Only( + OmicronZoneExternalSnatIpV6 { + id: ExternalIpUuid::new_v4(), + snat_cfg: SourceNatConfigV6::new( + Ipv6Addr::LOCALHOST, + 0, + 0x4000 - 1, + ) + .unwrap(), + }, + ), }, ); Ok(()) diff --git a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt index 8c0077cc540..ce0345e1f66 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt @@ -394,10 +394,14 @@ mismatched zone type: after: Nexus( Nexus { internal_address: [fd01:1122:3344:105::4]:12221, lockstep_port: 12232, - external_ip: OmicronZoneExternalFloatingIp { - id: 6ebcade9-3a69-465e-99e9-6bf8eb9a8390 (external_ip), - ip: 192.0.2.2, - }, + external_ips: OmicronZoneExternalFloatingIps( + { + 192.0.2.2: OmicronZoneExternalFloatingIp { + id: 6ebcade9-3a69-465e-99e9-6bf8eb9a8390 (external_ip), + ip: 192.0.2.2, + }, + }, + ), nic: NetworkInterface { id: 93efbe06-a16a-449f-995b-82382534fcca, kind: Service { diff --git a/nexus/src/lib.rs b/nexus/src/lib.rs index fe1e946eed2..e86c4c20869 100644 --- a/nexus/src/lib.rs +++ b/nexus/src/lib.rs @@ -483,22 +483,35 @@ impl nexus_test_interface::NexusServer for Server { let (ipv4_service_ranges, ipv6_service_ranges): (Vec<_>, Vec<_>) = blueprint .in_service_zones() - .filter_map(|(_, zc)| match &zc.zone_type { - BlueprintZoneType::BoundaryNtp( - blueprint_zone_type::BoundaryNtp { - external_ip, .. - }, - ) => Some(IpRange::from(external_ip.snat_cfg.ip)), - BlueprintZoneType::ExternalDns( - blueprint_zone_type::ExternalDns { - dns_address, .. - }, - ) => Some(IpRange::from(dns_address.addr.ip())), - BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { - external_ip, - .. - }) => Some(IpRange::from(external_ip.ip)), - _ => None, + .flat_map(|(_, zc)| { + let ranges: Vec = match &zc.zone_type { + BlueprintZoneType::BoundaryNtp( + blueprint_zone_type::BoundaryNtp { + external_ip, + .. + }, + ) => external_ip + .iter() + .map(|snat| IpRange::from(snat.snat_cfg.ip)) + .collect(), + BlueprintZoneType::ExternalDns( + blueprint_zone_type::ExternalDns { + dns_addresses, + .. + }, + ) => dns_addresses + .iter() + .map(|a| IpRange::from(a.addr.ip())) + .collect(), + BlueprintZoneType::Nexus( + blueprint_zone_type::Nexus { external_ips, .. }, + ) => external_ips + .iter() + .map(|e| IpRange::from(e.ip)) + .collect(), + _ => Vec::new(), + }; + ranges }) .partition(|r| r.is_ipv4()); diff --git a/nexus/test-utils/src/starter.rs b/nexus/test-utils/src/starter.rs index a8d4f3d4d46..24dcad350aa 100644 --- a/nexus/test-utils/src/starter.rs +++ b/nexus/test-utils/src/starter.rs @@ -52,7 +52,10 @@ use nexus_types::deployment::BlueprintZoneType; use nexus_types::deployment::CockroachDbPreserveDowngrade; use nexus_types::deployment::LastAllocatedSubnetIpOffset; use nexus_types::deployment::OmicronZoneExternalFloatingAddr; +use nexus_types::deployment::OmicronZoneExternalFloatingAddrs; use nexus_types::deployment::OmicronZoneExternalFloatingIp; +use nexus_types::deployment::OmicronZoneExternalFloatingIps; +use nexus_types::deployment::OmicronZoneExternalSnat; use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::OximeterReadMode; use nexus_types::deployment::PendingMgsUpdates; @@ -744,15 +747,17 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> { .deployment .external_dns_servers .clone(), - external_ip: OmicronZoneExternalFloatingIp { - id: ExternalIpUuid::new_v4(), - ip: config - .deployment - .dropshot_external - .dropshot - .bind_address - .ip(), - }, + external_ips: OmicronZoneExternalFloatingIps::from_single( + OmicronZoneExternalFloatingIp { + id: ExternalIpUuid::new_v4(), + ip: config + .deployment + .dropshot_external + .dropshot + .bind_address + .ip(), + }, + ), external_tls: config.deployment.dropshot_external.tls, internal_address, lockstep_port, @@ -1132,15 +1137,17 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> { slot: 0, vni: Vni::SERVICES_VNI, }, - external_ip: OmicronZoneExternalSnatIp { - id: ExternalIpUuid::new_v4(), - snat_cfg: SourceNatConfigGeneric::new( - external_ip, - 0, - 16383, - ) - .unwrap(), - }, + external_ip: OmicronZoneExternalSnat::from_single( + OmicronZoneExternalSnatIp { + id: ExternalIpUuid::new_v4(), + snat_cfg: SourceNatConfigGeneric::new( + external_ip, + 0, + 16383, + ) + .unwrap(), + }, + ), }, ), image_source: BlueprintZoneImageSource::InstallDataset, @@ -1229,10 +1236,13 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> { zone_type: BlueprintZoneType::ExternalDns( blueprint_zone_type::ExternalDns { dataset: OmicronZoneDataset { pool_name }, - dns_address: OmicronZoneExternalFloatingAddr { - id: ExternalIpUuid::new_v4(), - addr: dns_address.into(), - }, + dns_addresses: + OmicronZoneExternalFloatingAddrs::from_single( + OmicronZoneExternalFloatingAddr { + id: ExternalIpUuid::new_v4(), + addr: dns_address.into(), + }, + ), http_address: dropshot_address, nic: NetworkInterface { id: Uuid::new_v4(), diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index be58f89a1fb..023f70a8717 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -102,15 +102,21 @@ pub use clickhouse::ClickhouseClusterConfig; use gateway_types::rot::RotSlot; pub use network_resources::AddNetworkResourceError; pub use network_resources::OmicronZoneExternalFloatingAddr; +pub use network_resources::OmicronZoneExternalFloatingAddrs; pub use network_resources::OmicronZoneExternalFloatingIp; +pub use network_resources::OmicronZoneExternalFloatingIps; pub use network_resources::OmicronZoneExternalIp; pub use network_resources::OmicronZoneExternalIpEntry; pub use network_resources::OmicronZoneExternalIpKey; +pub use network_resources::OmicronZoneExternalSnat; pub use network_resources::OmicronZoneExternalSnatIp; +pub use network_resources::OmicronZoneExternalSnatIpV4; +pub use network_resources::OmicronZoneExternalSnatIpV6; pub use network_resources::OmicronZoneNetworkResources; pub use network_resources::OmicronZoneNic; pub use network_resources::OmicronZoneNicEntry; pub use network_resources::OmicronZoneNicIp; +pub use network_resources::ZoneExternalSnatError; use omicron_common::api::external::Error; pub use planning_input::ClickhouseMode; pub use planning_input::ClickhousePolicy; @@ -349,28 +355,37 @@ impl Blueprint { let entries = self .in_service_zones() .filter_map(|(sled_id, zone_config)| { - let (nic_mac, vni, kind) = match &zone_config.zone_type { - BlueprintZoneType::BoundaryNtp(ntp) => ( - ntp.nic.mac, - ntp.nic.vni, - ServiceZoneNatKind::BoundaryNtp { - snat_cfg: ntp.external_ip.snat_cfg, - }, - ), - BlueprintZoneType::ExternalDns(dns) => ( - dns.nic.mac, - dns.nic.vni, - ServiceZoneNatKind::ExternalDns { - external_ip: dns.dns_address.addr.ip(), - }, - ), - BlueprintZoneType::Nexus(nexus) => ( - nexus.nic.mac, - nexus.nic.vni, - ServiceZoneNatKind::Nexus { - external_ip: nexus.external_ip.ip, - }, - ), + let (nic_mac, vni, kinds) = match &zone_config.zone_type { + BlueprintZoneType::BoundaryNtp(ntp) => { + let kinds = ntp + .external_ip + .iter() + .map(|eip| ServiceZoneNatKind::BoundaryNtp { + snat_cfg: eip.snat_cfg, + }) + .collect::>(); + (ntp.nic.mac, ntp.nic.vni, kinds) + } + BlueprintZoneType::ExternalDns(dns) => { + let kinds = dns + .dns_addresses + .iter() + .map(|addr| ServiceZoneNatKind::ExternalDns { + external_ip: addr.addr.ip(), + }) + .collect::>(); + (dns.nic.mac, dns.nic.vni, kinds) + } + BlueprintZoneType::Nexus(nexus) => { + let kinds = nexus + .external_ips + .iter() + .map(|ip| ServiceZoneNatKind::Nexus { + external_ip: ip.ip, + }) + .collect::>(); + (nexus.nic.mac, nexus.nic.vni, kinds) + } // None of these zone types have external NAT. BlueprintZoneType::Clickhouse(_) @@ -393,14 +408,20 @@ impl Blueprint { .expect("sled must exist if we have in-service zones") .subnet; - Some(ServiceZoneNatEntry { - zone_id: zone_config.id, - sled_underlay_ip: *get_sled_address(sled_subnet).ip(), - nic_mac, - vni, - kind, - }) + // Return a list of entries for all IPs in the zone. + let entries = kinds + .into_iter() + .map(|kind| ServiceZoneNatEntry { + zone_id: zone_config.id, + sled_underlay_ip: *get_sled_address(sled_subnet).ip(), + nic_mac, + vni, + kind, + }) + .collect::>(); + Some(entries) }) + .flatten() .collect::>(); entries.try_into() @@ -728,11 +749,14 @@ impl Blueprint { self.all_in_service_and_expunged_zones( BlueprintExpungedZoneAccessReason::ExternalDnsExternalIps, ) - .filter_map(|(_id, zone)| match &zone.zone_type { - BlueprintZoneType::ExternalDns(dns) => { - Some(dns.dns_address.addr.ip()) - } - _ => None, + .flat_map(|(_id, zone)| { + let addrs = match &zone.zone_type { + BlueprintZoneType::ExternalDns(dns) => { + Some(dns.dns_addresses.iter().map(|a| a.addr.ip())) + } + _ => None, + }; + addrs.into_iter().flatten() }) .collect() } diff --git a/nexus/types/src/deployment/execution/utils.rs b/nexus/types/src/deployment/execution/utils.rs index 4d12d72259f..4d4b4f8790b 100644 --- a/nexus/types/src/deployment/execution/utils.rs +++ b/nexus/types/src/deployment/execution/utils.rs @@ -86,9 +86,11 @@ pub fn blueprint_nexus_external_ips( ) -> Vec { blueprint .in_service_nexus_zones() - .filter_map(|(_sled_id, _zone_config, nexus_config)| { - (nexus_config.nexus_generation == active_generation) - .then_some(nexus_config.external_ip.ip) + .filter(|(_sled_id, _zone_config, nexus_config)| { + nexus_config.nexus_generation == active_generation + }) + .flat_map(|(_sled_id, _zone_config, nexus_config)| { + nexus_config.external_ips.iter().map(|e| e.ip) }) .collect() } @@ -100,11 +102,14 @@ pub fn blueprint_external_dns_nameserver_ips( ) -> Vec { blueprint .in_service_zones() - .filter_map(|(_, z)| match z.zone_type { - BlueprintZoneType::ExternalDns( - blueprint_zone_type::ExternalDns { dns_address, .. }, - ) => Some(dns_address.addr.ip()), - _ => None, + .flat_map(|(_, z)| { + let addrs = match &z.zone_type { + BlueprintZoneType::ExternalDns( + blueprint_zone_type::ExternalDns { dns_addresses, .. }, + ) => Some(dns_addresses.iter().map(|a| a.addr.ip())), + _ => None, + }; + addrs.into_iter().flatten() }) .collect() } diff --git a/nexus/types/src/deployment/network_resources.rs b/nexus/types/src/deployment/network_resources.rs index a0f575290a6..a74faf366de 100644 --- a/nexus/types/src/deployment/network_resources.rs +++ b/nexus/types/src/deployment/network_resources.rs @@ -4,6 +4,8 @@ use anyhow::anyhow; use daft::Diffable; +use iddqd::IdOrdItem; +use iddqd::IdOrdMap; use iddqd::TriHashItem; use iddqd::TriHashMap; use iddqd::tri_upcast; @@ -16,7 +18,14 @@ use omicron_uuid_kinds::VnicUuid; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; +use sled_agent_types::inventory::ExternalDnsAddrs; +use sled_agent_types::inventory::NexusExternalIps; use sled_agent_types::inventory::SourceNatConfigGeneric; +use sled_agent_types::inventory::SourceNatConfigV4; +use sled_agent_types::inventory::SourceNatConfigV6; +use sled_agent_types::inventory::ZoneExternalAddrsError; +use sled_agent_types::inventory::ZoneSnatConfig; +use sled_agent_types::inventory::check_external_ip_count; use std::net::IpAddr; use std::net::Ipv4Addr; use std::net::Ipv6Addr; @@ -217,16 +226,7 @@ impl OmicronZoneNetworkResources { /// External IP variants possible for Omicron-managed zones. #[derive( - Debug, - Clone, - Copy, - Hash, - PartialOrd, - Ord, - PartialEq, - Eq, - Serialize, - Deserialize, + Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Serialize, Deserialize, )] pub enum OmicronZoneExternalIp { Floating(OmicronZoneExternalFloatingIp), @@ -286,15 +286,17 @@ pub enum OmicronZoneExternalIpKey { /// This is a slimmer `nexus_db_model::ExternalIp` that only stores the fields /// necessary for blueprint planning, and requires that the zone have a single /// IP. +// +// NOTE: It's important that we continue to derive Ord and Eq. They're used in +// those trait implementations for the newtype `OmicronZoneExternalFloatingIps`. #[derive( Debug, Clone, Copy, - Hash, - PartialOrd, - Ord, PartialEq, Eq, + Ord, + PartialOrd, JsonSchema, Serialize, Deserialize, @@ -305,15 +307,29 @@ pub struct OmicronZoneExternalFloatingIp { pub ip: IpAddr, } +impl IdOrdItem for OmicronZoneExternalFloatingIp { + type Key<'a> = IpAddr; + + fn key(&self) -> Self::Key<'_> { + self.ip + } + + iddqd::id_upcast!(); +} + /// Floating external address with port allocated to an Omicron-managed zone. +// +// NOTE: It's important that we continue to derive Ord and Eq. They're used in +// those trait implementations for the newtype +// `OmicronZoneExternalFloatingAddrs`. #[derive( Debug, Clone, Copy, PartialEq, Eq, - PartialOrd, Ord, + PartialOrd, JsonSchema, Serialize, Deserialize, @@ -324,6 +340,16 @@ pub struct OmicronZoneExternalFloatingAddr { pub addr: SocketAddr, } +impl IdOrdItem for OmicronZoneExternalFloatingAddr { + type Key<'a> = IpAddr; + + fn key(&self) -> Self::Key<'_> { + self.addr.ip() + } + + iddqd::id_upcast!(); +} + impl OmicronZoneExternalFloatingAddr { pub fn into_ip(self) -> OmicronZoneExternalFloatingIp { OmicronZoneExternalFloatingIp { id: self.id, ip: self.addr.ip() } @@ -354,6 +380,418 @@ pub struct OmicronZoneExternalSnatIp { pub snat_cfg: SourceNatConfigGeneric, } +/// An IPv4 SNAT external IP allocated to an Omicron-managed zone. +/// +/// The family-typed analog of [`OmicronZoneExternalSnatIp`], used in the +/// variants of [`OmicronZoneExternalSnat`] so the enum can't hold an address of +/// the wrong family. +#[derive( + Debug, + Clone, + Copy, + Hash, + PartialOrd, + Ord, + PartialEq, + Eq, + JsonSchema, + Serialize, + Deserialize, + Diffable, +)] +pub struct OmicronZoneExternalSnatIpV4 { + pub id: ExternalIpUuid, + pub snat_cfg: SourceNatConfigV4, +} + +impl OmicronZoneExternalSnatIpV4 { + /// Widen to a family-agnostic [`OmicronZoneExternalSnatIp`]. + pub fn to_generic(self) -> OmicronZoneExternalSnatIp { + OmicronZoneExternalSnatIp { + id: self.id, + snat_cfg: self.snat_cfg.into(), + } + } +} + +/// An IPv6 SNAT external IP allocated to an Omicron-managed zone. +/// +/// The family-typed analog of [`OmicronZoneExternalSnatIp`], used in the +/// variants of [`OmicronZoneExternalSnat`] so the enum can't hold an address of +/// the wrong family. +#[derive( + Debug, + Clone, + Copy, + Hash, + PartialOrd, + Ord, + PartialEq, + Eq, + JsonSchema, + Serialize, + Deserialize, + Diffable, +)] +pub struct OmicronZoneExternalSnatIpV6 { + pub id: ExternalIpUuid, + pub snat_cfg: SourceNatConfigV6, +} + +impl OmicronZoneExternalSnatIpV6 { + /// Widen to a family-agnostic [`OmicronZoneExternalSnatIp`]. + pub fn to_generic(self) -> OmicronZoneExternalSnatIp { + OmicronZoneExternalSnatIp { + id: self.id, + snat_cfg: self.snat_cfg.into(), + } + } +} + +/// A set of `OmicronZoneExternalFloatingIp`s allocated to a single zone. +/// +/// The set of IPs is always non-empty, and there are no duplicate IP addresses. +/// Also, the size is bounded above by `MAX_ZONE_EXTERNAL_IPS`. +/// +/// NOTE: This is the reconfigurator analog of the inventory `NexusExternalIps` +/// type. +#[derive( + Debug, Clone, PartialEq, Eq, JsonSchema, Serialize, Deserialize, Diffable, +)] +#[daft(leaf)] +#[serde( + try_from = "IdOrdMap", + into = "IdOrdMap" +)] +pub struct OmicronZoneExternalFloatingIps( + #[schemars(length( + min = 1, + max = "sled_agent_types::inventory::MAX_ZONE_EXTERNAL_IPS" + ))] + IdOrdMap, +); + +impl std::cmp::PartialOrd for OmicronZoneExternalFloatingIps { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl std::cmp::Ord for OmicronZoneExternalFloatingIps { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.0.iter().cmp(other.0.iter()) + } +} + +impl OmicronZoneExternalFloatingIps { + /// Construct from a set of external IPs, validating the count. + /// + /// Uniqueness of the IP addresses is guaranteed by the `IdOrdMap` key, so + /// the only remaining invariant to check is that the number of addresses is + /// in `[1, MAX_ZONE_EXTERNAL_IPS]`. + pub fn new( + ips: IdOrdMap, + ) -> Result { + check_external_ip_count(ips.len())?; + Ok(Self(ips)) + } + + /// Construct from a single external IP. + pub fn from_single(ip: OmicronZoneExternalFloatingIp) -> Self { + Self(IdOrdMap::from_iter_unique([ip]).unwrap()) + } + + /// Iterate over the external IPs. + pub fn iter(&self) -> impl Iterator { + self.0.iter() + } + + /// Convert self into the inventory-specific `NexusExternalIps` type. + /// + /// # Panics + /// + /// This panics if the conversion can't be made. That should be impossible. + /// Both types have the same invariants: + /// + /// - There's at least one IP + /// - There are no more than `MAX_ZONE_EXTERNAL_IPS` IPs + /// - All the IP addresses are unique. + /// + /// The only difference between this type and `NexusExternalIps` is that + /// this one carries the UUID for each IP address as well. + pub(crate) fn into_nexus_external_ips_or_panic(self) -> NexusExternalIps { + NexusExternalIps::new(self.0.into_iter().map(|ip| ip.ip).collect()) + .unwrap() + } +} + +impl TryFrom> + for OmicronZoneExternalFloatingIps +{ + type Error = ZoneExternalAddrsError; + + fn try_from( + value: IdOrdMap, + ) -> Result { + Self::new(value) + } +} + +impl From + for IdOrdMap +{ + fn from(ips: OmicronZoneExternalFloatingIps) -> Self { + ips.0 + } +} + +/// A set of `OmicronZoneExternalFloatingAddrs`s allocated to a single zone. +/// +/// The set of IPs is always non-empty, and there are no duplicate IP addresses. +/// Also, the size is bounded above by `MAX_ZONE_EXTERNAL_IPS`. +/// +/// NOTE: This is the reconfigurator analog of the inventory `ExternalDnsAddrs` +/// type. +#[derive( + Debug, Clone, Eq, PartialEq, JsonSchema, Serialize, Deserialize, Diffable, +)] +#[daft(leaf)] +#[serde( + try_from = "IdOrdMap", + into = "IdOrdMap" +)] +pub struct OmicronZoneExternalFloatingAddrs( + #[schemars(length( + min = 1, + max = "sled_agent_types::inventory::MAX_ZONE_EXTERNAL_IPS" + ))] + IdOrdMap, +); + +impl std::cmp::PartialOrd for OmicronZoneExternalFloatingAddrs { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl std::cmp::Ord for OmicronZoneExternalFloatingAddrs { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.0.iter().cmp(other.0.iter()) + } +} + +impl OmicronZoneExternalFloatingAddrs { + /// Construct from a set of external addresses, validating the count. + /// + /// Uniqueness of the IP addresses (ignoring port) is guaranteed by the + /// `IdOrdMap` key, so the only remaining invariant to check is that the + /// number of addresses is in `[1, MAX_ZONE_EXTERNAL_IPS]`. + pub fn new( + addrs: IdOrdMap, + ) -> Result { + check_external_ip_count(addrs.len())?; + Ok(Self(addrs)) + } + + /// Construct from a single external address. + pub fn from_single(addr: OmicronZoneExternalFloatingAddr) -> Self { + Self(IdOrdMap::from_iter_unique([addr]).unwrap()) + } + + /// Iterate over the external addresses. + pub fn iter( + &self, + ) -> impl Iterator { + self.0.iter() + } + + /// Convert self into the inventory-specific `ExternalDnsAddrs` type. + /// + /// # Panics + /// + /// This panics if the conversion can't be made. That should be impossible. + /// Both types have the same invariants: + /// + /// - There's at least one IP + /// - There are no more than `MAX_ZONE_EXTERNAL_IPS` IPs + /// - All the IP addresses are unique, ignoring the port numbers. + /// + /// The only difference between this type and `ExternalDnsAddrs` is that + /// this one carries the UUID for each IP address as well. + pub(crate) fn into_external_dns_addrs_or_panic(self) -> ExternalDnsAddrs { + ExternalDnsAddrs::new( + self.0.into_iter().map(|addr| addr.addr).collect(), + ) + .unwrap() + } +} + +impl TryFrom> + for OmicronZoneExternalFloatingAddrs +{ + type Error = ZoneExternalAddrsError; + + fn try_from( + value: IdOrdMap, + ) -> Result { + Self::new(value) + } +} + +impl From + for IdOrdMap +{ + fn from(addrs: OmicronZoneExternalFloatingAddrs) -> Self { + addrs.0 + } +} + +/// SNAT configuration for a boundary NTP zone in a blueprint. +/// +/// Boundary NTP reaches upstream servers via source NAT and needs a source +/// address per IP version it wants to reach them on: at most one per family, +/// and at least one overall. This is the blueprint-layer analog of the +/// sled-agent wire type `ZoneSnatConfig`, but each entry additionally carries +/// its allocated `ExternalIpUuid`. +#[derive( + Debug, + Clone, + Copy, + Eq, + PartialEq, + Ord, + PartialOrd, + JsonSchema, + Serialize, + Deserialize, + Diffable, +)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum OmicronZoneExternalSnat { + Ipv4Only(OmicronZoneExternalSnatIpV4), + Ipv6Only(OmicronZoneExternalSnatIpV6), + DualStack { + ipv4: OmicronZoneExternalSnatIpV4, + ipv6: OmicronZoneExternalSnatIpV6, + }, +} + +impl OmicronZoneExternalSnat { + /// Construct from a single SNAT IP, inferring the family from its address. + pub fn from_single(snat: OmicronZoneExternalSnatIp) -> Self { + match snat.snat_cfg.ip { + IpAddr::V4(_) => { + let snat_cfg = snat + .snat_cfg + .try_as_ipv4() + .expect("just matched an IPv4 address"); + OmicronZoneExternalSnat::Ipv4Only(OmicronZoneExternalSnatIpV4 { + id: snat.id, + snat_cfg, + }) + } + IpAddr::V6(_) => { + let snat_cfg = snat + .snat_cfg + .try_as_ipv6() + .expect("just matched an IPv6 address"); + OmicronZoneExternalSnat::Ipv6Only(OmicronZoneExternalSnatIpV6 { + id: snat.id, + snat_cfg, + }) + } + } + } + + /// Build from a set of SNAT IPs: at most one per IP family, and at least + /// one overall. + pub fn from_ips( + ips: impl IntoIterator, + ) -> Result { + let mut v4: Option = None; + let mut v6: Option = None; + for ip in ips { + match ip.snat_cfg.ip { + IpAddr::V4(_) => { + let snat_cfg = ip + .snat_cfg + .try_as_ipv4() + .expect("just matched an IPv4 address"); + let entry = + OmicronZoneExternalSnatIpV4 { id: ip.id, snat_cfg }; + if v4.replace(entry).is_some() { + return Err(ZoneExternalSnatError::DuplicateIpv4); + } + } + IpAddr::V6(_) => { + let snat_cfg = ip + .snat_cfg + .try_as_ipv6() + .expect("just matched an IPv6 address"); + let entry = + OmicronZoneExternalSnatIpV6 { id: ip.id, snat_cfg }; + if v6.replace(entry).is_some() { + return Err(ZoneExternalSnatError::DuplicateIpv6); + } + } + } + } + match (v4, v6) { + (Some(ipv4), None) => Ok(OmicronZoneExternalSnat::Ipv4Only(ipv4)), + (None, Some(ipv6)) => Ok(OmicronZoneExternalSnat::Ipv6Only(ipv6)), + (Some(ipv4), Some(ipv6)) => { + Ok(OmicronZoneExternalSnat::DualStack { ipv4, ipv6 }) + } + (None, None) => Err(ZoneExternalSnatError::Empty), + } + } + + /// Iterate over the SNAT IPs (one per family), widened to the + /// family-agnostic [`OmicronZoneExternalSnatIp`]. + pub fn iter(&self) -> impl Iterator { + let (first, second) = match *self { + OmicronZoneExternalSnat::Ipv4Only(v4) => (v4.to_generic(), None), + OmicronZoneExternalSnat::Ipv6Only(v6) => (v6.to_generic(), None), + OmicronZoneExternalSnat::DualStack { ipv4, ipv6 } => { + (ipv4.to_generic(), Some(ipv6.to_generic())) + } + }; + std::iter::once(first).chain(second) + } +} + +impl From for ZoneSnatConfig { + /// Convert to the sled-agent wire [`ZoneSnatConfig`], dropping the + /// allocation IDs (which sled-agent does not need). + fn from(snat: OmicronZoneExternalSnat) -> Self { + match snat { + OmicronZoneExternalSnat::Ipv4Only(v4) => { + ZoneSnatConfig::Ipv4Only(v4.snat_cfg) + } + OmicronZoneExternalSnat::Ipv6Only(v6) => { + ZoneSnatConfig::Ipv6Only(v6.snat_cfg) + } + OmicronZoneExternalSnat::DualStack { ipv4, ipv6 } => { + ZoneSnatConfig::DualStack { + ipv4: ipv4.snat_cfg, + ipv6: ipv6.snat_cfg, + } + } + } + } +} + +/// Errors building an [`OmicronZoneExternalSnat`] from a set of SNAT IPs. +#[derive(Clone, Copy, Debug, Error)] +pub enum ZoneExternalSnatError { + #[error("must provide at least one SNAT address")] + Empty, + #[error("multiple IPv4 SNAT addresses provided")] + DuplicateIpv4, + #[error("multiple IPv6 SNAT addresses provided")] + DuplicateIpv6, +} + /// The private IP address(es) of an Omicron zone's network interface. #[derive( Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, @@ -541,6 +979,7 @@ mod tests { use super::*; use omicron_common::api::internal::shared::PrivateIpv4Config; use omicron_common::api::internal::shared::PrivateIpv6Config; + use proptest::prelude::*; fn v4_config() -> PrivateIpv4Config { PrivateIpv4Config::new( @@ -769,4 +1208,144 @@ mod tests { .expect("NIC with no external IP should be accepted"); } } + + #[test] + fn omicron_zone_external_floating_ips_reject_duplicate_ip_on_deserialize() { + let json = r#"[ + {"id":"bf8c8086-cb70-4b33-82a1-ce749fcdd8de","ip":"192.0.2.1"}, + {"id":"d0c6f5fc-7414-46d7-8992-f553d3fc303f","ip":"192.0.2.1"} + ]"#; + let result: Result = + serde_json::from_str(json); + assert!( + result.is_err(), + "a duplicate IP should fail to deserialize, got {result:?}", + ); + } + + #[test] + fn omicron_zone_external_floating_ips_reject_bad_count() { + let empty = + OmicronZoneExternalFloatingIps::new(IdOrdMap::new()).unwrap_err(); + assert!( + matches!(empty, ZoneExternalAddrsError::Empty), + "got {empty:?}", + ); + + let too_many = IdOrdMap::from_iter_unique( + (0..=sled_agent_types::inventory::MAX_ZONE_EXTERNAL_IPS).map(|i| { + OmicronZoneExternalFloatingIp { + id: ExternalIpUuid::new_v4(), + ip: IpAddr::V4(Ipv4Addr::new(192, 0, 2, i as u8)), + } + }), + ) + .expect("distinct IPs build a valid map"); + let err = OmicronZoneExternalFloatingIps::new(too_many).unwrap_err(); + assert!( + matches!(err, ZoneExternalAddrsError::TooMany { .. }), + "got {err:?}", + ); + } + + // The size of the pool of IP addresses we draw from in the proptests below. + // + // This is large enough so that we get up to and beyond the limit of + // `MAX_ZONE_EXTERNAL_IPS`, but also small enough that randomly drawing IPs + // generates collisions pretty frequently. That tests the duplicate IP + // rejection code. + const IP_POOL_SIZE: usize = + sled_agent_types::inventory::MAX_ZONE_EXTERNAL_IPS + 4; + + // Get an IP address from the pool, my mapping the index to an IP. We have + // both IPv4 and IPv6. + fn pool_ip(index: usize) -> IpAddr { + if index.is_multiple_of(2) { + IpAddr::V4(Ipv4Addr::new(192, 0, 2, index as u8)) + } else { + IpAddr::V6(Ipv6Addr::new( + 0x2001, + 0xdb8, + 0, + 0, + 0, + 0, + 0, + index as u16, + )) + } + } + + fn arbitrary_floating_ips() + -> impl Strategy> { + let element = (any::(), 0..IP_POOL_SIZE).prop_map( + |(id, index)| OmicronZoneExternalFloatingIp { + id, + ip: pool_ip(index), + }, + ); + proptest::collection::vec(element, 0..=IP_POOL_SIZE) + } + + fn arbitrary_floating_addrs() + -> impl Strategy> { + // Ports are irrelevant to the constructors, so just draw randomly. + let element = (any::(), 0..IP_POOL_SIZE, any::()) + .prop_map(|(id, index, port)| OmicronZoneExternalFloatingAddr { + id, + addr: SocketAddr::new(pool_ip(index), port), + }); + proptest::collection::vec(element, 0..=IP_POOL_SIZE) + } + + proptest! { + /// We should always be able to convert the blueprint + /// `OmicronZoneExternalFloatingIps` into the inventory type + /// `NexusExternalIps`. That's technically a panicking conversion, and + /// there's some cross-crate coupling here, so use a proptest to make + /// sure it fails loudly if anything changes. + #[test] + fn floating_ips_always_convert_to_inventory( + ips in arbitrary_floating_ips(), + ) { + if let Ok(map) = IdOrdMap::from_iter_unique(ips) { + let blueprint = OmicronZoneExternalFloatingIps::new(map.clone()); + let inventory = + NexusExternalIps::new(map.iter().map(|ip| ip.ip).collect()); + + prop_assert_eq!(blueprint.is_ok(), inventory.is_ok()); + + if let (Ok(blueprint), Ok(inventory)) = (blueprint, inventory) { + prop_assert_eq!( + blueprint.into_nexus_external_ips_or_panic(), + inventory, + ); + } + } + } + + /// The same test for `OmicronZoneExternalFloatingAddrs` and the + /// inventory `ExternalDnsAddrs`. + #[test] + fn floating_addrs_always_convert_to_inventory( + addrs in arbitrary_floating_addrs(), + ) { + if let Ok(map) = IdOrdMap::from_iter_unique(addrs) { + let blueprint = + OmicronZoneExternalFloatingAddrs::new(map.clone()); + let inventory = ExternalDnsAddrs::new( + map.iter().map(|addr| addr.addr).collect(), + ); + + prop_assert_eq!(blueprint.is_ok(), inventory.is_ok()); + + if let (Ok(blueprint), Ok(inventory)) = (blueprint, inventory) { + prop_assert_eq!( + blueprint.into_external_dns_addrs_or_panic(), + inventory, + ); + } + } + } + } } diff --git a/nexus/types/src/deployment/zone_type.rs b/nexus/types/src/deployment/zone_type.rs index 3e88ee94e67..f983078cbfc 100644 --- a/nexus/types/src/deployment/zone_type.rs +++ b/nexus/types/src/deployment/zone_type.rs @@ -15,9 +15,7 @@ use omicron_common::disk::DatasetName; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; -use sled_agent_types::inventory::ExternalDnsAddrs; use sled_agent_types::inventory::NetworkInterface; -use sled_agent_types::inventory::NexusExternalIps; use sled_agent_types::inventory::OmicronZoneDataset; use sled_agent_types::inventory::OmicronZoneType; use sled_agent_types::inventory::ZoneKind; @@ -118,20 +116,41 @@ impl BlueprintZoneType { self.durable_dataset().map(|dataset| &dataset.dataset.pool_name) } + /// Return this zone's external IPs (one or more) and its OPTE service vNIC, + /// if it has external networking. + /// + /// A zone has at most one service vNIC but may have multiple external IPs + /// (Nexus, external DNS) or a source-NAT address per IP family (boundary + /// NTP), so the external IPs are returned as a (small) collection. pub fn external_networking( &self, - ) -> Option<(OmicronZoneExternalIp, &NetworkInterface)> { + ) -> Option<(Vec, &NetworkInterface)> { match self { - BlueprintZoneType::Nexus(nexus) => Some(( - OmicronZoneExternalIp::Floating(nexus.external_ip), - &nexus.nic, - )), - BlueprintZoneType::ExternalDns(dns) => Some(( - OmicronZoneExternalIp::Floating(dns.dns_address.into_ip()), - &dns.nic, - )), + BlueprintZoneType::Nexus(nexus) => { + let ips = nexus + .external_ips + .iter() + .copied() + .map(OmicronZoneExternalIp::Floating) + .collect(); + Some((ips, &nexus.nic)) + } + BlueprintZoneType::ExternalDns(dns) => { + let ips = dns + .dns_addresses + .iter() + .copied() + .map(|addr| OmicronZoneExternalIp::Floating(addr.into_ip())) + .collect(); + Some((ips, &dns.nic)) + } BlueprintZoneType::BoundaryNtp(ntp) => { - Some((OmicronZoneExternalIp::Snat(ntp.external_ip), &ntp.nic)) + let ips = ntp + .external_ip + .iter() + .map(OmicronZoneExternalIp::Snat) + .collect(); + Some((ips, &ntp.nic)) } BlueprintZoneType::Clickhouse(_) | BlueprintZoneType::ClickhouseKeeper(_) @@ -266,7 +285,7 @@ impl From for OmicronZoneType { dns_servers: zone.dns_servers, domain: zone.domain, nic: zone.nic, - snat: zone.external_ip.snat_cfg.into(), + snat: zone.external_ip.into(), }, BlueprintZoneType::Clickhouse(zone) => Self::Clickhouse { address: zone.address, @@ -297,9 +316,9 @@ impl From for OmicronZoneType { BlueprintZoneType::ExternalDns(zone) => Self::ExternalDns { dataset: zone.dataset, http_address: zone.http_address, - dns_addresses: ExternalDnsAddrs::from_single( - zone.dns_address.addr, - ), + dns_addresses: zone + .dns_addresses + .into_external_dns_addrs_or_panic(), nic: zone.nic, }, BlueprintZoneType::InternalDns(zone) => Self::InternalDns { @@ -315,9 +334,9 @@ impl From for OmicronZoneType { BlueprintZoneType::Nexus(zone) => Self::Nexus { internal_address: zone.internal_address, lockstep_port: zone.lockstep_port, - external_ips: NexusExternalIps::from_single( - zone.external_ip.ip, - ), + external_ips: zone + .external_ips + .into_nexus_external_ips_or_panic(), nic: zone.nic, external_tls: zone.external_tls, external_dns_servers: zone.external_dns_servers, @@ -350,9 +369,9 @@ impl BlueprintZoneType { } pub mod blueprint_zone_type { - use crate::deployment::OmicronZoneExternalFloatingAddr; - use crate::deployment::OmicronZoneExternalFloatingIp; - use crate::deployment::OmicronZoneExternalSnatIp; + use crate::deployment::OmicronZoneExternalFloatingAddrs; + use crate::deployment::OmicronZoneExternalFloatingIps; + use crate::deployment::OmicronZoneExternalSnat; use daft::Diffable; use omicron_generation_kinds::NexusGeneration; use schemars::JsonSchema; @@ -383,7 +402,8 @@ pub mod blueprint_zone_type { pub domain: Option, /// The service vNIC providing outbound connectivity using OPTE. pub nic: NetworkInterface, - pub external_ip: OmicronZoneExternalSnatIp, + /// The source NAT configuration (one address per IP family). + pub external_ip: OmicronZoneExternalSnat, } /// Used in single-node clickhouse setups @@ -505,8 +525,8 @@ pub mod blueprint_zone_type { pub dataset: OmicronZoneDataset, /// The address at which the external DNS server API is reachable. pub http_address: SocketAddrV6, - /// The address at which the external DNS server is reachable. - pub dns_address: OmicronZoneExternalFloatingAddr, + /// The addresses at which the external DNS server is reachable. + pub dns_addresses: OmicronZoneExternalFloatingAddrs, /// The service vNIC providing external connectivity using OPTE. pub nic: NetworkInterface, } @@ -574,8 +594,8 @@ pub mod blueprint_zone_type { /// The port at which the lockstep server is reachable. This shares the /// same IP address with `internal_address`. pub lockstep_port: u16, - /// The address at which the external nexus server is reachable. - pub external_ip: OmicronZoneExternalFloatingIp, + /// The addresses at which the external nexus server is reachable. + pub external_ips: OmicronZoneExternalFloatingIps, /// The service vNIC providing external connectivity using OPTE. pub nic: NetworkInterface, /// Whether Nexus's external endpoint should use TLS diff --git a/openapi/nexus-lockstep.json b/openapi/nexus-lockstep.json index 4ce2d1a52c9..58871a5c8ba 100644 --- a/openapi/nexus-lockstep.json +++ b/openapi/nexus-lockstep.json @@ -3134,7 +3134,12 @@ "type": "string" }, "external_ip": { - "$ref": "#/components/schemas/OmicronZoneExternalSnatIp" + "description": "The source NAT configuration (one address per IP family).", + "allOf": [ + { + "$ref": "#/components/schemas/OmicronZoneExternalSnat" + } + ] }, "nic": { "description": "The service vNIC providing outbound connectivity using OPTE.", @@ -3302,11 +3307,11 @@ "dataset": { "$ref": "#/components/schemas/OmicronZoneDataset" }, - "dns_address": { - "description": "The address at which the external DNS server is reachable.", + "dns_addresses": { + "description": "The addresses at which the external DNS server is reachable.", "allOf": [ { - "$ref": "#/components/schemas/OmicronZoneExternalFloatingAddr" + "$ref": "#/components/schemas/OmicronZoneExternalFloatingAddrs" } ] }, @@ -3331,7 +3336,7 @@ }, "required": [ "dataset", - "dns_address", + "dns_addresses", "http_address", "nic", "type" @@ -3405,11 +3410,11 @@ "format": "ip" } }, - "external_ip": { - "description": "The address at which the external nexus server is reachable.", + "external_ips": { + "description": "The addresses at which the external nexus server is reachable.", "allOf": [ { - "$ref": "#/components/schemas/OmicronZoneExternalFloatingIp" + "$ref": "#/components/schemas/OmicronZoneExternalFloatingIps" } ] }, @@ -3452,7 +3457,7 @@ }, "required": [ "external_dns_servers", - "external_ip", + "external_ips", "external_tls", "internal_address", "lockstep_port", @@ -6643,6 +6648,27 @@ "id" ] }, + "OmicronZoneExternalFloatingAddrs": { + "title": "IdOrdMap", + "description": "A set of `OmicronZoneExternalFloatingAddrs`s allocated to a single zone.\n\nThe set of IPs is always non-empty, and there are no duplicate IP addresses. Also, the size is bounded above by `MAX_ZONE_EXTERNAL_IPS`.\n\nNOTE: This is the reconfigurator analog of the inventory `ExternalDnsAddrs` type.", + "x-rust-type": { + "crate": "iddqd", + "parameters": [ + { + "$ref": "#/components/schemas/OmicronZoneExternalFloatingAddr" + } + ], + "path": "iddqd::IdOrdMap", + "version": "*" + }, + "type": "array", + "items": { + "$ref": "#/components/schemas/OmicronZoneExternalFloatingAddr" + }, + "minItems": 1, + "maxItems": 16, + "uniqueItems": true + }, "OmicronZoneExternalFloatingIp": { "description": "Floating external IP allocated to an Omicron-managed zone.\n\nThis is a slimmer `nexus_db_model::ExternalIp` that only stores the fields necessary for blueprint planning, and requires that the zone have a single IP.", "type": "object", @@ -6660,15 +6686,125 @@ "ip" ] }, - "OmicronZoneExternalSnatIp": { - "description": "SNAT (outbound) external IP allocated to an Omicron-managed zone.\n\nThis is a slimmer `nexus_db_model::ExternalIp` that only stores the fields necessary for blueprint planning, and requires that the zone have a single IP.", + "OmicronZoneExternalFloatingIps": { + "title": "IdOrdMap", + "description": "A set of `OmicronZoneExternalFloatingIp`s allocated to a single zone.\n\nThe set of IPs is always non-empty, and there are no duplicate IP addresses. Also, the size is bounded above by `MAX_ZONE_EXTERNAL_IPS`.\n\nNOTE: This is the reconfigurator analog of the inventory `NexusExternalIps` type.", + "x-rust-type": { + "crate": "iddqd", + "parameters": [ + { + "$ref": "#/components/schemas/OmicronZoneExternalFloatingIp" + } + ], + "path": "iddqd::IdOrdMap", + "version": "*" + }, + "type": "array", + "items": { + "$ref": "#/components/schemas/OmicronZoneExternalFloatingIp" + }, + "minItems": 1, + "maxItems": 16, + "uniqueItems": true + }, + "OmicronZoneExternalSnat": { + "description": "SNAT configuration for a boundary NTP zone in a blueprint.\n\nBoundary NTP reaches upstream servers via source NAT and needs a source address per IP version it wants to reach them on: at most one per family, and at least one overall. This is the blueprint-layer analog of the sled-agent wire type `ZoneSnatConfig`, but each entry additionally carries its allocated `ExternalIpUuid`.", + "oneOf": [ + { + "description": "An IPv4 SNAT external IP allocated to an Omicron-managed zone.\n\nThe family-typed analog of [`OmicronZoneExternalSnatIp`], used in the variants of [`OmicronZoneExternalSnat`] so the enum can't hold an address of the wrong family.", + "type": "object", + "properties": { + "id": { + "$ref": "#/components/schemas/ExternalIpUuid" + }, + "snat_cfg": { + "$ref": "#/components/schemas/SourceNatConfigV4" + }, + "type": { + "type": "string", + "enum": [ + "ipv4_only" + ] + } + }, + "required": [ + "id", + "snat_cfg", + "type" + ] + }, + { + "description": "An IPv6 SNAT external IP allocated to an Omicron-managed zone.\n\nThe family-typed analog of [`OmicronZoneExternalSnatIp`], used in the variants of [`OmicronZoneExternalSnat`] so the enum can't hold an address of the wrong family.", + "type": "object", + "properties": { + "id": { + "$ref": "#/components/schemas/ExternalIpUuid" + }, + "snat_cfg": { + "$ref": "#/components/schemas/SourceNatConfigV6" + }, + "type": { + "type": "string", + "enum": [ + "ipv6_only" + ] + } + }, + "required": [ + "id", + "snat_cfg", + "type" + ] + }, + { + "type": "object", + "properties": { + "ipv4": { + "$ref": "#/components/schemas/OmicronZoneExternalSnatIpV4" + }, + "ipv6": { + "$ref": "#/components/schemas/OmicronZoneExternalSnatIpV6" + }, + "type": { + "type": "string", + "enum": [ + "dual_stack" + ] + } + }, + "required": [ + "ipv4", + "ipv6", + "type" + ] + } + ] + }, + "OmicronZoneExternalSnatIpV4": { + "description": "An IPv4 SNAT external IP allocated to an Omicron-managed zone.\n\nThe family-typed analog of [`OmicronZoneExternalSnatIp`], used in the variants of [`OmicronZoneExternalSnat`] so the enum can't hold an address of the wrong family.", + "type": "object", + "properties": { + "id": { + "$ref": "#/components/schemas/ExternalIpUuid" + }, + "snat_cfg": { + "$ref": "#/components/schemas/SourceNatConfigV4" + } + }, + "required": [ + "id", + "snat_cfg" + ] + }, + "OmicronZoneExternalSnatIpV6": { + "description": "An IPv6 SNAT external IP allocated to an Omicron-managed zone.\n\nThe family-typed analog of [`OmicronZoneExternalSnatIp`], used in the variants of [`OmicronZoneExternalSnat`] so the enum can't hold an address of the wrong family.", "type": "object", "properties": { "id": { "$ref": "#/components/schemas/ExternalIpUuid" }, "snat_cfg": { - "$ref": "#/components/schemas/SourceNatConfigGeneric" + "$ref": "#/components/schemas/SourceNatConfigV6" } }, "required": [ @@ -9737,34 +9873,6 @@ "type": "string", "format": "uuid" }, - "SourceNatConfigGeneric": { - "description": "An IP address and port range used for source NAT, i.e., making outbound network connections from guests or services.", - "type": "object", - "properties": { - "first_port": { - "description": "The first port used for source NAT, inclusive.", - "type": "integer", - "format": "uint16", - "minimum": 0 - }, - "ip": { - "description": "The external address provided to the instance or service.", - "type": "string", - "format": "ip" - }, - "last_port": { - "description": "The last port used for source NAT, also inclusive.", - "type": "integer", - "format": "uint16", - "minimum": 0 - } - }, - "required": [ - "first_port", - "ip", - "last_port" - ] - }, "SourceNatConfigV4": { "description": "An IP address and port range used for source NAT, i.e., making outbound network connections from guests or services.", "type": "object", diff --git a/sled-agent/rack-setup/src/plan/service.rs b/sled-agent/rack-setup/src/plan/service.rs index f9c68900a13..821c91565f1 100644 --- a/sled-agent/rack-setup/src/plan/service.rs +++ b/sled-agent/rack-setup/src/plan/service.rs @@ -24,9 +24,10 @@ use nexus_types::deployment::{ BlueprintSledConfig, BlueprintSledUpdateDisposition, BlueprintSource, BlueprintZoneConfig, BlueprintZoneDisposition, BlueprintZoneImageSource, BlueprintZoneType, CockroachDbPreserveDowngrade, - OmicronZoneExternalFloatingAddr, OmicronZoneExternalFloatingIp, - OmicronZoneExternalSnatIp, OximeterReadMode, PendingMgsUpdates, - blueprint_zone_type, + OmicronZoneExternalFloatingAddr, OmicronZoneExternalFloatingAddrs, + OmicronZoneExternalFloatingIp, OmicronZoneExternalFloatingIps, + OmicronZoneExternalSnat, OmicronZoneExternalSnatIp, OximeterReadMode, + PendingMgsUpdates, blueprint_zone_type, }; use nexus_types::external_api::sled::SledState; use omicron_common::address::{ @@ -600,7 +601,10 @@ impl ServicePlan { pool_name: *dataset_name.pool(), }, http_address, - dns_address, + dns_addresses: + OmicronZoneExternalFloatingAddrs::from_single( + dns_address, + ), nic, }, ), @@ -634,9 +638,12 @@ impl ServicePlan { blueprint_zone_type::Nexus { internal_address, lockstep_port: NEXUS_LOCKSTEP_PORT, - external_ip: from_ipaddr_to_external_floating_ip( - external_ip, - ), + external_ips: + OmicronZoneExternalFloatingIps::from_single( + from_ipaddr_to_external_floating_ip( + external_ip, + ), + ), nic, // Tell Nexus to use TLS if and only if the caller // provided TLS certificates. This effectively @@ -824,10 +831,11 @@ impl ServicePlan { dns_servers: config.dns_servers.clone(), domain: None, nic, - external_ip: + external_ip: OmicronZoneExternalSnat::from_single( from_source_nat_config_to_external_snat_ip( snat_cfg, ), + ), }, ), ServiceName::BoundaryNtp, diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 53278932687..0c771ef3336 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -1860,11 +1860,12 @@ impl ServiceManager { // a single address, preferring the IPv4 one (else IPv6) to // match `opte_interface_set_up_install`. let dns_address = dns_addresses.temporary_primary_address(); - let private_ip = Self::private_ip_for_external_address( - dns_address.ip(), + let private_ips = Self::private_ips_for_external_addresses( + std::iter::once(&dns_address.ip()), &nic.ip_config, config.zone_type.kind(), )?; + let private_ip = private_ips[0]; let private_dns_address = SocketAddr::new(private_ip, dns_address.port()).to_string(); @@ -2215,23 +2216,29 @@ impl ServiceManager { })?; let opte_iface_name = port.name(); - // Fetch the private IP of the same IP version as the external - // IP address. + // Fetch the private IPs for each of the external addresses. // - // TODO(#11006): Nexus should be reachable on the private IP - // address for *all* of its external IPs. For now we bind a - // single address, preferring the IPv4 one (else IPv6) to match - // match `opte_interface_set_up_install`. - let external_ip = external_ips.temporary_primary_address(); - let private_ip = Self::private_ip_for_external_address( - external_ip, + // Consume the first private address, which always exists, and + // then collect any additional addresses into a list. + let nexus_port = if *external_tls { 443 } else { 80 }; + let mut private_ips = Self::private_ips_for_external_addresses( + external_ips.iter(), &nic.ip_config, config.zone_type.kind(), - )?; + )? + .into_iter(); + let bind_address = SocketAddr::new( + private_ips.next().expect( + "Always at least one external address for Nexus", + ), + nexus_port, + ); + let dropshot_external_additional_addresses = private_ips + .map(|ip| SocketAddr::new(ip, nexus_port)) + .collect(); // Nexus takes a separate config file for parameters // which cannot be known at packaging time. - let nexus_port = if *external_tls { 443 } else { 80 }; let deployment_config = DeploymentConfig { id: *id, rack_id: info.rack_id, @@ -2240,9 +2247,7 @@ impl ServiceManager { dropshot_external: ConfigDropshotWithTls { tls: *external_tls, dropshot: dropshot::ConfigDropshot { - bind_address: SocketAddr::new( - private_ip, nexus_port, - ), + bind_address, default_request_body_max_bytes: 1048576, default_handler_task_mode: HandlerTaskMode::Detached, @@ -2250,9 +2255,7 @@ impl ServiceManager { compression: dropshot::CompressionConfig::Gzip, }, }, - // TODO(#9288): populate additional external addresses here - // once sled-agent assigns dual-stack external IPs to Nexus. - dropshot_external_additional_addresses: vec![], + dropshot_external_additional_addresses, dropshot_internal: dropshot::ConfigDropshot { bind_address: (*internal_address).into(), default_request_body_max_bytes: 1048576, @@ -3796,35 +3799,62 @@ impl ServiceManager { } } - fn private_ip_for_external_address( - external_ip: IpAddr, - ip_config: &PrivateIpConfig, + // Return the private IP addresses for each external IP address. + // + // When given at least one address, this returns 1 or 2 private IPs for + // those external addresses, i.e., at least one address and as many as one + // per family. + fn private_ips_for_external_addresses<'a>( + external_ips: impl Iterator + 'a, + ip_config: &'a PrivateIpConfig, kind: ZoneKind, - ) -> Result { - let maybe_private_ip = if external_ip.is_ipv6() { - ip_config.ipv6_addr().copied().map(IpAddr::V6) - } else { - ip_config.ipv4_addr().copied().map(IpAddr::V4) - }; - maybe_private_ip.ok_or_else(|| { - let external_ip_version = - if external_ip.is_ipv6() { "6" } else { "4" }; - let private_ip_stack = if ip_config.is_ipv4_only() { - "IPv4" - } else if ip_config.is_ipv6_only() { - "IPv6" + ) -> Result, Error> { + let mut private_ipv4 = None; + let mut private_ipv6 = None; + for external_ip in external_ips { + if external_ip.is_ipv6() { + let Some(pip_v6) = ip_config.ipv6_addr().copied() else { + return Err(Error::BadServiceRequest { + service: kind.report_str().to_string(), + message: + "External IP address is IPv6, but VPC-private \ + IP configuration does not have an IPv6 \ + address" + .to_string(), + }); + }; + + // We always have at most one private IPv6 address, so + // "replacing" it is fine. + let _ = private_ipv6.insert(IpAddr::V6(pip_v6)); } else { - "dual-stack" - }; - Error::BadServiceRequest { - service: kind.report_str().to_string(), - message: format!( - "External IP address is IPv{}, but VPC-private \ - IP configuration is {}", - external_ip_version, private_ip_stack, - ), + let Some(pip_v4) = ip_config.ipv4_addr().copied() else { + return Err(Error::BadServiceRequest { + service: kind.report_str().to_string(), + message: + "External IP address is IPv4, but VPC-private \ + IP configuration does not have an IPv4 \ + address" + .to_string(), + }); + }; + + // We always have at most one private IPv4 address, so + // "replacing" it is fine. + let _ = private_ipv4.insert(IpAddr::V4(pip_v4)); } - }) + } + let out = + private_ipv4.into_iter().chain(private_ipv6).collect::>(); + if out.is_empty() { + return Err(Error::BadServiceRequest { + service: kind.report_str().to_string(), + message: "`private_ips_for_external_addresses()` requires \ + at least one IP address, but none were provided" + .to_string(), + }); + } + Ok(out) } } diff --git a/sled-agent/src/sim/server.rs b/sled-agent/src/sim/server.rs index 79f7a7c7c09..7fe5b6bb7da 100644 --- a/sled-agent/src/sim/server.rs +++ b/sled-agent/src/sim/server.rs @@ -31,6 +31,7 @@ use nexus_types::deployment::{ }; use nexus_types::deployment::{ BlueprintZoneConfig, BlueprintZoneDisposition, BlueprintZoneType, + OmicronZoneExternalFloatingAddrs, OmicronZoneExternalFloatingIps, }; use omicron_common::FileKv; use omicron_common::address::IpRange; @@ -500,9 +501,12 @@ pub async fn run_standalone_server( SocketAddr::V6(a) => a, }, lockstep_port: nexus_lockstep_port, - external_ip: from_ipaddr_to_external_floating_ip( - external_ip, - ), + external_ips: + OmicronZoneExternalFloatingIps::from_single( + from_ipaddr_to_external_floating_ip( + external_ip, + ), + ), nic: NetworkInterface { id: Uuid::new_v4(), kind: NetworkInterfaceKind::Service { @@ -557,9 +561,12 @@ pub async fn run_standalone_server( blueprint_zone_type::ExternalDns { dataset: OmicronZoneDataset { pool_name }, http_address: external_dns_internal_addr, - dns_address: from_sockaddr_to_external_floating_addr( - SocketAddr::V6(external_dns_internal_addr), - ), + dns_addresses: + OmicronZoneExternalFloatingAddrs::from_single( + from_sockaddr_to_external_floating_addr( + SocketAddr::V6(external_dns_internal_addr), + ), + ), nic: NetworkInterface { id: Uuid::new_v4(), kind: NetworkInterfaceKind::Service { diff --git a/sled-agent/types/versions/src/impls/inventory.rs b/sled-agent/types/versions/src/impls/inventory.rs index b981c68788f..7f9b5ef1024 100644 --- a/sled-agent/types/versions/src/impls/inventory.rs +++ b/sled-agent/types/versions/src/impls/inventory.rs @@ -1134,6 +1134,26 @@ impl SourceNatConfigGeneric { } } +impl From for SourceNatConfigGeneric { + fn from(c: SourceNatConfigV4) -> Self { + SourceNatConfig { + ip: IpAddr::V4(c.ip), + first_port: c.first_port, + last_port: c.last_port, + } + } +} + +impl From for SourceNatConfigGeneric { + fn from(c: SourceNatConfigV6) -> Self { + SourceNatConfig { + ip: IpAddr::V6(c.ip), + first_port: c.first_port, + last_port: c.last_port, + } + } +} + #[cfg(any(test, feature = "testing"))] impl proptest::arbitrary::Arbitrary for SourceNatConfig where @@ -1209,18 +1229,6 @@ impl NexusExternalIps { pub fn iter(&self) -> impl Iterator { self.0.iter() } - - /// Return the "primary" address, either IPv4 or IPv6 in that order. - /// - /// NOTE: This is a temporary method used while we don't fully support - /// multiple IP addresses. It should be removed when that support is done. - pub fn temporary_primary_address(&self) -> IpAddr { - self.iter() - .find(|ip| ip.is_ipv4()) - .or_else(|| self.iter().next()) - .copied() - .expect("NexusExternalIps is non-empty by construction") - } } impl From<&NexusExternalIps> for crate::latest::instance::ExternalIpConfig { diff --git a/sled-agent/types/versions/src/latest.rs b/sled-agent/types/versions/src/latest.rs index 7f4936b69b3..9425afba235 100644 --- a/sled-agent/types/versions/src/latest.rs +++ b/sled-agent/types/versions/src/latest.rs @@ -204,12 +204,15 @@ pub mod inventory { pub use crate::v51::inventory::ConfigReconcilerInventoryStatus; pub use crate::v51::inventory::ExternalDnsAddrs; pub use crate::v51::inventory::Inventory; + pub use crate::v51::inventory::MAX_ZONE_EXTERNAL_IPS; pub use crate::v51::inventory::NexusExternalIps; pub use crate::v51::inventory::OmicronSledConfig; pub use crate::v51::inventory::OmicronZoneConfig; pub use crate::v51::inventory::OmicronZoneType; pub use crate::v51::inventory::OmicronZonesConfig; + pub use crate::v51::inventory::ZoneExternalAddrsError; pub use crate::v51::inventory::ZoneSnatConfig; + pub use crate::v51::inventory::check_external_ip_count; pub use crate::impls::inventory::FmdHostCaseDisplay; pub use crate::impls::inventory::FmdInventoryDisplay; diff --git a/sled-agent/types/versions/src/multiple_zone_external_ips/inventory.rs b/sled-agent/types/versions/src/multiple_zone_external_ips/inventory.rs index 20a93a057f9..6657137cdfb 100644 --- a/sled-agent/types/versions/src/multiple_zone_external_ips/inventory.rs +++ b/sled-agent/types/versions/src/multiple_zone_external_ips/inventory.rs @@ -55,10 +55,12 @@ use crate::v50; // requests. It also should be enforced by the database or Neuxs, when we allow // operators control over which IPs DNS listens on, or which IP Pools Nexus // draws from. That's part of #10574. -const MAX_ZONE_EXTERNAL_IPS: usize = 16; +pub const MAX_ZONE_EXTERNAL_IPS: usize = 16; -// Helper to check the length of an array of IPs / socket addrs. -fn check_length(count: usize) -> Result<(), ZoneExternalAddrsError> { +/// Helper to check the length of an array of IPs / socket addrs. +pub fn check_external_ip_count( + count: usize, +) -> Result<(), ZoneExternalAddrsError> { if count == 0 { return Err(ZoneExternalAddrsError::Empty); } @@ -81,7 +83,7 @@ pub struct NexusExternalIps( impl NexusExternalIps { /// Construct from a list of IPs. pub fn new(ips: BTreeSet) -> Result { - check_length(ips.len())?; + check_external_ip_count(ips.len())?; Ok(Self(ips)) } } @@ -113,7 +115,7 @@ pub struct ExternalDnsAddrs( impl ExternalDnsAddrs { /// Construct from a list of addresses. pub fn new(addrs: Vec) -> Result { - check_length(addrs.len())?; + check_external_ip_count(addrs.len())?; let mut inner = BTreeMap::new(); for (ip, port) in addrs.iter().map(|addr| (addr.ip(), addr.port())) { if inner.insert(ip, port).is_some() {