diff --git a/CHANGELOG.md b/CHANGELOG.md index da84c5837..1101aeb35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ All notable changes to this project will be documented in this file. - Fix multicast subscriber tunnel source resolution for NAT environments — resolve local interface IP instead of using public IP - Added multicast filters to access-pass list, enabling filtering by publisher/subscriber role and identifying access passes not authorized for a specific multicast group. - Device interface `--bandwidth` and `--cir` flags now accept Kbps, Mbps, or Gbps units; `interface list` displays those values as human-readable strings + - Add duplicate IP check to prevent a user from assigning the same IP more than once - Client - Fix BGP `OnClose` deleting routes from all peers instead of only the closing peer, preventing multicast teardown from nuking unicast routes - Skip route deletion on `OnClose` for `NoInstall` peers (multicast) since they never install kernel routes diff --git a/e2e/compatibility_test.go b/e2e/compatibility_test.go index 1e0e0e599..b655bcfc9 100644 --- a/e2e/compatibility_test.go +++ b/e2e/compatibility_test.go @@ -64,7 +64,7 @@ var knownIncompatibilities = map[string]knownIncompat{ // All multicast operations that depend on multicast_group_create. When the group // can't be created (< 0.8.1), these all fail with "MulticastGroup not found". - "write/multicast_group_wait_activated": {minVersion: "0.8.1"}, + "write/multicast_group_wait_activated": {minVersion: "0.8.1"}, // multicast_group_update: In addition to the dependency above, v0.8.1-v0.8.8 parsed // --max-bandwidth as a plain integer. v0.8.9 added validate_parse_bandwidth (a855ca7a) // which accepts unit strings like "200Mbps". diff --git a/e2e/interface_validation_test.go b/e2e/interface_validation_test.go index 884962417..e3db7168e 100644 --- a/e2e/interface_validation_test.go +++ b/e2e/interface_validation_test.go @@ -226,7 +226,7 @@ func TestE2E_InterfaceValidation(t *testing.T) { output, err := dn.Manager.Exec(t.Context(), []string{ "doublezero", "device", "interface", "create", testDeviceCode, testInterfaceName, - "--ip-net", "45.33.100.50/31", + "--ip-net", "198.51.100.1/31", "--bandwidth", "10G", }) diff --git a/smartcontract/cli/src/device/interface/create.rs b/smartcontract/cli/src/device/interface/create.rs index c2cfa1bb5..961b9500e 100644 --- a/smartcontract/cli/src/device/interface/create.rs +++ b/smartcontract/cli/src/device/interface/create.rs @@ -8,7 +8,7 @@ use crate::{ use clap::Args; use doublezero_program_common::{types::network_v4::NetworkV4, validate_iface}; use doublezero_sdk::commands::device::{ - get::GetDeviceCommand, interface::create::CreateDeviceInterfaceCommand, + get::GetDeviceCommand, interface::create::CreateDeviceInterfaceCommand, list::ListDeviceCommand, }; use std::io::Write; @@ -77,6 +77,26 @@ impl CreateDeviceInterfaceCliCommand { )) })?; + if let Some(ref ip_net) = self.ip_net { + let devices = client.list_device(ListDeviceCommand)?; + for dev in devices.values() { + if dev.contributor_pk != device.contributor_pk { + continue; + } + for iface in &dev.interfaces { + let iface = iface.into_current_version(); + if iface.ip_net == *ip_net { + eyre::bail!( + "IP {} is already assigned to interface {} on device {}", + ip_net, + iface.name, + dev.code + ); + } + } + } + } + let (signature, _) = client.create_device_interface(CreateDeviceInterfaceCommand { pubkey: device_pk, name: self.name.clone(), @@ -114,6 +134,129 @@ mod tests { use mockall::predicate; use solana_sdk::{pubkey::Pubkey, signature::Signature}; + #[test] + fn test_cli_device_interface_create_fails_duplicate_ip_across_contributor_devices() { + use std::collections::HashMap; + + let mut client = create_test_client(); + + let contributor_pk = Pubkey::new_unique(); + let device1_pubkey = Pubkey::new_unique(); + let device1 = Device { + account_type: AccountType::Device, + index: 1, + bump_seed: 255, + reference_count: 0, + code: "device1".to_string(), + contributor_pk, + location_pk: Pubkey::default(), + exchange_pk: Pubkey::new_unique(), + device_type: DeviceType::Hybrid, + public_ip: [1, 2, 3, 4].into(), + dz_prefixes: "1.2.3.4/32".parse().unwrap(), + status: DeviceStatus::Activated, + metrics_publisher_pk: Pubkey::default(), + owner: device1_pubkey, + mgmt_vrf: "default".to_string(), + interfaces: vec![], + max_users: 255, + users_count: 0, + device_health: doublezero_serviceability::state::device::DeviceHealth::ReadyForUsers, + desired_status: + doublezero_serviceability::state::device::DeviceDesiredStatus::Activated, + unicast_users_count: 0, + multicast_users_count: 0, + max_unicast_users: 0, + max_multicast_users: 0, + }; + + let device2_pubkey = Pubkey::new_unique(); + let device2 = Device { + account_type: AccountType::Device, + index: 2, + bump_seed: 254, + reference_count: 0, + code: "device2".to_string(), + contributor_pk, + location_pk: Pubkey::default(), + exchange_pk: Pubkey::new_unique(), + device_type: DeviceType::Hybrid, + public_ip: [5, 6, 7, 8].into(), + dz_prefixes: "5.6.7.8/32".parse().unwrap(), + status: DeviceStatus::Activated, + metrics_publisher_pk: Pubkey::default(), + owner: device2_pubkey, + mgmt_vrf: "default".to_string(), + interfaces: vec![CurrentInterfaceVersion { + status: InterfaceStatus::Activated, + name: "Loopback100".to_string(), + interface_type: InterfaceType::Loopback, + loopback_type: LoopbackType::Ipv4, + interface_cyoa: InterfaceCYOA::None, + interface_dia: InterfaceDIA::None, + bandwidth: 0, + cir: 0, + mtu: 1500, + routing_mode: RoutingMode::Static, + vlan_id: 0, + ip_net: "185.189.47.80/32".parse().unwrap(), + node_segment_idx: 0, + user_tunnel_endpoint: false, + } + .to_interface()], + max_users: 255, + users_count: 0, + device_health: doublezero_serviceability::state::device::DeviceHealth::ReadyForUsers, + desired_status: + doublezero_serviceability::state::device::DeviceDesiredStatus::Activated, + unicast_users_count: 0, + multicast_users_count: 0, + max_unicast_users: 0, + max_multicast_users: 0, + }; + + let mut devices = HashMap::new(); + devices.insert(device2_pubkey, device2); + + client.expect_check_requirements().returning(|_| Ok(())); + client.expect_get_device().returning({ + let device1 = device1.clone(); + move |_| Ok((device1_pubkey, device1.clone())) + }); + client + .expect_list_device() + .returning(move |_| Ok(devices.clone())); + + let mut output = Vec::new(); + let res = CreateDeviceInterfaceCliCommand { + device: device1_pubkey.to_string(), + name: "Loopback100".to_string(), + loopback_type: Some(types::LoopbackType::Ipv4), + interface_cyoa: None, + interface_dia: None, + ip_net: Some("185.189.47.80/32".parse().unwrap()), + bandwidth: 0, + cir: 0, + mtu: 1500, + routing_mode: types::RoutingMode::Static, + vlan_id: 0, + user_tunnel_endpoint: None, + wait: false, + } + .execute(&client, &mut output); + + assert!(res.is_err()); + let err = res.unwrap_err().to_string(); + assert!( + err.contains("185.189.47.80/32"), + "Expected IP in error, got: {err}" + ); + assert!( + err.contains("device2"), + "Expected device code in error, got: {err}" + ); + } + #[test] fn test_cli_device_interface_create() { let mut client = create_test_client(); diff --git a/smartcontract/cli/src/device/interface/update.rs b/smartcontract/cli/src/device/interface/update.rs index 4304468b6..1e2f8c551 100644 --- a/smartcontract/cli/src/device/interface/update.rs +++ b/smartcontract/cli/src/device/interface/update.rs @@ -6,9 +6,9 @@ use crate::{ validators::{validate_parse_bandwidth, validate_pubkey_or_code}, }; use clap::Args; -use doublezero_program_common::validate_iface; +use doublezero_program_common::{types::network_v4::NetworkV4, validate_iface}; use doublezero_sdk::commands::device::{ - get::GetDeviceCommand, interface::update::UpdateDeviceInterfaceCommand, + get::GetDeviceCommand, interface::update::UpdateDeviceInterfaceCommand, list::ListDeviceCommand, }; use std::io::Write; @@ -91,6 +91,44 @@ impl UpdateDeviceInterfaceCliCommand { )); } + let parsed_ip_net: Option = self + .ip_net + .as_ref() + .map(|s| s.parse()) + .transpose() + .map_err(|e| eyre::eyre!("Invalid IP network: {}", e))?; + + if let Some(ref ip_net) = parsed_ip_net { + let devices = client.list_device(ListDeviceCommand)?; + for (pk, dev) in &devices { + if dev.contributor_pk != device.contributor_pk { + continue; + } + for iface in &dev.interfaces { + let iface_v = iface.into_current_version(); + // Skip the interface being updated + if *pk == device_pk && iface_v.name.eq_ignore_ascii_case(&self.name) { + continue; + } + if iface_v.ip_net == *ip_net { + eyre::bail!( + "IP {} is already assigned to interface {} on device {}", + ip_net, + iface_v.name, + dev.code + ); + } + } + } + } + + let parsed_status = self + .status + .as_ref() + .map(|s| s.parse()) + .transpose() + .map_err(|e| eyre::eyre!("Invalid status: {}", e))?; + let signature = client.update_device_interface(UpdateDeviceInterfaceCommand { pubkey: device_pk, name: self.name.clone(), @@ -103,8 +141,8 @@ impl UpdateDeviceInterfaceCliCommand { routing_mode: self.routing_mode.map(|rm| rm.into()), vlan_id: self.vlan_id, user_tunnel_endpoint: self.user_tunnel_endpoint, - status: self.status.as_ref().map(|s| s.parse().unwrap()), - ip_net: self.ip_net.as_ref().map(|s| s.parse().unwrap()), + status: parsed_status, + ip_net: parsed_ip_net, node_segment_idx: self.node_segment_idx, })?; writeln!(out, "Signature: {signature}")?; @@ -128,6 +166,7 @@ mod tests { }; use mockall::predicate; use solana_sdk::{pubkey::Pubkey, signature::Signature}; + use std::collections::HashMap; #[test] fn test_cli_device_interface_update_success() { @@ -203,12 +242,18 @@ mod tests { .expect_check_requirements() .with(predicate::eq(CHECK_ID_JSON | CHECK_BALANCE)) .returning(|_| Ok(())); + let device1_for_list = device1.clone(); client .expect_get_device() .with(predicate::eq(GetDeviceCommand { pubkey_or_code: device1_pubkey.to_string(), })) .returning(move |_| Ok((device1_pubkey, device1.clone()))); + client.expect_list_device().returning(move |_| { + let mut devices = HashMap::new(); + devices.insert(device1_pubkey, device1_for_list.clone()); + Ok(devices) + }); client .expect_update_device_interface() @@ -255,4 +300,143 @@ mod tests { let output_str = String::from_utf8(output).unwrap(); assert_eq!(output_str, format!("Signature: {signature}\n")); } + + #[test] + fn test_cli_device_interface_update_fails_duplicate_ip_across_contributor_devices() { + let mut client = create_test_client(); + + let contributor_pk = Pubkey::new_unique(); + let device1_pubkey = Pubkey::new_unique(); + let device1 = Device { + account_type: AccountType::Device, + index: 1, + bump_seed: 2, + reference_count: 0, + code: "device1".to_string(), + contributor_pk, + location_pk: Pubkey::default(), + exchange_pk: Pubkey::default(), + device_type: DeviceType::Hybrid, + public_ip: [1, 2, 3, 4].into(), + dz_prefixes: "1.2.3.4/32".parse().unwrap(), + status: DeviceStatus::Activated, + metrics_publisher_pk: Pubkey::default(), + owner: Pubkey::default(), + mgmt_vrf: "default".to_string(), + interfaces: vec![CurrentInterfaceVersion { + status: InterfaceStatus::Activated, + name: "Loopback0".to_string(), + interface_type: InterfaceType::Loopback, + loopback_type: LoopbackType::Ipv4, + interface_cyoa: InterfaceCYOA::None, + interface_dia: doublezero_serviceability::state::interface::InterfaceDIA::None, + bandwidth: 0, + cir: 0, + mtu: 1500, + routing_mode: RoutingMode::Static, + vlan_id: 0, + ip_net: "10.0.0.1/32".parse().unwrap(), + node_segment_idx: 0, + user_tunnel_endpoint: false, + } + .to_interface()], + max_users: 255, + users_count: 0, + device_health: doublezero_serviceability::state::device::DeviceHealth::ReadyForUsers, + desired_status: + doublezero_serviceability::state::device::DeviceDesiredStatus::Activated, + unicast_users_count: 0, + multicast_users_count: 0, + max_unicast_users: 0, + max_multicast_users: 0, + }; + + let device2_pubkey = Pubkey::new_unique(); + let device2 = Device { + account_type: AccountType::Device, + index: 2, + bump_seed: 3, + reference_count: 0, + code: "device2".to_string(), + contributor_pk, + location_pk: Pubkey::default(), + exchange_pk: Pubkey::default(), + device_type: DeviceType::Hybrid, + public_ip: [5, 6, 7, 8].into(), + dz_prefixes: "5.6.7.8/32".parse().unwrap(), + status: DeviceStatus::Activated, + metrics_publisher_pk: Pubkey::default(), + owner: Pubkey::default(), + mgmt_vrf: "default".to_string(), + interfaces: vec![CurrentInterfaceVersion { + status: InterfaceStatus::Activated, + name: "Loopback100".to_string(), + interface_type: InterfaceType::Loopback, + loopback_type: LoopbackType::Ipv4, + interface_cyoa: InterfaceCYOA::None, + interface_dia: doublezero_serviceability::state::interface::InterfaceDIA::None, + bandwidth: 0, + cir: 0, + mtu: 1500, + routing_mode: RoutingMode::Static, + vlan_id: 0, + ip_net: "185.189.47.80/32".parse().unwrap(), + node_segment_idx: 0, + user_tunnel_endpoint: false, + } + .to_interface()], + max_users: 255, + users_count: 0, + device_health: doublezero_serviceability::state::device::DeviceHealth::ReadyForUsers, + desired_status: + doublezero_serviceability::state::device::DeviceDesiredStatus::Activated, + unicast_users_count: 0, + multicast_users_count: 0, + max_unicast_users: 0, + max_multicast_users: 0, + }; + + let mut devices = HashMap::new(); + devices.insert(device1_pubkey, device1.clone()); + devices.insert(device2_pubkey, device2); + + client.expect_check_requirements().returning(|_| Ok(())); + client + .expect_get_device() + .returning(move |_| Ok((device1_pubkey, device1.clone()))); + client + .expect_list_device() + .returning(move |_| Ok(devices.clone())); + + let mut output = Vec::new(); + let res = UpdateDeviceInterfaceCliCommand { + pubkey_or_code: device1_pubkey.to_string(), + name: "Loopback0".to_string(), + loopback_type: None, + interface_cyoa: None, + interface_dia: None, + bandwidth: None, + cir: None, + mtu: None, + routing_mode: None, + vlan_id: None, + user_tunnel_endpoint: None, + status: None, + ip_net: Some("185.189.47.80/32".to_string()), + node_segment_idx: None, + wait: false, + } + .execute(&client, &mut output); + + assert!(res.is_err()); + let err = res.unwrap_err().to_string(); + assert!( + err.contains("185.189.47.80/32"), + "Expected IP in error, got: {err}" + ); + assert!( + err.contains("device2"), + "Expected device code in error, got: {err}" + ); + } }