From 6079a7969a34addfba456921d5643d9743f34d38 Mon Sep 17 00:00:00 2001 From: Chet Nichols III Date: Fri, 14 Aug 2026 15:14:07 -0700 Subject: [PATCH] feat(instance): enable IPv6 VPC prefix selection As it stood, automatic VPC prefix selection only accepted `Ipv4Only` at the RPC and REST boundaries, even though the allocator and instance model already understood `Ipv6Only` and `DualStack`. The agent also assumed every tenant interface had the old IPv4 compatibility fields, so letting those modes through would have produced empty addresses in NVUE, status, and DHCP configuration. So, this enables all three family modes end to end. Core emits the family-neutral `addresses` list for IPv6-only interfaces, the agent treats the IPv4 compatibility tuple as optional, and REST preserves the selected mode through create, update, batch create, and reconciliation. Existing IPv4 and dual-written payloads keep working during the rollout. This does not add DHCPv6 or admin-network IPv6 support; those stay with their existing follow-up work. Tests cover the RPC/REST boundaries, allocation and persistence, compatibility projections, DHCP validation, status, and IPv6-only FNN rendering. This supports https://github.com/NVIDIA/infra-controller/issues/2402 Signed-off-by: Chet Nichols III --- Cargo.lock | 1 + crates/agent/src/dhcp_server_grpc_client.rs | 4 +- crates/agent/src/ethernet_virtualization.rs | 63 ++- crates/agent/src/nvue.rs | 42 +- crates/agent/src/periodic_config_fetcher.rs | 51 ++- .../api-core/src/ethernet_virtualization.rs | 119 ++++-- .../src/tests/common/api_fixtures/mod.rs | 31 +- crates/api-core/src/tests/instance.rs | 368 +++++++++++------- .../api-core/src/tests/instance_allocate.rs | 14 + crates/api-db/src/instance_address.rs | 8 +- .../api-model/src/instance/status/network.rs | 111 ++++-- .../proto/dhcp_server_control.proto | 11 +- crates/dhcp-server/src/grpc_server.rs | 84 +++- crates/dhcp-server/src/modes/dpu.rs | 157 +++++++- crates/dhcp-server/src/packet_handler.rs | 36 +- .../src/machine_state_machine.rs | 4 +- crates/network/src/virtualization.rs | 19 +- crates/rpc-utils/src/dhcp.rs | 131 +++++-- crates/rpc/proto/forge.proto | 36 +- .../rpc/src/model/instance/config/network.rs | 27 +- .../rpc/src/model/instance/status/network.rs | 76 +++- crates/test-harness/Cargo.toml | 9 +- crates/test-harness/src/machine_dpu.rs | 31 +- rest-api/api/pkg/api/handler/instance.go | 4 +- rest-api/api/pkg/api/handler/instance_test.go | 36 +- rest-api/api/pkg/api/handler/instancebatch.go | 2 +- .../api/pkg/api/handler/instancebatch_test.go | 10 +- rest-api/api/pkg/api/model/interface.go | 30 +- rest-api/api/pkg/api/model/interface_test.go | 47 ++- rest-api/docs/index.html | 14 +- rest-api/openapi/spec.yaml | 12 +- rest-api/proto/core/gen/v1/nico_nico.pb.go | 36 +- rest-api/proto/core/src/v1/nico_nico.proto | 36 +- .../model_interface_create_request.go | 2 +- .../pkg/activity/instance/instance.go | 22 +- .../pkg/activity/instance/instance_test.go | 41 ++ 36 files changed, 1242 insertions(+), 483 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b53a2f58f0..5d9b71eb19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3352,6 +3352,7 @@ dependencies = [ "carbide-api-db", "carbide-api-model", "carbide-macros", + "carbide-network", "carbide-network-segment-controller", "carbide-site-explorer", "carbide-sqlx-testing", diff --git a/crates/agent/src/dhcp_server_grpc_client.rs b/crates/agent/src/dhcp_server_grpc_client.rs index f6328fdd2c..65942b1360 100644 --- a/crates/agent/src/dhcp_server_grpc_client.rs +++ b/crates/agent/src/dhcp_server_grpc_client.rs @@ -81,8 +81,8 @@ impl From for proto::InterfaceInfoV6 { impl From for proto::InterfaceInfo { fn from(i: ModelInterfaceInfo) -> Self { proto::InterfaceInfo { - address: i.address.to_string(), - gateway: i.gateway.to_string(), + address: i.address.map(|address| address.to_string()), + gateway: i.gateway.map(|gateway| gateway.to_string()), prefix: i.prefix, fqdn: i.fqdn, booturl: i.booturl, diff --git a/crates/agent/src/ethernet_virtualization.rs b/crates/agent/src/ethernet_virtualization.rs index 41e575bc5e..626d3f50b8 100644 --- a/crates/agent/src/ethernet_virtualization.rs +++ b/crates/agent/src/ethernet_virtualization.rs @@ -480,8 +480,8 @@ pub(super) async fn update_nvue( } }; - // For dual-stack FNN, the DPU-side IPv6 address is the network address - // of the /127 linknet (the ::0 end). The ::1 end is the host. + // For FNN interfaces with IPv6, the DPU-side address is the network + // address of the /127 linknet (the ::0 end). The ::1 end is the host. ifs.push(nvue::PortConfig { interface_name: name, is_phy: net.function_type == rpc::InterfaceFunctionType::Physical as i32, @@ -1137,7 +1137,7 @@ pub(super) async fn interfaces( mac_address: Some(factory_mac_address.to_string()), addresses, prefixes, - gateways: vec![iface.gateway.clone()], + gateways: build_dual_stack_list(iface.gateway.clone(), None), network_security_group: None, internal_uuid: iface.internal_uuid.clone(), }); @@ -1224,7 +1224,7 @@ pub(super) async fn interfaces( mac_address: mac, addresses, prefixes, - gateways: vec![iface.gateway.clone()], + gateways: build_dual_stack_list(iface.gateway.clone(), None), network_security_group, internal_uuid: iface.internal_uuid.clone(), }); @@ -1239,7 +1239,7 @@ pub(super) fn tenant_peers(network_config: &rpc::ManagedHostNetworkConfigRespons network_config .tenant_interfaces .iter() - .map(|iface| iface.ip.as_str()) + .filter_map(|iface| (!iface.ip.is_empty()).then_some(iface.ip.as_str())) .collect() } @@ -1838,7 +1838,6 @@ mod tests { use ::rpc::{common as rpc_common, forge as rpc}; use carbide_network::virtualization::{VpcVirtualizationType, get_svi_ip}; use carbide_rpc_utils::dhcp::{DhcpConfig, HostConfig}; - use carbide_utils::none_if_empty::NoneIfEmpty; use eyre::WrapErr; use ipnetwork::IpNetwork; @@ -3716,36 +3715,32 @@ mod tests { } } - #[test] - fn test_dual_stack_addresses_building() { - // Verify the iterator-based pattern used to build dual-stack address/prefix vectors. - let ip = "10.0.0.1".to_string(); - let ip6 = Some("2001:db8::1".to_string()); - let interface_prefix = "10.0.0.0/31".to_string(); - let interface_prefix_v6 = Some("2001:db8::/127".to_string()); - - let addresses: Vec = std::iter::once(ip.clone()) - .chain(ip6.none_if_empty()) - .collect(); - assert_eq!(addresses, vec!["10.0.0.1", "2001:db8::1"]); + #[tokio::test] + #[allow(deprecated)] + async fn ipv6_only_status_omits_empty_ipv4_compatibility_values() { + let network_config = rpc::ManagedHostNetworkConfigResponse { + tenant_interfaces: vec![rpc::FlatInterfaceConfig { + function_type: rpc::InterfaceFunctionType::Physical.into(), + vlan_id: 100, + ipv6_interface_config: Some(rpc::FlatInterfaceIpv6Config { + ip: "2001:db8::1".to_string(), + interface_prefix: "2001:db8::/127".to_string(), + svi_ip: None, + }), + ..Default::default() + }], + ..Default::default() + }; - let prefixes: Vec = std::iter::once(interface_prefix) - .chain(interface_prefix_v6.none_if_empty()) - .collect(); - assert_eq!(prefixes, vec!["10.0.0.0/31", "2001:db8::/127"]); + assert!(tenant_peers(&network_config).is_empty()); - // Verify empty ip6 is not included. - let empty_ip6: Option = Some("".to_string()); - let addresses2: Vec = std::iter::once(ip) - .chain(empty_ip6.none_if_empty()) - .collect(); - assert_eq!(addresses2, vec!["10.0.0.1"]); + let observations = interfaces(&network_config, "02:00:00:00:00:01".parse().unwrap(), None) + .await + .unwrap(); - // Verify None ip6 is not included. - let none_ip6: Option = None; - let addresses3: Vec = std::iter::once("10.0.0.1".to_string()) - .chain(none_ip6.none_if_empty()) - .collect(); - assert_eq!(addresses3, vec!["10.0.0.1"]); + assert_eq!(observations.len(), 1); + assert_eq!(observations[0].addresses, vec!["2001:db8::1"]); + assert_eq!(observations[0].prefixes, vec!["2001:db8::/127"]); + assert!(observations[0].gateways.is_empty()); } } diff --git a/crates/agent/src/nvue.rs b/crates/agent/src/nvue.rs index fc4c945864..3d3c3ad3d8 100644 --- a/crates/agent/src/nvue.rs +++ b/crates/agent/src/nvue.rs @@ -387,7 +387,9 @@ pub fn build(conf: NvueConfig) -> eyre::Result { RoutingProfile: interface_routing_profile.unwrap_or_default(), IsPhy: network.is_phy, L2VNI: network.vni.map(|x| x.to_string()).unwrap_or("".to_string()), - IPs: vec![network.gateway_cidr.clone()], + IPs: std::iter::once(network.gateway_cidr.clone()) + .filter(|address| !address.is_empty()) + .collect(), IPsIpv6: network .ipv6_port_config .as_ref() @@ -1262,7 +1264,7 @@ pub struct PortConfig { pub vni: Option, // In FNN, admin network has both an l2vni and an l3vni pub l3_vni: Option, pub gateway_cidr: String, - /// Optional IPv6 configuration for dual-stack interfaces. + /// Optional IPv6 configuration for interfaces that include IPv6. pub ipv6_port_config: Option, pub vpc_prefixes: Vec, pub vpc_peer_prefixes: Vec, @@ -1554,7 +1556,7 @@ struct TmplVpc { struct TmplHostInterfaces { ID: u32, HostIP: String, - /// IPv6 host address (if dual-stack). + /// IPv6 host address, when configured. HostIPv6: Option, // HostRoute in the context of FNN-L3 is the /31 prefix allocation. @@ -2316,6 +2318,40 @@ mod tests { ); } + #[test] + fn test_build_fnn_ipv6_only_interface() { + let mut conf = dual_stack_fnn_config(); + let port = conf + .ct_port_configs + .first_mut() + .expect("fixture should have a port"); + port.host_ip.clear(); + port.host_route.clear(); + port.gateway_cidr.clear(); + port.svi_ip = None; + port.vpc_prefixes + .retain(|prefix| matches!(prefix.parse::(), Ok(IpNet::V6(_)))); + + let vlan = conf + .ct_access_vlans + .first_mut() + .expect("fixture should have an access VLAN"); + vlan.ip.clear(); + vlan.network.clear(); + + let output = build(conf).expect("build should succeed"); + let docs: serde_yaml::Value = + serde_yaml::from_str(&output).expect("output should be valid YAML"); + let set = &docs.as_sequence().unwrap()[1]["set"]; + + assert_eq!( + yaml_mapping_keys(&set["interface"]["pf0vf0_if"]["ip"]["address"]), + address_set(&["2001:db8::0/127"]), + ); + let neighbors = &set["vrf"]["vpc_100"]["router"]["bgp"]["neighbor"]; + assert_eq!(yaml_mapping_keys(neighbors), address_set(&["2001:db8::1"]),); + } + #[test] fn test_build_fnn_dual_stack_l2_interface() { let mut conf = dual_stack_fnn_config(); diff --git a/crates/agent/src/periodic_config_fetcher.rs b/crates/agent/src/periodic_config_fetcher.rs index 9634fb1154..7187d1f48b 100644 --- a/crates/agent/src/periodic_config_fetcher.rs +++ b/crates/agent/src/periodic_config_fetcher.rs @@ -333,19 +333,21 @@ fn normalize_interface_addresses( } } - let ipv4 = ipv4.ok_or_else(|| { - eyre::eyre!( - "IPv4 address configuration is required; IPv6-only agent support is tracked by https://github.com/NVIDIA/infra-controller/issues/2402" - ) - })?; - - interface.gateway.clone_from(&ipv4.gateway); - interface.ip.clone_from(&ipv4.ip); - interface - .interface_prefix - .clone_from(&ipv4.interface_prefix); - interface.prefix.clone_from(&ipv4.prefix); - interface.svi_ip.clone_from(&ipv4.svi_ip); + if let Some(ipv4) = ipv4 { + interface.gateway.clone_from(&ipv4.gateway); + interface.ip.clone_from(&ipv4.ip); + interface + .interface_prefix + .clone_from(&ipv4.interface_prefix); + interface.prefix.clone_from(&ipv4.prefix); + interface.svi_ip.clone_from(&ipv4.svi_ip); + } else { + interface.gateway.clear(); + interface.ip.clear(); + interface.interface_prefix.clear(); + interface.prefix.clear(); + interface.svi_ip = None; + } let prefixless_legacy_ipv6 = interface .ipv6_interface_config .clone() @@ -594,6 +596,21 @@ mod tests { } } + // Describes an authoritative IPv6-only projection with cleared IPv4 compatibility fields. + #[allow(deprecated)] + fn expected_ipv6_interface() -> rpc::FlatInterfaceConfig { + rpc::FlatInterfaceConfig { + vlan_id: 100, + ipv6_interface_config: Some(rpc::FlatInterfaceIpv6Config { + ip: "2001:db8::1".to_string(), + interface_prefix: "2001:db8::/127".to_string(), + svi_ip: Some("2001:db8::2/64".to_string()), + }), + addresses: vec![ipv6_address()], + ..Default::default() + } + } + // Exercises authoritative clearing of the deprecated optional SVI field. #[test] #[allow(deprecated)] @@ -643,6 +660,9 @@ mod tests { "reversed dual-stack list is selected by family without reordering" { legacy_interface(vec![ipv6_address(), ipv4_address()]) => Yields(dual_stack), } + "IPv6-only list clears stale IPv4 compatibility fields" { + legacy_interface(vec![ipv6_address()]) => Yields(expected_ipv6_interface()), + } "absent V4 SVI clears the legacy value" { legacy_interface(vec![ipv4_without_svi]) => Yields(expected_ipv4_without_svi), } @@ -663,9 +683,6 @@ mod tests { unknown.address_family = 99; scenarios!(run = normalized_interface; - "V4 is required during the compatibility phase" { - legacy_interface(vec![ipv6_address()]) => Fails, - } "explicit family is required" { legacy_interface(vec![unspecified]) => Fails, } @@ -688,6 +705,7 @@ mod tests { tenant_interfaces: vec![ legacy_interface(vec![ipv4_address()]), legacy_interface(vec![ipv6_address(), ipv4_address()]), + legacy_interface(vec![ipv6_address()]), ], ..Default::default() }; @@ -708,6 +726,7 @@ mod tests { svi_ip: Some("2001:db8::2/64".to_string()), }) ); + assert_eq!(response.tenant_interfaces[2], expected_ipv6_interface()); } #[test] diff --git a/crates/api-core/src/ethernet_virtualization.rs b/crates/api-core/src/ethernet_virtualization.rs index 7e4ec10d84..de78983856 100644 --- a/crates/api-core/src/ethernet_virtualization.rs +++ b/crates/api-core/src/ethernet_virtualization.rs @@ -177,9 +177,9 @@ pub(crate) async fn validate_instance_interface_routing_profiles( Ok(()) } -/// Groups the optional IPv4 prefix with the optional IPv6 prefix for a -/// dual-stack network segment, and provides convenience methods for -/// extracting addresses and interface prefixes from an InstanceInterfaceConfig. +/// Groups the optional IPv4 and IPv6 prefixes for a network segment and +/// provides convenience methods for extracting addresses and interface +/// prefixes from an InstanceInterfaceConfig. struct PrefixPair<'a> { v4: Option<&'a NetworkPrefix>, v6: Option<&'a NetworkPrefix>, @@ -297,14 +297,18 @@ fn interface_address_configs( config: &rpc::FlatInterfaceConfig, ipv6_segment_prefix: Option<&str>, ) -> Vec { - let mut addresses = vec![rpc::InterfaceAddressConfig { - address_family: rpc::AddressFamily::V4.into(), - gateway: config.gateway.clone(), - ip: config.ip.clone(), - interface_prefix: config.interface_prefix.clone(), - prefix: config.prefix.clone(), - svi_ip: config.svi_ip.clone(), - }]; + let mut addresses = Vec::with_capacity(2); + + if !config.interface_prefix.is_empty() { + addresses.push(rpc::InterfaceAddressConfig { + address_family: rpc::AddressFamily::V4.into(), + gateway: config.gateway.clone(), + ip: config.ip.clone(), + interface_prefix: config.interface_prefix.clone(), + prefix: config.prefix.clone(), + svi_ip: config.svi_ip.clone(), + }); + } if let Some(ipv6) = config.ipv6_interface_config.as_ref() && !ipv6.interface_prefix.is_empty() @@ -548,7 +552,8 @@ pub(crate) async fn admin_network( } #[allow(clippy::too_many_arguments)] -// This writer keeps the deprecated fields populated for older agents during the rollout. +// This writer keeps the deprecated IPv4 fields populated when IPv4 is configured so older +// agents can consume IPv4-only and dual-stack payloads during the rollout. #[allow(deprecated)] pub(crate) async fn tenant_network( txn: &mut PgConnection, @@ -568,12 +573,15 @@ pub(crate) async fn tenant_network( let is_l2_segment = segment.status.can_stretch.unwrap_or(true); let ds = PrefixPair::from_segment_prefixes(&segment.prefixes, instance_id, segment.id)?; - let address = ds.v4_address(iface).ok_or_else(|| CarbideError::Internal { - message: format!( - "No IPv4 address is available for instance {instance_id} on segment {}", - segment.id, - ), - })?; + let address = match ds.v4() { + Some(_) => Some(ds.v4_address(iface).ok_or_else(|| CarbideError::Internal { + message: format!( + "no IPv4 address is available for instance {instance_id} on segment {}", + segment.id, + ), + })?), + None => None, + }; // If not, default to a /32 -- backwards compatibility for instances // configured before interface_prefixes were introduced. @@ -581,13 +589,17 @@ pub(crate) async fn tenant_network( // TODO(chet): This can eventually be phased out once all of the // InstanceInterfaceConfigs stored contain the prefix. let interface_prefix = - ds.v4_interface_prefix(iface, address)? - .ok_or_else(|| CarbideError::Internal { - message: format!( - "No IPv4 prefix is available for instance {instance_id} on segment {}", - segment.id, - ), - })?; + match address { + Some(address) => Some(ds.v4_interface_prefix(iface, address)?.ok_or_else(|| { + CarbideError::Internal { + message: format!( + "no IPv4 prefix is available for instance {instance_id} on segment {}", + segment.id, + ), + } + })?), + None => None, + }; let v6_address = ds.v6_address(iface); let v6_interface_prefix = ds.v6_interface_prefix(iface); @@ -776,8 +788,12 @@ pub(crate) async fn tenant_network( .v4() .map(|p| p.gateway_cidr().unwrap_or_default()) .unwrap_or_default(), - ip: address.to_string(), - interface_prefix: interface_prefix.to_string(), + ip: address + .map(|address| address.to_string()) + .unwrap_or_default(), + interface_prefix: interface_prefix + .map(|prefix| prefix.to_string()) + .unwrap_or_default(), vpc_prefixes, prefix: ds.v4().map(|p| p.prefix.to_string()).unwrap_or_default(), // FIXME: Right now we are sending instance IP as hostname. This should be replaced by @@ -886,6 +902,25 @@ mod test { } } + fn ipv6_interface_config() -> rpc::FlatInterfaceIpv6Config { + rpc::FlatInterfaceIpv6Config { + ip: "2001:db8::1".to_string(), + interface_prefix: "2001:db8::/127".to_string(), + svi_ip: Some("2001:db8::2/64".to_string()), + } + } + + fn ipv6_address_config() -> rpc::InterfaceAddressConfig { + rpc::InterfaceAddressConfig { + address_family: rpc::AddressFamily::V6.into(), + gateway: "2001:db8::/127".to_string(), + ip: "2001:db8::1".to_string(), + interface_prefix: "2001:db8::/127".to_string(), + prefix: "2001:db8::/64".to_string(), + svi_ip: Some("2001:db8::2/64".to_string()), + } + } + #[test] fn interface_address_configs_mirror_legacy_fields_in_family_order() { value_scenarios!( @@ -895,6 +930,18 @@ mod test { "IPv4-only config" { (legacy_interface_config(None), None) => vec![ipv4_address_config()], } + "no configured address families" { + (rpc::FlatInterfaceConfig::default(), None) => vec![], + } + "IPv6-only config" { + ( + rpc::FlatInterfaceConfig { + ipv6_interface_config: Some(ipv6_interface_config()), + ..Default::default() + }, + Some("2001:db8::/64"), + ) => vec![ipv6_address_config()], + } "IPv6 segment without an interface address" { (legacy_interface_config(None), Some("2001:db8::/64")) => vec![ipv4_address_config()], } @@ -910,23 +957,9 @@ mod test { } "dual-stack config" { ( - legacy_interface_config(Some(rpc::FlatInterfaceIpv6Config { - ip: "2001:db8::1".to_string(), - interface_prefix: "2001:db8::/127".to_string(), - svi_ip: Some("2001:db8::2/64".to_string()), - })), + legacy_interface_config(Some(ipv6_interface_config())), Some("2001:db8::/64"), - ) => vec![ - ipv4_address_config(), - rpc::InterfaceAddressConfig { - address_family: rpc::AddressFamily::V6.into(), - gateway: "2001:db8::/127".to_string(), - ip: "2001:db8::1".to_string(), - interface_prefix: "2001:db8::/127".to_string(), - prefix: "2001:db8::/64".to_string(), - svi_ip: Some("2001:db8::2/64".to_string()), - }, - ], + ) => vec![ipv4_address_config(), ipv6_address_config()], } ); } diff --git a/crates/api-core/src/tests/common/api_fixtures/mod.rs b/crates/api-core/src/tests/common/api_fixtures/mod.rs index b81a3900a1..bdd1176947 100644 --- a/crates/api-core/src/tests/common/api_fixtures/mod.rs +++ b/crates/api-core/src/tests/common/api_fixtures/mod.rs @@ -37,6 +37,7 @@ use carbide_machine_controller::handler::{ MachineStateHandler, MachineStateHandlerBuilder, PowerOptionConfig, ReachabilityParams, }; use carbide_machine_controller::io::MachineStateControllerIO; +use carbide_network::virtualization::build_dual_stack_list; use carbide_network_segment_controller::context::NetworkSegmentStateHandlerServices; use carbide_network_segment_controller::handler::NetworkSegmentStateHandler; use carbide_network_segment_controller::io::NetworkSegmentStateControllerIO; @@ -2254,9 +2255,18 @@ pub(in crate::tests) async fn network_configured_with_health_and_ext_services( function_type: iface.function_type, virtual_function_id: None, mac_address: None, - addresses: vec![iface.ip.clone()], - prefixes: vec![iface.interface_prefix.clone()], - gateways: vec![iface.gateway.clone()], + addresses: build_dual_stack_list( + iface.ip.clone(), + iface.ipv6_interface_config.as_ref().map(|v6| v6.ip.clone()), + ), + prefixes: build_dual_stack_list( + iface.interface_prefix.clone(), + iface + .ipv6_interface_config + .as_ref() + .map(|v6| v6.interface_prefix.clone()), + ), + gateways: build_dual_stack_list(iface.gateway.clone(), None), network_security_group: None, internal_uuid: iface.internal_uuid.clone(), }] @@ -2267,9 +2277,18 @@ pub(in crate::tests) async fn network_configured_with_health_and_ext_services( function_type: iface.function_type, virtual_function_id: iface.virtual_function_id, mac_address: None, - addresses: vec![iface.ip.clone()], - prefixes: vec![iface.interface_prefix.clone()], - gateways: vec![iface.gateway.clone()], + addresses: build_dual_stack_list( + iface.ip.clone(), + iface.ipv6_interface_config.as_ref().map(|v6| v6.ip.clone()), + ), + prefixes: build_dual_stack_list( + iface.interface_prefix.clone(), + iface + .ipv6_interface_config + .as_ref() + .map(|v6| v6.interface_prefix.clone()), + ), + gateways: build_dual_stack_list(iface.gateway.clone(), None), network_security_group: None, internal_uuid: iface.internal_uuid.clone(), }); diff --git a/crates/api-core/src/tests/instance.rs b/crates/api-core/src/tests/instance.rs index eb3b3d50b9..b739da01c3 100644 --- a/crates/api-core/src/tests/instance.rs +++ b/crates/api-core/src/tests/instance.rs @@ -3090,7 +3090,7 @@ async fn test_vpc_prefix_handling(pool: PgPool) { } /// Verifies automatic selection remains deterministic and tenant-scoped across -/// persistence while gated IPv6 modes exercise the same resolver internally. +/// every supported address-family mode. #[crate::sqlx_test] async fn test_auto_vpc_prefix_selection_uses_static_first_fit(pool: PgPool) { let fixture = create_auto_vpc_selection_fixture(pool).await; @@ -3099,19 +3099,14 @@ async fn test_auto_vpc_prefix_selection_uses_static_first_fit(pool: PgPool) { // persisted RPC projection, and final exhaustion. assert_static_ipv4_first_fit(&fixture).await; assert_explicit_prefix_tenant_ownership(&fixture).await; - let allocated_instance = allocate_and_assert_auto_vpc_instance(&fixture).await; + allocate_and_assert_auto_vpc_instance(&fixture).await; assert_ipv4_candidates_exhausted(&fixture).await; - // Add fresh family capacity and exercise the IPv6 internals that remain - // gated at the request layer. + // Add fresh family capacity and exercise IPv6-only and dual-stack through + // the public allocation boundary. let dual_stack_prefixes = add_dual_stack_prefix_capacity(&fixture).await; - assert_ipv6_only_resolution( - &fixture, - &allocated_instance, - dual_stack_prefixes.ipv6_prefix_id, - ) - .await; - assert_dual_stack_resolution(&fixture, &allocated_instance, &dual_stack_prefixes).await; + assert_ipv6_only_resolution(&fixture, dual_stack_prefixes.ipv6_prefix_id).await; + assert_dual_stack_resolution(&fixture, &dual_stack_prefixes).await; } /// Verifies a rare overlap from outside parent-prefix serialization retries @@ -3449,13 +3444,6 @@ struct AutoVpcSelectionFixture { tenant_organization_id: TenantOrganizationId, } -/// Retains persisted instance and host identity because internal IPv6-only and -/// dual-stack address allocation requires both. -struct AutoVpcAllocatedInstance { - managed_host: TestManagedHost, - instance_id: InstanceId, -} - /// Holds fresh per-family candidates added after IPv4 exhaustion so future /// family-mode checks cannot perturb the initial first-fit coverage. struct AutoVpcDualStackPrefixes { @@ -3588,35 +3576,39 @@ async fn assert_explicit_prefix_tenant_ownership(fixture: &AutoVpcSelectionFixtu txn.rollback().await.unwrap(); } +/// Allocates one instance through the public automatic VPC selector. +async fn allocate_auto_vpc_instance( + fixture: &AutoVpcSelectionFixture, + family_mode: rpc::forge::InstanceInterfaceIpFamilyMode, + name: &str, +) -> (TestManagedHost, rpc::Instance) { + let managed_host = create_managed_host(&fixture.env).await; + let (_, instance) = managed_host + .instance_builer(&fixture.env) + .tenant_org(FIXTURE_TENANT_ORG_ID) + .network(automatic_rpc_network_config(fixture.vpc_id, family_mode)) + .metadata(rpc::Metadata { + name: name.to_string(), + description: "tests/instance".to_string(), + labels: Vec::new(), + }) + .build_and_return() + .await; + + (managed_host, instance.into_inner()) +} + /// Allocates and re-reads through the public boundary so intent and resolution /// are proven to persist beyond the allocation response. -async fn allocate_and_assert_auto_vpc_instance( - fixture: &AutoVpcSelectionFixture, -) -> AutoVpcAllocatedInstance { +async fn allocate_and_assert_auto_vpc_instance(fixture: &AutoVpcSelectionFixture) { // Allocate the final original IPv4 linknet through the public selector and // verify its immediate projection. - let managed_host = create_managed_host(&fixture.env).await; - let instance = fixture - .env - .api - .allocate_instance( - InstanceAllocationRequest::builder(false) - .machine_id(managed_host.id) - .config( - InstanceConfig::default_tenant_and_os() - .tenant(fixture_tenant_config()) - .network(automatic_ipv4_rpc_network_config(fixture.vpc_id)), - ) - .metadata(rpc::Metadata { - name: "automatic-vpc-prefix-selection".to_string(), - description: "tests/instance".to_string(), - labels: Vec::new(), - }) - .tonic_request(), - ) - .await - .unwrap() - .into_inner(); + let (_, instance) = allocate_auto_vpc_instance( + fixture, + rpc::forge::InstanceInterfaceIpFamilyMode::Ipv4Only, + "automatic-vpc-prefix-selection", + ) + .await; assert_ipv4_auto_rpc_resolution(&instance, fixture.vpc_id, fixture.higher_ipv4_prefix_id); // Re-read through FindInstancesByIds to verify persisted intent and resolution. @@ -3627,11 +3619,6 @@ async fn allocate_and_assert_auto_vpc_instance( fixture.vpc_id, fixture.higher_ipv4_prefix_id, ); - - AutoVpcAllocatedInstance { - managed_host, - instance_id, - } } /// Confirms original IPv4 capacity is exhausted so later family-mode checks @@ -3682,34 +3669,56 @@ async fn add_dual_stack_prefix_capacity( } } -/// Exercises IPv6-only resolution and odd-address assignment internally while -/// its public path remains gated pending end-to-end IPv6 support. +/// Exercises IPv6-only resolution, persistence, and DPU config rendering +/// through the public allocation boundary. +#[allow(deprecated)] async fn assert_ipv6_only_resolution( fixture: &AutoVpcSelectionFixture, - allocated_instance: &AutoVpcAllocatedInstance, ipv6_prefix_id: VpcPrefixId, ) { - // Exercise IPv6-only allocation directly (the request layer still rejects - // this mode) and keep its selected prefix in the legacy primary arm. - let mut ipv6_only_config = - automatic_network_config(fixture.vpc_id, InstanceInterfaceIpFamilyMode::Ipv6Only); - let mut txn = fixture.env.db_txn().await; - allocate_network( - &mut ipv6_only_config, - &fixture.tenant_organization_id, - &mut txn, + let (managed_host, instance) = allocate_auto_vpc_instance( + fixture, + rpc::forge::InstanceInterfaceIpFamilyMode::Ipv6Only, + "automatic-ipv6-only-selection", ) - .await - .unwrap(); - let ipv6_only_interface = &ipv6_only_config.interfaces[0]; - assert_eq!( - ipv6_only_interface.network_details, - Some(NetworkDetails::VpcPrefixId(ipv6_prefix_id)), + .await; + let ipv6_only_segment_id = assert_auto_rpc_resolution( + &instance, + fixture.vpc_id, + rpc::forge::InstanceInterfaceIpFamilyMode::Ipv6Only, + None, + Some(ipv6_prefix_id), ); - assert!(ipv6_only_interface.ipv6_interface_config.is_none()); - // The generated segment must contain exactly the selected IPv6 linknet. - let ipv6_only_segment_id = ipv6_only_interface.network_segment_id.unwrap(); + // Re-read through the public API to prove the family intent and selected + // prefix persisted beyond the allocation response. + let instance_id = instance.id.unwrap(); + let persisted = fixture.env.one_instance(instance_id).await; + assert_auto_rpc_resolution( + persisted.inner(), + fixture.vpc_id, + rpc::forge::InstanceInterfaceIpFamilyMode::Ipv6Only, + None, + Some(ipv6_prefix_id), + ); + let status = persisted.status(); + let status_interface = &status.network().interfaces[0]; + assert_eq!(status_interface.addresses.len(), 1); + assert!( + status_interface.addresses[0] + .parse::() + .unwrap() + .is_ipv6() + ); + assert_eq!(status_interface.prefixes.len(), 1); + assert!( + status_interface.prefixes[0] + .parse::() + .unwrap() + .is_ipv6() + ); + + let mut txn = fixture.env.db_txn().await; let ipv6_only_segment = db::network_segment::find_by( txn.as_mut(), ObjectColumnFilter::One(IdColumn, &ipv6_only_segment_id), @@ -3720,69 +3729,113 @@ async fn assert_ipv6_only_resolution( assert_eq!(ipv6_only_segment[0].prefixes.len(), 1); assert!(ipv6_only_segment[0].prefixes[0].prefix.is_ipv6()); - // Reuse the persisted instance and host to exercise internal address assignment. - let host = allocated_instance - .managed_host - .host() - .db_machine(&mut txn) - .await; - ipv6_only_config = db::instance_network_config::with_allocated_ips( - ipv6_only_config, - txn.as_mut(), - allocated_instance.instance_id, - &host, - ) - .await - .unwrap(); let ipv6_only_addresses = db::instance_address::find_by_segment_id(txn.as_mut(), &ipv6_only_segment_id) .await .unwrap(); - // Config and persistence must each contain one address, with an odd IPv6 host persisted. - assert_eq!(ipv6_only_config.interfaces[0].ip_addrs.len(), 1); + // Persistence contains one odd IPv6 host address from the selected /127. assert_eq!(ipv6_only_addresses.len(), 1); assert!(matches!( ipv6_only_addresses[0].address, IpAddr::V6(address) if address.to_bits() & 1 == 1 )); txn.commit().await.unwrap(); + + // Core publishes only the real V6 family and leaves deprecated V4 fields + // empty rather than fabricating an unusable V4 address configuration. + let managed_config = fixture + .env + .api + .get_managed_host_network_config(Request::new( + rpc::forge::ManagedHostNetworkConfigRequest { + dpu_machine_id: managed_host.dpu().id.into(), + }, + )) + .await + .unwrap() + .into_inner(); + let [tenant_interface] = managed_config.tenant_interfaces.as_slice() else { + panic!("expected one IPv6-only tenant interface"); + }; + assert!(tenant_interface.gateway.is_empty()); + assert!(tenant_interface.ip.is_empty()); + assert!(tenant_interface.interface_prefix.is_empty()); + assert!(tenant_interface.prefix.is_empty()); + assert!(tenant_interface.svi_ip.is_none()); + let [address] = tenant_interface.addresses.as_slice() else { + panic!("expected one IPv6 tenant address configuration"); + }; + assert_eq!( + rpc::forge::AddressFamily::try_from(address.address_family).unwrap(), + rpc::forge::AddressFamily::V6, + ); + assert!(address.ip.parse::().unwrap().is_ipv6()); + assert_eq!( + tenant_interface + .ipv6_interface_config + .as_ref() + .map(|config| config.ip.as_str()), + Some(address.ip.as_str()), + ); } -/// Exercises dual-stack resolution and per-family address assignment internally -/// while its public path remains gated pending end-to-end IPv6 support. +/// Exercises dual-stack resolution and per-family address assignment through +/// the public allocation boundary. +// This test deliberately verifies the deprecated V4 compatibility projection. +#[allow(deprecated)] async fn assert_dual_stack_resolution( fixture: &AutoVpcSelectionFixture, - allocated_instance: &AutoVpcAllocatedInstance, prefixes: &AutoVpcDualStackPrefixes, ) { - // IPv6-only consumed the first /127. Resolve dual stack internally with IPv4 - // as primary and the remaining IPv6 /127 augmenting its generated segment. - let mut dual_stack_config = - automatic_network_config(fixture.vpc_id, InstanceInterfaceIpFamilyMode::DualStack); - let mut txn = fixture.env.db_txn().await; - allocate_network( - &mut dual_stack_config, - &fixture.tenant_organization_id, - &mut txn, + // IPv6-only consumed the first /127. Dual stack uses IPv4 as primary and + // attaches the remaining IPv6 /127 to the same generated segment. + let (managed_host, instance) = allocate_auto_vpc_instance( + fixture, + rpc::forge::InstanceInterfaceIpFamilyMode::DualStack, + "automatic-dual-stack-selection", ) - .await - .unwrap(); - let dual_stack_interface = &dual_stack_config.interfaces[0]; + .await; + let dual_stack_segment_id = assert_auto_rpc_resolution( + &instance, + fixture.vpc_id, + rpc::forge::InstanceInterfaceIpFamilyMode::DualStack, + Some(prefixes.ipv4_prefix_id), + Some(prefixes.ipv6_prefix_id), + ); + + let instance_id = instance.id.unwrap(); + let persisted = fixture.env.one_instance(instance_id).await; + assert_auto_rpc_resolution( + persisted.inner(), + fixture.vpc_id, + rpc::forge::InstanceInterfaceIpFamilyMode::DualStack, + Some(prefixes.ipv4_prefix_id), + Some(prefixes.ipv6_prefix_id), + ); + let status = persisted.status(); + let status_interface = &status.network().interfaces[0]; + assert_eq!(status_interface.addresses.len(), 2); assert_eq!( - dual_stack_interface.network_details, - Some(NetworkDetails::VpcPrefixId(prefixes.ipv4_prefix_id)), + status_interface + .addresses + .iter() + .filter(|address| address.parse::().unwrap().is_ipv4()) + .count(), + 1, ); + assert_eq!(status_interface.prefixes.len(), 2); assert_eq!( - dual_stack_interface - .ipv6_interface_config - .as_ref() - .map(|config| config.vpc_prefix_id), - Some(prefixes.ipv6_prefix_id), + status_interface + .prefixes + .iter() + .filter(|prefix| prefix.parse::().unwrap().is_ipv4()) + .count(), + 1, ); // Both selected family linknets must share one generated segment. - let dual_stack_segment_id = dual_stack_interface.network_segment_id.unwrap(); + let mut txn = fixture.env.db_txn().await; let dual_stack_segment = db::network_segment::find_by( txn.as_mut(), ObjectColumnFilter::One(IdColumn, &dual_stack_segment_id), @@ -3808,27 +3861,12 @@ async fn assert_dual_stack_resolution( 1, ); - // Reuse the persisted instance and host to exercise internal address assignment. - let host = allocated_instance - .managed_host - .host() - .db_machine(&mut txn) - .await; - dual_stack_config = db::instance_network_config::with_allocated_ips( - dual_stack_config, - txn.as_mut(), - allocated_instance.instance_id, - &host, - ) - .await - .unwrap(); let dual_stack_addresses = db::instance_address::find_by_segment_id(txn.as_mut(), &dual_stack_segment_id) .await .unwrap(); - // Config must expose two addresses; persistence must contain one per family. - assert_eq!(dual_stack_config.interfaces[0].ip_addrs.len(), 2); + // Persistence contains one address per family. assert_eq!(dual_stack_addresses.len(), 2); assert_eq!( dual_stack_addresses @@ -3842,10 +3880,44 @@ async fn assert_dual_stack_resolution( IpAddr::V6(address) if address.to_bits() & 1 == 1 ))); txn.commit().await.unwrap(); + + let managed_config = fixture + .env + .api + .get_managed_host_network_config(Request::new( + rpc::forge::ManagedHostNetworkConfigRequest { + dpu_machine_id: managed_host.dpu().id.into(), + }, + )) + .await + .unwrap() + .into_inner(); + let [tenant_interface] = managed_config.tenant_interfaces.as_slice() else { + panic!("expected one dual-stack tenant interface"); + }; + let [ipv4_address, ipv6_address] = tenant_interface.addresses.as_slice() else { + panic!("expected IPv4 and IPv6 tenant address configurations"); + }; + assert_eq!( + [ipv4_address.address_family(), ipv6_address.address_family()], + [rpc::forge::AddressFamily::V4, rpc::forge::AddressFamily::V6], + ); + assert!(!tenant_interface.gateway.is_empty()); + assert!(!tenant_interface.ip.is_empty()); + assert!(!tenant_interface.interface_prefix.is_empty()); + assert!(!tenant_interface.prefix.is_empty()); + assert_eq!(tenant_interface.gateway, ipv4_address.gateway); + assert_eq!(tenant_interface.ip, ipv4_address.ip); + assert_eq!( + tenant_interface.interface_prefix, + ipv4_address.interface_prefix, + ); + assert_eq!(tenant_interface.prefix, ipv4_address.prefix); + assert_eq!(tenant_interface.svi_ip, ipv4_address.svi_ip); } -/// Builds unresolved selector intent so allocator internals (including gated -/// IPv6 modes) can be exercised without RPC orchestration. +/// Builds unresolved selector intent so allocator internals can be exercised +/// without RPC orchestration. fn automatic_network_config( vpc_id: VpcId, family_mode: InstanceInterfaceIpFamilyMode, @@ -3924,6 +3996,13 @@ async fn wait_until_prefix_allocator_blocked_by( /// Builds unresolved external selector intent so public-boundary tests do not /// pre-resolve a prefix themselves. fn automatic_ipv4_rpc_network_config(vpc_id: VpcId) -> rpc::InstanceNetworkConfig { + automatic_rpc_network_config(vpc_id, rpc::forge::InstanceInterfaceIpFamilyMode::Ipv4Only) +} + +fn automatic_rpc_network_config( + vpc_id: VpcId, + family_mode: rpc::forge::InstanceInterfaceIpFamilyMode, +) -> rpc::InstanceNetworkConfig { rpc::InstanceNetworkConfig { interfaces: vec![rpc::InstanceInterfaceConfig { function_type: rpc::InterfaceFunctionType::Physical as i32, @@ -3931,7 +4010,7 @@ fn automatic_ipv4_rpc_network_config(vpc_id: VpcId) -> rpc::InstanceNetworkConfi network_details: Some(rpc::forge::instance_interface_config::NetworkDetails::Vpc( rpc::forge::InstanceInterfaceVpcSelection { vpc_id: Some(vpc_id), - family_mode: rpc::forge::InstanceInterfaceIpFamilyMode::Ipv4Only as i32, + family_mode: family_mode as i32, }, )), device: None, @@ -3949,11 +4028,13 @@ fn automatic_ipv4_rpc_network_config(vpc_id: VpcId) -> rpc::InstanceNetworkConfi /// Verifies config retains caller VPC intent while status exposes its resolved /// prefix, protecting the distinction between intent and active allocation. -fn assert_ipv4_auto_rpc_resolution( +fn assert_auto_rpc_resolution( instance: &rpc::Instance, vpc_id: VpcId, - vpc_prefix_id: VpcPrefixId, -) { + family_mode: rpc::forge::InstanceInterfaceIpFamilyMode, + ipv4_vpc_prefix_id: Option, + ipv6_vpc_prefix_id: Option, +) -> NetworkSegmentId { // Config retains VPC-level caller intent while carrying the generated segment. let interface = &instance .config @@ -3968,11 +4049,8 @@ fn assert_ipv4_auto_rpc_resolution( other => panic!("expected automatic VPC selection, got {other:?}"), }; assert_eq!(selection.vpc_id, Some(vpc_id)); - assert_eq!( - selection.family_mode, - rpc::forge::InstanceInterfaceIpFamilyMode::Ipv4Only as i32, - ); - assert!(interface.network_segment_id.is_some()); + assert_eq!(selection.family_mode, family_mode as i32); + let network_segment_id = interface.network_segment_id.unwrap(); // Status publishes the active family-keyed prefix separately. let status_interface = &instance @@ -3985,8 +4063,24 @@ fn assert_ipv4_auto_rpc_resolution( .interfaces[0]; assert_eq!(status_interface.vpc_id, Some(vpc_id)); let resolved = status_interface.resolved_vpc_prefixes.as_ref().unwrap(); - assert_eq!(resolved.ipv4_vpc_prefix_id, Some(vpc_prefix_id)); - assert_eq!(resolved.ipv6_vpc_prefix_id, None); + assert_eq!(resolved.ipv4_vpc_prefix_id, ipv4_vpc_prefix_id); + assert_eq!(resolved.ipv6_vpc_prefix_id, ipv6_vpc_prefix_id); + + network_segment_id +} + +fn assert_ipv4_auto_rpc_resolution( + instance: &rpc::Instance, + vpc_id: VpcId, + vpc_prefix_id: VpcPrefixId, +) { + assert_auto_rpc_resolution( + instance, + vpc_id, + rpc::forge::InstanceInterfaceIpFamilyMode::Ipv4Only, + Some(vpc_prefix_id), + None, + ); } async fn create_tenant_overlay_prefix(env: &TestEnv, vpc_id: VpcId) -> VpcPrefixId { diff --git a/crates/api-core/src/tests/instance_allocate.rs b/crates/api-core/src/tests/instance_allocate.rs index a31246b5e2..3fb098a26c 100644 --- a/crates/api-core/src/tests/instance_allocate.rs +++ b/crates/api-core/src/tests/instance_allocate.rs @@ -1041,6 +1041,13 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( && prefix_id.eq(&host_inband_segment_1.prefixes[0].id) ) ); + assert_eq!( + interface_in_segment_1 + .interface_prefixes + .get(&host_inband_segment_1.prefixes[0].id), + Some(&host_inband_segment_1.prefixes[0].prefix), + "HostInband allocation should persist the interface's segment prefix", + ); assert!( interface_in_segment_2 @@ -1051,6 +1058,13 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( && prefix_id.eq(&host_inband_segment_2.prefixes[0].id) ) ); + assert_eq!( + interface_in_segment_2 + .interface_prefixes + .get(&host_inband_segment_2.prefixes[0].id), + Some(&host_inband_segment_2.prefixes[0].prefix), + "HostInband allocation should persist the interface's segment prefix", + ); let mut txn = env.db_txn().await; for segment_id in [host_inband_segment_1.id, host_inband_segment_2.id] { diff --git a/crates/api-db/src/instance_address.rs b/crates/api-db/src/instance_address.rs index 8f6f07de81..501d7be3a0 100644 --- a/crates/api-db/src/instance_address.rs +++ b/crates/api-db/src/instance_address.rs @@ -648,7 +648,10 @@ impl AssignIpsFrom<(&Machine, &NetworkPrefix)> for InstanceInterfaceConfig { )); }; + let assigned_address = IpNetwork::new(address, network_prefix.prefix.prefix())?; self.ip_addrs.insert(network_prefix.id, address); + self.interface_prefixes + .insert(network_prefix.id, network_prefix.prefix); self.host_inband_mac_address = Some(inband_host_interface.mac_address); @@ -671,10 +674,7 @@ impl AssignIpsFrom<(&Machine, &NetworkPrefix)> for InstanceInterfaceConfig { .insert(network_prefix.id, gateway_as_network); } - Ok(vec![IpNetwork::new( - address, - network_prefix.prefix.prefix(), - )?]) + Ok(vec![assigned_address]) } } diff --git a/crates/api-model/src/instance/status/network.rs b/crates/api-model/src/instance/status/network.rs index b26fe32d84..36a13580e1 100644 --- a/crates/api-model/src/instance/status/network.rs +++ b/crates/api-model/src/instance/status/network.rs @@ -364,20 +364,22 @@ pub struct InstanceInterfaceStatus { /// The list of IP addresses that had been assigned to this interface, /// based on the requested subnet. + /// IPv4 precedes IPv6 when both families are assigned. /// The list will be empty if interface configuration hasn't been completed pub addresses: Vec, - // The list of IP prefixes that have been assigned to this interface - // out of the requested subnet (where the prefix allocated to the interface - // may be a /30 in the case of FNN, or just a /32 in the case of ETV). - // - // This is similar to `gateways`, in that there is one `prefix` for each - // address in `addresses`. + /// The IP prefixes assigned to this interface, with one prefix for each + /// entry in `addresses` in the same IPv4-before-IPv6 order. A prefix may be + /// a /30 for FNN or a /32 for ETV. /// /// The list will be empty if interface configuration hasn't been completed pub prefixes: Vec, - /// The list of gateways, in CIDR notation, one for each address in `addresses`. + /// The explicitly configured gateways, in CIDR notation. There is at most + /// one gateway per address family, associated with the same-family address + /// and prefix. A family without an explicit gateway is omitted, so this + /// list can be shorter than `addresses` and is not positionally aligned. + /// IPv4 precedes IPv6 when both gateways are explicitly configured. pub gateways: Vec, /// The logical VPC this interface belongs to. @@ -394,37 +396,45 @@ impl InstanceInterfaceStatus { /// Create a "synthetic" InstanceInterfaceStatus using an InstanceInterfaceConfig as a seed. /// Host-inband interfaces do not get real network status observations, so we construct status /// ourselves from the host interface's config. - pub fn from_host_inband_interface(mut value: InstanceInterfaceConfig) -> Self { + pub fn from_host_inband_interface(value: InstanceInterfaceConfig) -> Self { let resolved_vpc_prefixes = value.resolved_vpc_prefixes(); - let (prefix_ids, addresses): (Vec<_>, Vec<_>) = value.ip_addrs.into_iter().unzip(); + let mut address_entries = value.ip_addrs.into_iter().collect::>(); + address_entries.sort_by_key(|(_, address)| (address.is_ipv6(), *address)); - // For each NetworkPrefixId we saw in ip_addrs, get that entry from the - // network_segment_gateways map. Collecting them into an Option> returns None - // if any of them were not found. - let gateways = prefix_ids + // Interface prefixes were added after the original host-inband status + // path. Fall back to the segment gateway's prefix for legacy IPv4 + // configs that do not contain the newer per-interface value. + let prefixes = address_entries .iter() - .map(|id| { - if let Some(gw) = value.network_segment_gateways.remove(id) { - Some(gw) - } else { + .map(|(id, _)| { + value.interface_prefixes.get(id).copied().or_else(|| { + value.network_segment_gateways.get(id).map(|gateway| { + // Unwrap safety: the prefix length comes from an + // already validated IpNetwork. + IpNetwork::new(gateway.network(), gateway.prefix()).unwrap() + }) + }) + .or_else(|| { tracing::warn!( network_prefix_id = %id, - "Missing gateway in InstanceInterfaceConfig; gateways field will be empty", + "Missing prefix in InstanceInterfaceConfig; prefixes field will be empty", ); None - } + }) }) .collect::>>() .unwrap_or_default(); - // Build a map of prefixes by taking the gateway field (which already is an IpNetwork e.g. - // 10.1.2.1/24) and building an IpNetwork from the gateway's prefix (e.g. 10.1.2.0/24) - let prefixes = gateways + // Gateways are optional per family (IPv6 normally learns one through + // Router Advertisements), so retain only the explicitly configured + // values while preserving family order. + let gateways = address_entries .iter() - // Unwrap safety: This only fails if the prefix length passed to IpNetwork::new() is - // invalid, which can't happen because we're getting it from another (valid) - // IpNetwork. - .map(|gw| IpNetwork::new(gw.network(), gw.prefix()).unwrap()) + .filter_map(|(id, _)| value.network_segment_gateways.get(id).copied()) + .collect(); + let addresses = address_entries + .into_iter() + .map(|(_, address)| address) .collect(); Self { @@ -508,22 +518,24 @@ pub struct InstanceInterfaceStatusObservation { /// The list of IP addresses that had been assigned to this interface, /// based on the requested subnet. + /// IPv4 precedes IPv6 when both families are assigned. /// The list will be empty if interface configuration hasn't been completed #[serde(default)] pub addresses: Vec, - // The list of IP prefixes that have been assigned to this interface - // out of the requested subnet (where the prefix allocated to the interface - // may be a /30 in the case of FNN, or just a /32 in the case of ETV). - // - // This is similar to `gateways`, in that there is one `prefix` for each - // address in `addresses`. + /// The IP prefixes assigned to this interface, with one prefix for each + /// entry in `addresses` in the same IPv4-before-IPv6 order. A prefix may be + /// a /30 for FNN or a /32 for ETV. /// /// The list will be empty if interface configuration hasn't been completed #[serde(default)] pub prefixes: Vec, - /// The list of gateways, in CIDR notation, one for each address in `addresses`. + /// The explicitly configured gateways, in CIDR notation. There is at most + /// one gateway per address family, associated with the same-family address + /// and prefix. A family without an explicit gateway is omitted, so this + /// list can be shorter than `addresses` and is not positionally aligned. + /// IPv4 precedes IPv6 when both gateways are explicitly configured. #[serde(default)] pub gateways: Vec, @@ -1130,4 +1142,37 @@ mod tests { ); assert_eq!(status, expected_host_inband_status()) } + + #[test] + fn host_inband_status_orders_dual_stack_fields_by_family() { + let mut interface = host_inband_network_config().interfaces.remove(0); + let ipv6_prefix_id = NetworkPrefixId::new(); + interface + .ip_addrs + .insert(ipv6_prefix_id, "2001:db8::2".parse().unwrap()); + interface + .interface_prefixes + .insert(ipv6_prefix_id, "2001:db8::/64".parse().unwrap()); + + let status = InstanceInterfaceStatus::from_host_inband_interface(interface); + + assert_eq!( + status.addresses, + vec![ + "127.0.1.2".parse::().unwrap(), + "2001:db8::2".parse::().unwrap(), + ], + ); + assert_eq!( + status.prefixes, + vec![ + "127.0.1.0/24".parse::().unwrap(), + "2001:db8::/64".parse::().unwrap(), + ], + ); + assert_eq!( + status.gateways, + vec!["127.0.1.1/24".parse::().unwrap()] + ); + } } diff --git a/crates/dhcp-server/proto/dhcp_server_control.proto b/crates/dhcp-server/proto/dhcp_server_control.proto index 649d3cb3b7..4acfd32232 100644 --- a/crates/dhcp-server/proto/dhcp_server_control.proto +++ b/crates/dhcp-server/proto/dhcp_server_control.proto @@ -56,12 +56,13 @@ message DhcpConfig { } // Mirrors utils::models::dhcp::InterfaceInfo. -// IPv4 addresses are encoded as dotted-decimal strings. IPv6 details are -// absent for v4-only hosts. +// IPv4 addresses are encoded as dotted-decimal strings and are absent for +// IPv6-only hosts. Address, gateway, and prefix are either all present or all +// absent. IPv6 details are absent for v4-only hosts. message InterfaceInfo { - string address = 1; - string gateway = 2; - string prefix = 3; + optional string address = 1; + optional string gateway = 2; + optional string prefix = 3; string fqdn = 4; optional string booturl = 5; optional uint32 mtu = 6; diff --git a/crates/dhcp-server/src/grpc_server.rs b/crates/dhcp-server/src/grpc_server.rs index 62757cac1c..cb61697e0d 100644 --- a/crates/dhcp-server/src/grpc_server.rs +++ b/crates/dhcp-server/src/grpc_server.rs @@ -116,10 +116,22 @@ impl TryFrom for ModelInterfaceInfo { type Error = DhcpError; fn try_from(i: proto::InterfaceInfo) -> Result { + let (address, gateway, prefix) = match (i.address, i.gateway, i.prefix) { + (Some(address), Some(gateway), Some(prefix)) => { + (Some(address.parse()?), Some(gateway.parse()?), Some(prefix)) + } + (None, None, None) => (None, None, None), + _ => { + return Err(DhcpError::InvalidInput( + "IPv4 address, gateway, and prefix must be configured together".to_string(), + )); + } + }; + Ok(ModelInterfaceInfo { - address: i.address.parse()?, - gateway: i.gateway.parse()?, - prefix: i.prefix, + address, + gateway, + prefix, fqdn: i.fqdn, booturl: i.booturl, mtu: i.mtu, @@ -253,3 +265,69 @@ pub(super) async fn run_grpc_server(addr: SocketAddr, ctrl_tx: mpsc::Sender, Option, Option); + + fn summarize_interface(interface: proto::InterfaceInfo) -> Result { + ModelInterfaceInfo::try_from(interface) + .map(|interface| (interface.address, interface.gateway, interface.prefix)) + .map_err(drop) + } + + #[test] + fn interface_ipv4_fields_are_all_present_or_all_absent() { + scenarios!(run = summarize_interface; + "complete IPv4 configuration" { + proto::InterfaceInfo { + address: Some("192.0.2.10".to_string()), + gateway: Some("192.0.2.1".to_string()), + prefix: Some("192.0.2.0/24".to_string()), + ..Default::default() + } => Yields(( + Some(Ipv4Addr::new(192, 0, 2, 10)), + Some(Ipv4Addr::new(192, 0, 2, 1)), + Some("192.0.2.0/24".to_string()), + )), + } + "IPv6-only configuration" { + proto::InterfaceInfo { + ipv6: Some(proto::InterfaceInfoV6 { + address: Some("2001:db8::10".to_string()), + prefix: "2001:db8::/64".to_string(), + }), + ..Default::default() + } => Yields((None, None, None)), + } + "missing IPv4 address" { + proto::InterfaceInfo { + gateway: Some("192.0.2.1".to_string()), + prefix: Some("192.0.2.0/24".to_string()), + ..Default::default() + } => Fails, + } + "missing IPv4 gateway" { + proto::InterfaceInfo { + address: Some("192.0.2.10".to_string()), + prefix: Some("192.0.2.0/24".to_string()), + ..Default::default() + } => Fails, + } + "missing IPv4 prefix" { + proto::InterfaceInfo { + address: Some("192.0.2.10".to_string()), + gateway: Some("192.0.2.1".to_string()), + ..Default::default() + } => Fails, + } + ); + } +} diff --git a/crates/dhcp-server/src/modes/dpu.rs b/crates/dhcp-server/src/modes/dpu.rs index 70468ef591..dd92eb782f 100644 --- a/crates/dhcp-server/src/modes/dpu.rs +++ b/crates/dhcp-server/src/modes/dpu.rs @@ -29,23 +29,37 @@ use crate::{Config, HostConfig}; #[derive(Debug)] pub(crate) struct Dpu {} -fn from_host_conf(value: &InterfaceInfo, interface_id: MachineInterfaceId) -> DhcpRecord { +fn from_host_conf( + value: &InterfaceInfo, + interface_id: MachineInterfaceId, +) -> Result { + let address = value + .address + .ok_or_else(|| DhcpError::InvalidInput("IPv4 address is not configured".to_string()))?; + let gateway = value + .gateway + .ok_or_else(|| DhcpError::InvalidInput("IPv4 gateway is not configured".to_string()))?; + let prefix = value + .prefix + .clone() + .ok_or_else(|| DhcpError::InvalidInput("IPv4 prefix is not configured".to_string()))?; + // Fill only needed fields. Rest are left empty or none. - DhcpRecord { + Ok(DhcpRecord { machine_id: None, machine_interface_id: Some(interface_id), segment_id: None, subdomain_id: None, fqdn: value.fqdn.clone(), mac_address: "dummy".to_string(), - address: value.address.to_string(), + address: address.to_string(), mtu: 0, - prefix: value.prefix.clone(), - gateway: Some(value.gateway.to_string()), + prefix, + gateway: Some(gateway.to_string()), booturl: value.booturl.clone(), last_invalidation_time: None, ntp_servers: vec![], - } + }) } #[async_trait] @@ -78,7 +92,7 @@ impl DhcpMode for Dpu { )); }; - Ok(from_host_conf(ip_details, host_config.host_interface_id)) + from_host_conf(ip_details, host_config.host_interface_id) } /// Here circuit is interface name. This is what dhcp-relay used to fill. @@ -91,6 +105,21 @@ impl DhcpMode for Dpu { } } +fn validate_host_config(host_config: &HostConfig) -> Result<(), DhcpError> { + for (circuit_id, interface) in &host_config.host_ip_addresses { + if !matches!( + (&interface.address, &interface.gateway, &interface.prefix), + (Some(_), Some(_), Some(_)) | (None, None, None) + ) { + return Err(DhcpError::InvalidInput(format!( + "IPv4 address, gateway, and prefix for {circuit_id} must be configured together" + ))); + } + } + + Ok(()) +} + /// This config is fetched by dpu-agent from controller periodically. In case of any change in /// this configuration, dpu-agent MUST restart dhcp-server. pub(crate) async fn get_host_config( @@ -104,6 +133,120 @@ pub(crate) async fn get_host_config( let f = tokio::fs::read_to_string(host_config).await?; let host_config: HostConfig = serde_yaml::from_str(&f)?; + validate_host_config(&host_config)?; Ok(Some(host_config)) } + +#[cfg(test)] +mod tests { + use std::net::Ipv4Addr; + + use carbide_rpc_utils::dhcp::InterfaceInfoV6; + use carbide_test_support::Outcome::*; + use carbide_test_support::scenarios; + + use super::*; + + fn summarize_ipv4_config( + interface: InterfaceInfo, + ) -> Result<(String, Option, String), ()> { + let interface_id = "11111111-1111-1111-1111-111111111111".parse().unwrap(); + from_host_conf(&interface, interface_id) + .map(|record| (record.address, record.gateway, record.prefix)) + .map_err(drop) + } + + fn validate_interface_presence(interface: InterfaceInfo) -> Result<(), ()> { + let host_config = HostConfig { + host_interface_id: "11111111-1111-1111-1111-111111111111".parse().unwrap(), + host_ip_addresses: [("vlan100".to_string(), interface)].into(), + }; + validate_host_config(&host_config).map_err(drop) + } + + #[test] + fn host_config_requires_ipv4_for_dhcpv4() { + scenarios!(run = summarize_ipv4_config; + "complete IPv4 configuration" { + InterfaceInfo { + address: Some(Ipv4Addr::new(192, 0, 2, 10)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + prefix: Some("192.0.2.0/24".to_string()), + ..Default::default() + } => Yields(( + "192.0.2.10".to_string(), + Some("192.0.2.1".to_string()), + "192.0.2.0/24".to_string(), + )), + } + "IPv6-only configuration" { + InterfaceInfo { + ipv6: Some(InterfaceInfoV6 { + address: Some("2001:db8::10".parse().unwrap()), + prefix: "2001:db8::/64".to_string(), + }), + ..Default::default() + } => Fails, + } + "missing IPv4 gateway" { + InterfaceInfo { + address: Some(Ipv4Addr::new(192, 0, 2, 10)), + prefix: Some("192.0.2.0/24".to_string()), + ..Default::default() + } => Fails, + } + "missing IPv4 prefix" { + InterfaceInfo { + address: Some(Ipv4Addr::new(192, 0, 2, 10)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + ..Default::default() + } => Fails, + } + ); + } + + #[test] + fn host_config_rejects_partial_ipv4_configuration() { + scenarios!(run = validate_interface_presence; + "complete IPv4 configuration" { + InterfaceInfo { + address: Some(Ipv4Addr::new(192, 0, 2, 10)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + prefix: Some("192.0.2.0/24".to_string()), + ..Default::default() + } => Yields(()), + } + "IPv6-only configuration" { + InterfaceInfo { + ipv6: Some(InterfaceInfoV6 { + address: Some("2001:db8::10".parse().unwrap()), + prefix: "2001:db8::/64".to_string(), + }), + ..Default::default() + } => Yields(()), + } + "missing IPv4 address" { + InterfaceInfo { + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + prefix: Some("192.0.2.0/24".to_string()), + ..Default::default() + } => Fails, + } + "missing IPv4 gateway" { + InterfaceInfo { + address: Some(Ipv4Addr::new(192, 0, 2, 10)), + prefix: Some("192.0.2.0/24".to_string()), + ..Default::default() + } => Fails, + } + "missing IPv4 prefix" { + InterfaceInfo { + address: Some(Ipv4Addr::new(192, 0, 2, 10)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + ..Default::default() + } => Fails, + } + ); + } +} diff --git a/crates/dhcp-server/src/packet_handler.rs b/crates/dhcp-server/src/packet_handler.rs index 2b9782c2fd..1b157debc1 100644 --- a/crates/dhcp-server/src/packet_handler.rs +++ b/crates/dhcp-server/src/packet_handler.rs @@ -619,26 +619,34 @@ mod test { #[test] fn test_get_mtu() { let interface_mtu_none = crate::packet_handler::InterfaceInfo { - address: ::from_str("10.12.1.2") - .ok() - .unwrap(), - gateway: ::from_str("10.12.1.2") - .ok() - .unwrap(), - prefix: "24".to_string(), + address: Some( + ::from_str("10.12.1.2") + .ok() + .unwrap(), + ), + gateway: Some( + ::from_str("10.12.1.2") + .ok() + .unwrap(), + ), + prefix: Some("24".to_string()), fqdn: "fqdn1".to_string(), booturl: None, mtu: None, ipv6: None, }; let interface_mtu_9000 = crate::packet_handler::InterfaceInfo { - address: ::from_str("20.22.2.2") - .ok() - .unwrap(), - gateway: ::from_str("20.22.2.2") - .ok() - .unwrap(), - prefix: "16".to_string(), + address: Some( + ::from_str("20.22.2.2") + .ok() + .unwrap(), + ), + gateway: Some( + ::from_str("20.22.2.2") + .ok() + .unwrap(), + ), + prefix: Some("16".to_string()), fqdn: "fqdn2".to_string(), booturl: None, mtu: Some(9000), diff --git a/crates/machine-a-tron/src/machine_state_machine.rs b/crates/machine-a-tron/src/machine_state_machine.rs index 853e438bf2..9c326e33b4 100644 --- a/crates/machine-a-tron/src/machine_state_machine.rs +++ b/crates/machine-a-tron/src/machine_state_machine.rs @@ -1160,7 +1160,7 @@ impl MachineStateMachine { mac_address: self.machine_info.host_mac_address().map(|a| a.to_string()), addresses, prefixes, - gateways: vec![iface.gateway.clone()], + gateways: build_dual_stack_list(iface.gateway.clone(), None), network_security_group: None, internal_uuid: None, }] @@ -1186,7 +1186,7 @@ impl MachineStateMachine { mac_address: self.machine_info.host_mac_address().map(|a| a.to_string()), addresses, prefixes, - gateways: vec![iface.gateway.clone()], + gateways: build_dual_stack_list(iface.gateway.clone(), None), network_security_group: iface.network_security_group.as_ref().map(|s| { rpc::forge::NetworkSecurityGroupStatus { source: s.source, diff --git a/crates/network/src/virtualization.rs b/crates/network/src/virtualization.rs index 71d6cdc11f..d57aa3a089 100644 --- a/crates/network/src/virtualization.rs +++ b/crates/network/src/virtualization.rs @@ -236,11 +236,12 @@ impl fmt::Display for VpcVirtualizationType { } } -/// Concatenate a required IPv4 value with an optional IPv6 value into a vector. -/// Empty IPv6 strings are filtered out. +/// Concatenate IPv4 and IPv6 values in family order. Empty strings and `None` +/// represent absent families and are omitted. pub fn build_dual_stack_list(v4: String, v6: Option) -> Vec { std::iter::once(v4) - .chain(v6.filter(|s| !s.is_empty())) + .chain(v6) + .filter(|value| !value.is_empty()) .collect() } @@ -554,12 +555,16 @@ mod tests { ("10.0.0.1".to_string(), Some(String::new())) => vec!["10.0.0.1".to_string()], } - "v4 is kept even when empty (it is required)" { - (String::new(), None) => vec![String::new()], + "both families absent" { + (String::new(), None) => vec![], } - "empty v4 with a real v6 keeps both" { - (String::new(), Some("2001:db8::1".to_string())) => vec![String::new(), "2001:db8::1".to_string()], + "v6 only when v4 is absent" { + (String::new(), Some("2001:db8::1".to_string())) => vec!["2001:db8::1".to_string()], + } + + "empty strings for both families are absent" { + (String::new(), Some(String::new())) => vec![], } ); } diff --git a/crates/rpc-utils/src/dhcp.rs b/crates/rpc-utils/src/dhcp.rs index a3d97a7ff6..7bef40f757 100644 --- a/crates/rpc-utils/src/dhcp.rs +++ b/crates/rpc-utils/src/dhcp.rs @@ -121,17 +121,18 @@ pub struct HostConfig { pub host_ip_addresses: BTreeMap, } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct InterfaceInfo { - pub address: Ipv4Addr, - pub gateway: Ipv4Addr, - pub prefix: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub address: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub gateway: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub prefix: Option, pub fqdn: String, pub booturl: Option, #[serde(skip_serializing_if = "Option::is_none")] pub mtu: Option, - // TODO(ipv6-only): the v4 fields above are still required. IPv6-only - // hosts will need those fields to become optional in a later milestone. #[serde(default, skip_serializing_if = "Option::is_none")] pub ipv6: Option, } @@ -142,19 +143,7 @@ pub struct InterfaceInfoV6 { pub address: Option, pub prefix: String, } -impl Default for InterfaceInfo { - fn default() -> Self { - InterfaceInfo { - address: Ipv4Addr::UNSPECIFIED, - gateway: Ipv4Addr::UNSPECIFIED, - prefix: Default::default(), - fqdn: Default::default(), - booturl: None, - mtu: None, - ipv6: None, - } - } -} + impl HostConfig { pub fn try_from( value: ManagedHostNetworkConfigResponse, @@ -214,12 +203,35 @@ impl HostConfig { impl TryFrom<::rpc::forge::FlatInterfaceConfig> for InterfaceInfo { type Error = DhcpDataError; fn try_from(value: ::rpc::forge::FlatInterfaceConfig) -> Result { - let gateway = Ipv4Network::from_str(&value.gateway)?.ip(); + let empty_ipv4_fields = [ + value.ip.is_empty(), + value.gateway.is_empty(), + value.prefix.is_empty(), + ] + .into_iter() + .filter(|empty| *empty) + .count(); + + if empty_ipv4_fields != 0 && empty_ipv4_fields != 3 { + return Err(DhcpDataError::ParameterMissing( + "complete IPv4 interface configuration", + )); + } + + let (address, gateway, prefix) = if empty_ipv4_fields == 3 { + (None, None, None) + } else { + ( + Some(value.ip.parse()?), + Some(Ipv4Network::from_str(&value.gateway)?.ip()), + Some(value.prefix), + ) + }; Ok(InterfaceInfo { - address: value.ip.parse()?, + address, gateway, - prefix: value.prefix, + prefix, fqdn: value.fqdn, booturl: value.booturl, mtu: value.mtu, @@ -354,9 +366,9 @@ mod tests { #[derive(Debug, PartialEq)] struct InterfaceSummary { - address: Ipv4Addr, - gateway: Ipv4Addr, - prefix: String, + address: Option, + gateway: Option, + prefix: Option, fqdn: String, booturl: Option, mtu: Option, @@ -397,6 +409,14 @@ mod tests { } } + #[allow(deprecated)] + fn ipv6_only_interface_config() -> FlatInterfaceConfig { + let mut config = + interface_config(InterfaceFunctionType::Virtual, 100, Some(3), false, "", ""); + config.prefix.clear(); + config + } + fn host_network_config( use_admin_network: bool, admin_interface: Option, @@ -520,16 +540,24 @@ mod tests { "192.0.2.50", "192.0.2.1/24", ) => Yields(InterfaceSummary { - address: Ipv4Addr::new(192, 0, 2, 50), - gateway: Ipv4Addr::new(192, 0, 2, 1), - prefix: "192.0.2.0/24".to_string(), + address: Some(Ipv4Addr::new(192, 0, 2, 50)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + prefix: Some("192.0.2.0/24".to_string()), + fqdn: "host.example.com".to_string(), + booturl: Some("http://boot.example.com/ipxe".to_string()), + mtu: Some(9000), + }), + ipv6_only_interface_config() => Yields(InterfaceSummary { + address: None, + gateway: None, + prefix: None, fqdn: "host.example.com".to_string(), booturl: Some("http://boot.example.com/ipxe".to_string()), mtu: Some(9000), }), } - "invalid addresses" { + "invalid or incomplete addresses" { interface_config( InterfaceFunctionType::Virtual, 100, @@ -546,6 +574,14 @@ mod tests { "192.0.2.50", "not a network", ) => FailsWith("ip-network"), + interface_config( + InterfaceFunctionType::Virtual, + 100, + Some(3), + false, + "", + "192.0.2.1/24", + ) => FailsWith("parameter-missing"), } ); } @@ -575,9 +611,9 @@ mod tests { host_ip_addresses: vec![( "vlan100".to_string(), InterfaceSummary { - address: Ipv4Addr::new(192, 0, 2, 10), - gateway: Ipv4Addr::new(192, 0, 2, 1), - prefix: "192.0.2.0/24".to_string(), + address: Some(Ipv4Addr::new(192, 0, 2, 10)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + prefix: Some("192.0.2.0/24".to_string()), fqdn: "host.example.com".to_string(), booturl: Some("http://boot.example.com/ipxe".to_string()), mtu: Some(9000), @@ -608,9 +644,9 @@ mod tests { host_ip_addresses: vec![( "vf3sf".to_string(), InterfaceSummary { - address: Ipv4Addr::new(192, 0, 2, 20), - gateway: Ipv4Addr::new(192, 0, 2, 1), - prefix: "192.0.2.0/24".to_string(), + address: Some(Ipv4Addr::new(192, 0, 2, 20)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + prefix: Some("192.0.2.0/24".to_string()), fqdn: "host.example.com".to_string(), booturl: Some("http://boot.example.com/ipxe".to_string()), mtu: Some(9000), @@ -641,9 +677,9 @@ mod tests { host_ip_addresses: vec![( "p0".to_string(), InterfaceSummary { - address: Ipv4Addr::new(192, 0, 2, 30), - gateway: Ipv4Addr::new(192, 0, 2, 1), - prefix: "192.0.2.0/24".to_string(), + address: Some(Ipv4Addr::new(192, 0, 2, 30)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + prefix: Some("192.0.2.0/24".to_string()), fqdn: "host.example.com".to_string(), booturl: Some("http://boot.example.com/ipxe".to_string()), mtu: Some(9000), @@ -738,9 +774,9 @@ mod tests { #[test] fn interface_info_ipv6_round_trip_and_defaults_when_absent() { let interface = InterfaceInfo { - address: Ipv4Addr::new(192, 0, 2, 10), - gateway: Ipv4Addr::new(192, 0, 2, 1), - prefix: "192.0.2.0/24".to_string(), + address: Some(Ipv4Addr::new(192, 0, 2, 10)), + gateway: Some(Ipv4Addr::new(192, 0, 2, 1)), + prefix: Some("192.0.2.0/24".to_string()), fqdn: "host.example.com".to_string(), booturl: None, mtu: Some(9000), @@ -765,7 +801,20 @@ mod tests { }"#; let old_interface: InterfaceInfo = serde_json::from_str(old_wire).expect("old interface deserializes"); + assert_eq!(old_interface.address, interface.address); + assert_eq!(old_interface.gateway, interface.gateway); + assert_eq!(old_interface.prefix, interface.prefix); assert_eq!(old_interface.ipv6, None); + + let ipv6_only_wire = r#"{ + "fqdn": "host.example.com", + "booturl": null + }"#; + let ipv6_only_interface: InterfaceInfo = serde_json::from_str(ipv6_only_wire) + .expect("interface without IPv4 fields deserializes"); + assert_eq!(ipv6_only_interface.address, None); + assert_eq!(ipv6_only_interface.gateway, None); + assert_eq!(ipv6_only_interface.prefix, None); } #[test] diff --git a/crates/rpc/proto/forge.proto b/crates/rpc/proto/forge.proto index 80ceff15e2..992aebadad 100644 --- a/crates/rpc/proto/forge.proto +++ b/crates/rpc/proto/forge.proto @@ -3559,9 +3559,6 @@ message InstanceInterfaceVpcSelection { } // Address families requested for automatic VPC prefix and address selection. -// -// Core models and persists every mode. External IPv6-only and dual-stack -// requests are temporarily rejected until downstream DPU support is complete. enum InstanceInterfaceIpFamilyMode { // Invalid for an automatic VPC selection request. INSTANCE_INTERFACE_IP_FAMILY_MODE_UNSPECIFIED = 0; @@ -3651,18 +3648,23 @@ message InstanceInterfaceStatus { // The list of IP addresses that had been assigned to this interface, // based on the requested subnet. + // IPv4 precedes IPv6 when both families are assigned. // The list will be empty if interface configuration hasn't been completed repeated string addresses = 3; - // The list of gateways, in CIDR notation, one for each address in `addresses`. + // The explicitly configured gateways, in CIDR notation. There is at most one + // gateway per address family, associated with the same-family address and + // prefix. A family without an explicit gateway is omitted, so this list can + // be shorter than `addresses` and is not positionally aligned. IPv4 precedes + // IPv6 when both gateways are explicitly configured. repeated string gateways = 4; // The list of IP prefixes that have been assigned to this interface // out of the requested subnet (where the prefix allocated to the interface // may be a /30 in the case of FNN, or just a /32 in the case of ETV). // - // This is similar to `gateways`, in that there is one `prefix` for each - // address in `addresses`. + // There is one prefix for each entry in `addresses`, in the same + // IPv4-before-IPv6 order. repeated string prefixes = 5; optional string device = 6; @@ -5021,7 +5023,7 @@ message FlatInterfaceConfig { // MTU size optional uint32 mtu = 18; - // IPv6 configuration for dual-stack FNN interfaces. + // IPv6 configuration for FNN interfaces that include IPv6. optional FlatInterfaceIpv6Config ipv6_interface_config = 19; // Route imports and tagging details for exports used by FNN configs. @@ -5034,8 +5036,9 @@ message FlatInterfaceConfig { // Family-neutral replacement for gateway, ip, interface_prefix, prefix, svi_ip, // and ipv6_interface_config. An empty list identifies a legacy payload. Writers - // must emit at most one entry per family, ordered V4 before V6. Readers must - // select entries by address_family rather than position. + // must emit at most one entry per family, ordered V4 before V6. A family absent + // from this list has empty deprecated compatibility fields. Readers must select + // entries by address_family rather than position. repeated InterfaceAddressConfig addresses = 22; // The details of the network security group associated with @@ -5057,8 +5060,8 @@ message FlatInterfaceRoutingProfile { repeated PrefixFilterPolicyEntry allowed_anycast_prefixes = 1; } -// IPv6 configuration for a dual-stack FNN interface, sent from the API to -// the DPU agent as part of FlatInterfaceConfig. +// IPv6 configuration for an FNN interface that includes IPv6, sent from the +// API to the DPU agent as part of FlatInterfaceConfig. message FlatInterfaceIpv6Config { option (carbide.codegen.v1.message_derive) = "serde::Serialize"; // Host IPv6 address (e.g. "2001:db8::1"). @@ -5799,18 +5802,23 @@ message InstanceInterfaceStatusObservation { // The list of IP addresses that had been assigned to this interface, // based on the requested subnet. + // IPv4 precedes IPv6 when both families are assigned. // The list will be empty if interface configuration hasn't been completed repeated string addresses = 4; - // The list of gateways, in CIDR notation, one for each address in `addresses`. + // The explicitly configured gateways, in CIDR notation. There is at most one + // gateway per address family, associated with the same-family address and + // prefix. A family without an explicit gateway is omitted, so this list can + // be shorter than `addresses` and is not positionally aligned. IPv4 precedes + // IPv6 when both gateways are explicitly configured. repeated string gateways = 5; // The list of IP prefixes that have been assigned to this interface // out of the requested subnet (where the prefix allocated to the interface // may be a /30 in the case of FNN, or just a /32 in the case of ETV). // - // This is similar to `gateways`, in that there is one `prefix` for each - // address in `addresses`. + // There is one prefix for each entry in `addresses`, in the same + // IPv4-before-IPv6 order. repeated string prefixes = 6; // The NSG details for the observed NSG of the interface diff --git a/crates/rpc/src/model/instance/config/network.rs b/crates/rpc/src/model/instance/config/network.rs index 4ca3541f74..87a64df4c0 100644 --- a/crates/rpc/src/model/instance/config/network.rs +++ b/crates/rpc/src/model/instance/config/network.rs @@ -307,17 +307,6 @@ impl TryFrom for InstanceNetworkConfig { )); } - // Core models and allocation support every family. - // TODO: Accept automatic IPv6 modes once downstream DPU support is - // complete end to end. - if let Some(selection) = vpc_selection - && selection.family_mode != InstanceInterfaceIpFamilyMode::Ipv4Only - { - return Err(RpcDataConversionError::InvalidArgument( - "automatic VPC selection currently supports only IPV4_ONLY".to_string(), - )); - } - if iface.ip_address.is_some() && matches!(network_details, Some(NetworkDetails::NetworkSegment(..))) { @@ -626,8 +615,8 @@ mod tests { .is_ok() } - /// Typed family conversion models future IPv6 modes even while the - /// external allocation boundary temporarily accepts only IPv4. + /// Typed family conversion accepts every concrete mode and rejects the + /// unspecified sentinel. #[test] fn convert_vpc_selection_family_modes() { value_scenarios!( @@ -647,8 +636,8 @@ mod tests { ); } - /// The inbound RPC boundary rejects unspecified, unknown, missing, and - /// not-yet-supported family requests while accepting IPv4 automatic mode. + /// The inbound RPC boundary accepts every concrete family mode while + /// rejecting unspecified, unknown, and incomplete requests. #[test] fn validate_inbound_vpc_selection_modes() { let vpc_id = VpcId::new(); @@ -658,11 +647,11 @@ mod tests { "IPv4 only" { (Some(vpc_id), forge::InstanceInterfaceIpFamilyMode::Ipv4Only as i32) => true, } - "IPv6 only is not yet supported" { - (Some(vpc_id), forge::InstanceInterfaceIpFamilyMode::Ipv6Only as i32) => false, + "IPv6 only" { + (Some(vpc_id), forge::InstanceInterfaceIpFamilyMode::Ipv6Only as i32) => true, } - "dual stack is not yet supported" { - (Some(vpc_id), forge::InstanceInterfaceIpFamilyMode::DualStack as i32) => false, + "dual stack" { + (Some(vpc_id), forge::InstanceInterfaceIpFamilyMode::DualStack as i32) => true, } "unspecified" { (Some(vpc_id), forge::InstanceInterfaceIpFamilyMode::Unspecified as i32) => false, diff --git a/crates/rpc/src/model/instance/status/network.rs b/crates/rpc/src/model/instance/status/network.rs index e08880e4fa..58f5e9897c 100644 --- a/crates/rpc/src/model/instance/status/network.rs +++ b/crates/rpc/src/model/instance/status/network.rs @@ -107,6 +107,30 @@ impl TryFrom for InstanceInterfaceStatu }) .try_collect()?; + let gateways = observation + .gateways + .iter() + .map(|gateway| { + IpNetwork::try_from(gateway.as_str()) + .map_err(|_| Self::Error::InvalidCidr(gateway.to_string())) + }) + .collect::, _>>()?; + let mut seen_ipv4 = false; + let mut seen_ipv6 = false; + for gateway in &gateways { + let seen = if gateway.is_ipv4() { + &mut seen_ipv4 + } else { + &mut seen_ipv6 + }; + if *seen { + return Err(RpcDataConversionError::InvalidArgument( + "gateways must contain at most one entry per address family".to_string(), + )); + } + *seen = true; + } + let internal_uuid = if let Some(internal_uuid) = &observation.internal_uuid { Some(internal_uuid.try_into().map_err(|_| { RpcDataConversionError::InvalidUuid("internal_uuid", internal_uuid.to_string()) @@ -126,14 +150,7 @@ impl TryFrom for InstanceInterfaceStatu .map_err(|_| Self::Error::InvalidCidr(ip_network.to_string())) }) .collect::, Self::Error>>()?, - gateways: observation - .gateways - .iter() - .map(|gw| { - IpNetwork::try_from(gw.as_str()) - .map_err(|_| Self::Error::InvalidCidr(gw.to_string())) - }) - .collect::, Self::Error>>()?, + gateways, mac_address: observation .mac_address .map(|addr| { @@ -153,11 +170,54 @@ impl TryFrom for InstanceInterfaceStatu #[cfg(test)] mod tests { + use carbide_test_support::Outcome::{Fails, Yields}; + use carbide_test_support::scenarios; use carbide_uuid::vpc::{VpcId, VpcPrefixId}; use model::instance::config::network::InstanceInterfaceResolvedVpcPrefixes; use super::*; + fn status_observation_with_gateways( + gateways: Vec<&str>, + ) -> rpc::InstanceInterfaceStatusObservation { + rpc::InstanceInterfaceStatusObservation { + function_type: rpc::InterfaceFunctionType::Physical as i32, + gateways: gateways.into_iter().map(str::to_string).collect(), + ..Default::default() + } + } + + #[test] + fn status_observation_allows_at_most_one_gateway_per_family() { + scenarios!( + run = |gateways| { + InstanceInterfaceStatusObservation::try_from( + status_observation_with_gateways(gateways), + ) + .map(|_| ()) + .map_err(drop) + }; + "no gateways" { + vec![] => Yields(()), + } + "one IPv4 gateway" { + vec!["192.0.2.1/24"] => Yields(()), + } + "one IPv6 gateway" { + vec!["2001:db8::1/64"] => Yields(()), + } + "one gateway per family" { + vec!["192.0.2.1/24", "2001:db8::1/64"] => Yields(()), + } + "duplicate IPv4 gateways" { + vec!["192.0.2.1/24", "198.51.100.1/24"] => Fails, + } + "duplicate IPv6 gateways" { + vec!["2001:db8::1/64", "2001:db8:1::1/64"] => Fails, + } + ); + } + /// Status conversion keeps both family-keyed prefix IDs for a resolved /// dual-stack interface in its single logical VPC. #[test] diff --git a/crates/test-harness/Cargo.toml b/crates/test-harness/Cargo.toml index 951de5e4fb..a582b23626 100644 --- a/crates/test-harness/Cargo.toml +++ b/crates/test-harness/Cargo.toml @@ -26,9 +26,14 @@ repository.workspace = true [dependencies] carbide-api-core = { path = "../api-core", features = ["test-support"] } carbide-api-db = { path = "../api-db", default-features = false } -carbide-api-model = { path = "../api-model", default-features = false, features = ["test-support"] } -carbide-site-explorer = { path = "../site-explorer", features = ["test-support"] } +carbide-api-model = { path = "../api-model", default-features = false, features = [ + "test-support", +] } +carbide-site-explorer = { path = "../site-explorer", features = [ + "test-support", +] } carbide-macros = { path = "../macros", default-features = false } +carbide-network = { path = "../network" } carbide-network-segment-controller = { path = "../network-segment-controller", default-features = false } carbide-sqlx-testing = { path = "../sqlx-testing", default-features = false } carbide-utils = { path = "../utils", features = ["test-support"] } diff --git a/crates/test-harness/src/machine_dpu.rs b/crates/test-harness/src/machine_dpu.rs index 941eb1b7ae..b88dc8b557 100644 --- a/crates/test-harness/src/machine_dpu.rs +++ b/crates/test-harness/src/machine_dpu.rs @@ -18,6 +18,7 @@ use std::sync::Arc; use carbide_api_core::test_support::Api; +use carbide_network::virtualization::build_dual_stack_list; use carbide_uuid::machine::MachineId; use mac_address::MacAddress; use model::hardware_info::HardwareInfo; @@ -142,9 +143,18 @@ async fn record_dpu_network_status(api: &Api, dpu_machine_id: MachineId) { function_type: iface.function_type, virtual_function_id: None, mac_address: None, - addresses: vec![iface.ip.clone()], - prefixes: vec![iface.interface_prefix.clone()], - gateways: vec![iface.gateway.clone()], + addresses: build_dual_stack_list( + iface.ip.clone(), + iface.ipv6_interface_config.as_ref().map(|v6| v6.ip.clone()), + ), + prefixes: build_dual_stack_list( + iface.interface_prefix.clone(), + iface + .ipv6_interface_config + .as_ref() + .map(|v6| v6.interface_prefix.clone()), + ), + gateways: build_dual_stack_list(iface.gateway.clone(), None), network_security_group: None, internal_uuid: iface.internal_uuid.clone(), }] @@ -157,9 +167,18 @@ async fn record_dpu_network_status(api: &Api, dpu_machine_id: MachineId) { function_type: iface.function_type, virtual_function_id: iface.virtual_function_id, mac_address: None, - addresses: vec![iface.ip.clone()], - prefixes: vec![iface.interface_prefix.clone()], - gateways: vec![iface.gateway.clone()], + addresses: build_dual_stack_list( + iface.ip.clone(), + iface.ipv6_interface_config.as_ref().map(|v6| v6.ip.clone()), + ), + prefixes: build_dual_stack_list( + iface.interface_prefix.clone(), + iface + .ipv6_interface_config + .as_ref() + .map(|v6| v6.interface_prefix.clone()), + ), + gateways: build_dual_stack_list(iface.gateway.clone(), None), network_security_group: None, internal_uuid: iface.internal_uuid.clone(), }, diff --git a/rest-api/api/pkg/api/handler/instance.go b/rest-api/api/pkg/api/handler/instance.go index a7f351cb54..dc3b3693fd 100644 --- a/rest-api/api/pkg/api/handler/instance.go +++ b/rest-api/api/pkg/api/handler/instance.go @@ -836,7 +836,7 @@ func (cih CreateInstanceHandler) Handle(c echo.Context) error { dbInterfaces = append(dbInterfaces, cdbm.Interface{ VpcID: &interfaceVpcID, Vpc: interfaceVpc, - VpcIPFamilyMode: cutil.GetPtr(cdbm.InterfaceVpcIPFamilyModeIPv4Only), + VpcIPFamilyMode: cutil.GetPtr(ifc.VpcIPFamilyMode()), InlineRoutingProfile: ifc.InlineRoutingProfile.ToDB(), Device: ifc.Device, DeviceInstance: ifc.DeviceInstance, @@ -2879,7 +2879,7 @@ func (uih UpdateInstanceHandler) Handle(c echo.Context) error { dbInterfaces = append(dbInterfaces, cdbm.Interface{ VpcID: &interfaceVpcID, Vpc: interfaceVpc, - VpcIPFamilyMode: cutil.GetPtr(cdbm.InterfaceVpcIPFamilyModeIPv4Only), + VpcIPFamilyMode: cutil.GetPtr(ifc.VpcIPFamilyMode()), InlineRoutingProfile: ifc.InlineRoutingProfile.ToDB(), Device: ifc.Device, DeviceInstance: ifc.DeviceInstance, diff --git a/rest-api/api/pkg/api/handler/instance_test.go b/rest-api/api/pkg/api/handler/instance_test.go index eeb5e4efa1..e0f7521257 100644 --- a/rest-api/api/pkg/api/handler/instance_test.go +++ b/rest-api/api/pkg/api/handler/instance_test.go @@ -693,7 +693,7 @@ func assertInterfaceRoutingProfilePrefixes(t *testing.T, actual *corev1.Instance // assertInterfaceVpcSelection verifies that an Interface carries the expected // Controller-managed VPC selection intent. -func assertInterfaceVpcSelection(t *testing.T, actual *corev1.InstanceInterfaceConfig, controllerVpcID uuid.UUID) { +func assertInterfaceVpcSelection(t *testing.T, actual *corev1.InstanceInterfaceConfig, controllerVpcID uuid.UUID, familyMode cdbm.InterfaceVpcIPFamilyMode) { t.Helper() require.NotNil(t, actual) @@ -702,7 +702,19 @@ func assertInterfaceVpcSelection(t *testing.T, actual *corev1.InstanceInterfaceC require.NotNil(t, selection.Vpc) require.NotNil(t, selection.Vpc.VpcId) assert.Equal(t, controllerVpcID.String(), selection.Vpc.VpcId.Value) - assert.Equal(t, corev1.InstanceInterfaceIpFamilyMode_INSTANCE_INTERFACE_IP_FAMILY_MODE_IPV4_ONLY, selection.Vpc.FamilyMode) + + var expectedFamilyMode corev1.InstanceInterfaceIpFamilyMode + switch familyMode { + case cdbm.InterfaceVpcIPFamilyModeIPv4Only: + expectedFamilyMode = corev1.InstanceInterfaceIpFamilyMode_INSTANCE_INTERFACE_IP_FAMILY_MODE_IPV4_ONLY + case cdbm.InterfaceVpcIPFamilyModeIPv6Only: + expectedFamilyMode = corev1.InstanceInterfaceIpFamilyMode_INSTANCE_INTERFACE_IP_FAMILY_MODE_IPV6_ONLY + case cdbm.InterfaceVpcIPFamilyModeDualStack: + expectedFamilyMode = corev1.InstanceInterfaceIpFamilyMode_INSTANCE_INTERFACE_IP_FAMILY_MODE_DUAL_STACK + default: + require.FailNow(t, "unsupported VPC IP family mode", "mode: %s", familyMode) + } + assert.Equal(t, expectedFamilyMode, selection.Vpc.FamilyMode) } func TestBuildInstanceNetworkConfig(t *testing.T) { @@ -1688,7 +1700,7 @@ func TestCreateInstanceHandler_Handle(t *testing.T) { verifyChildSpanner: true, }, { - name: "test Instance create API endpoint preserves VPC selection intent", + name: "test Instance create API endpoint preserves IPv6-only and dual-stack VPC selection intent", fields: fields{ dbSession: dbSession, tc: tc, @@ -1705,12 +1717,12 @@ func TestCreateInstanceHandler_Handle(t *testing.T) { Interfaces: []model.APIInterfaceCreateOrUpdateRequest{ { VpcID: cutil.GetPtr(vpc9.ID.String()), - IPFamilies: []model.IPFamily{model.IPFamilyIPv4}, + IPFamilies: []model.IPFamily{model.IPFamilyIPv6}, IsPhysical: true, }, { VpcID: cutil.GetPtr(vpc9Secondary.ID.String()), - IPFamilies: []model.IPFamily{model.IPFamilyIPv4}, + IPFamilies: []model.IPFamily{model.IPFamilyIPv4, model.IPFamilyIPv6}, }, }, }, @@ -3887,7 +3899,7 @@ func TestCreateInstanceHandler_Handle(t *testing.T) { require.NotNil(t, dbIfcs[i].VpcID) assert.Equal(t, *tt.args.reqData.Interfaces[i].VpcID, dbIfcs[i].VpcID.String()) require.NotNil(t, dbIfcs[i].VpcIPFamilyMode) - assert.Equal(t, cdbm.InterfaceVpcIPFamilyModeIPv4Only, *dbIfcs[i].VpcIPFamilyMode) + assert.Equal(t, tt.args.reqData.Interfaces[i].VpcIPFamilyMode(), *dbIfcs[i].VpcIPFamilyMode) assert.Nil(t, dbIfcs[i].VpcPrefixID) } } @@ -3925,7 +3937,7 @@ func TestCreateInstanceHandler_Handle(t *testing.T) { if reqIfc.VpcID != nil { expectedControllerVpcID, ok := tt.expectedControllerVpcIDs[*reqIfc.VpcID] require.True(t, ok) - assertInterfaceVpcSelection(t, req.Config.Network.Interfaces[i], expectedControllerVpcID) + assertInterfaceVpcSelection(t, req.Config.Network.Interfaces[i], expectedControllerVpcID, reqIfc.VpcIPFamilyMode()) } } } @@ -6046,7 +6058,7 @@ func TestUpdateInstanceHandler_Handle(t *testing.T) { wantErr: false, }, { - name: "test Instance update API endpoint preserves requested VPC selection intent", + name: "test Instance update API endpoint preserves requested IPv6-only and dual-stack VPC selection intent", fields: fields{ dbSession: dbSession, tc: tc, @@ -6059,12 +6071,12 @@ func TestUpdateInstanceHandler_Handle(t *testing.T) { Interfaces: []model.APIInterfaceCreateOrUpdateRequest{ { VpcID: cutil.GetPtr(vpcSelection.ID.String()), - IPFamilies: []model.IPFamily{model.IPFamilyIPv4}, + IPFamilies: []model.IPFamily{model.IPFamilyIPv6}, IsPhysical: true, }, { VpcID: cutil.GetPtr(vpcSelectionSecondary.ID.String()), - IPFamilies: []model.IPFamily{model.IPFamilyIPv4}, + IPFamilies: []model.IPFamily{model.IPFamilyIPv4, model.IPFamilyIPv6}, }, }, }, @@ -7149,7 +7161,7 @@ func TestUpdateInstanceHandler_Handle(t *testing.T) { require.NotNil(t, persistedIfcs[i].VpcID) assert.Equal(t, *reqIfc.VpcID, persistedIfcs[i].VpcID.String()) require.NotNil(t, persistedIfcs[i].VpcIPFamilyMode) - assert.Equal(t, cdbm.InterfaceVpcIPFamilyModeIPv4Only, *persistedIfcs[i].VpcIPFamilyMode) + assert.Equal(t, reqIfc.VpcIPFamilyMode(), *persistedIfcs[i].VpcIPFamilyMode) assert.Nil(t, persistedIfcs[i].VpcPrefixID) } } @@ -7395,7 +7407,7 @@ func TestUpdateInstanceHandler_Handle(t *testing.T) { } expectedControllerVpcID, ok := tt.expectedControllerVpcIDs[interfaceVpcID] require.True(t, ok) - assertInterfaceVpcSelection(t, siteIfc, expectedControllerVpcID) + assertInterfaceVpcSelection(t, siteIfc, expectedControllerVpcID, *reqInsIfcs[i].VpcIPFamilyMode) default: assert.Failf(t, "unexpected Interface network details", "%T", networkDetails) } diff --git a/rest-api/api/pkg/api/handler/instancebatch.go b/rest-api/api/pkg/api/handler/instancebatch.go index fab4a051c1..63e0ff0c8c 100644 --- a/rest-api/api/pkg/api/handler/instancebatch.go +++ b/rest-api/api/pkg/api/handler/instancebatch.go @@ -691,7 +691,7 @@ func (bcih BatchCreateInstanceHandler) Handle(c echo.Context) error { dbInterfaces = append(dbInterfaces, cdbm.Interface{ VpcID: &interfaceVpcID, Vpc: interfaceVpc, - VpcIPFamilyMode: cutil.GetPtr(cdbm.InterfaceVpcIPFamilyModeIPv4Only), + VpcIPFamilyMode: cutil.GetPtr(ifc.VpcIPFamilyMode()), InlineRoutingProfile: ifc.InlineRoutingProfile.ToDB(), Device: ifc.Device, DeviceInstance: ifc.DeviceInstance, diff --git a/rest-api/api/pkg/api/handler/instancebatch_test.go b/rest-api/api/pkg/api/handler/instancebatch_test.go index 960e3c7b4a..5b6f2d7cd2 100644 --- a/rest-api/api/pkg/api/handler/instancebatch_test.go +++ b/rest-api/api/pkg/api/handler/instancebatch_test.go @@ -317,7 +317,7 @@ func TestBatchCreateInstanceHandler_Handle(t *testing.T) { wantErr: false, }, { - name: "test batch instance create API endpoint preserves VPC selection intent", + name: "test batch instance create API endpoint preserves IPv6-only and dual-stack VPC selection intent", fields: fields{ dbSession: dbSession, tc: tc, @@ -336,12 +336,12 @@ func TestBatchCreateInstanceHandler_Handle(t *testing.T) { Interfaces: []model.APIInterfaceCreateOrUpdateRequest{ { VpcID: cutil.GetPtr(vpcFNN.ID.String()), - IPFamilies: []model.IPFamily{model.IPFamilyIPv4}, + IPFamilies: []model.IPFamily{model.IPFamilyIPv6}, IsPhysical: true, }, { VpcID: cutil.GetPtr(vpcFNNSecondary.ID.String()), - IPFamilies: []model.IPFamily{model.IPFamilyIPv4}, + IPFamilies: []model.IPFamily{model.IPFamilyIPv4, model.IPFamilyIPv6}, }, }, }, @@ -1396,7 +1396,7 @@ func TestBatchCreateInstanceHandler_Handle(t *testing.T) { require.NotNil(t, dbIfcs[j].VpcID) assert.Equal(t, *reqIfc.VpcID, dbIfcs[j].VpcID.String()) require.NotNil(t, dbIfcs[j].VpcIPFamilyMode) - assert.Equal(t, cdbm.InterfaceVpcIPFamilyModeIPv4Only, *dbIfcs[j].VpcIPFamilyMode) + assert.Equal(t, reqIfc.VpcIPFamilyMode(), *dbIfcs[j].VpcIPFamilyMode) assert.Nil(t, dbIfcs[j].VpcPrefixID) } } @@ -1446,7 +1446,7 @@ func TestBatchCreateInstanceHandler_Handle(t *testing.T) { if reqIfc.VpcID != nil { expectedControllerVpcID, ok := tt.expectedControllerVpcIDs[*reqIfc.VpcID] require.True(t, ok) - assertInterfaceVpcSelection(t, instReq.Config.Network.Interfaces[j], expectedControllerVpcID) + assertInterfaceVpcSelection(t, instReq.Config.Network.Interfaces[j], expectedControllerVpcID, reqIfc.VpcIPFamilyMode()) } } } diff --git a/rest-api/api/pkg/api/model/interface.go b/rest-api/api/pkg/api/model/interface.go index 64ce352e15..adae47615f 100644 --- a/rest-api/api/pkg/api/model/interface.go +++ b/rest-api/api/pkg/api/model/interface.go @@ -181,22 +181,27 @@ func (ifcr *APIInterfaceCreateOrUpdateRequest) Validate() error { } } - normalizedFamilies := make([]IPFamily, 0, len(ifcr.IPFamilies)) - seenFamilies := make(map[IPFamily]struct{}, len(ifcr.IPFamilies)) - // TODO: Allow IPv6-only and dual-stack selection when supported by the Controller API. + seenIPv4 := false + seenIPv6 := false for _, family := range ifcr.IPFamilies { switch family { case IPFamilyIPv4: + seenIPv4 = true + case IPFamilyIPv6: + seenIPv6 = true default: return validation.Errors{ "ipFamilies": fmt.Errorf("invalid IP family `%s`", family), } } + } - if _, exists := seenFamilies[family]; !exists { - normalizedFamilies = append(normalizedFamilies, family) - seenFamilies[family] = struct{}{} - } + normalizedFamilies := make([]IPFamily, 0, 2) + if seenIPv4 { + normalizedFamilies = append(normalizedFamilies, IPFamilyIPv4) + } + if seenIPv6 { + normalizedFamilies = append(normalizedFamilies, IPFamilyIPv6) } ifcr.IPFamilies = normalizedFamilies } else if ifcr.IPFamilies != nil { @@ -250,6 +255,17 @@ func (ifcr *APIInterfaceCreateOrUpdateRequest) Validate() error { return nil } +// VpcIPFamilyMode converts validated VPC-selection families to the DB model. +func (ifcr APIInterfaceCreateOrUpdateRequest) VpcIPFamilyMode() cdbm.InterfaceVpcIPFamilyMode { + if len(ifcr.IPFamilies) == 2 { + return cdbm.InterfaceVpcIPFamilyModeDualStack + } + if len(ifcr.IPFamilies) == 1 && ifcr.IPFamilies[0] == IPFamilyIPv6 { + return cdbm.InterfaceVpcIPFamilyModeIPv6Only + } + return cdbm.InterfaceVpcIPFamilyModeIPv4Only +} + // APIInterface is the data structure to capture Interface type APIInterface struct { // ID is the unique UUID v4 identifier for the Interface diff --git a/rest-api/api/pkg/api/model/interface_test.go b/rest-api/api/pkg/api/model/interface_test.go index 63686634d4..b87a7a4af5 100644 --- a/rest-api/api/pkg/api/model/interface_test.go +++ b/rest-api/api/pkg/api/model/interface_test.go @@ -306,10 +306,10 @@ func TestAPIInterfaceCreateRequest_Validate(t *testing.T) { name: "test valid Interface VPC request normalizes duplicate IP families", fields: fields{ VpcID: cutil.GetPtr(uuid.NewString()), - IPFamilies: []IPFamily{IPFamilyIPv4, IPFamilyIPv4}, + IPFamilies: []IPFamily{IPFamilyIPv6, IPFamilyIPv4, IPFamilyIPv6, IPFamilyIPv4}, }, wantErr: false, - wantIPFamilies: []IPFamily{IPFamilyIPv4}, + wantIPFamilies: []IPFamily{IPFamilyIPv4, IPFamilyIPv6}, }, { name: "test invalid Interface Subnet request", @@ -405,22 +405,22 @@ func TestAPIInterfaceCreateRequest_Validate(t *testing.T) { wantErrorMessage: "invalid IP family `IPX`", }, { - name: "test invalid Interface IPv6-only VPC request", + name: "test valid Interface IPv6-only VPC request", fields: fields{ VpcID: cutil.GetPtr(uuid.NewString()), IPFamilies: []IPFamily{IPFamilyIPv6}, }, - wantErr: true, - wantErrorMessage: "invalid IP family `IPv6`", + wantErr: false, + wantIPFamilies: []IPFamily{IPFamilyIPv6}, }, { - name: "test invalid Interface dual-stack VPC request", + name: "test valid Interface dual-stack VPC request", fields: fields{ VpcID: cutil.GetPtr(uuid.NewString()), IPFamilies: []IPFamily{IPFamilyIPv4, IPFamilyIPv6}, }, - wantErr: true, - wantErrorMessage: "invalid IP family `IPv6`", + wantErr: false, + wantIPFamilies: []IPFamily{IPFamilyIPv4, IPFamilyIPv6}, }, { name: "test valid Interface device and deviceInterface request", @@ -614,3 +614,34 @@ func TestAPIInterfaceCreateRequest_Validate(t *testing.T) { }) } } + +func TestAPIInterfaceCreateOrUpdateRequest_VpcIPFamilyMode(t *testing.T) { + tests := []struct { + name string + families []IPFamily + wantFamily cdbm.InterfaceVpcIPFamilyMode + }{ + { + name: "IPv4-only", + families: []IPFamily{IPFamilyIPv4}, + wantFamily: cdbm.InterfaceVpcIPFamilyModeIPv4Only, + }, + { + name: "IPv6-only", + families: []IPFamily{IPFamilyIPv6}, + wantFamily: cdbm.InterfaceVpcIPFamilyModeIPv6Only, + }, + { + name: "dual-stack", + families: []IPFamily{IPFamilyIPv4, IPFamilyIPv6}, + wantFamily: cdbm.InterfaceVpcIPFamilyModeDualStack, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := APIInterfaceCreateOrUpdateRequest{IPFamilies: tt.families} + assert.Equal(t, tt.wantFamily, req.VpcIPFamilyMode()) + }) + } +} diff --git a/rest-api/docs/index.html b/rest-api/docs/index.html index 192338f661..475ab771dc 100644 --- a/rest-api/docs/index.html +++ b/rest-api/docs/index.html @@ -9408,8 +9408,8 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa fiNpIH bAoMjv">

