Merge 1022b04 into 1968c26

rust/agama-dbus-server/src/network.rs

@@ -47,7 +47,7 @@ mod nm;
 pub mod system;
 
 pub use action::Action;
-pub use adapter::Adapter;
+pub use adapter::{Adapter, NetworkAdapterError};
 pub use dbus::NetworkService;
 pub use model::NetworkState;
 pub use nm::NetworkManagerAdapter;

rust/agama-dbus-server/src/network/action.rs

@@ -4,7 +4,7 @@ use tokio::sync::oneshot;
 use uuid::Uuid;
 use zbus::zvariant::OwnedObjectPath;
 
-use super::error::NetworkStateError;
+use super::{error::NetworkStateError, NetworkAdapterError};
 
 pub type Responder<T> = oneshot::Sender<T>;
 pub type ControllerConnection = (Connection, Vec<String>);
@@ -24,7 +24,9 @@ pub enum Action {
     /// Gets a connection
     GetConnection(Uuid, Responder<Option<Connection>>),
     /// Gets a connection
-    GetConnectionPath(String, Responder<Option<OwnedObjectPath>>),
+    GetConnectionPath(Uuid, Responder<Option<OwnedObjectPath>>),
+    /// Gets a connection
+    GetConnectionPathById(String, Responder<Option<OwnedObjectPath>>),
     /// Get connections paths
     GetConnectionsPaths(Responder<Vec<OwnedObjectPath>>),
     /// Gets a controller connection
@@ -44,7 +46,7 @@ pub enum Action {
     /// Update a connection (replacing the old one).
     UpdateConnection(Box<Connection>),
     /// Remove the connection with the given Uuid.
-    RemoveConnection(String),
+    RemoveConnection(Uuid),
     /// Apply the current configuration.
-    Apply,
+    Apply(Responder<Result<(), NetworkAdapterError>>),
 }

rust/agama-dbus-server/src/network/adapter.rs

@@ -1,10 +1,27 @@
 use crate::network::NetworkState;
+use agama_lib::error::ServiceError;
 use async_trait::async_trait;
-use std::error::Error;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum NetworkAdapterError {
+    #[error("Could not read the network configuration: {0}")]
+    Read(ServiceError),
+    #[error("Could not update the network configuration: {0}")]
+    Write(ServiceError),
+    #[error("Checkpoint handling error: {0}")]
+    Checkpoint(ServiceError), // only relevant for adapters that implement a checkpoint mechanism
+}
 
 /// A trait for the ability to read/write from/to a network service
 #[async_trait]
 pub trait Adapter {
-    async fn read(&self) -> Result<NetworkState, Box<dyn Error>>;
-    async fn write(&self, network: &NetworkState) -> Result<(), Box<dyn Error>>;
+    async fn read(&self) -> Result<NetworkState, NetworkAdapterError>;
+    async fn write(&self, network: &NetworkState) -> Result<(), NetworkAdapterError>;
+}
+
+impl From<NetworkAdapterError> for zbus::fdo::Error {
+    fn from(value: NetworkAdapterError) -> zbus::fdo::Error {
+        zbus::fdo::Error::Failed(value.to_string())
+    }
 }

rust/agama-dbus-server/src/network/dbus/interfaces/connection_configs.rs

@@ -198,6 +198,20 @@ impl Wireless {
             .await?;
         Ok(())
     }
+
+    /// Whether the network is hidden or not.
+    #[dbus_interface(property)]
+    pub async fn hidden(&self) -> zbus::fdo::Result<bool> {
+        let config = self.get_config::<WirelessConfig>().await?;
+        Ok(config.hidden)
+    }
+
+    #[dbus_interface(property)]
+    pub async fn set_hidden(&mut self, hidden: bool) -> zbus::fdo::Result<()> {
+        self.update_config::<WirelessConfig, _>(|c| c.hidden = hidden)
+            .await?;
+        Ok(())
+    }
 }
 
 #[async_trait]

rust/agama-dbus-server/src/network/dbus/interfaces/connections.rs

@@ -60,14 +60,31 @@ impl Connections {
         Ok(path)
     }
 
