From 5d8bf7ed3e5fd6dc88ff9d4bd38e8c73282c1ba8 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 11 Apr 2025 09:52:02 +0100 Subject: [PATCH 01/10] Adding bridge support for the http api and CLI --- .../share/examples/network/bond.json | 14 ++++ .../share/examples/network/bridge.json | 13 ++++ rust/agama-lib/share/profile.schema.json | 65 +++++++++++++++++-- rust/agama-lib/src/network/store.rs | 21 +++++- rust/agama-network/src/model.rs | 63 +++++++++++++++++- rust/agama-network/src/nm/client.rs | 14 ++-- rust/agama-network/src/nm/dbus.rs | 11 +++- rust/agama-network/src/settings.rs | 34 ++++++++++ rust/agama-server/src/network/web.rs | 43 ++++++++---- 9 files changed, 248 insertions(+), 30 deletions(-) create mode 100644 rust/agama-lib/share/examples/network/bond.json create mode 100644 rust/agama-lib/share/examples/network/bridge.json diff --git a/rust/agama-lib/share/examples/network/bond.json b/rust/agama-lib/share/examples/network/bond.json new file mode 100644 index 0000000000..2ffa967169 --- /dev/null +++ b/rust/agama-lib/share/examples/network/bond.json @@ -0,0 +1,14 @@ +{ + "network": { + "connections": [ + { + "id": "bond0", + "bond": { + "ports": ["eth0", "eth1"], + "mode": "active-backup", + "options": "primary=eth1" + } + } + ] + } +} diff --git a/rust/agama-lib/share/examples/network/bridge.json b/rust/agama-lib/share/examples/network/bridge.json new file mode 100644 index 0000000000..5326e72ac8 --- /dev/null +++ b/rust/agama-lib/share/examples/network/bridge.json @@ -0,0 +1,13 @@ +{ + "network": { + "connections": [ + { + "id": "br0", + "bridge": { + "ports": ["eth0", "eth1"], + "stp": false + } + } + ] + } +} diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index 5a605a1338..d3ed15e864 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -434,6 +434,45 @@ } } }, + "bridge": { + "type": "object", + "title": "Bridge configuration", + "additionalProperties": false, + "properties": { + "stp": { + "title": "whether the Spanning Tree Protocol is enabled or not", + "type": "boolean" + }, + "forwardDelay": { + "title": "Spanning Tree Protocol forward delay, in seconds", + "type": "integer", + "minimum": "0" + }, + "priority": { + "title": "Spanning Tree Protocol priority. Lower values are 'better'", + "type": "integer", + "minimum": "0" + }, + "maxAge": { + "title": "Spanning Tree Protocol maximum message age, in seconds", + "type": "integer", + "minimum": "0" + }, + "helloTime": { + "title": "Spanning Tree Protocol hello time, in seconds", + "type": "integer", + "minimum": "0" + }, + "ports": { + "type": "array", + "items": { + "title": "A list of the interfaces or connections to be part of the bridge", + "type": "string" + } + } + } + }, + "match": { "type": "object", "title": "Match settings", @@ -669,7 +708,11 @@ } }, "required": ["name"], - "oneOf": [{ "required": ["body"] }, { "required": ["url"] }, { "required": ["content"]}] + "oneOf": [ + { "required": ["body"] }, + { "required": ["url"] }, + { "required": ["content"] } + ] }, "postPartitioning": { "title": "User-defined installation script that runs after the partitioning finishes", @@ -697,7 +740,11 @@ } }, "required": ["name"], - "oneOf": [{ "required": ["body"] }, { "required": ["url"] }, { "required": ["content"]}] + "oneOf": [ + { "required": ["body"] }, + { "required": ["url"] }, + { "required": ["content"] } + ] }, "postScript": { "title": "User-defined installation script that runs after the installation finishes", @@ -730,7 +777,11 @@ } }, "required": ["name"], - "oneOf": [{ "required": ["body"] }, { "required": ["url"] }, { "required": ["content"]}] + "oneOf": [ + { "required": ["body"] }, + { "required": ["url"] }, + { "required": ["content"] } + ] }, "initScript": { "title": "User-defined installation script that runs during the first boot of the target system, once the installation is finished", @@ -758,7 +809,11 @@ } }, "required": ["name"], - "oneOf": [{ "required": ["body"] }, { "required": ["url"] }, { "required": ["content"]}] + "oneOf": [ + { "required": ["body"] }, + { "required": ["url"] }, + { "required": ["content"] } + ] }, "file": { "title": "User-defined file to deploy", @@ -795,7 +850,7 @@ } }, "required": ["destination"], - "oneOf": [{ "required": ["url"] }, { "required": ["content"]}] + "oneOf": [{ "required": ["url"] }, { "required": ["content"] }] } } } diff --git a/rust/agama-lib/src/network/store.rs b/rust/agama-lib/src/network/store.rs index 6bdd5fceeb..2d0a4d07bc 100644 --- a/rust/agama-lib/src/network/store.rs +++ b/rust/agama-lib/src/network/store.rs @@ -94,6 +94,16 @@ fn add_ordered_connection( } } + if let Some(bridge) = &conn.bridge { + for port in &bridge.ports { + if let Some(conn) = find_connection(port, conns) { + add_ordered_connection(conn, conns, ordered); + } else if !ordered.contains(&conn.id) { + ordered.push(port.clone()); + } + } + } + if !ordered.contains(&conn.id) { ordered.push(conn.id.to_owned()) } @@ -119,7 +129,7 @@ fn default_connection(id: &str) -> NetworkConnection { #[cfg(test)] mod tests { use super::ordered_connections; - use crate::network::settings::{BondSettings, NetworkConnection}; + use crate::network::settings::{BondSettings, BridgeSettings, NetworkConnection}; #[test] fn test_ordered_connections() { @@ -132,6 +142,15 @@ mod tests { ..Default::default() }; + let bridge = NetworkConnection { + id: "br0".to_string(), + bridge: Some(BridgeSettings { + ports: vec!["eth0".to_string(), "eth1".to_string(), "eth3".to_string()], + ..Default::default() + }), + ..Default::default() + }; + let eth0 = NetworkConnection { id: "eth0".to_string(), ..Default::default() diff --git a/rust/agama-network/src/model.rs b/rust/agama-network/src/model.rs index ad0a8a0ed1..53006d052f 100644 --- a/rust/agama-network/src/model.rs +++ b/rust/agama-network/src/model.rs @@ -23,7 +23,9 @@ //! * This module contains the types that represent the network concepts. They are supposed to be //! agnostic from the real network service (e.g., NetworkManager). use crate::error::NetworkStateError; -use crate::settings::{BondSettings, IEEE8021XSettings, NetworkConnection, WirelessSettings}; +use crate::settings::{ + BondSettings, BridgeSettings, IEEE8021XSettings, NetworkConnection, WirelessSettings, +}; use crate::types::{BondMode, ConnectionState, DeviceState, DeviceType, Status, SSID}; use agama_utils::openapi::schemas; use cidr::IpInet; @@ -625,6 +627,10 @@ impl TryFrom for Connection { let config = BondConfig::try_from(bond_config)?; connection.config = config.into(); } + if let Some(bridge_config) = conn.bridge { + let config = BridgeConfig::try_from(bridge_config)?; + connection.config = config.into(); + } if let Some(ieee_8021x_config) = conn.ieee_8021x { connection.ieee_8021x_config = Some(IEEE8021XConfig::try_from(ieee_8021x_config)?); @@ -692,6 +698,9 @@ impl TryFrom for NetworkConnection { ConnectionConfig::Bond(config) => { connection.bond = Some(BondSettings::try_from(config)?); } + ConnectionConfig::Bridge(config) => { + connection.bridge = Some(BridgeSettings::try_from(config)?); + } _ => {} } @@ -720,6 +729,12 @@ pub enum PortConfig { Bridge(BridgePortConfig), } +impl From for ConnectionConfig { + fn from(value: BridgeConfig) -> Self { + Self::Bridge(value) + } +} + impl From for ConnectionConfig { fn from(value: BondConfig) -> Self { Self::Bond(value) @@ -1696,7 +1711,8 @@ impl TryFrom for BondSettings { #[derive(Debug, Default, PartialEq, Clone, Serialize, utoipa::ToSchema)] pub struct BridgeConfig { - pub stp: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub stp: Option, #[serde(skip_serializing_if = "Option::is_none")] pub priority: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -1709,6 +1725,49 @@ pub struct BridgeConfig { pub ageing_time: Option, } +impl TryFrom for BridgeConfig { + type Error = NetworkStateError; + + fn try_from(value: ConnectionConfig) -> Result { + match value { + ConnectionConfig::Bridge(config) => Ok(config), + _ => Err(NetworkStateError::UnexpectedConfiguration), + } + } +} + +impl TryFrom for BridgeConfig { + type Error = NetworkStateError; + + fn try_from(settings: BridgeSettings) -> Result { + let stp = settings.stp; + let priority = settings.priority; + let forward_delay = settings.forward_delay; + let hello_time = settings.forward_delay; + + Ok(BridgeConfig { + stp, + priority, + forward_delay, + hello_time, + ..Default::default() + }) + } +} + +impl TryFrom for BridgeSettings { + type Error = NetworkStateError; + + fn try_from(bridge: BridgeConfig) -> Result { + Ok(BridgeSettings { + stp: bridge.stp, + priority: bridge.priority, + forward_delay: bridge.forward_delay, + hello_time: bridge.hello_time, + ..Default::default() + }) + } +} #[derive(Debug, Default, PartialEq, Clone, Serialize, utoipa::ToSchema)] pub struct BridgePortConfig { #[serde(skip_serializing_if = "Option::is_none")] diff --git a/rust/agama-network/src/nm/client.rs b/rust/agama-network/src/nm/client.rs index 836c88765f..cca8caf760 100644 --- a/rust/agama-network/src/nm/client.rs +++ b/rust/agama-network/src/nm/client.rs @@ -182,7 +182,7 @@ impl<'a> NetworkManagerClient<'a> { /// Returns the list of network connections. pub async fn connections(&self) -> Result, NmError> { let mut controlled_by: HashMap = HashMap::new(); - let mut uuids_map: HashMap = HashMap::new(); + let mut uuids_map: HashMap = HashMap::new(); let proxy = SettingsProxy::new(&self.connection).await?; let paths = proxy.list_connections().await?; @@ -214,10 +214,10 @@ impl<'a> NetworkManagerClient<'a> { Self::add_secrets(&mut connection.config, &proxy).await?; if let Some(controller) = controller { - controlled_by.insert(connection.uuid, controller.to_string()); + controlled_by.insert(connection.uuid, controller); } if let Some(iname) = &connection.interface { - uuids_map.insert(iname.to_string(), connection.uuid); + uuids_map.insert(connection.uuid, iname.to_string()); } if self.settings_active_connection(path).await?.is_none() { connection.set_down() @@ -231,16 +231,16 @@ impl<'a> NetworkManagerClient<'a> { } for conn in connections.iter_mut() { - let Some(interface_name) = controlled_by.get(&conn.uuid) else { + let Some(conn_uuid) = controlled_by.get(&conn.uuid) else { continue; }; - if let Some(uuid) = uuids_map.get(interface_name) { - conn.controller = Some(*uuid); + if let Ok(controller) = Uuid::parse_str(&conn_uuid) { + conn.controller = Some(controller); } else { tracing::warn!( "Could not found a connection for the interface '{}' (required by connection '{}')", - interface_name, + conn_uuid, conn.id ); } diff --git a/rust/agama-network/src/nm/dbus.rs b/rust/agama-network/src/nm/dbus.rs index 3c05395072..b0db543145 100644 --- a/rust/agama-network/src/nm/dbus.rs +++ b/rust/agama-network/src/nm/dbus.rs @@ -547,7 +547,9 @@ fn bond_config_to_dbus(config: &BondConfig) -> HashMap<&str, zvariant::Value> { fn bridge_config_to_dbus(bridge: &BridgeConfig) -> HashMap<&str, zvariant::Value> { let mut hash = HashMap::new(); - hash.insert("stp", bridge.stp.into()); + if let Some(stp) = bridge.stp { + hash.insert("stp", stp.into()); + } if let Some(prio) = bridge.priority { hash.insert("priority", prio.into()); } @@ -573,7 +575,7 @@ fn bridge_config_from_dbus(conn: &OwnedNestedHash) -> Result, + #[serde(skip_serializing_if = "Option::is_none")] + pub priority: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub forward_delay: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub hello_time: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub max_age: Option, + #[serde(skip_serializing_if = "Vec::is_empty", default)] + pub ports: Vec, +} + +impl Default for BridgeSettings { + fn default() -> Self { + Self { + stp: None, + priority: None, + forward_delay: None, + hello_time: None, + max_age: None, + ports: vec![], + } + } +} + /// IEEE 802.1x (EAP) settings #[derive(Clone, Debug, Serialize, Deserialize, utoipa::ToSchema)] #[serde(rename_all = "camelCase")] @@ -215,6 +244,9 @@ pub struct NetworkConnection { /// Bonding settings if part of a bond #[serde(skip_serializing_if = "Option::is_none")] pub bond: Option, + /// Bridge settings if part of a bridge + #[serde(skip_serializing_if = "Option::is_none")] + pub bridge: Option, /// MAC address of the connection's interface #[serde(rename = "mac-address", skip_serializing_if = "Option::is_none")] pub mac_address: Option, @@ -250,6 +282,8 @@ impl NetworkConnection { DeviceType::Wireless } else if self.bond.is_some() { DeviceType::Bond + } else if self.bridge.is_some() { + DeviceType::Bridge } else { DeviceType::Ethernet } diff --git a/rust/agama-server/src/network/web.rs b/rust/agama-server/src/network/web.rs index f8fa1f38be..c8bd9b6ca5 100644 --- a/rust/agama-server/src/network/web.rs +++ b/rust/agama-server/src/network/web.rs @@ -30,6 +30,7 @@ use axum::{ routing::{delete, get, patch, post}, Json, Router, }; +use uuid::Uuid; use agama_lib::{ error::ServiceError, @@ -210,19 +211,37 @@ async fn connections( State(state): State, ) -> Result>, NetworkError> { let connections = state.network.get_connections().await?; - let mut result = vec![]; - - for conn in connections { - let state = conn.state.clone(); - let network_connection = NetworkConnection::try_from(conn)?; - let connection_with_state = NetworkConnectionWithState { - connection: network_connection, - state, - }; - result.push(connection_with_state); - } - Ok(Json(result)) + let network_connections = connections + .iter() + .filter(|c| c.controller.is_none()) + .map(|c| { + let state = c.state.clone(); + let mut conn = NetworkConnection::try_from(c.clone()).unwrap(); + if let Some(ref mut bond) = conn.bond { + bond.ports = ports_for(connections.to_owned(), c.uuid); + } + if let Some(ref mut bridge) = conn.bridge { + bridge.ports = ports_for(connections.to_owned(), c.uuid); + }; + let connection_with_state = NetworkConnectionWithState { + connection: conn, + state, + }; + + return connection_with_state; + }) + .collect(); + + Ok(Json(network_connections)) +} + +fn ports_for(connections: Vec, uuid: Uuid) -> Vec { + return connections + .iter() + .filter(|c| c.controller == Some(uuid) && c.interface.is_some()) + .map(|c| c.interface.to_owned().unwrap_or_default()) + .collect(); } #[utoipa::path( From e5ec0a880fdb85975d582d8decc99daa2c24d711 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 16 Apr 2025 08:28:00 +0100 Subject: [PATCH 02/10] Set ports in case of a bridge or bond connection --- rust/agama-network/src/system.rs | 15 +++++++++++++++ rust/agama-server/src/network/web.rs | 17 ++++++++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/rust/agama-network/src/system.rs b/rust/agama-network/src/system.rs index 41f04f3e07..82d1a0e402 100644 --- a/rust/agama-network/src/system.rs +++ b/rust/agama-network/src/system.rs @@ -209,6 +209,21 @@ impl NetworkSystemClient { Ok(result?) } + pub async fn set_ports( + &self, + connection: Connection, + ports: Vec, + ) -> Result<(), NetworkSystemError> { + let (tx, rx) = oneshot::channel(); + self.actions.send(Action::SetPorts( + connection.uuid, + Box::new(ports.clone()), + tx, + ))?; + let result = rx.await?; + Ok(result?) + } + /// Applies the network configuration. pub async fn apply(&self) -> Result<(), NetworkSystemError> { let (tx, rx) = oneshot::channel(); diff --git a/rust/agama-server/src/network/web.rs b/rust/agama-server/src/network/web.rs index c8bd9b6ca5..701180b9da 100644 --- a/rust/agama-server/src/network/web.rs +++ b/rust/agama-server/src/network/web.rs @@ -254,15 +254,26 @@ fn ports_for(connections: Vec, uuid: Uuid) -> Vec { )] async fn add_connection( State(state): State, - Json(conn): Json, + Json(net_conn): Json, ) -> Result, NetworkError> { - let conn = Connection::try_from(conn)?; + let bond = net_conn.clone().bond; + let bridge = net_conn.clone().bridge; + let conn = Connection::try_from(net_conn)?; let id = conn.id.clone(); state.network.add_connection(conn).await?; + match state.network.get_connection(&id).await? { None => Err(NetworkError::CannotAddConnection(id.clone())), - Some(conn) => Ok(Json(conn)), + Some(conn) => { + if let Some(bond) = bond { + state.network.set_ports(conn.clone(), bond.ports).await?; + } + if let Some(bridge) = bridge { + state.network.set_ports(conn.clone(), bridge.ports).await?; + } + Ok(Json(conn)) + } } } From 814e88a4247771a4bf8e059267d6ab16318b6cdc Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 9 May 2025 12:41:50 +0100 Subject: [PATCH 03/10] Added support for setting bridge ports in the model --- rust/agama-network/src/model.rs | 37 +++++++++++++++++---------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/rust/agama-network/src/model.rs b/rust/agama-network/src/model.rs index 53006d052f..c474d94764 100644 --- a/rust/agama-network/src/model.rs +++ b/rust/agama-network/src/model.rs @@ -222,28 +222,29 @@ impl NetworkState { controller: &Connection, ports: Vec, ) -> Result<(), NetworkStateError> { - if let ConnectionConfig::Bond(_) = &controller.config { - let mut controlled = vec![]; - for port in ports { - let connection = self - .get_connection_by_interface(&port) - .or_else(|| self.get_connection(&port)) - .ok_or(NetworkStateError::UnknownConnection(port))?; - controlled.push(connection.uuid); - } + match &controller.config { + ConnectionConfig::Bond(_) | ConnectionConfig::Bridge(_) => { + let mut controlled = vec![]; + for port in ports { + let connection = self + .get_connection_by_interface(&port) + .or_else(|| self.get_connection(&port)) + .ok_or(NetworkStateError::UnknownConnection(port))?; + controlled.push(connection.uuid); + } - for conn in self.connections.iter_mut() { - if controlled.contains(&conn.uuid) { - conn.controller = Some(controller.uuid); - } else if conn.controller == Some(controller.uuid) { - conn.controller = None; + for conn in self.connections.iter_mut() { + if controlled.contains(&conn.uuid) { + conn.controller = Some(controller.uuid); + } else if conn.controller == Some(controller.uuid) { + conn.controller = None; + } } + Ok(()) } - Ok(()) - } else { - Err(NetworkStateError::NotControllerConnection( + _ => Err(NetworkStateError::NotControllerConnection( controller.id.to_owned(), - )) + )), } } } From 459173ee1ab0a65d9ee18d50b3fa16b8e7c99e7f Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Sun, 11 May 2025 22:00:40 +0100 Subject: [PATCH 04/10] Some small fix for bridge and bond handling --- rust/agama-network/src/nm/client.rs | 6 +++--- rust/agama-network/src/nm/dbus.rs | 28 ++++++++++++++++++++++++---- rust/agama-server/src/network/web.rs | 15 +++++++++++++-- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/rust/agama-network/src/nm/client.rs b/rust/agama-network/src/nm/client.rs index cca8caf760..bdf96fc503 100644 --- a/rust/agama-network/src/nm/client.rs +++ b/rust/agama-network/src/nm/client.rs @@ -231,16 +231,16 @@ impl<'a> NetworkManagerClient<'a> { } for conn in connections.iter_mut() { - let Some(conn_uuid) = controlled_by.get(&conn.uuid) else { + let Some(id) = controlled_by.get(&conn.uuid) else { continue; }; - if let Ok(controller) = Uuid::parse_str(&conn_uuid) { + if let controller = conn.uuid { conn.controller = Some(controller); } else { tracing::warn!( "Could not found a connection for the interface '{}' (required by connection '{}')", - conn_uuid, + conn.uuid, conn.id ); } diff --git a/rust/agama-network/src/nm/dbus.rs b/rust/agama-network/src/nm/dbus.rs index b0db543145..6da85a3ee9 100644 --- a/rust/agama-network/src/nm/dbus.rs +++ b/rust/agama-network/src/nm/dbus.rs @@ -66,7 +66,7 @@ pub fn connection_to_dbus<'a>( } if let Some(controller) = controller { - let slave_type = match controller.config { + let port_type = match controller.config { ConnectionConfig::Bond(_) => BOND_KEY, ConnectionConfig::Bridge(_) => BRIDGE_KEY, _ => { @@ -74,14 +74,14 @@ pub fn connection_to_dbus<'a>( "" } }; - connection_dbus.insert("slave-type", slave_type.into()); + connection_dbus.insert("port-type", port_type.into()); let master = controller .interface .as_deref() .unwrap_or(controller.id.as_str()); connection_dbus.insert("master", master.into()); } else { - connection_dbus.insert("slave-type", "".into()); + connection_dbus.insert("port-type", "".into()); connection_dbus.insert("master", "".into()); } @@ -256,6 +256,18 @@ pub fn merge_dbus_connections<'a>( Ok(merged) } +fn is_bridge(conn: NestedHash) -> bool { + if let Some(connection) = conn.get("connection") { + if let Some(port_type) = connection.get("port-type") { + if port_type.to_string().as_str() == "bridge" { + return true; + } + } + } + + return false; +} + /// Cleans up the NestedHash that represents a connection. /// /// By now it just removes the "addresses" key from the "ipv4" and "ipv6" objects, which is @@ -263,6 +275,10 @@ pub fn merge_dbus_connections<'a>( /// /// * `conn`: connection represented as a NestedHash. pub fn cleanup_dbus_connection(conn: &mut NestedHash) { + if !is_bridge(conn.to_owned()) { + conn.remove("bridge-port"); + } + if let Some(connection) = conn.get_mut("connection") { if connection.get("interface-name").is_some_and(is_empty_value) { connection.remove("interface-name"); @@ -272,9 +288,13 @@ pub fn cleanup_dbus_connection(conn: &mut NestedHash) { connection.remove("master"); } - if connection.get("slave-type").is_some_and(is_empty_value) { + if connection.get("slave-type").is_some() { connection.remove("slave-type"); } + + if connection.get("port-type").is_some_and(is_empty_value) { + connection.remove("port-type"); + } } if let Some(ipv4) = conn.get_mut("ipv4") { diff --git a/rust/agama-server/src/network/web.rs b/rust/agama-server/src/network/web.rs index 701180b9da..713c1ce114 100644 --- a/rust/agama-server/src/network/web.rs +++ b/rust/agama-server/src/network/web.rs @@ -261,7 +261,7 @@ async fn add_connection( let conn = Connection::try_from(net_conn)?; let id = conn.id.clone(); - state.network.add_connection(conn).await?; + state.network.add_connection(conn.clone()).await?; match state.network.get_connection(&id).await? { None => Err(NetworkError::CannotAddConnection(id.clone())), @@ -337,6 +337,9 @@ async fn update_connection( .get_connection(&id) .await? .ok_or_else(|| NetworkError::UnknownConnection(id.clone()))?; + let bond = conn.clone().bond; + let bridge = conn.clone().bridge; + let mut conn = Connection::try_from(conn)?; if orig_conn.id != id { // FIXME: why? @@ -345,7 +348,15 @@ async fn update_connection( conn.uuid = orig_conn.uuid; } - state.network.update_connection(conn).await?; + state.network.update_connection(conn.clone()).await?; + + if let Some(bond) = bond { + state.network.set_ports(conn.clone(), bond.ports).await?; + } + if let Some(bridge) = bridge { + state.network.set_ports(conn.clone(), bridge.ports).await?; + } + Ok(StatusCode::NO_CONTENT) } From 61cffffcee7d8c4bc77e861f306bf818eaae1950 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 14 May 2025 07:36:21 +0100 Subject: [PATCH 05/10] Small fixes --- rust/agama-lib/src/hostname/client.rs | 1 - rust/agama-lib/src/network/store.rs | 12 +++++ rust/agama-network/src/model.rs | 3 ++ rust/agama-network/src/nm/client.rs | 14 +----- rust/agama-network/src/nm/dbus.rs | 6 ++- rust/agama-server/src/network/web.rs | 19 ++++---- rust/agama-server/tests/network_service.rs | 52 ++++++++++++++++++++-- 7 files changed, 82 insertions(+), 25 deletions(-) diff --git a/rust/agama-lib/src/hostname/client.rs b/rust/agama-lib/src/hostname/client.rs index d2d5a2b23d..adc87d53af 100644 --- a/rust/agama-lib/src/hostname/client.rs +++ b/rust/agama-lib/src/hostname/client.rs @@ -43,7 +43,6 @@ impl<'a> HostnameClient<'a> { let settings = HostnameSettings { hostname: Some(hostname), static_hostname: Some(static_hostname), - ..Default::default() }; Ok(settings) diff --git a/rust/agama-lib/src/network/store.rs b/rust/agama-lib/src/network/store.rs index 2d0a4d07bc..a591b529dd 100644 --- a/rust/agama-lib/src/network/store.rs +++ b/rust/agama-lib/src/network/store.rs @@ -176,6 +176,18 @@ mod tests { "bond0".to_string(), "eth2".to_string() ] + ); + + let conns = vec![bridge]; + let ordered = ordered_connections(&conns); + assert_eq!( + ordered, + vec![ + "eth0".to_string(), + "eth1".to_string(), + "eth3".to_string(), + "br0".to_string() + ] ) } } diff --git a/rust/agama-network/src/model.rs b/rust/agama-network/src/model.rs index c474d94764..64360e203e 100644 --- a/rust/agama-network/src/model.rs +++ b/rust/agama-network/src/model.rs @@ -236,6 +236,9 @@ impl NetworkState { for conn in self.connections.iter_mut() { if controlled.contains(&conn.uuid) { conn.controller = Some(controller.uuid); + if conn.interface.is_none() { + conn.interface = Some(conn.clone().id); + } } else if conn.controller == Some(controller.uuid) { conn.controller = None; } diff --git a/rust/agama-network/src/nm/client.rs b/rust/agama-network/src/nm/client.rs index bdf96fc503..93318cb261 100644 --- a/rust/agama-network/src/nm/client.rs +++ b/rust/agama-network/src/nm/client.rs @@ -231,19 +231,9 @@ impl<'a> NetworkManagerClient<'a> { } for conn in connections.iter_mut() { - let Some(id) = controlled_by.get(&conn.uuid) else { - continue; + if controlled_by.contains_key(&conn.uuid) { + conn.controller = Some(conn.uuid); }; - - if let controller = conn.uuid { - conn.controller = Some(controller); - } else { - tracing::warn!( - "Could not found a connection for the interface '{}' (required by connection '{}')", - conn.uuid, - conn.id - ); - } } Ok(connections) diff --git a/rust/agama-network/src/nm/dbus.rs b/rust/agama-network/src/nm/dbus.rs index 6da85a3ee9..be5a4a97da 100644 --- a/rust/agama-network/src/nm/dbus.rs +++ b/rust/agama-network/src/nm/dbus.rs @@ -80,6 +80,8 @@ pub fn connection_to_dbus<'a>( .as_deref() .unwrap_or(controller.id.as_str()); connection_dbus.insert("master", master.into()); + connection_dbus.remove("autoconnect"); + connection_dbus.insert("autoconnect", false.into()); } else { connection_dbus.insert("port-type", "".into()); connection_dbus.insert("master", "".into()); @@ -122,6 +124,7 @@ pub fn connection_to_dbus<'a>( } ConnectionConfig::Bond(bond) => { connection_dbus.insert("type", BOND_KEY.into()); + connection_dbus.insert("autoconnect-slaves", 1.into()); if !connection_dbus.contains_key("interface-name") { connection_dbus.insert("interface-name", conn.id.as_str().into()); } @@ -136,6 +139,7 @@ pub fn connection_to_dbus<'a>( } ConnectionConfig::Bridge(bridge) => { connection_dbus.insert("type", BRIDGE_KEY.into()); + connection_dbus.insert("autoconnect-slaves", 1.into()); result.insert(BRIDGE_KEY, bridge_config_to_dbus(bridge)); } ConnectionConfig::Infiniband(infiniband) => { @@ -265,7 +269,7 @@ fn is_bridge(conn: NestedHash) -> bool { } } - return false; + false } /// Cleans up the NestedHash that represents a connection. diff --git a/rust/agama-server/src/network/web.rs b/rust/agama-server/src/network/web.rs index 713c1ce114..894facaca9 100644 --- a/rust/agama-server/src/network/web.rs +++ b/rust/agama-server/src/network/web.rs @@ -214,9 +214,8 @@ async fn connections( let network_connections = connections .iter() - .filter(|c| c.controller.is_none()) .map(|c| { - let state = c.state.clone(); + let state = c.state; let mut conn = NetworkConnection::try_from(c.clone()).unwrap(); if let Some(ref mut bond) = conn.bond { bond.ports = ports_for(connections.to_owned(), c.uuid); @@ -224,12 +223,10 @@ async fn connections( if let Some(ref mut bridge) = conn.bridge { bridge.ports = ports_for(connections.to_owned(), c.uuid); }; - let connection_with_state = NetworkConnectionWithState { + NetworkConnectionWithState { connection: conn, state, - }; - - return connection_with_state; + } }) .collect(); @@ -239,8 +236,14 @@ async fn connections( fn ports_for(connections: Vec, uuid: Uuid) -> Vec { return connections .iter() - .filter(|c| c.controller == Some(uuid) && c.interface.is_some()) - .map(|c| c.interface.to_owned().unwrap_or_default()) + .filter(|c| c.controller == Some(uuid)) + .map(|c| { + if let Some(interface) = c.interface.to_owned() { + interface + } else { + c.clone().id + } + }) .collect(); } diff --git a/rust/agama-server/tests/network_service.rs b/rust/agama-server/tests/network_service.rs index c23a16c8aa..292c1e7378 100644 --- a/rust/agama-server/tests/network_service.rs +++ b/rust/agama-server/tests/network_service.rs @@ -21,7 +21,7 @@ pub mod common; use agama_lib::error::ServiceError; -use agama_lib::network::settings::{BondSettings, NetworkConnection}; +use agama_lib::network::settings::{BondSettings, BridgeSettings, NetworkConnection}; use agama_lib::network::types::{DeviceType, SSID}; use agama_lib::network::{ model::{self, AccessPoint, GeneralState, NetworkState, StateConfig}, @@ -185,7 +185,7 @@ async fn test_add_bond_connection() -> Result<(), Box> { let state = build_state().await; let network_service = build_service(state.clone()).await?; - let eth0 = NetworkConnection { + let eth2 = NetworkConnection { id: "eth2".to_string(), ..Default::default() }; @@ -207,7 +207,7 @@ async fn test_add_bond_connection() -> Result<(), Box> { .uri("/connections") .header("Content-Type", "application/json") .method(Method::POST) - .body(serde_json::to_string(ð0)?) + .body(serde_json::to_string(ð2)?) .unwrap(); let response = network_service.clone().oneshot(request).await?; @@ -236,6 +236,52 @@ async fn test_add_bond_connection() -> Result<(), Box> { assert!(body.contains(r#""id":"bond0""#)); assert!(body.contains(r#""mode":"active-backup""#)); assert!(body.contains(r#""primary=eth0""#)); + assert!(body.contains(r#""ports":["eth0"]"#)); + + Ok(()) +} + +#[test] +async fn test_add_bridge_connection() -> Result<(), Box> { + let state = build_state().await; + let network_service = build_service(state.clone()).await?; + + let br0 = NetworkConnection { + id: "br0".to_string(), + method4: Some("manual".to_string()), + method6: Some("disabled".to_string()), + interface: Some("br0".to_string()), + bridge: Some(BridgeSettings { + ports: vec!["eth0".to_string()], + stp: Some(false), + ..Default::default() + }), + ..Default::default() + }; + + let request = Request::builder() + .uri("/connections") + .header("Content-Type", "application/json") + .method(Method::POST) + .body(serde_json::to_string(&br0)?) + .unwrap(); + + let response = network_service.clone().oneshot(request).await?; + assert_eq!(response.status(), StatusCode::OK); + + let request = Request::builder() + .uri("/connections") + .method(Method::GET) + .body(Body::empty()) + .unwrap(); + + let response = network_service.clone().oneshot(request).await?; + assert_eq!(response.status(), StatusCode::OK); + let body = body_to_string(response.into_body()).await; + assert!(body.contains(r#""id":"eth0""#)); + assert!(body.contains(r#""id":"br0""#)); + assert!(body.contains(r#""ports":["eth0"]"#)); + assert!(body.contains(r#""stp":false"#)); Ok(()) } From e9a9a38ed06e2c0499316edf2e8d4457e380dea5 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 14 May 2025 14:30:52 +0100 Subject: [PATCH 06/10] add BridgeSettings to the OpenAPI spec --- rust/agama-server/src/web/docs/network.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/agama-server/src/web/docs/network.rs b/rust/agama-server/src/web/docs/network.rs index e65a28f5f3..8e4ecb52dd 100644 --- a/rust/agama-server/src/web/docs/network.rs +++ b/rust/agama-server/src/web/docs/network.rs @@ -50,6 +50,7 @@ impl ApiDocBuilder for NetworkApiDocBuilder { fn components(&self) -> Components { ComponentsBuilder::new() .schema_from::() + .schema_from::() .schema_from::() .schema_from::() .schema_from::() From df2462af3cee269694306ddb85ef0836477066b1 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 14 May 2025 15:46:00 +0100 Subject: [PATCH 07/10] Fix bridge json schema --- rust/agama-lib/share/profile.schema.json | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index d3ed15e864..ba90cee616 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -446,22 +446,22 @@ "forwardDelay": { "title": "Spanning Tree Protocol forward delay, in seconds", "type": "integer", - "minimum": "0" + "minimum": 0 }, "priority": { "title": "Spanning Tree Protocol priority. Lower values are 'better'", "type": "integer", - "minimum": "0" + "minimum": 0 }, "maxAge": { "title": "Spanning Tree Protocol maximum message age, in seconds", "type": "integer", - "minimum": "0" + "minimum": 0 }, "helloTime": { "title": "Spanning Tree Protocol hello time, in seconds", "type": "integer", - "minimum": "0" + "minimum": 0 }, "ports": { "type": "array", @@ -472,7 +472,6 @@ } } }, - "match": { "type": "object", "title": "Match settings", From ab61b1ff133c0c338de4532c791d8226dae26e63 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 14 May 2025 16:15:44 +0100 Subject: [PATCH 08/10] Added changelog --- rust/package/agama.changes | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rust/package/agama.changes b/rust/package/agama.changes index b3d2d089ad..1394c08f9b 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Wed May 14 15:20:42 UTC 2025 - Knut Anderssen + +- Add support for bridge connections (gh#openSUSE/agama#2258). + ------------------------------------------------------------------- Mon May 19 14:02:04 UTC 2025 - Imobach Gonzalez Sosa From d2b4c5553f034829d3e188dda37d7c7cd6013f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Fri, 16 May 2025 18:05:28 +0100 Subject: [PATCH 09/10] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Imobach González Sosa --- rust/agama-lib/share/profile.schema.json | 2 +- rust/agama-network/src/model.rs | 2 +- rust/agama-server/src/network/web.rs | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index ba90cee616..d23227b4a2 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -449,7 +449,7 @@ "minimum": 0 }, "priority": { - "title": "Spanning Tree Protocol priority. Lower values are 'better'", + "title": "Spanning Tree Protocol priority (lower values are 'better')", "type": "integer", "minimum": 0 }, diff --git a/rust/agama-network/src/model.rs b/rust/agama-network/src/model.rs index 64360e203e..031481030b 100644 --- a/rust/agama-network/src/model.rs +++ b/rust/agama-network/src/model.rs @@ -237,7 +237,7 @@ impl NetworkState { if controlled.contains(&conn.uuid) { conn.controller = Some(controller.uuid); if conn.interface.is_none() { - conn.interface = Some(conn.clone().id); + conn.interface = Some(conn.id.clone()); } } else if conn.controller == Some(controller.uuid) { conn.controller = None; diff --git a/rust/agama-server/src/network/web.rs b/rust/agama-server/src/network/web.rs index 894facaca9..4f5fa1d246 100644 --- a/rust/agama-server/src/network/web.rs +++ b/rust/agama-server/src/network/web.rs @@ -259,8 +259,8 @@ async fn add_connection( State(state): State, Json(net_conn): Json, ) -> Result, NetworkError> { - let bond = net_conn.clone().bond; - let bridge = net_conn.clone().bridge; + let bond = net_conn.bond.clone(); + let bridge = net_conn.bridge.clone(); let conn = Connection::try_from(net_conn)?; let id = conn.id.clone(); @@ -340,8 +340,8 @@ async fn update_connection( .get_connection(&id) .await? .ok_or_else(|| NetworkError::UnknownConnection(id.clone()))?; - let bond = conn.clone().bond; - let bridge = conn.clone().bridge; + let bond = conn.bond.clone(); + let bridge = conn.bridge.clone(); let mut conn = Connection::try_from(conn)?; if orig_conn.id != id { From f1ae5d6d3c4da76e116c74bd8cb528892fad45d5 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 19 May 2025 09:32:12 +0100 Subject: [PATCH 10/10] Changes based on CR --- rust/agama-network/src/nm/dbus.rs | 35 ++++++++++++++++++++++++++-- rust/agama-network/src/system.rs | 9 +++---- rust/agama-server/src/network/web.rs | 8 +++---- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/rust/agama-network/src/nm/dbus.rs b/rust/agama-network/src/nm/dbus.rs index be5a4a97da..638e9ac087 100644 --- a/rust/agama-network/src/nm/dbus.rs +++ b/rust/agama-network/src/nm/dbus.rs @@ -274,8 +274,13 @@ fn is_bridge(conn: NestedHash) -> bool { /// Cleans up the NestedHash that represents a connection. /// -/// By now it just removes the "addresses" key from the "ipv4" and "ipv6" objects, which is -/// replaced with "address-data". However, if "addresses" is present, it takes precedence. +/// If the connections is not a "bridge-port" anymore it removes the "bridge-port" key. +/// +/// It also removes empty files from the "connection" object like the "interface-name", "master", +/// "slave-type", "port-type" keys. +/// +/// Finally, it removes removes the "addresses" and "dns" keys from the "ipv4" and "ipv6" objects, +/// which are replaced with "address-data". /// /// * `conn`: connection represented as a NestedHash. pub fn cleanup_dbus_connection(conn: &mut NestedHash) { @@ -1588,6 +1593,32 @@ mod test { Ok(()) } + #[test] + fn test_connection_from_dbus_bridge() -> anyhow::Result<()> { + dbg!("TESTING BRIDGE"); + let uuid = Uuid::new_v4().to_string(); + let connection_section = HashMap::from([hi("id", "br0")?, hi("uuid", uuid)?]); + + let bridge_config = Value::new(HashMap::from([ + ("stp".to_string(), Value::from(true)), + ("priority".to_string(), Value::from(10_u32)), + ("forward-delay".to_string(), Value::from(5_u32)), + ])); + + let dbus_conn = HashMap::from([ + ("connection".to_string(), connection_section), + (BRIDGE_KEY.to_string(), bridge_config.try_into().unwrap()), + ]); + + let connection = connection_from_dbus(dbus_conn); + if let ConnectionConfig::Bridge(config) = connection.unwrap().config { + assert_eq!(config.stp, Some(true)); + assert_eq!(config.forward_delay, Some(5_u32)); + } + + Ok(()) + } + #[test] fn test_connection_from_dbus_infiniband() -> anyhow::Result<()> { let uuid = Uuid::new_v4().to_string(); diff --git a/rust/agama-network/src/system.rs b/rust/agama-network/src/system.rs index 82d1a0e402..64a80cc623 100644 --- a/rust/agama-network/src/system.rs +++ b/rust/agama-network/src/system.rs @@ -211,15 +211,12 @@ impl NetworkSystemClient { pub async fn set_ports( &self, - connection: Connection, + uuid: Uuid, ports: Vec, ) -> Result<(), NetworkSystemError> { let (tx, rx) = oneshot::channel(); - self.actions.send(Action::SetPorts( - connection.uuid, - Box::new(ports.clone()), - tx, - ))?; + self.actions + .send(Action::SetPorts(uuid, Box::new(ports.clone()), tx))?; let result = rx.await?; Ok(result?) } diff --git a/rust/agama-server/src/network/web.rs b/rust/agama-server/src/network/web.rs index 4f5fa1d246..2e7eadc3a2 100644 --- a/rust/agama-server/src/network/web.rs +++ b/rust/agama-server/src/network/web.rs @@ -270,10 +270,10 @@ async fn add_connection( None => Err(NetworkError::CannotAddConnection(id.clone())), Some(conn) => { if let Some(bond) = bond { - state.network.set_ports(conn.clone(), bond.ports).await?; + state.network.set_ports(conn.uuid, bond.ports).await?; } if let Some(bridge) = bridge { - state.network.set_ports(conn.clone(), bridge.ports).await?; + state.network.set_ports(conn.uuid, bridge.ports).await?; } Ok(Json(conn)) } @@ -354,10 +354,10 @@ async fn update_connection( state.network.update_connection(conn.clone()).await?; if let Some(bond) = bond { - state.network.set_ports(conn.clone(), bond.ports).await?; + state.network.set_ports(conn.uuid, bond.ports).await?; } if let Some(bridge) = bridge { - state.network.set_ports(conn.clone(), bridge.ports).await?; + state.network.set_ports(conn.uuid, bridge.ports).await?; } Ok(StatusCode::NO_CONTENT)