ID of the VPC Prefix to attach to the Interface

vpcId
string <uuid>

ID of the VPC from which the Controller should select a prefix. ipFamilies must also be specified, and ipAddress cannot be specified.

-
ipFamilies
Array of strings non-empty
Items Value: "IPv4"

Address families requested for Controller prefix selection. Required with vpcId and prohibited otherwise. Only IPv4 is currently accepted.

+
ipFamilies
Array of strings non-empty
Items Enum: "IPv4" "IPv6"

Address families requested for Controller prefix selection. Required with vpcId and prohibited otherwise. Specify IPv4, IPv6, or both for dual-stack allocation. Duplicate values are accepted and normalized in IPv4, then IPv6 order.

ipAddress
string or null

Explicitly requested IP address for the interface. It can only be specified with an explicit vpcPrefixId. The least-significant host bit must be 1.

InterfaceInlineRoutingProfile (object) or null
Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa fiNpIH bAoMjv">

ID of the VPC Prefix to attach to the Interface

vpcId
string <uuid>

ID of the VPC from which the Controller should select a prefix. ipFamilies must also be specified, and ipAddress cannot be specified.

-
ipFamilies
Array of strings non-empty
Items Value: "IPv4"

Address families requested for Controller prefix selection. Required with vpcId and prohibited otherwise. Only IPv4 is currently accepted.