-    /// Returns the D-Bus path of the network connection.
+    /// Returns the D-Bus path of the network connection by its UUID.
+    ///
+    /// * `uuid`: connection UUID.
+    pub async fn get_connection(&self, uuid: &str) -> zbus::fdo::Result<OwnedObjectPath> {
+        let uuid: Uuid = uuid
+            .parse()
+            .map_err(|_| NetworkStateError::InvalidUuid(uuid.to_string()))?;
+        let actions = self.actions.lock().await;
+        let (tx, rx) = oneshot::channel();
+        actions.send(Action::GetConnectionPath(uuid, tx)).unwrap();
+        let path = rx
+            .await
+            .unwrap()
+            .ok_or(NetworkStateError::UnknownConnection(uuid.to_string()))?;
+        Ok(path)
+    }
+
+    /// Returns the D-Bus path of the network connection by its ID.
     ///
     /// * `id`: connection ID.
-    pub async fn get_connection(&self, id: &str) -> zbus::fdo::Result<OwnedObjectPath> {
+    pub async fn get_connection_by_id(&self, id: &str) -> zbus::fdo::Result<OwnedObjectPath> {
         let actions = self.actions.lock().await;
         let (tx, rx) = oneshot::channel();
         actions
-            .send(Action::GetConnectionPath(id.to_string(), tx))
+            .send(Action::GetConnectionPathById(id.to_string(), tx))
             .unwrap();
         let path = rx
             .await
@@ -79,11 +96,12 @@ impl Connections {
     /// Removes a network connection.
     ///
     /// * `uuid`: connection UUID..
-    pub async fn remove_connection(&mut self, id: &str) -> zbus::fdo::Result<()> {
+    pub async fn remove_connection(&mut self, uuid: &str) -> zbus::fdo::Result<()> {
+        let uuid = uuid
+            .parse()
+            .map_err(|_| NetworkStateError::InvalidUuid(uuid.to_string()))?;
         let actions = self.actions.lock().await;
-        actions
-            .send(Action::RemoveConnection(id.to_string()))
-            .unwrap();
+        actions.send(Action::RemoveConnection(uuid)).unwrap();
         Ok(())
     }
 
@@ -92,7 +110,9 @@ impl Connections {
     /// It includes adding, updating and removing connections as needed.
     pub async fn apply(&self) -> zbus::fdo::Result<()> {
         let actions = self.actions.lock().await;
-        actions.send(Action::Apply).unwrap();
+        let (tx, rx) = oneshot::channel();
+        actions.send(Action::Apply(tx)).unwrap();
+        rx.await.unwrap()?;
         Ok(())
     }
 

rust/agama-dbus-server/src/network/dbus/tree.rs

@@ -1,4 +1,5 @@
 use agama_lib::error::ServiceError;
+use uuid::Uuid;
 use zbus::zvariant::{ObjectPath, OwnedObjectPath};
 
 use crate::network::{action::Action, dbus::interfaces, model::*};
@@ -77,15 +78,15 @@ impl Tree {
     /// * `notify`: whether to notify the added connection
     pub async fn add_connection(
         &mut self,
-        conn: &mut Connection,
+        conn: &Connection,
     ) -> Result<OwnedObjectPath, ServiceError> {
         let uuid = conn.uuid;
-        let (id, path) = self.objects.register_connection(conn);
-        if id != conn.id {
-            conn.id = id.clone();
-        }
-        let path: OwnedObjectPath = path.into();
-        log::info!("Publishing network connection '{}' on '{}'", id, &path);
+        let path: OwnedObjectPath = self.objects.register_connection(conn.uuid).into();
+        log::info!(
+            "Publishing network connection '{}' on '{}'",
+            &conn.id,
+            &path
+        );
 
         self.add_interface(
             &path,
@@ -114,13 +115,13 @@ impl Tree {
 
     /// Removes a connection from the tree
     ///
-    /// * `id`: connection ID.
-    pub async fn remove_connection(&mut self, id: &str) -> Result<(), ServiceError> {
-        let Some(path) = self.objects.connection_path(id) else {
+    /// * `uuid`: connection UUID.
+    pub async fn remove_connection(&mut self, uuid: Uuid) -> Result<(), ServiceError> {
+        let Some(path) = self.objects.connection_path(uuid) else {
             return Ok(());
         };
         self.remove_connection_on(path.as_str()).await?;
-        self.objects.deregister_connection(id).unwrap();
+        self.objects.deregister_connection(uuid).unwrap();
         Ok(())
     }
 
@@ -134,8 +135,8 @@ impl Tree {
         self.objects.connections_paths()
     }
 
-    pub fn connection_path(&self, id: &str) -> Option<OwnedObjectPath> {
-        self.objects.connection_path(id).map(|o| o.into())
+    pub fn connection_path(&self, uuid: Uuid) -> Option<OwnedObjectPath> {
+        self.objects.connection_path(uuid).map(|o| o.into())
     }
 
     /// Adds connections to the D-Bus tree.
@@ -187,6 +188,7 @@ impl Tree {
         _ = object_server.remove::<interfaces::Bond, _>(path).await;
         _ = object_server.remove::<interfaces::Wireless, _>(path).await;
         object_server.remove::<interfaces::Ip, _>(path).await?;
+        object_server.remove::<interfaces::Match, _>(path).await?;
         object_server
             .remove::<interfaces::Connection, _>(path)
             .await?;
@@ -210,7 +212,7 @@ struct ObjectsRegistry {
     /// device_name (eth0) -> object_path
     devices: HashMap<String, OwnedObjectPath>,
     /// id -> object_path
-    connections: HashMap<String, OwnedObjectPath>,
+    connections: HashMap<Uuid, OwnedObjectPath>,
 }
 
 impl ObjectsRegistry {
@@ -227,30 +229,26 @@ impl ObjectsRegistry {
     /// It returns the connection Id and the D-Bus path. Bear in mind that the Id can be different
     /// in case the original one already existed.
     ///
-    /// * `conn`: network connection.
-    pub fn register_connection(&mut self, conn: &Connection) -> (String, ObjectPath) {
+    /// * `uuid`: network connection's UUID.
+    pub fn register_connection(&mut self, uuid: Uuid) -> ObjectPath {
         let path = format!("{}/{}", CONNECTIONS_PATH, self.connections.len());
         let path = ObjectPath::try_from(path).unwrap();
-        let mut id = conn.id.clone();
-        if self.connection_path(&id).is_some() {
-            id = self.propose_id(&id);
-        };
-        self.connections.insert(id.clone(), path.clone().into());
-        (id, path)
+        self.connections.insert(uuid, path.clone().into());
+        path
     }
 
     /// Returns the path for a connection.
     ///
-    /// * `id`: connection ID.
-    pub fn connection_path(&self, id: &str) -> Option<ObjectPath> {
-        self.connections.get(id).map(|p| p.as_ref())
+    /// * `uuid`: connection ID.
+    pub fn connection_path(&self, uuid: Uuid) -> Option<ObjectPath> {
+        self.connections.get(&uuid).map(|p| p.as_ref())
     }
 
     /// Deregisters a network connection.
     ///
     /// * `id`: connection ID.
-    pub fn deregister_connection(&mut self, id: &str) -> Option<OwnedObjectPath> {
-        self.connections.remove(id)
+    pub fn deregister_connection(&mut self, uuid: Uuid) -> Option<OwnedObjectPath> {
+        self.connections.remove(&uuid)
     }
 
     /// Returns all devices paths.
@@ -262,18 +260,4 @@ impl ObjectsRegistry {
     pub fn connections_paths(&self) -> Vec<OwnedObjectPath> {
         self.connections.values().cloned().collect()
     }
-
-    /// Proposes a connection ID.
-    ///
-    /// * `id`: original connection ID.
-    fn propose_id(&self, id: &str) -> String {
-        let prefix = format!("{}-", id);
-        let filtered: Vec<_> = self
-            .connections
-            .keys()
-            .filter_map(|i| i.strip_prefix(&prefix).and_then(|n| n.parse::<u32>().ok()))
-            .collect();
-        let index = filtered.into_iter().max().unwrap_or(0);
-        format!("{}{}", prefix, index + 1)
-    }
 }

rust/agama-dbus-server/src/network/error.rs

@@ -1,6 +1,5 @@
 //! Error types.
 use thiserror::Error;
-use uuid::Uuid;
 
 /// Errors that are related to the network configuration.
 #[derive(Error, Debug)]
@@ -16,7 +15,7 @@ pub enum NetworkStateError {
     #[error("Invalid wireless mode: '{0}'")]
     InvalidWirelessMode(String),
     #[error("Connection '{0}' already exists")]
-    ConnectionExists(Uuid),
+    ConnectionExists(String),
     #[error("Invalid security wireless protocol: '{0}'")]
     InvalidSecurityProtocol(String),
     #[error("Adapter error: '{0}'")]

rust/agama-dbus-server/src/network/model.rs

@@ -43,11 +43,18 @@ impl NetworkState {
 
     /// Get connection by UUID
     ///
-    /// * `id`: connection UUID
+    /// * `uuid`: connection UUID
     pub fn get_connection_by_uuid(&self, uuid: Uuid) -> Option<&Connection> {
         self.connections.iter().find(|c| c.uuid == uuid)
     }
 
+    /// Get connection by UUID as mutable
+    ///
+    /// * `uuid`: connection UUID
+    pub fn get_connection_by_uuid_mut(&mut self, uuid: Uuid) -> Option<&mut Connection> {
+        self.connections.iter_mut().find(|c| c.uuid == uuid)
+    }
+
     /// Get connection by interface
     ///
     /// * `name`: connection interface name
@@ -85,7 +92,7 @@ impl NetworkState {
     /// It uses the `id` to decide whether the connection already exists.
     pub fn add_connection(&mut self, conn: Connection) -> Result<(), NetworkStateError> {
         if self.get_connection(&conn.id).is_some() {
-            return Err(NetworkStateError::ConnectionExists(conn.uuid));
+            return Err(NetworkStateError::ConnectionExists(conn.id));
         }
         self.connections.push(conn);
 
@@ -109,9 +116,9 @@ impl NetworkState {
     /// Removes a connection from the state.
     ///
     /// Additionally, it registers the connection to be removed when the changes are applied.
-    pub fn remove_connection(&mut self, id: &str) -> Result<(), NetworkStateError> {
-        let Some(conn) = self.get_connection_mut(id) else {
-            return Err(NetworkStateError::UnknownConnection(id.to_string()));
+    pub fn remove_connection(&mut self, uuid: Uuid) -> Result<(), NetworkStateError> {
+        let Some(conn) = self.get_connection_by_uuid_mut(uuid) else {
+            return Err(NetworkStateError::UnknownConnection(uuid.to_string()));
         };
 
         conn.remove();
@@ -268,18 +275,18 @@ mod tests {
     #[test]
     fn test_remove_connection() {
         let mut state = NetworkState::default();
-        let id = "eth0".to_string();
-        let conn0 = Connection::new(id, DeviceType::Ethernet);
+        let conn0 = Connection::new("eth0".to_string(), DeviceType::Ethernet);
+        let uuid = conn0.uuid;
         state.add_connection(conn0).unwrap();
-        state.remove_connection("eth0").unwrap();
+        state.remove_connection(uuid).unwrap();
         let found = state.get_connection("eth0").unwrap();
         assert!(found.is_removed());
     }
 
     #[test]
     fn test_remove_unknown_connection() {
         let mut state = NetworkState::default();
-        let error = state.remove_connection("eth0").unwrap_err();
+        let error = state.remove_connection(Uuid::new_v4()).unwrap_err();
         assert!(matches!(error, NetworkStateError::UnknownConnection(_)));
     }
 
@@ -369,7 +376,7 @@ pub struct Device {
 }
 
 /// Represents an availble network connection.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct Connection {
     pub id: String,
     pub uuid: Uuid,
@@ -383,12 +390,6 @@ pub struct Connection {
     pub config: ConnectionConfig,
 }
 
-impl PartialEq for Connection {
-    fn eq(&self, other: &Self) -> bool {
-        self.id == other.id && self.uuid == other.uuid && self.ip_config == other.ip_config
-    }
-}
-
 impl Connection {
     pub fn new(id: String, device_type: DeviceType) -> Self {
         let config = match device_type {
@@ -494,7 +495,7 @@ impl From<WirelessConfig> for ConnectionConfig {
 #[error("Invalid MAC address: {0}")]
 pub struct InvalidMacAddress(String);
 
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Default, Clone, PartialEq)]
 pub enum MacAddress {
     MacAddress(macaddr::MacAddr6),
     Preserve,