From c86e7b3771690c61cb3303cae571f499be81a6f9 Mon Sep 17 00:00:00 2001 From: Jorik Cronenberg Date: Wed, 22 Nov 2023 14:55:16 +0100 Subject: [PATCH] Add basic routes to migration --- rust/migrate-wicked/src/interface.rs | 123 +++++++++++++----- .../system-connections/eth0.nmconnection | 29 +++++ .../wicked_xml/routes.xml} | 32 +++++ .../system-connections/eth0.nmconnection | 21 --- 4 files changed, 151 insertions(+), 54 deletions(-) create mode 100644 rust/migrate-wicked/tests/routes/system-connections/eth0.nmconnection rename rust/migrate-wicked/tests/{single_gateway/wicked_xml/single_gateway.xml => routes/wicked_xml/routes.xml} (56%) delete mode 100644 rust/migrate-wicked/tests/single_gateway/system-connections/eth0.nmconnection diff --git a/rust/migrate-wicked/src/interface.rs b/rust/migrate-wicked/src/interface.rs index f2d75bcd14..85f9fe1936 100644 --- a/rust/migrate-wicked/src/interface.rs +++ b/rust/migrate-wicked/src/interface.rs @@ -1,4 +1,4 @@ -use agama_dbus_server::network::model::{self, IpConfig, Ipv4Method, Ipv6Method, Parent}; +use agama_dbus_server::network::model::{self, IpConfig, IpRoute, Ipv4Method, Ipv6Method, Parent}; use cidr::IpInet; use serde::{Deserialize, Deserializer, Serialize}; use serde_with::{ @@ -513,8 +513,8 @@ impl Interface { }; let mut addresses: Vec = vec![]; - let mut gateway4 = None; - let mut gateway6 = None; + let mut new_routes4: Vec = vec![]; + let mut new_routes6: Vec = vec![]; if let Some(ipv4_static) = &self.ipv4_static { if let Some(addresses_in) = &ipv4_static.addresses { for addr in addresses_in { @@ -523,18 +523,13 @@ impl Interface { } if let Some(routes) = &ipv4_static.routes { for route in routes { - if let Some(nexthops) = &route.nexthops { - // TODO fix when implementing better route handling - // the logged warning isn't really true for multiple hops - // as gateways just can't have multiple nexthops AFAICT - if gateway4.is_some() || nexthops.len() > 1 { - connection_result.warnings.push(anyhow::anyhow!( - "Multipath routing isn't natively supported by NetworkManager" - )); - } else { - gateway4 = Some(IpAddr::from_str(&nexthops[0].gateway).unwrap()); + new_routes4.push(match interface_route_to_ip_route(route) { + Ok(route) => route, + Err(e) => { + connection_result.warnings.push(e); + continue; } - } + }); } } } @@ -546,34 +541,68 @@ impl Interface { } if let Some(routes) = &ipv6_static.routes { for route in routes { - if let Some(nexthops) = &route.nexthops { - // TODO fix when implementing better route handling - // the logged warning isn't really true for multiple hops - // as gateways just can't have multiple nexthops AFAICT - if gateway6.is_some() || nexthops.len() > 1 { - connection_result.warnings.push(anyhow::anyhow!( - "Multipath routing isn't natively supported by NetworkManager" - )); - } else { - gateway6 = Some(IpAddr::from_str(&nexthops[0].gateway).unwrap()); + new_routes6.push(match interface_route_to_ip_route(route) { + Ok(route) => route, + Err(e) => { + connection_result.warnings.push(e); + continue; } - } + }); } } } + let routes4 = if !new_routes4.is_empty() { + Some(new_routes4) + } else { + None + }; + let routes6 = if !new_routes6.is_empty() { + Some(new_routes6) + } else { + None + }; + connection_result.ip_config = IpConfig { addresses, method4, method6, - gateway4, - gateway6, + routes4, + routes6, ..Default::default() }; Ok(connection_result) } } +fn interface_route_to_ip_route(route: &Route) -> Result { + let mut next_hop: Option = None; + if let Some(nexthops) = &route.nexthops { + if nexthops.len() > 1 { + return Err(anyhow::anyhow!( + "Multipath routing isn't natively supported by NetworkManager" + )); + } else { + next_hop = Some(IpAddr::from_str(&nexthops[0].gateway).unwrap()); + } + } + let destination = if route.destination.is_some() { + IpInet::from_str(route.destination.clone().unwrap().as_str())? + } else if next_hop.is_some() { + // In this case the route is a default route, so we need to give the destination a /0 as prefix length + // Note the ip address doesn't actually really matter to NetworkManager + IpInet::new(next_hop.unwrap(), 0)? + } else { + return Err(anyhow::anyhow!("Error occurred when parsing a route")); + }; + let metric = route.priority; + Ok(IpRoute { + destination, + next_hop, + metric, + }) +} + #[cfg(test)] mod tests { use super::*; @@ -638,22 +667,50 @@ mod tests { .to_string(), "128" ); - assert!(static_connection.base().ip_config.gateway4.is_some()); - assert_eq!( + assert!(static_connection.base().ip_config.routes4.is_some()); + assert!( static_connection .base() .ip_config - .gateway4 + .routes4 + .clone() + .unwrap() + .len() + == 1 + ); + assert_eq!( + static_connection.base().ip_config.routes4.clone().unwrap()[0] + .destination + .to_string(), + "127.0.0.1/0" + ); + assert_eq!( + static_connection.base().ip_config.routes4.clone().unwrap()[0] + .next_hop .unwrap() .to_string(), "127.0.0.1" ); - assert!(static_connection.base().ip_config.gateway6.is_some()); - assert_eq!( + assert!(static_connection.base().ip_config.routes6.is_some()); + assert!( static_connection .base() .ip_config - .gateway6 + .routes6 + .clone() + .unwrap() + .len() + == 1 + ); + assert_eq!( + static_connection.base().ip_config.routes6.clone().unwrap()[0] + .destination + .to_string(), + "::1/0" + ); + assert_eq!( + static_connection.base().ip_config.routes6.clone().unwrap()[0] + .next_hop .unwrap() .to_string(), "::1" diff --git a/rust/migrate-wicked/tests/routes/system-connections/eth0.nmconnection b/rust/migrate-wicked/tests/routes/system-connections/eth0.nmconnection new file mode 100644 index 0000000000..2640014dc5 --- /dev/null +++ b/rust/migrate-wicked/tests/routes/system-connections/eth0.nmconnection @@ -0,0 +1,29 @@ +[connection] +id=eth0 +uuid=e7736944-bd8b-4da3-8be9-84b333b2ccfa +type=ethernet +interface-name=eth0 + +[ethernet] + +[match] + +[ipv4] +address1=192.168.101.5/24 +address2=192.168.102.5/24 +method=manual +route1=192.168.101.0/24,192.168.101.1 +route2=192.168.102.0/24,192.168.102.1,1 +route3=192.168.102.0/24,192.168.102.2,2 +route4=192.168.103.0/24,192.168.101.1 +route5=192.168.104.0/24,192.168.102.1 +route6=192.168.102.1/0,192.168.102.1,1 + +[ipv6] +addr-gen-mode=default +address1=2001:db8:1::5/64 +method=manual +route1=2001:db8:1::1/0,2001:db8:1::1 +route2=2001:db8:1::/64,2001:db8:1::1,1 + +[proxy] diff --git a/rust/migrate-wicked/tests/single_gateway/wicked_xml/single_gateway.xml b/rust/migrate-wicked/tests/routes/wicked_xml/routes.xml similarity index 56% rename from rust/migrate-wicked/tests/single_gateway/wicked_xml/single_gateway.xml rename to rust/migrate-wicked/tests/routes/wicked_xml/routes.xml index bba5be0016..5e640e3621 100644 --- a/rust/migrate-wicked/tests/single_gateway/wicked_xml/single_gateway.xml +++ b/rust/migrate-wicked/tests/routes/wicked_xml/routes.xml @@ -18,9 +18,35 @@ 192.168.101.0/24 + + 192.168.101.1 + 192.168.102.0/24 + + 192.168.102.1 + + 1 + + + 192.168.102.0/24 + + 192.168.102.2 + + 2 + + + 192.168.103.0/24 + + 192.168.101.1 + + + + 192.168.104.0/24 + + 192.168.102.1 + @@ -42,6 +68,12 @@ 2001:db8:1::1 + + + 2001:db8:1::/64 + + 2001:db8:1::1 + 1 diff --git a/rust/migrate-wicked/tests/single_gateway/system-connections/eth0.nmconnection b/rust/migrate-wicked/tests/single_gateway/system-connections/eth0.nmconnection deleted file mode 100644 index 242e2b77a2..0000000000 --- a/rust/migrate-wicked/tests/single_gateway/system-connections/eth0.nmconnection +++ /dev/null @@ -1,21 +0,0 @@ -[connection] -id=eth0 -uuid=676a03ec-54a6-4e45-b7ab-2eb0e1c18a75 -type=ethernet -interface-name=eth0 - -[ethernet] - -[match] - -[ipv4] -address1=192.168.101.5/24,192.168.102.1 -address2=192.168.102.5/24 -method=manual - -[ipv6] -addr-gen-mode=default -address1=2001:db8:1::5/64,2001:db8:1::1 -method=manual - -[proxy]