+
ipFamilies
Array of strings non-empty
Items Enum: "IPv4" "IPv6"

Address families requested for Controller prefix selection. Required with vpcId and prohibited otherwise. Specify IPv4, IPv6, or both for dual-stack allocation. Duplicate values are accepted and normalized in IPv4, then IPv6 order.

ipAddress
string or null

Explicitly requested IP address for the interface. It can only be specified with an explicit vpcPrefixId. The least-significant host bit must be 1.

InterfaceInlineRoutingProfile (object) or null
Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa fiNpIH bAoMjv">

ID of the VPC Prefix to attach to the Interface

vpcId
string <uuid>

ID of the VPC from which the Controller should select a prefix. ipFamilies must also be specified, and ipAddress cannot be specified.

-
ipFamilies
Array of strings non-empty
Items Value: "IPv4"

Address families requested for Controller prefix selection. Required with vpcId and prohibited otherwise. Only IPv4 is currently accepted.

+
ipFamilies
Array of strings non-empty
Items Enum: "IPv4" "IPv6"

Address families requested for Controller prefix selection. Required with vpcId and prohibited otherwise. Specify IPv4, IPv6, or both for dual-stack allocation. Duplicate values are accepted and normalized in IPv4, then IPv6 order.

ipAddress
string or null

Explicitly requested IP address for the interface. It can only be specified with an explicit vpcPrefixId. The least-significant host bit must be 1.

InterfaceInlineRoutingProfile (object) or null
Typical API Call Flow for Tenant