From 4e4d354c6754dd9449cd22e9bba5486813dda6bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 22 Apr 2024 12:49:52 +0100 Subject: [PATCH 01/24] Add XML files for ISCSI-related interfaces --- ...use.Agama.Storage1.ISCSI.Initiator.bus.xml | 1 + ...opensuse.Agama.Storage1.ISCSI.Node.bus.xml | 46 +++++++++++++++++++ ...use.Agama.Storage1.ISCSI.Initiator.doc.xml | 19 ++++++++ ...opensuse.Agama.Storage1.ISCSI.Node.doc.xml | 20 ++++++++ 4 files changed, 86 insertions(+) create mode 120000 doc/dbus/bus/org.opensuse.Agama.Storage1.ISCSI.Initiator.bus.xml create mode 100644 doc/dbus/bus/org.opensuse.Agama.Storage1.ISCSI.Node.bus.xml create mode 100644 doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml create mode 100644 doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Node.doc.xml diff --git a/doc/dbus/bus/org.opensuse.Agama.Storage1.ISCSI.Initiator.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Storage1.ISCSI.Initiator.bus.xml new file mode 120000 index 0000000000..ec1e9c8218 --- /dev/null +++ b/doc/dbus/bus/org.opensuse.Agama.Storage1.ISCSI.Initiator.bus.xml @@ -0,0 +1 @@ +org.opensuse.Agama.Storage1.bus.xml \ No newline at end of file diff --git a/doc/dbus/bus/org.opensuse.Agama.Storage1.ISCSI.Node.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Storage1.ISCSI.Node.bus.xml new file mode 100644 index 0000000000..75c9267c19 --- /dev/null +++ b/doc/dbus/bus/org.opensuse.Agama.Storage1.ISCSI.Node.bus.xml @@ -0,0 +1,46 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml b/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml new file mode 100644 index 0000000000..43d07f0035 --- /dev/null +++ b/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + diff --git a/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Node.doc.xml b/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Node.doc.xml new file mode 100644 index 0000000000..ec83237365 --- /dev/null +++ b/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Node.doc.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + + + + + From e35b1e39b0ca1161f5027b038d6e5e4a90b08018 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 25 Apr 2024 15:55:50 +0100 Subject: [PATCH 02/24] rust: add an ISCSIClient --- rust/agama-lib/src/storage.rs | 2 +- rust/agama-lib/src/storage/client.rs | 1 + rust/agama-lib/src/storage/client/iscsi.rs | 278 +++++++++++++++++++++ rust/agama-lib/src/storage/proxies.rs | 74 ++++++ 4 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 rust/agama-lib/src/storage/client/iscsi.rs diff --git a/rust/agama-lib/src/storage.rs b/rust/agama-lib/src/storage.rs index 13e4ecc6b1..df40feed1c 100644 --- a/rust/agama-lib/src/storage.rs +++ b/rust/agama-lib/src/storage.rs @@ -6,6 +6,6 @@ pub mod proxies; mod settings; mod store; -pub use client::StorageClient; +pub use client::{iscsi::ISCSIClient, StorageClient}; pub use settings::StorageSettings; pub use store::StorageStore; diff --git a/rust/agama-lib/src/storage/client.rs b/rust/agama-lib/src/storage/client.rs index f71b5b8ccf..5aa22e469a 100644 --- a/rust/agama-lib/src/storage/client.rs +++ b/rust/agama-lib/src/storage/client.rs @@ -15,6 +15,7 @@ use zbus::fdo::ObjectManagerProxy; use zbus::names::{InterfaceName, OwnedInterfaceName}; use zbus::zvariant::{OwnedObjectPath, OwnedValue}; use zbus::Connection; +pub mod iscsi; type DbusObject = ( OwnedObjectPath, diff --git a/rust/agama-lib/src/storage/client/iscsi.rs b/rust/agama-lib/src/storage/client/iscsi.rs new file mode 100644 index 0000000000..32bd9c4740 --- /dev/null +++ b/rust/agama-lib/src/storage/client/iscsi.rs @@ -0,0 +1,278 @@ +use core::fmt; +use std::collections::HashMap; + +use crate::{ + dbus::get_property, + error::ServiceError, + storage::proxies::{InitiatorProxy, NodeProxy}, +}; +use serde::{Deserialize, Serialize}; +use thiserror::Error; +use zbus::{ + fdo::ObjectManagerProxy, + zvariant::{self, ObjectPath, OwnedObjectPath, OwnedValue, Value}, + Connection, +}; + +#[derive(Serialize)] +pub struct Initiator { + name: String, + ibft: bool, +} + +#[derive(Default, Serialize)] +/// ISCSI node +pub struct ISCSINode { + /// Artificial ID to match it against the D-Bus backend. + pub id: u32, + /// Target name. + pub target: String, + /// Target IP address (in string-like form). + pub address: String, + /// Target port. + pub port: u32, + /// Interface name. + pub interface: String, + /// Whether the node was initiated by iBFT + pub ibft: bool, + /// Whether the node is connected (there is a session). + pub connected: bool, + /// Startup status (TODO: document better) + pub startup: String, +} + +impl TryFrom<&HashMap> for ISCSINode { + type Error = ServiceError; + + fn try_from(value: &HashMap) -> Result { + Ok(ISCSINode { + id: 0, + target: get_property(value, "Target")?, + address: get_property(value, "Address")?, + interface: get_property(value, "Interface")?, + port: get_property(value, "Port")?, + ibft: get_property(value, "IBFT")?, + connected: get_property(value, "Connected")?, + startup: get_property(value, "Startup")?, + }) + } +} + +#[derive(Clone, Default, Serialize, Deserialize)] +pub struct ISCSIAuth { + pub username: Option, + pub password: Option, + pub reverse_username: Option, + pub reverse_password: Option, +} + +impl From for HashMap { + fn from(value: ISCSIAuth) -> Self { + let mut hash = HashMap::new(); + + if let Some(username) = value.username { + hash.insert("Username".to_string(), Value::new(username).to_owned()); + } + + if let Some(password) = value.password { + hash.insert("Password".to_string(), Value::new(password).to_owned()); + } + + if let Some(reverse_username) = value.reverse_username { + hash.insert( + "ReverseUsername".to_string(), + Value::new(reverse_username).to_owned(), + ); + } + + if let Some(reverse_password) = value.reverse_password { + hash.insert( + "ReversePassword".to_string(), + Value::new(reverse_password).to_owned(), + ); + } + + hash + } +} + +/// D-Bus client for the ISCSI part of the storage service. +#[derive(Clone)] +pub struct ISCSIClient<'a> { + connection: zbus::Connection, + initiator_proxy: InitiatorProxy<'a>, + object_manager_proxy: ObjectManagerProxy<'a>, +} + +impl<'a> ISCSIClient<'a> { + pub async fn new(connection: Connection) -> Result, ServiceError> { + let initiator_proxy = InitiatorProxy::builder(&connection) + .destination("org.opensuse.Agama.Storage1")? + .path("/org/opensuse/Agama/Storage1")? + .build() + .await?; + + let object_manager_proxy = ObjectManagerProxy::builder(&connection) + .destination("org.opensuse.Agama.Storage1")? + .path("/org/opensuse/Agama/Storage1")? + .build() + .await?; + + Ok(Self { + connection, + initiator_proxy, + object_manager_proxy, + }) + } + + /// Performs an iSCSI discovery. + /// + /// It returns true when the discovery was successful. + /// + /// * `address`: target address in string-like form. + /// * `port`: target port. + /// * `auth`: authentication options. + pub async fn discover<'b>( + &self, + address: &str, + port: u32, + auth: ISCSIAuth, + ) -> Result { + let mut options_hash: HashMap<&str, zvariant::Value> = HashMap::new(); + + if let (Some(ref username), Some(ref password)) = (auth.username, auth.password) { + options_hash.insert("Username", username.to_string().into()); + options_hash.insert("Password", password.to_string().into()); + } + + if let (Some(ref username), Some(ref password)) = + (auth.reverse_username, auth.reverse_password) + { + options_hash.insert("ReverseUsername", username.to_string().into()); + options_hash.insert("ReversePassword", password.to_string().into()); + } + + let mut options_ref: HashMap<&str, &zvariant::Value<'_>> = HashMap::new(); + for (key, value) in options_hash.iter() { + options_ref.insert(key, value); + } + + let result = self + .initiator_proxy + .discover(address, port as u32, options_ref) + .await?; + Ok(result == 0) + } + + /// Returns the initiator data. + pub async fn get_initiator(&self) -> Result { + let ibft = self.initiator_proxy.ibft().await?; + let name = self.initiator_proxy.initiator_name().await?; + Ok(Initiator { name, ibft }) + } + + /// Returns the iSCSI nodes. + pub async fn get_nodes(&self) -> Result, ServiceError> { + let managed_objects = self.object_manager_proxy.get_managed_objects().await?; + + let mut nodes: Vec = vec![]; + for (path, ifaces) in managed_objects { + if let Some(properties) = ifaces.get("org.opensuse.Agama.Storage1.ISCSI.Node") { + let id = extract_node_id(&path).unwrap_or(0); + match ISCSINode::try_from(properties) { + Ok(mut node) => { + node.id = id; + nodes.push(node); + } + Err(error) => { + log::warn!("Not a valid iSCSI node: {}", error); + } + } + } + } + Ok(nodes) + } + + pub async fn login( + &self, + id: u32, + auth: ISCSIAuth, + startup: String, + ) -> Result { + let proxy = self.get_node_proxy(id).await?; + + let mut options: HashMap = auth.into(); + options.insert("Startup".to_string(), Value::new(startup).to_owned()); + + // FIXME: duplicated code (see discover) + let mut options_ref: HashMap<&str, &zvariant::Value<'_>> = HashMap::new(); + for (key, value) in options.iter() { + options_ref.insert(key, value); + } + let result = proxy.login(options_ref).await?; + let result = + LoginResult::try_from(result).map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?; + Ok(result) + } + + pub async fn logout(&self, id: u32) -> Result { + let proxy = self.get_node_proxy(id).await?; + let result = proxy.logout().await?; + Ok(result == 0) + } + + pub async fn delete_node(&self, id: u32) -> Result<(), ServiceError> { + let path = format!("/org/opensuse/Agama/Storage1/iscsi_nodes/{}", id); + let path = ObjectPath::from_string_unchecked(path); + self.initiator_proxy.delete(&path).await?; + Ok(()) + } + + pub async fn get_node_proxy(&self, id: u32) -> Result { + let proxy = NodeProxy::builder(&self.connection) + .path(format!("/org/opensuse/Agama/Storage1/iscsi_nodes/{}", id))? + .build() + .await?; + Ok(proxy) + } +} + +fn extract_node_id(path: &OwnedObjectPath) -> Option { + let id = path.split("/").nth(6)?; + id.parse::().ok() +} + +#[derive(Serialize)] +pub enum LoginResult { + Success = 0, + InvalidStartup = 1, + Failed = 2, +} + +#[derive(Debug, Error, PartialEq)] +#[error("Invalid iSCSI login result: {0}")] +pub struct InvalidLoginResult(u32); + +impl TryFrom for LoginResult { + type Error = InvalidLoginResult; + + fn try_from(value: u32) -> Result { + match value { + v if v == Self::Success as u32 => Ok(Self::Success), + v if v == Self::InvalidStartup as u32 => Ok(Self::InvalidStartup), + v if v == Self::Failed as u32 => Ok(Self::Failed), + _ => Err(InvalidLoginResult(value)), + } + } +} + +// TODO: the error description should come from the backend (as in deregister) +impl fmt::Display for LoginResult { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Success => write!(f, "Success"), + Self::InvalidStartup => write!(f, "Invalid startup value"), + Self::Failed => write!(f, "Could not login into the iSCSI node"), + } + } +} diff --git a/rust/agama-lib/src/storage/proxies.rs b/rust/agama-lib/src/storage/proxies.rs index 052b4692da..02c87d3ebb 100644 --- a/rust/agama-lib/src/storage/proxies.rs +++ b/rust/agama-lib/src/storage/proxies.rs @@ -95,3 +95,77 @@ trait Device { #[dbus_proxy(property, name = "SID")] fn sid(&self) -> zbus::Result; } + +#[dbus_proxy( + interface = "org.opensuse.Agama.Storage1.ISCSI.Initiator", + default_service = "org.opensuse.Agama.Storage1", + assume_defaults = true +)] +trait Initiator { + /// Delete method + fn delete(&self, node: &zbus::zvariant::ObjectPath<'_>) -> zbus::Result; + + /// Discover method + fn discover( + &self, + address: &str, + port: u32, + options: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result; + + /// IBFT property + #[dbus_proxy(property, name = "IBFT")] + fn ibft(&self) -> zbus::Result; + + /// InitiatorName property + #[dbus_proxy(property)] + fn initiator_name(&self) -> zbus::Result; + #[dbus_proxy(property)] + fn set_initiator_name(&self, value: &str) -> zbus::Result<()>; +} + +#[dbus_proxy( + interface = "org.opensuse.Agama.Storage1.ISCSI.Node", + default_service = "org.opensuse.Agama.Storage1", + assume_defaults = true +)] +trait Node { + /// Login method + fn login( + &self, + options: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result; + + /// Logout method + fn logout(&self) -> zbus::Result; + + /// Address property + #[dbus_proxy(property)] + fn address(&self) -> zbus::Result; + + /// Connected property + #[dbus_proxy(property)] + fn connected(&self) -> zbus::Result; + + /// IBFT property + #[dbus_proxy(property, name = "IBFT")] + fn ibft(&self) -> zbus::Result; + + /// Interface property + #[dbus_proxy(property)] + fn interface(&self) -> zbus::Result; + + /// Port property + #[dbus_proxy(property)] + fn port(&self) -> zbus::Result; + + /// Startup property + #[dbus_proxy(property)] + fn startup(&self) -> zbus::Result; + #[dbus_proxy(property)] + fn set_startup(&self, value: &str) -> zbus::Result<()>; + + /// Target property + #[dbus_proxy(property)] + fn target(&self) -> zbus::Result; +} From d5999c1dccc8832f393881639b594b288bedae0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 26 Apr 2024 13:15:20 +0100 Subject: [PATCH 03/24] rust: add a basic HTTP API for iSCSI handling --- rust/agama-server/src/storage/web.rs | 5 + rust/agama-server/src/storage/web/iscsi.rs | 122 +++++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 rust/agama-server/src/storage/web/iscsi.rs diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index 7bbdcf63be..14467cebbd 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -24,8 +24,11 @@ use axum::{ use serde::Serialize; use tokio_stream::{Stream, StreamExt}; +mod iscsi; + use crate::{ error::Error, + storage::web::iscsi::iscsi_service, web::{ common::{issues_router, progress_router, service_status_router, EventStreams}, Event, @@ -77,6 +80,7 @@ pub async fn storage_service(dbus: zbus::Connection) -> Result Result { + client: ISCSIClient<'a>, +} + +pub async fn iscsi_service(dbus: &zbus::Connection) -> Result, ServiceError> { + let client = ISCSIClient::new(dbus.clone()).await?; + let state = ISCSIState { client }; + let router = Router::new() + .route("/initiator", get(initiator)) + .route("/nodes", get(nodes)) + .route("/nodes/:id", delete(delete_node)) + .route("/nodes/:id/login", post(login_node)) + .route("/nodes/:id/logout", post(logout_node)) + .route("/discover", post(discover)) + .with_state(state); + Ok(router) +} + +async fn initiator(State(state): State>) -> Result, Error> { + let initiator = state.client.get_initiator().await?; + Ok(Json(initiator)) +} + +async fn nodes(State(state): State>) -> Result>, Error> { + let nodes = state.client.get_nodes().await?; + Ok(Json(nodes)) +} + +#[derive(Deserialize)] +struct DiscoverParams { + address: String, + port: u32, + #[serde(default)] + options: ISCSIAuth, +} + +async fn discover( + State(state): State>, + Json(params): Json, +) -> Result { + let result = state + .client + .discover(¶ms.address, params.port, params.options) + .await?; + if result { + Ok(StatusCode::NO_CONTENT) + } else { + Ok(StatusCode::BAD_REQUEST) + } +} + +async fn delete_node( + State(state): State>, + Path(id): Path, +) -> Result { + state.client.delete_node(id).await?; + Ok(StatusCode::NO_CONTENT) +} + +#[derive(Deserialize)] +struct LoginParams { + #[serde(flatten)] + auth: ISCSIAuth, + startup: String, +} + +#[derive(Serialize)] +struct LoginError { + code: LoginResult, +} + +async fn login_node( + State(state): State>, + Path(id): Path, + Json(params): Json, +) -> Result { + let result = state.client.login(id, params.auth, params.startup).await?; + match result { + LoginResult::Success => Ok((StatusCode::NO_CONTENT, ().into_response())), + error => Ok(( + StatusCode::UNPROCESSABLE_ENTITY, + Json(error).into_response(), + )), + } +} + +async fn logout_node( + State(state): State>, + Path(id): Path, +) -> Result { + if state.client.logout(id).await? { + Ok(StatusCode::NO_CONTENT) + } else { + Ok(StatusCode::UNPROCESSABLE_ENTITY) + } +} From 8883fae94c0b3b71c4f5c8258df9c121637587e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 6 May 2024 11:13:31 +0100 Subject: [PATCH 04/24] rust: add tests for get_*_property functions --- rust/agama-lib/src/dbus.rs | 41 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/rust/agama-lib/src/dbus.rs b/rust/agama-lib/src/dbus.rs index 822016d3c9..8fef639679 100644 --- a/rust/agama-lib/src/dbus.rs +++ b/rust/agama-lib/src/dbus.rs @@ -43,3 +43,44 @@ where Ok(None) } } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use zbus::zvariant::{self, OwnedValue, Str}; + + use crate::dbus::{get_optional_property, get_property}; + + #[test] + fn test_get_property() { + let data: HashMap = HashMap::from([ + ("Id".to_string(), (1 as u8).into()), + ("Device".to_string(), Str::from_static("/dev/sda").into()), + ]); + let id: u8 = get_property(&data, "Id").unwrap(); + assert_eq!(id, 1); + + let device: String = get_property(&data, "Device").unwrap(); + assert_eq!(device, "/dev/sda".to_string()); + } + + #[test] + fn test_get_property_wrong_type() { + let data: HashMap = + HashMap::from([("Id".to_string(), (1 as u8).into())]); + let result: Result = get_property(&data, "Id"); + assert_eq!(result, Err(zvariant::Error::IncorrectType)); + } + + #[test] + fn test_get_optional_property() { + let data: HashMap = + HashMap::from([("Id".to_string(), (1 as u8).into())]); + let id: Option = get_optional_property(&data, "Id").unwrap(); + assert_eq!(id, Some(1)); + + let device: Option = get_optional_property(&data, "Device").unwrap(); + assert_eq!(device, None); + } +} From da32772c16c5839e97b694e5fe4a14fdbdde1db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 6 May 2024 11:32:24 +0100 Subject: [PATCH 05/24] rust: add an UpdateFromDBus trait --- rust/agama-lib/src/dbus.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/rust/agama-lib/src/dbus.rs b/rust/agama-lib/src/dbus.rs index 8fef639679..b073734a57 100644 --- a/rust/agama-lib/src/dbus.rs +++ b/rust/agama-lib/src/dbus.rs @@ -44,6 +44,30 @@ where } } +#[macro_export] +macro_rules! property_from_dbus { + ($self:ident, $field:ident, $key:expr, $dbus:ident, $type:ty) => { + if let Some(v) = get_optional_property($dbus, $key)? { + $self.$field = v; + } + }; +} + +/// Merges an struct when the values coming from D-Bus. +/// +/// NOTE: Instead of using a trait, we could follow a different approach by +/// exporting the struct to a HashMap (implementing From/Into), +/// merging both hashes and creating a new struct from the merged values. +pub trait UpdateFromDBus { + /// Updates the struct when the given values. + /// + /// * `value`: values from D-Bus. + fn update_from_dbus( + &mut self, + value: &HashMap, + ) -> Result<(), zbus::zvariant::Error>; +} + #[cfg(test)] mod tests { use std::collections::HashMap; From dc04603ede9774bdef2f64e988c14070329de8f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 6 May 2024 11:33:23 +0100 Subject: [PATCH 06/24] rust: add a function to convert non-owned to owned hashmaps --- rust/agama-lib/src/dbus.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/rust/agama-lib/src/dbus.rs b/rust/agama-lib/src/dbus.rs index b073734a57..4fffe10c90 100644 --- a/rust/agama-lib/src/dbus.rs +++ b/rust/agama-lib/src/dbus.rs @@ -68,6 +68,19 @@ pub trait UpdateFromDBus { ) -> Result<(), zbus::zvariant::Error>; } +/// Converts a hash map containing zbus non-owned values to hash map with owned ones. +/// +/// * `source`: hash map containing non-onwed values ([zbus::zvariant::Value]). +pub fn to_owned_hash(source: &HashMap<&str, Value<'_>>) -> HashMap { + let mut owned = HashMap::new(); + for (key, value) in source.iter() { + if let Ok(owned_value) = value.try_into() { + owned.insert(key.to_string(), owned_value); + } + } + owned +} + #[cfg(test)] mod tests { use std::collections::HashMap; From 4f7b24b100451b06d86e37dffd81b072ee7f102b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 6 May 2024 12:47:45 +0100 Subject: [PATCH 07/24] rust: add a stream of added/changed/removed D-Bus objects --- rust/agama-server/src/dbus.rs | 340 ++++++++++++++++++++++++++++++++++ rust/agama-server/src/lib.rs | 1 + 2 files changed, 341 insertions(+) create mode 100644 rust/agama-server/src/dbus.rs diff --git a/rust/agama-server/src/dbus.rs b/rust/agama-server/src/dbus.rs new file mode 100644 index 0000000000..a222a53f4a --- /dev/null +++ b/rust/agama-server/src/dbus.rs @@ -0,0 +1,340 @@ +//! This module defines some reusable functions/structs related to D-Bus that might be useful when +//! implementing agama-server features. + +use std::{ + collections::{hash_map::Entry, HashMap}, + pin::Pin, + sync::Arc, + task::{Context, Poll}, +}; + +use agama_lib::{ + dbus::{to_owned_hash, UpdateFromDBus}, + error::ServiceError, +}; +use futures_util::{ready, Stream}; +use pin_project::pin_project; +use tokio::sync::mpsc::unbounded_channel; +use tokio_stream::{wrappers::UnboundedReceiverStream, StreamExt, StreamMap}; +use zbus::{ + fdo::{InterfacesAdded, InterfacesRemoved, PropertiesChanged}, + zvariant::{ObjectPath, OwnedObjectPath, OwnedValue}, + MatchRule, Message, MessageStream, MessageType, +}; + +#[derive(Debug)] +pub enum DBusObjectChange { + Added(OwnedObjectPath, HashMap), + Changed(OwnedObjectPath, HashMap), + Removed(OwnedObjectPath), +} + +const PROPERTIES_CHANGED: &str = "properties_changed"; +const OBJECTS_MANAGER: &str = "objects_manager"; + +/// This stream listens for changes in a collection of D-Bus objects and emits +/// an [DBusObjectChange] when an object is added, updated or removed. It is required +/// that the collection implements the ObjectManager interface. +/// +/// Initially, it was intended to emit the proxy representing the object. However, as +/// retrieving the proxy is an async operation too, it might lead to a deadlock. +/// See the [zbus::MessageStream](https://docs.rs/zbus/4.2.0/zbus/struct.MessageStream.html) +/// and [issue#350](https://github.com/dbus2/zbus/issues/350). +/// +/// TODO: allow filtering by multiple D-Bus interfaces and properties. +#[pin_project] +pub struct DBusObjectChangesStream { + connection: zbus::Connection, + manager_path: OwnedObjectPath, + namespace: OwnedObjectPath, + interface: String, + #[pin] + inner: StreamMap<&'static str, MessageStream>, +} + +impl DBusObjectChangesStream { + /// Creates a new stream. + /// + /// * `connection`: D-Bus connection to listen on. + /// * `manager_path`: D-Bus path of the object implementing the ObjectManager. + /// * `namespace`: namespace to watch (corresponds to a "path_namespace" in the MatchRule). + /// * `interface`: name of the interface to watch. + pub async fn new( + connection: &zbus::Connection, + manager_path: &ObjectPath<'_>, + namespace: &ObjectPath<'_>, + interface: &str, + ) -> Result { + let manager_path = OwnedObjectPath::from(manager_path.to_owned()); + let namespace = OwnedObjectPath::from(namespace.to_owned()); + let connection = connection.clone(); + let mut inner = StreamMap::new(); + inner.insert( + OBJECTS_MANAGER, + Self::build_added_and_removed_stream(&connection, &manager_path).await?, + ); + inner.insert( + PROPERTIES_CHANGED, + Self::build_properties_changed_stream(&connection, &namespace).await?, + ); + + Ok(Self { + connection, + manager_path, + namespace, + interface: interface.to_string(), + inner, + }) + } + + /// Handles the case where a property changes. + /// + /// * message: property change message. + /// * interface: name of the interface to watch. + fn handle_properties_changed( + message: Result, zbus::Error>, + interface: &str, + ) -> Option { + let Ok(message) = message else { + return None; + }; + let properties = PropertiesChanged::from_message(message)?; + let args = properties.args().ok()?; + + if args.interface_name.as_str() == interface { + let path = OwnedObjectPath::from(properties.path().unwrap().clone()); + let data = to_owned_hash(&args.changed_properties); + Some(DBusObjectChange::Changed(path, data)) + } else { + None + } + } + + /// Handles the addition or removal of an object. + /// + /// * message: add/remove message. + /// * interface: name of the interface to watch. + fn handle_added_or_removed( + message: Result, zbus::Error>, + interface: &str, + ) -> Option { + let Ok(message) = message else { + return None; + }; + + if let Some(added) = InterfacesAdded::from_message(message.clone()) { + let args = added.args().ok()?; + let data = args.interfaces_and_properties.get(&interface)?; + let data = to_owned_hash(data); + let path = OwnedObjectPath::from(args.object_path().clone()); + return Some(DBusObjectChange::Added(path, data)); + } + + if let Some(removed) = InterfacesRemoved::from_message(message) { + let args = removed.args().ok()?; + if args.interfaces.contains(&interface) { + let path = OwnedObjectPath::from(args.object_path().clone()); + return Some(DBusObjectChange::Removed(path)); + } + } + + None + } + + /// Builds a stream of added/removed objects within the collection. + /// + /// * `connection`: D-Bus connection. + /// * `manager_path`: . + async fn build_added_and_removed_stream( + connection: &zbus::Connection, + manager_path: &OwnedObjectPath, + ) -> Result { + let rule = MatchRule::builder() + .msg_type(MessageType::Signal) + .path(manager_path.clone())? + .interface("org.freedesktop.DBus.ObjectManager")? + .build(); + let stream = MessageStream::for_match_rule(rule, &connection, None).await?; + Ok(stream) + } + + /// Builds a stream of properties changed within the collection. + /// + /// * `connection`: D-Bus connection. + /// * `namespace`: namespace to watch for. + async fn build_properties_changed_stream( + connection: &zbus::Connection, + namespace: &OwnedObjectPath, + ) -> Result { + let rule = MatchRule::builder() + .msg_type(MessageType::Signal) + .path_namespace(namespace.clone())? + .interface("org.freedesktop.DBus.Properties")? + .member("PropertiesChanged")? + .build(); + let stream = MessageStream::for_match_rule(rule, &connection, None).await?; + Ok(stream) + } +} + +impl Stream for DBusObjectChangesStream { + type Item = DBusObjectChange; + + fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + let mut pinned = self.project(); + Poll::Ready(loop { + let item = ready!(pinned.inner.as_mut().poll_next(cx)); + let next_value = match item { + Some((PROPERTIES_CHANGED, message)) => { + Self::handle_properties_changed(message, &pinned.interface) + } + Some((OBJECTS_MANAGER, message)) => { + Self::handle_added_or_removed(message, &pinned.interface) + } + _ => None, + }; + if next_value.is_some() { + break next_value; + } + }) + } +} + +#[derive(Debug, Clone)] +pub enum ObjectEvent { + Added(T), + Changed(T), + Removed(T), +} + +/// This stream listens for changes in a collection of D-Bus objects and emits +/// the updated objects. +/// +/// By implementing a cache, it avoids holding a bunch of proxy objects. +#[pin_project] +pub struct ObjectsStream { + dbus: zbus::Connection, + cache: ObjectsCache, + #[pin] + inner: UnboundedReceiverStream, +} + +impl Stream for ObjectsStream +where + T: Default + Clone + UpdateFromDBus, +{ + type Item = ObjectEvent; + + fn poll_next( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + let mut pinned = self.project(); + let change = ready!(pinned.inner.as_mut().poll_next(cx)); + + match change { + Some(change) => match change { + DBusObjectChange::Added(path, values) => { + let object = Self::handle_added(pinned.cache, path, values).unwrap(); + Poll::Ready(Some(ObjectEvent::Added(object.clone()))) + } + DBusObjectChange::Changed(path, updated) => { + let object = Self::handle_changed(pinned.cache, path, updated).unwrap(); + Poll::Ready(Some(ObjectEvent::Changed(object.clone()))) + } + DBusObjectChange::Removed(path) => { + let object = Self::handle_removed(pinned.cache, path).unwrap(); + Poll::Ready(Some(ObjectEvent::Removed(object))) + } + }, + None => Poll::Ready(None), + } + } +} + +impl ObjectsStream +where + T: Default + Clone + UpdateFromDBus, +{ + /// Creates a new stream. + /// + /// * `connection`: D-Bus connection to listen on. + /// * `manager_path`: D-Bus path of the object implementing the ObjectManager. + /// * `namespace`: namespace to watch (corresponds to a "path_namespace" in the MatchRule). + /// * `interface`: name of the interface to watch. + pub async fn new( + dbus: &zbus::Connection, + manager_path: &ObjectPath<'_>, + namespace: &ObjectPath<'_>, + interface: &str, + ) -> Result { + let (tx, rx) = unbounded_channel(); + let mut stream = + DBusObjectChangesStream::new(&dbus, manager_path, namespace, interface).await?; + + tokio::spawn(async move { + while let Some(change) = stream.next().await { + let _ = tx.send(change); + } + }); + let rx = UnboundedReceiverStream::new(rx); + + Ok(Self { + dbus: dbus.clone(), + cache: Default::default(), + inner: rx, + }) + } + + fn handle_added( + cache: &mut ObjectsCache, + path: OwnedObjectPath, + values: HashMap, + ) -> Result<&mut T, ServiceError> { + let object = cache.find_or_create_device(&path); + object.update_from_dbus(&values)?; + Ok(object) + } + + fn handle_changed( + cache: &mut ObjectsCache, + path: OwnedObjectPath, + values: HashMap, + ) -> Result<&mut T, ServiceError> { + let object = cache.find_or_create_device(&path); + object.update_from_dbus(&values)?; + Ok(object) + } + + fn handle_removed(cache: &mut ObjectsCache, path: OwnedObjectPath) -> Option { + cache.remove_device(&path) + } +} + +struct ObjectsCache { + objects: HashMap, +} + +impl ObjectsCache +where + T: Default, +{ + fn find_or_create_device(&mut self, path: &OwnedObjectPath) -> &mut T { + match self.objects.entry(path.clone()) { + Entry::Vacant(entry) => entry.insert(T::default()), + Entry::Occupied(entry) => entry.into_mut(), + } + } + + fn remove_device(&mut self, path: &OwnedObjectPath) -> Option { + self.objects.remove(&path) + } +} + +impl Default for ObjectsCache { + fn default() -> Self { + Self { + objects: HashMap::new(), + } + } +} diff --git a/rust/agama-server/src/lib.rs b/rust/agama-server/src/lib.rs index 46f0cfc909..fe2efcdef8 100644 --- a/rust/agama-server/src/lib.rs +++ b/rust/agama-server/src/lib.rs @@ -1,4 +1,5 @@ pub mod cert; +pub mod dbus; pub mod error; pub mod l10n; pub mod manager; From 657b10c5b1471ecce9289d07453cae987af35be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 6 May 2024 13:00:09 +0100 Subject: [PATCH 08/24] rust: implement UpdateFromDBus for ISCSINode --- rust/agama-lib/src/storage.rs | 5 ++++- rust/agama-lib/src/storage/client/iscsi.rs | 21 +++++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/rust/agama-lib/src/storage.rs b/rust/agama-lib/src/storage.rs index df40feed1c..f6f7fae89f 100644 --- a/rust/agama-lib/src/storage.rs +++ b/rust/agama-lib/src/storage.rs @@ -6,6 +6,9 @@ pub mod proxies; mod settings; mod store; -pub use client::{iscsi::ISCSIClient, StorageClient}; +pub use client::{ + iscsi::{ISCSIClient, ISCSINode}, + StorageClient, +}; pub use settings::StorageSettings; pub use store::StorageStore; diff --git a/rust/agama-lib/src/storage/client/iscsi.rs b/rust/agama-lib/src/storage/client/iscsi.rs index 32bd9c4740..88cdb3e44a 100644 --- a/rust/agama-lib/src/storage/client/iscsi.rs +++ b/rust/agama-lib/src/storage/client/iscsi.rs @@ -2,8 +2,9 @@ use core::fmt; use std::collections::HashMap; use crate::{ - dbus::get_property, + dbus::{get_optional_property, get_property, UpdateFromDBus}, error::ServiceError, + property_from_dbus, storage::proxies::{InitiatorProxy, NodeProxy}, }; use serde::{Deserialize, Serialize}; @@ -20,7 +21,7 @@ pub struct Initiator { ibft: bool, } -#[derive(Default, Serialize)] +#[derive(Clone, Debug, Default, Serialize)] /// ISCSI node pub struct ISCSINode { /// Artificial ID to match it against the D-Bus backend. @@ -58,6 +59,22 @@ impl TryFrom<&HashMap> for ISCSINode { } } +impl UpdateFromDBus for ISCSINode { + fn update_from_dbus( + &mut self, + value: &HashMap, + ) -> Result<(), zbus::zvariant::Error> { + property_from_dbus!(self, target, "Target", value, str); + property_from_dbus!(self, address, "Address", value, str); + property_from_dbus!(self, interface, "Interface", value, str); + property_from_dbus!(self, startup, "Startup", value, str); + property_from_dbus!(self, id, "Id", value, u32); + property_from_dbus!(self, port, "Port", value, u32); + property_from_dbus!(self, connected, "Connected", value, bool); + Ok(()) + } +} + #[derive(Clone, Default, Serialize, Deserialize)] pub struct ISCSIAuth { pub username: Option, From fd9c9150777bbefeea857e980293bd750fb34cec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 6 May 2024 15:33:52 +0100 Subject: [PATCH 09/24] rust: emit iSCSI events --- rust/agama-server/src/storage/web.rs | 13 +++++---- rust/agama-server/src/storage/web/iscsi.rs | 34 +++++++++++++++++++++- rust/agama-server/src/web/event.rs | 11 ++++++- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index 14467cebbd..9390531bdc 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -28,7 +28,7 @@ mod iscsi; use crate::{ error::Error, - storage::web::iscsi::iscsi_service, + storage::web::iscsi::{iscsi_service, iscsi_stream}, web::{ common::{issues_router, progress_router, service_status_router, EventStreams}, Event, @@ -36,10 +36,13 @@ use crate::{ }; pub async fn storage_streams(dbus: zbus::Connection) -> Result { - let result: EventStreams = vec![( - "devices_dirty", - Box::pin(devices_dirty_stream(dbus.clone()).await?), - )]; // TODO: + let result: EventStreams = vec![ + ( + "devices_dirty", + Box::pin(devices_dirty_stream(dbus.clone()).await?), + ), + ("iscsi_stream", Box::pin(iscsi_stream(&dbus).await?)), + ]; Ok(result) } diff --git a/rust/agama-server/src/storage/web/iscsi.rs b/rust/agama-server/src/storage/web/iscsi.rs index c068280410..d377267af7 100644 --- a/rust/agama-server/src/storage/web/iscsi.rs +++ b/rust/agama-server/src/storage/web/iscsi.rs @@ -5,6 +5,7 @@ //! * `iscsi_service` which returns the Axum service. //! * `iscsi_stream` which offers an stream that emits the iSCSI-related events coming from D-Bus. +use crate::{error::Error, web::Event}; use agama_lib::{ error::ServiceError, storage::{ @@ -19,15 +20,46 @@ use axum::{ routing::{delete, get, post}, Json, Router, }; +use futures_util::Stream; use serde::{Deserialize, Serialize}; +use tokio_stream::StreamExt; +use zbus::zvariant::ObjectPath; -use crate::error::Error; +use crate::dbus::{ObjectEvent, ObjectsStream}; + +/// Returns the stream of iSCSI-related events. +/// +/// It relies on [ObjectsStream]. +/// +/// * `dbus`: D-Bus connection to use. +pub async fn iscsi_stream( + dbus: &zbus::Connection, +) -> Result + Send, Error> { + let stream: ObjectsStream = ObjectsStream::new( + &dbus, + &ObjectPath::from_str_unchecked("/org/opensuse/Agama/Storage1"), + &ObjectPath::from_str_unchecked("/org/opensuse/Agama/Storage1/iscsi_nodes"), + "org.opensuse.Agama.Storage1.ISCSI.Node", + ) + .await?; + let stream = stream.map(|device| match device { + ObjectEvent::Added(node) => Event::ISCSINodeAdded { node }, + ObjectEvent::Removed(node) => Event::ISCSINodeRemoved { node }, + ObjectEvent::Changed(node) => Event::ISCSINodeChanged { node }, + }); + Ok(stream) +} #[derive(Clone)] struct ISCSIState<'a> { client: ISCSIClient<'a>, } +/// Sets up and returns the Axum service for the iSCSI part of the storage module. +/// +/// It acts as a proxy to Agama D-Bus service. +/// +/// * `dbus`: D-Bus connection to use. pub async fn iscsi_service(dbus: &zbus::Connection) -> Result, ServiceError> { let client = ISCSIClient::new(dbus.clone()).await?; let state = ISCSIState { client }; diff --git a/rust/agama-server/src/web/event.rs b/rust/agama-server/src/web/event.rs index ffa15aa4b7..85c9fc08ec 100644 --- a/rust/agama-server/src/web/event.rs +++ b/rust/agama-server/src/web/event.rs @@ -1,7 +1,7 @@ use crate::{l10n::web::LocaleConfig, network::model::NetworkChange}; use agama_lib::{ manager::InstallationPhase, product::RegistrationRequirement, progress::Progress, - software::SelectedBy, users::FirstUser, + software::SelectedBy, storage::ISCSINode, users::FirstUser, }; use serde::Serialize; use std::collections::HashMap; @@ -66,6 +66,15 @@ pub enum Event { path: String, errors: Vec, }, + ISCSINodeAdded { + node: ISCSINode, + }, + ISCSINodeChanged { + node: ISCSINode, + }, + ISCSINodeRemoved { + node: ISCSINode, + }, } pub type EventsSender = Sender; From cdb5a75e73a49364ab14ec962afbc0596a4a0d08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 7 May 2024 14:52:46 +0100 Subject: [PATCH 10/24] rust: simplify the ISCSI events handling --- rust/agama-lib/src/dbus.rs | 18 +-- rust/agama-lib/src/storage/client/iscsi.rs | 19 +-- rust/agama-server/src/dbus.rs | 127 +-------------- rust/agama-server/src/storage/web/iscsi.rs | 18 +-- .../src/storage/web/iscsi/stream.rs | 144 ++++++++++++++++++ 5 files changed, 157 insertions(+), 169 deletions(-) create mode 100644 rust/agama-server/src/storage/web/iscsi/stream.rs diff --git a/rust/agama-lib/src/dbus.rs b/rust/agama-lib/src/dbus.rs index 4fffe10c90..9d113f4545 100644 --- a/rust/agama-lib/src/dbus.rs +++ b/rust/agama-lib/src/dbus.rs @@ -53,23 +53,11 @@ macro_rules! property_from_dbus { }; } -/// Merges an struct when the values coming from D-Bus. -/// -/// NOTE: Instead of using a trait, we could follow a different approach by -/// exporting the struct to a HashMap (implementing From/Into), -/// merging both hashes and creating a new struct from the merged values. -pub trait UpdateFromDBus { - /// Updates the struct when the given values. - /// - /// * `value`: values from D-Bus. - fn update_from_dbus( - &mut self, - value: &HashMap, - ) -> Result<(), zbus::zvariant::Error>; -} - /// Converts a hash map containing zbus non-owned values to hash map with owned ones. /// +/// NOTE: we could follow a different approach like building our own type (e.g. +/// using the newtype idiom) and offering a better API. +/// /// * `source`: hash map containing non-onwed values ([zbus::zvariant::Value]). pub fn to_owned_hash(source: &HashMap<&str, Value<'_>>) -> HashMap { let mut owned = HashMap::new(); diff --git a/rust/agama-lib/src/storage/client/iscsi.rs b/rust/agama-lib/src/storage/client/iscsi.rs index 88cdb3e44a..be6e1e84e6 100644 --- a/rust/agama-lib/src/storage/client/iscsi.rs +++ b/rust/agama-lib/src/storage/client/iscsi.rs @@ -2,9 +2,8 @@ use core::fmt; use std::collections::HashMap; use crate::{ - dbus::{get_optional_property, get_property, UpdateFromDBus}, + dbus::get_property, error::ServiceError, - property_from_dbus, storage::proxies::{InitiatorProxy, NodeProxy}, }; use serde::{Deserialize, Serialize}; @@ -59,22 +58,6 @@ impl TryFrom<&HashMap> for ISCSINode { } } -impl UpdateFromDBus for ISCSINode { - fn update_from_dbus( - &mut self, - value: &HashMap, - ) -> Result<(), zbus::zvariant::Error> { - property_from_dbus!(self, target, "Target", value, str); - property_from_dbus!(self, address, "Address", value, str); - property_from_dbus!(self, interface, "Interface", value, str); - property_from_dbus!(self, startup, "Startup", value, str); - property_from_dbus!(self, id, "Id", value, u32); - property_from_dbus!(self, port, "Port", value, u32); - property_from_dbus!(self, connected, "Connected", value, bool); - Ok(()) - } -} - #[derive(Clone, Default, Serialize, Deserialize)] pub struct ISCSIAuth { pub username: Option, diff --git a/rust/agama-server/src/dbus.rs b/rust/agama-server/src/dbus.rs index a222a53f4a..c1fb2c66f3 100644 --- a/rust/agama-server/src/dbus.rs +++ b/rust/agama-server/src/dbus.rs @@ -8,17 +8,13 @@ use std::{ task::{Context, Poll}, }; -use agama_lib::{ - dbus::{to_owned_hash, UpdateFromDBus}, - error::ServiceError, -}; +use agama_lib::{dbus::to_owned_hash, error::ServiceError}; use futures_util::{ready, Stream}; use pin_project::pin_project; -use tokio::sync::mpsc::unbounded_channel; -use tokio_stream::{wrappers::UnboundedReceiverStream, StreamExt, StreamMap}; +use tokio_stream::StreamMap; use zbus::{ fdo::{InterfacesAdded, InterfacesRemoved, PropertiesChanged}, - zvariant::{ObjectPath, OwnedObjectPath, OwnedValue}, + zvariant::{ObjectPath, OwnedObjectPath}, MatchRule, Message, MessageStream, MessageType, }; @@ -200,118 +196,7 @@ impl Stream for DBusObjectChangesStream { } } -#[derive(Debug, Clone)] -pub enum ObjectEvent { - Added(T), - Changed(T), - Removed(T), -} - -/// This stream listens for changes in a collection of D-Bus objects and emits -/// the updated objects. -/// -/// By implementing a cache, it avoids holding a bunch of proxy objects. -#[pin_project] -pub struct ObjectsStream { - dbus: zbus::Connection, - cache: ObjectsCache, - #[pin] - inner: UnboundedReceiverStream, -} - -impl Stream for ObjectsStream -where - T: Default + Clone + UpdateFromDBus, -{ - type Item = ObjectEvent; - - fn poll_next( - self: std::pin::Pin<&mut Self>, - cx: &mut std::task::Context<'_>, - ) -> std::task::Poll> { - let mut pinned = self.project(); - let change = ready!(pinned.inner.as_mut().poll_next(cx)); - - match change { - Some(change) => match change { - DBusObjectChange::Added(path, values) => { - let object = Self::handle_added(pinned.cache, path, values).unwrap(); - Poll::Ready(Some(ObjectEvent::Added(object.clone()))) - } - DBusObjectChange::Changed(path, updated) => { - let object = Self::handle_changed(pinned.cache, path, updated).unwrap(); - Poll::Ready(Some(ObjectEvent::Changed(object.clone()))) - } - DBusObjectChange::Removed(path) => { - let object = Self::handle_removed(pinned.cache, path).unwrap(); - Poll::Ready(Some(ObjectEvent::Removed(object))) - } - }, - None => Poll::Ready(None), - } - } -} - -impl ObjectsStream -where - T: Default + Clone + UpdateFromDBus, -{ - /// Creates a new stream. - /// - /// * `connection`: D-Bus connection to listen on. - /// * `manager_path`: D-Bus path of the object implementing the ObjectManager. - /// * `namespace`: namespace to watch (corresponds to a "path_namespace" in the MatchRule). - /// * `interface`: name of the interface to watch. - pub async fn new( - dbus: &zbus::Connection, - manager_path: &ObjectPath<'_>, - namespace: &ObjectPath<'_>, - interface: &str, - ) -> Result { - let (tx, rx) = unbounded_channel(); - let mut stream = - DBusObjectChangesStream::new(&dbus, manager_path, namespace, interface).await?; - - tokio::spawn(async move { - while let Some(change) = stream.next().await { - let _ = tx.send(change); - } - }); - let rx = UnboundedReceiverStream::new(rx); - - Ok(Self { - dbus: dbus.clone(), - cache: Default::default(), - inner: rx, - }) - } - - fn handle_added( - cache: &mut ObjectsCache, - path: OwnedObjectPath, - values: HashMap, - ) -> Result<&mut T, ServiceError> { - let object = cache.find_or_create_device(&path); - object.update_from_dbus(&values)?; - Ok(object) - } - - fn handle_changed( - cache: &mut ObjectsCache, - path: OwnedObjectPath, - values: HashMap, - ) -> Result<&mut T, ServiceError> { - let object = cache.find_or_create_device(&path); - object.update_from_dbus(&values)?; - Ok(object) - } - - fn handle_removed(cache: &mut ObjectsCache, path: OwnedObjectPath) -> Option { - cache.remove_device(&path) - } -} - -struct ObjectsCache { +pub struct ObjectsCache { objects: HashMap, } @@ -319,14 +204,14 @@ impl ObjectsCache where T: Default, { - fn find_or_create_device(&mut self, path: &OwnedObjectPath) -> &mut T { + pub fn find_or_create(&mut self, path: &OwnedObjectPath) -> &mut T { match self.objects.entry(path.clone()) { Entry::Vacant(entry) => entry.insert(T::default()), Entry::Occupied(entry) => entry.into_mut(), } } - fn remove_device(&mut self, path: &OwnedObjectPath) -> Option { + pub fn remove(&mut self, path: &OwnedObjectPath) -> Option { self.objects.remove(&path) } } diff --git a/rust/agama-server/src/storage/web/iscsi.rs b/rust/agama-server/src/storage/web/iscsi.rs index d377267af7..1caa360a85 100644 --- a/rust/agama-server/src/storage/web/iscsi.rs +++ b/rust/agama-server/src/storage/web/iscsi.rs @@ -22,10 +22,9 @@ use axum::{ }; use futures_util::Stream; use serde::{Deserialize, Serialize}; -use tokio_stream::StreamExt; -use zbus::zvariant::ObjectPath; -use crate::dbus::{ObjectEvent, ObjectsStream}; +mod stream; +use stream::ISCSINodeStream; /// Returns the stream of iSCSI-related events. /// @@ -35,18 +34,7 @@ use crate::dbus::{ObjectEvent, ObjectsStream}; pub async fn iscsi_stream( dbus: &zbus::Connection, ) -> Result + Send, Error> { - let stream: ObjectsStream = ObjectsStream::new( - &dbus, - &ObjectPath::from_str_unchecked("/org/opensuse/Agama/Storage1"), - &ObjectPath::from_str_unchecked("/org/opensuse/Agama/Storage1/iscsi_nodes"), - "org.opensuse.Agama.Storage1.ISCSI.Node", - ) - .await?; - let stream = stream.map(|device| match device { - ObjectEvent::Added(node) => Event::ISCSINodeAdded { node }, - ObjectEvent::Removed(node) => Event::ISCSINodeRemoved { node }, - ObjectEvent::Changed(node) => Event::ISCSINodeChanged { node }, - }); + let stream = ISCSINodeStream::new(&dbus).await?; Ok(stream) } diff --git a/rust/agama-server/src/storage/web/iscsi/stream.rs b/rust/agama-server/src/storage/web/iscsi/stream.rs new file mode 100644 index 0000000000..e3b0f5c4cb --- /dev/null +++ b/rust/agama-server/src/storage/web/iscsi/stream.rs @@ -0,0 +1,144 @@ +use std::{collections::HashMap, task::Poll}; + +use agama_lib::{ + dbus::get_optional_property, error::ServiceError, property_from_dbus, storage::ISCSINode, +}; +use futures_util::{ready, Stream}; +use pin_project::pin_project; +use thiserror::Error; +use tokio::sync::mpsc::unbounded_channel; +use tokio_stream::{wrappers::UnboundedReceiverStream, StreamExt}; +use zbus::zvariant::{ObjectPath, OwnedObjectPath, OwnedValue}; + +use crate::{ + dbus::{DBusObjectChange, DBusObjectChangesStream, ObjectsCache}, + web::Event, +}; + +/// This stream listens for changes in the collection ISCSI nodes and emits +/// the updated objects. +/// +/// It relies on the [DBusObjectChangesStream] stream and uses a cache to avoid holding a bunch of +/// proxy objects. +#[pin_project] +pub struct ISCSINodeStream { + dbus: zbus::Connection, + cache: ObjectsCache, + #[pin] + inner: UnboundedReceiverStream, +} + +/// Internal stream error +#[derive(Debug, Error)] +enum ISCSINodeStreamError { + #[error("Service error: {0}")] + Service(#[from] ServiceError), + #[error("Unknown ISCSI node: {0}")] + UnknownNode(OwnedObjectPath), +} + +impl ISCSINodeStream { + /// Creates a new stream. + /// + /// * `connection`: D-Bus connection to listen on. + pub async fn new(dbus: &zbus::Connection) -> Result { + let (tx, rx) = unbounded_channel(); + let mut stream = DBusObjectChangesStream::new( + &dbus, + &ObjectPath::from_str_unchecked("/org/opensuse/Agama/Storage1"), + &ObjectPath::from_str_unchecked("/org/opensuse/Agama/Storage1/iscsi_nodes"), + "org.opensuse.Agama.Storage1.ISCSI.Node", + ) + .await?; + + tokio::spawn(async move { + while let Some(change) = stream.next().await { + let _ = tx.send(change); + } + }); + let rx = UnboundedReceiverStream::new(rx); + + Ok(Self { + dbus: dbus.clone(), + cache: Default::default(), + inner: rx, + }) + } + + fn update_node<'a>( + cache: &'a mut ObjectsCache, + path: &OwnedObjectPath, + values: &HashMap, + ) -> Result<&'a ISCSINode, ServiceError> { + let node = cache.find_or_create(&path); + if let Some((_, id)) = path.as_str().rsplit_once("/") { + node.id = id.parse().unwrap(); + } + property_from_dbus!(node, target, "Target", values, str); + property_from_dbus!(node, address, "Address", values, str); + property_from_dbus!(node, interface, "Interface", values, str); + property_from_dbus!(node, startup, "Startup", values, str); + property_from_dbus!(node, id, "Id", values, u32); + property_from_dbus!(node, port, "Port", values, u32); + property_from_dbus!(node, connected, "Connected", values, bool); + Ok(node) + } + + fn remove_node( + cache: &mut ObjectsCache, + path: &OwnedObjectPath, + ) -> Result { + cache + .remove(&path) + .ok_or_else(|| ISCSINodeStreamError::UnknownNode(path.clone())) + } + + fn handle_change( + cache: &mut ObjectsCache, + change: &DBusObjectChange, + ) -> Result { + match change { + DBusObjectChange::Added(path, values) => { + let node = Self::update_node(cache, path, &values)?; + Ok(Event::ISCSINodeAdded { node: node.clone() }) + } + DBusObjectChange::Changed(path, updated) => { + let node = Self::update_node(cache, path, &updated)?; + Ok(Event::ISCSINodeChanged { node: node.clone() }) + } + DBusObjectChange::Removed(path) => { + let node = Self::remove_node(cache, path)?; + Ok(Event::ISCSINodeRemoved { node }) + } + } + } +} + +impl Stream for ISCSINodeStream { + type Item = Event; + + fn poll_next( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + let mut pinned = self.project(); + + Poll::Ready(loop { + let change = ready!(pinned.inner.as_mut().poll_next(cx)); + let next_value = match change { + Some(change) => { + if let Ok(event) = Self::handle_change(pinned.cache, &change) { + Some(event) + } else { + log::warn!("Could not process change {:?}", &change); + None + } + } + None => break None, + }; + if next_value.is_some() { + break next_value; + } + }) + } +} From cafa40251af5daf8137982166d25861ffd17a4ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 11:37:01 +0100 Subject: [PATCH 11/24] rust: set the initiator name and the node startup --- rust/agama-lib/src/storage/client/iscsi.rs | 16 +++++++++++ rust/agama-server/src/storage/web/iscsi.rs | 31 ++++++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/rust/agama-lib/src/storage/client/iscsi.rs b/rust/agama-lib/src/storage/client/iscsi.rs index be6e1e84e6..37a02323bd 100644 --- a/rust/agama-lib/src/storage/client/iscsi.rs +++ b/rust/agama-lib/src/storage/client/iscsi.rs @@ -171,6 +171,13 @@ impl<'a> ISCSIClient<'a> { Ok(Initiator { name, ibft }) } + /// Sets the initiator name. + /// + /// * `name`: new name. + pub async fn set_initiator_name(&self, name: &str) -> Result<(), ServiceError> { + Ok(self.initiator_proxy.set_initiator_name(name).await?) + } + /// Returns the iSCSI nodes. pub async fn get_nodes(&self) -> Result, ServiceError> { let managed_objects = self.object_manager_proxy.get_managed_objects().await?; @@ -193,6 +200,15 @@ impl<'a> ISCSIClient<'a> { Ok(nodes) } + /// Sets the startup for a ISCSI node. + /// + /// * `id`: node ID. + /// * `startup`: new startup value. + pub async fn set_startup(&self, id: u32, startup: &str) -> Result<(), ServiceError> { + let proxy = self.get_node_proxy(id).await?; + Ok(proxy.set_startup(startup).await?) + } + pub async fn login( &self, id: u32, diff --git a/rust/agama-server/src/storage/web/iscsi.rs b/rust/agama-server/src/storage/web/iscsi.rs index 1caa360a85..a3127f92d4 100644 --- a/rust/agama-server/src/storage/web/iscsi.rs +++ b/rust/agama-server/src/storage/web/iscsi.rs @@ -52,9 +52,9 @@ pub async fn iscsi_service(dbus: &zbus::Connection) -> Result, Serv let client = ISCSIClient::new(dbus.clone()).await?; let state = ISCSIState { client }; let router = Router::new() - .route("/initiator", get(initiator)) + .route("/initiator", get(initiator).patch(update_initiator)) .route("/nodes", get(nodes)) - .route("/nodes/:id", delete(delete_node)) + .route("/nodes/:id", delete(delete_node).patch(update_node)) .route("/nodes/:id/login", post(login_node)) .route("/nodes/:id/logout", post(logout_node)) .route("/discover", post(discover)) @@ -67,6 +67,19 @@ async fn initiator(State(state): State>) -> Result>, + Json(params): Json, +) -> Result { + state.client.set_initiator_name(¶ms.name).await?; + Ok(StatusCode::NO_CONTENT) +} + async fn nodes(State(state): State>) -> Result>, Error> { let nodes = state.client.get_nodes().await?; Ok(Json(nodes)) @@ -95,6 +108,20 @@ async fn discover( } } +#[derive(Deserialize)] +struct NodeParams { + startup: String, +} + +async fn update_node( + State(state): State>, + Path(id): Path, + Json(params): Json, +) -> Result { + state.client.set_startup(id, ¶ms.startup).await?; + Ok(StatusCode::NO_CONTENT) +} + async fn delete_node( State(state): State>, Path(id): Path, From 9b5bd6a2b145cc75997f47537c55f0149d760f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 13:01:08 +0100 Subject: [PATCH 12/24] rust: emit ISCSI Initiator changes --- rust/agama-server/src/storage/web.rs | 14 ++++---- rust/agama-server/src/storage/web/iscsi.rs | 40 +++++++++++++++++++--- rust/agama-server/src/web/event.rs | 4 +++ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index 9390531bdc..123190d08f 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -36,13 +36,13 @@ use crate::{ }; pub async fn storage_streams(dbus: zbus::Connection) -> Result { - let result: EventStreams = vec![ - ( - "devices_dirty", - Box::pin(devices_dirty_stream(dbus.clone()).await?), - ), - ("iscsi_stream", Box::pin(iscsi_stream(&dbus).await?)), - ]; + let mut result: EventStreams = vec![( + "devices_dirty", + Box::pin(devices_dirty_stream(dbus.clone()).await?), + )]; + let mut iscsi = iscsi_stream(&dbus).await?; + + result.append(&mut iscsi); Ok(result) } diff --git a/rust/agama-server/src/storage/web/iscsi.rs b/rust/agama-server/src/storage/web/iscsi.rs index a3127f92d4..7aa891dfaf 100644 --- a/rust/agama-server/src/storage/web/iscsi.rs +++ b/rust/agama-server/src/storage/web/iscsi.rs @@ -5,11 +5,16 @@ //! * `iscsi_service` which returns the Axum service. //! * `iscsi_stream` which offers an stream that emits the iSCSI-related events coming from D-Bus. -use crate::{error::Error, web::Event}; +use crate::{ + error::Error, + web::{common::EventStreams, Event}, +}; use agama_lib::{ + dbus::{get_optional_property, to_owned_hash}, error::ServiceError, storage::{ client::iscsi::{ISCSIAuth, ISCSINode, Initiator, LoginResult}, + proxies::InitiatorProxy, ISCSIClient, }, }; @@ -20,21 +25,48 @@ use axum::{ routing::{delete, get, post}, Json, Router, }; -use futures_util::Stream; use serde::{Deserialize, Serialize}; mod stream; use stream::ISCSINodeStream; +use tokio_stream::{Stream, StreamExt}; +use zbus::fdo::{PropertiesChanged, PropertiesProxy}; /// Returns the stream of iSCSI-related events. /// /// It relies on [ObjectsStream]. /// /// * `dbus`: D-Bus connection to use. -pub async fn iscsi_stream( +pub async fn iscsi_stream(dbus: &zbus::Connection) -> Result { + let stream: EventStreams = vec![ + ("iscsi_nodes", Box::pin(ISCSINodeStream::new(dbus).await?)), + ("initiator", Box::pin(initiator_stream(dbus).await?)), + ]; + Ok(stream) +} + +async fn initiator_stream( dbus: &zbus::Connection, ) -> Result + Send, Error> { - let stream = ISCSINodeStream::new(&dbus).await?; + let proxy = PropertiesProxy::builder(dbus) + .destination("org.opensuse.Agama.Storage1")? + .path("/org/opensuse/Agama/Storage1")? + .build() + .await?; + let stream = proxy + .receive_properties_changed() + .await? + .filter_map(|change| { + let Ok(args) = change.args() else { + return None; + }; + + let changes = to_owned_hash(args.changed_properties()); + let name = get_optional_property(&changes, "InitiatorName").unwrap(); + let ibft = get_optional_property(&changes, "IBFT").unwrap(); + + Some(Event::ISCSIInitiatorChanged { ibft, name }) + }); Ok(stream) } diff --git a/rust/agama-server/src/web/event.rs b/rust/agama-server/src/web/event.rs index 85c9fc08ec..87a4c94787 100644 --- a/rust/agama-server/src/web/event.rs +++ b/rust/agama-server/src/web/event.rs @@ -75,6 +75,10 @@ pub enum Event { ISCSINodeRemoved { node: ISCSINode, }, + ISCSIInitiatorChanged { + name: Option, + ibft: Option, + }, } pub type EventsSender = Sender; From f07f7bf8a9f92dc38555e72815c1c295b17235f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 13:26:09 +0100 Subject: [PATCH 13/24] web: adapt the ISCSI client to the new HTTP API --- web/src/client/software.js | 4 +- web/src/client/storage.js | 194 +++++++----------- .../components/storage/iscsi/DiscoverForm.jsx | 31 +-- .../storage/iscsi/InitiatorPresenter.jsx | 22 +- .../storage/iscsi/InitiatorSection.jsx | 3 +- .../storage/iscsi/TargetsSection.jsx | 11 +- 6 files changed, 113 insertions(+), 152 deletions(-) diff --git a/web/src/client/software.js b/web/src/client/software.js index 7bbf0d31cf..1a319a0b60 100644 --- a/web/src/client/software.js +++ b/web/src/client/software.js @@ -295,7 +295,7 @@ class ProductBaseClient { return { success: response.ok, // still we can fail 400 due to dbus issue or 500 if backend stop working. maybe some message for this case? - message: "" + message: "", }; } @@ -318,7 +318,7 @@ class ProductBaseClient { return { success: response.ok, // still we can fail 400 due to dbus issue or 500 if backend stop working. maybe some message for this case? - message: "" + message: "", }; } diff --git a/web/src/client/storage.js b/web/src/client/storage.js index f06d1b7619..aea82d96d5 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -23,7 +23,7 @@ // cspell:ignore ptable import { compact, hex, uniq } from "~/utils"; -import { WithIssues, WithStatus, WithProgress } from "./mixins"; +import { WithIssues, WithProgress, WithStatus } from "./mixins"; import { HTTPClient } from "./http"; const STORAGE_OBJECT = "/org/opensuse/Agama/Storage1"; @@ -31,7 +31,7 @@ const STORAGE_JOBS_NAMESPACE = "/org/opensuse/Agama/Storage1/jobs"; const STORAGE_JOB_IFACE = "org.opensuse.Agama.Storage1.Job"; const PROPOSAL_IFACE = "org.opensuse.Agama.Storage1.Proposal"; const ISCSI_INITIATOR_IFACE = "org.opensuse.Agama.Storage1.ISCSI.Initiator"; -const ISCSI_NODES_NAMESPACE = "/org/opensuse/Agama/Storage1/iscsi_nodes"; +const ISCSI_NODES_NAMESPACE = "/storage/iscsi/nodes"; const ISCSI_NODE_IFACE = "org.opensuse.Agama.Storage1.ISCSI.Node"; const DASD_MANAGER_IFACE = "org.opensuse.Agama.Storage1.DASD.Manager"; const DASD_DEVICES_NAMESPACE = "/org/opensuse/Agama/Storage1/dasds"; @@ -1306,40 +1306,28 @@ class ZFCPManager { */ class ISCSIManager { /** - * @param {string} service - D-Bus service name - * @param {string} address - D-Bus address + * @param {import("./http").HTTPClient} client - HTTP client. */ - constructor(service, address) { - this.service = service; - this.address = address; - this.proxies = {}; + constructor(client) { + this.client = client; } /** - * @return {DBusClient} client + * Gets the iSCSI initiator + * + * @return {Promise} + * + * @typedef {object} ISCSIInitiator + * @property {string} name + * @property {boolean} ibft */ - client() { - // return this.assigned_client; - if (!this._client) { - this._client = new DBusClient(this.service, this.address); + async getInitiator() { + const response = await this.client.get("/storage/iscsi/initiator"); + if (!response.ok) { + console.error("Failed to get the iSCSI initiator", response); } - return this._client; - } - - async getInitiatorIbft() { - const proxy = await this.iscsiInitiatorProxy(); - return proxy.IBFT; - } - - /** - * Gets the iSCSI initiator name - * - * @returns {Promise} - */ - async getInitiatorName() { - const proxy = await this.iscsiInitiatorProxy(); - return proxy.InitiatorName; + return response.json(); } /** @@ -1347,9 +1335,8 @@ class ISCSIManager { * * @param {string} value */ - async setInitiatorName(value) { - const proxy = await this.iscsiInitiatorProxy(); - proxy.InitiatorName = value; + setInitiatorName(value) { + return this.client.patch("/storage/iscsi/initiator", { name: value }); } /** @@ -1368,8 +1355,12 @@ class ISCSIManager { * @property {string} startup */ async getNodes() { - const proxy = await this.iscsiNodesProxy(); - return Object.values(proxy).map(this.buildNode); + const response = await this.client.get("/storage/iscsi/nodes"); + if (!response.ok) { + console.error("Failed to get the list of iSCSI nodes", response); + } + + return response.json(); } /** @@ -1385,18 +1376,16 @@ class ISCSIManager { * @property {string} [reverseUsername] - Username for authentication by initiator * @property {string} [reversePassword] - Password for authentication by initiator * - * @returns {Promise} 0 on success, 1 on failure + * @returns {Promise} true on success, false on failure */ async discover(address, port, options = {}) { - const auth = removeUndefinedCockpitProperties({ - Username: { t: "s", v: options.username }, - Password: { t: "s", v: options.password }, - ReverseUsername: { t: "s", v: options.reverseUsername }, - ReversePassword: { t: "s", v: options.reversePassword } - }); - - const proxy = await this.iscsiInitiatorProxy(); - return proxy.Discover(address, port, auth); + const data = { + address, + port, + options, + }; + const response = await this.client.post("/storage/iscsi/discover", data); + return response.ok; } /** @@ -1405,25 +1394,21 @@ class ISCSIManager { * @param {ISCSINode} node * @param {String} startup */ - async setStartup(node, startup) { - const path = this.nodePath(node); - - const proxy = await this.client().proxy(ISCSI_NODE_IFACE, path); - proxy.Startup = startup; + setStartup(node, startup) { + this.client.patch(this.nodePath(node), { startup }); } /** * Deletes the given iSCSI node * * @param {ISCSINode} node - * @returns {Promise} 0 on success, 1 on failure if the given path is not exported, 2 on + * @returns {Promise} 0 on success, 1 on failure if the given path is not exported, 2 on * failure because any other reason. */ async delete(node) { - const path = this.nodePath(node); - - const proxy = await this.iscsiInitiatorProxy(); - return proxy.Delete(path); + // FIXME: return the proper error code + const response = await this.client.delete(this.nodePath(node)); + return response.ok; } /** @@ -1443,63 +1428,61 @@ class ISCSIManager { * valid, and 2 on failure because any other reason */ async login(node, options = {}) { - const path = this.nodePath(node); - - const dbusOptions = removeUndefinedCockpitProperties({ - Username: { t: "s", v: options.username }, - Password: { t: "s", v: options.password }, - ReverseUsername: { t: "s", v: options.reverseUsername }, - ReversePassword: { t: "s", v: options.reversePassword }, - Startup: { t: "s", v: options.startup } - }); + const path = this.nodePath(node) + "/login"; + const response = await this.client.post(path, options); + if (!response.ok) { + console.error("Could not login into the iSCSI node", response); + return response.json(); + } - const proxy = await this.client().proxy(ISCSI_NODE_IFACE, path); - return proxy.Login(dbusOptions); + return 0; } /** * Closes an iSCSI session * * @param {ISCSINode} node - * @returns {Promise} 0 on success, 1 on failure + * @returns {Promise} true on success, false on failure */ async logout(node) { - const path = this.nodePath(node); - // const iscsiNode = new ISCSINodeObject(this.client, path); - // return await iscsiNode.iface.logout(); - const proxy = await this.client().proxy(ISCSI_NODE_IFACE, path); - return proxy.Logout(); + const path = this.nodePath(node) + "/logout"; + const response = await this.client.post(path); + if (!response.ok) { + console.error("Could not logout from the iSCSI node", response); + return false; + } + + return true; } onInitiatorChanged(handler) { - return this.client().onObjectChanged(STORAGE_OBJECT, ISCSI_INITIATOR_IFACE, (changes) => { - const data = { - name: changes.InitiatorName?.v, - ibft: changes.IBFT?.v - }; + return this.client.onEvent("ISCSIInitiatorChanged", handler); + } - const filtered = Object.entries(data).filter(([, v]) => v !== undefined); - return handler(Object.fromEntries(filtered)); - }); + onNodeAdded(handler) { + return this.onNodeEvent("ISCSINodeAdded", handler); } - async onNodeAdded(handler) { - const proxy = await this.iscsiNodesProxy(); - proxy.addEventListener("added", (_, proxy) => handler(this.buildNode(proxy))); + onNodeChanged(handler) { + return this.onNodeEvent("ISCSINodeChanged", handler); } - async onNodeChanged(handler) { - const proxy = await this.iscsiNodesProxy(); - proxy.addEventListener("changed", (_, proxy) => handler(this.buildNode(proxy))); + onNodeRemoved(handler) { + return this.onNodeEvent("ISCSINodeRemoved", handler); } - async onNodeRemoved(handler) { - const proxy = await this.iscsiNodesProxy(); - proxy.addEventListener("removed", (_, proxy) => handler(this.buildNode(proxy))); + /** + * @private + * Registers a handler for the given iSCSI node event. + * + * @param {string} eventName - Event name + */ + onNodeEvent(eventName, handler) { + return this.client.onEvent(eventName, ({ node }) => handler(node)); } buildNode(proxy) { - const id = path => path.split("/").slice(-1)[0]; + const id = (path) => path.split("/").slice(-1)[0]; return { id: id(proxy.path), @@ -1509,39 +1492,10 @@ class ISCSIManager { interface: proxy.Interface, ibft: proxy.IBFT, connected: proxy.Connected, - startup: proxy.Startup + startup: proxy.Startup, }; } - /** - * @private - * Proxy for org.opensuse.Agama.Storage1.ISCSI.Initiator iface - * - * @returns {Promise} - */ - async iscsiInitiatorProxy() { - if (!this.proxies.iscsiInitiator) { - this.proxies.iscsiInitiator = await this.client().proxy(ISCSI_INITIATOR_IFACE, STORAGE_OBJECT); - } - - return this.proxies.iscsiInitiator; - } - - /** - * @private - * Proxy for objects implementing org.opensuse.Agama.Storage1.ISCSI.Node iface - * - * @note The ISCSI nodes are dynamically exported. - * - * @returns {Promise} - */ - async iscsiNodesProxy() { - if (!this.proxies.iscsiNodes) - this.proxies.iscsiNodes = await this.client().proxies(ISCSI_NODE_IFACE, ISCSI_NODES_NAMESPACE); - - return this.proxies.iscsiNodes; - } - /** * @private * Builds the D-Bus path for the given iSCSI node @@ -1570,7 +1524,7 @@ class StorageBaseClient { this.system = new DevicesManager(this.client, "system"); this.staging = new DevicesManager(this.client, "result"); this.proposal = new ProposalManager(this.client, this.system); - this.iscsi = new ISCSIManager(StorageBaseClient.SERVICE, client); + this.iscsi = new ISCSIManager(this.client); this.dasd = new DASDManager(StorageBaseClient.SERVICE, client); this.zfcp = new ZFCPManager(StorageBaseClient.SERVICE, client); } diff --git a/web/src/components/storage/iscsi/DiscoverForm.jsx b/web/src/components/storage/iscsi/DiscoverForm.jsx index 02df383c92..e66574e60a 100644 --- a/web/src/components/storage/iscsi/DiscoverForm.jsx +++ b/web/src/components/storage/iscsi/DiscoverForm.jsx @@ -55,11 +55,12 @@ export default function DiscoverForm({ onSubmit: onSubmitProp, onCancel }) { useEffect(() => { // Scroll the alert into view - if (isFailed) + if (isFailed) { alertRef.current.scrollIntoView({ behavior: "smooth", - block: "start" + block: "start", }); + } }); const updateData = (key, value) => setData({ ...data, [key]: value }); @@ -72,9 +73,9 @@ export default function DiscoverForm({ onSubmit: onSubmitProp, onCancel }) { setIsLoading(true); setSavedData(data); - const result = await onSubmitProp(data); + const success = await onSubmitProp(data); - if (result !== 0) { + if (!success) { setIsFailed(true); setIsLoading(false); } @@ -100,16 +101,18 @@ export default function DiscoverForm({ onSubmit: onSubmitProp, onCancel }) { // TRANSLATORS: popup title
- { isFailed && -
- -

{_("Make sure you provide the correct values")}

- -
} + {isFailed && + ( +
+ +

{_("Make sure you provide the correct values")}

+ +
+ )} { const actions = { - edit: { title: _("Edit"), onClick: openForm } + edit: { title: _("Edit"), onClick: openForm }, }; return [actions.edit]; }; const Content = () => { - if (isLoading) { + if (isLoading || !initiator) { return ( @@ -92,12 +92,14 @@ export default function InitiatorPresenter({ initiator, client }) { - { isFormOpen && - } + {isFormOpen && + ( + + )} ); } diff --git a/web/src/components/storage/iscsi/InitiatorSection.jsx b/web/src/components/storage/iscsi/InitiatorSection.jsx index e7728eaf11..ebe9c3eeb4 100644 --- a/web/src/components/storage/iscsi/InitiatorSection.jsx +++ b/web/src/components/storage/iscsi/InitiatorSection.jsx @@ -35,8 +35,7 @@ export default function InitiatorSection() { useEffect(() => { const loadInitiator = async () => { setInitiator(undefined); - const name = await cancellablePromise(client.iscsi.getInitiatorName()); - const ibft = await cancellablePromise(client.iscsi.getInitiatorIbft()); + const { name, ibft } = await cancellablePromise(client.iscsi.getInitiator()); setInitiator({ name, ibft, offloadCard: "" }); }; diff --git a/web/src/components/storage/iscsi/TargetsSection.jsx b/web/src/components/storage/iscsi/TargetsSection.jsx index 442d51675c..982f4fd08b 100644 --- a/web/src/components/storage/iscsi/TargetsSection.jsx +++ b/web/src/components/storage/iscsi/TargetsSection.jsx @@ -121,13 +121,16 @@ export default function TargetsSection() { const submitDiscoverForm = async (data) => { const { username, password, reverseUsername, reversePassword } = data; - const result = await client.iscsi.discover(data.address, parseInt(data.port), { - username, password, reverseUsername, reversePassword + const success = await client.iscsi.discover(data.address, parseInt(data.port), { + username, + password, + reverseUsername, + reversePassword, }); - if (result === 0) closeDiscoverForm(); + if (success) closeDiscoverForm(); - return result; + return success; }; const SectionContent = () => { From ff9f6ec93a775ca842a01753da55ae51ca4c580d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 14:15:30 +0100 Subject: [PATCH 14/24] web: adapt the ISCSI client tests --- web/src/client/storage.test.js | 206 ++++++++++++++++----------------- 1 file changed, 98 insertions(+), 108 deletions(-) diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index 5e0bc13e76..550989b4f4 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -22,9 +22,39 @@ // @ts-check // cspell:ignore ECKD dasda ddgdcbibhd wwpns +import { HTTPClient } from "./http"; import DBusClient from "./dbus"; import { StorageClient } from "./storage"; +const mockJsonFn = jest.fn(); +const mockGetFn = jest.fn().mockImplementation(() => { + return { ok: true, json: mockJsonFn }; +}); +const mockPostFn = jest.fn().mockImplementation(() => { + return { ok: true }; +}); +const mockDeleteFn = jest.fn().mockImplementation(() => { + return { + ok: true, + }; +}); +const mockPatchFn = jest.fn().mockImplementation(() => { + return { ok: true }; +}); + +jest.mock("./http", () => { + return { + HTTPClient: jest.fn().mockImplementation(() => { + return { + get: mockGetFn, + patch: mockPatchFn, + post: mockPostFn, + delete: mockDeleteFn, + }; + }), + }; +}); + /** * @typedef {import("~/client/storage").StorageDevice} StorageDevice */ @@ -565,54 +595,49 @@ const contexts = { Device: { t: "u", v: 2 }, Text: { t: "s", v: "Mount /dev/sdb1 as root" }, Subvol: { t: "b", v: false }, - Delete: { t: "b", v: false } - } - ] + Delete: { t: "b", v: false }, + }, + ], }; }, withAvailableDevices: () => { cockpitProxies.proposalCalculator.AvailableDevices = [ "/org/opensuse/Agama/Storage1/system/59", - "/org/opensuse/Agama/Storage1/system/62" + "/org/opensuse/Agama/Storage1/system/62", ]; }, withoutIssues: () => { cockpitProxies.issues = { - All: [] + All: [], }; }, withIssues: () => { cockpitProxies.issues = { - All: [["Issue 1", "", 1, 1], ["Issue 2", "", 1, 0], ["Issue 3", "", 2, 1]] - }; - }, - withoutISCSINodes: () => { - cockpitProxies.iscsiNodes = {}; - }, - withISCSINodes: () => { - cockpitProxies.iscsiNodes = { - "/org/opensuse/Agama/Storage1/iscsi_nodes/1": { - path: "/org/opensuse/Agama/Storage1/iscsi_nodes/1", - Target: "iqn.2023-01.com.example:37dac", - Address: "192.168.100.101", - Port: 3260, - Interface: "default", - IBFT: false, - Connected: false, - Startup: "" - }, - "/org/opensuse/Agama/Storage1/iscsi_nodes/2": { - path: "/org/opensuse/Agama/Storage1/iscsi_nodes/2", - Target: "iqn.2023-01.com.example:74afb", - Address: "192.168.100.102", - Port: 3260, - Interface: "default", - IBFT: true, - Connected: true, - Startup: "onboot" - } + All: [["Issue 1", "", 1, 1], ["Issue 2", "", 1, 0], ["Issue 3", "", 2, 1]], }; }, + withISCSINodes: () => [ + { + id: 1, + target: "iqn.2023-01.com.example:37dac", + address: "192.168.100.101", + port: 3260, + interface: "default", + ibft: false, + connected: false, + startup: "", + }, + { + id: 2, + target: "iqn.2023-01.com.example:74afb", + address: "192.168.100.102", + port: 3260, + interface: "default", + ibft: true, + connected: true, + startup: "onboot", + }, + ], withoutDASDDevices: () => { cockpitProxies.dasdDevices = {}; }, @@ -2248,40 +2273,41 @@ describe("#zfcp", () => { describe("#iscsi", () => { beforeEach(() => { - client = new StorageClient(); + client = new StorageClient(new HTTPClient(new URL("http://localhost"))); }); - describe("#getInitiatorName", () => { + describe("#getInitiator", () => { beforeEach(() => { - cockpitProxies.iscsiInitiator = { - InitiatorName: "iqn.1996-04.com.suse:01:351e6d6249" - }; + mockJsonFn.mockResolvedValue({ + name: "iqn.1996-04.com.suse:01:351e6d6249", + ibft: false, + }); }); - it("returns the current initiator name", async () => { - const initiatorName = await client.iscsi.getInitiatorName(); - expect(initiatorName).toEqual("iqn.1996-04.com.suse:01:351e6d6249"); + it("returns the current initiator", async () => { + const { name, ibft } = await client.iscsi.getInitiator(); + expect(name).toEqual("iqn.1996-04.com.suse:01:351e6d6249"); + expect(ibft).toEqual(false); }); }); describe("#setInitiatorName", () => { beforeEach(() => { cockpitProxies.iscsiInitiator = { - InitiatorName: "iqn.1996-04.com.suse:01:351e6d6249" + InitiatorName: "iqn.1996-04.com.suse:01:351e6d6249", }; }); it("sets the given initiator name", async () => { await client.iscsi.setInitiatorName("test"); - const initiatorName = await client.iscsi.getInitiatorName(); - expect(initiatorName).toEqual("test"); + expect(mockPatchFn).toHaveBeenCalledWith("/storage/iscsi/initiator", { name: "test" }); }); }); describe("#getNodes", () => { describe("if there is no exported iSCSI nodes yet", () => { beforeEach(() => { - contexts.withoutISCSINodes(); + mockJsonFn.mockResolvedValue([]); }); it("returns an empty list", async () => { @@ -2292,119 +2318,83 @@ describe("#iscsi", () => { describe("if there are exported iSCSI nodes", () => { beforeEach(() => { - contexts.withISCSINodes(); + mockJsonFn.mockResolvedValue(contexts.withISCSINodes()); }); it("returns a list with the exported iSCSI nodes", async () => { const result = await client.iscsi.getNodes(); expect(result.length).toEqual(2); expect(result).toContainEqual({ - id: "1", + id: 1, target: "iqn.2023-01.com.example:37dac", address: "192.168.100.101", - port: 3260, + port: 3260, interface: "default", ibft: false, connected: false, - startup: "" + startup: "", }); expect(result).toContainEqual({ - id: "2", + id: 2, target: "iqn.2023-01.com.example:74afb", address: "192.168.100.102", - port: 3260, + port: 3260, interface: "default", ibft: true, connected: true, - startup: "onboot" + startup: "onboot", }); }); }); }); describe("#discover", () => { - beforeEach(() => { - cockpitProxies.iscsiInitiator = { - Discover: jest.fn() - }; - }); - it("performs an iSCSI discovery with the given options", async () => { - await client.iscsi.discover("192.168.100.101", 3260, { + const options = { username: "test", password: "12345", reverseUsername: "target", - reversePassword: "nonsecret" - }); - - expect(cockpitProxies.iscsiInitiator.Discover).toHaveBeenCalledWith("192.168.100.101", 3260, { - Username: { t: "s", v: "test" }, - Password: { t: "s", v: "12345" }, - ReverseUsername: { t: "s", v: "target" }, - ReversePassword: { t: "s", v: "nonsecret" } - }); - }); - }); - - describe("#Delete", () => { - beforeEach(() => { - cockpitProxies.iscsiInitiator = { - Delete: jest.fn() + reversePassword: "nonsecret", }; + await client.iscsi.discover("192.168.100.101", 3260, options); + expect(mockPostFn).toHaveBeenCalledWith( + "/storage/iscsi/discover", + { address: "192.168.100.101", port: 3260, options }, + ); }); + }); + describe("#delete", () => { it("deletes the given iSCSI node", async () => { await client.iscsi.delete({ id: "1" }); - expect(cockpitProxies.iscsiInitiator.Delete).toHaveBeenCalledWith( - "/org/opensuse/Agama/Storage1/iscsi_nodes/1" + expect(mockDeleteFn).toHaveBeenCalledWith( + "/storage/iscsi/nodes/1", ); }); }); describe("#login", () => { - const nodeProxy = { - Login: jest.fn() - }; - - beforeEach(() => { - cockpitProxies.iscsiNode = { - "/org/opensuse/Agama/Storage1/iscsi_nodes/1": nodeProxy - }; - }); - it("performs an iSCSI login with the given options", async () => { - await client.iscsi.login({ id: "1" }, { + const auth = { username: "test", password: "12345", reverseUsername: "target", reversePassword: "nonsecret", - startup: "automatic" - }); + startup: "automatic", + }; + await client.iscsi.login({ id: "1" }, auth); - expect(nodeProxy.Login).toHaveBeenCalledWith({ - Username: { t: "s", v: "test" }, - Password: { t: "s", v: "12345" }, - ReverseUsername: { t: "s", v: "target" }, - ReversePassword: { t: "s", v: "nonsecret" }, - Startup: { t: "s", v: "automatic" } - }); + expect(mockPostFn).toHaveBeenCalledWith( + "/storage/iscsi/nodes/1/login", + auth, + ); }); }); describe("#logout", () => { - const nodeProxy = { - Logout: jest.fn() - }; - - beforeEach(() => { - cockpitProxies.iscsiNode = { - "/org/opensuse/Agama/Storage1/iscsi_nodes/1": nodeProxy - }; - }); - it("performs an iSCSI logout of the given node", async () => { await client.iscsi.logout({ id: "1" }); - expect(nodeProxy.Logout).toHaveBeenCalled(); + expect(mockPostFn).toHaveBeenCalledWith("/storage/iscsi/nodes/1/logout"); }); }); }); From 95d36f66740407e8e51f35b5310ea9e006444b15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 16:49:11 +0100 Subject: [PATCH 15/24] rust: refactor the extraction of the ISCSINode ID --- rust/agama-lib/src/dbus.rs | 14 +++++++++++++- rust/agama-lib/src/storage/client/iscsi.rs | 9 ++------- rust/agama-server/src/storage/web/iscsi/stream.rs | 5 +---- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/rust/agama-lib/src/dbus.rs b/rust/agama-lib/src/dbus.rs index 9d113f4545..fa9c8e2ec2 100644 --- a/rust/agama-lib/src/dbus.rs +++ b/rust/agama-lib/src/dbus.rs @@ -1,5 +1,5 @@ use std::collections::HashMap; -use zbus::zvariant::{self, OwnedValue, Value}; +use zbus::zvariant::{self, OwnedObjectPath, OwnedValue, Value}; /// Nested hash to send to D-Bus. pub type NestedHash<'a> = HashMap<&'a str, HashMap<&'a str, zvariant::Value<'a>>>; @@ -69,6 +69,18 @@ pub fn to_owned_hash(source: &HashMap<&str, Value<'_>>) -> HashMap Result { + path.as_str() + .rsplit_once("/") + .and_then(|(_, id)| id.parse::().ok()) + .ok_or_else(|| { + zvariant::Error::Message(format!("Could not extract the ID from {}", path.as_str())) + }) +} + #[cfg(test)] mod tests { use std::collections::HashMap; diff --git a/rust/agama-lib/src/storage/client/iscsi.rs b/rust/agama-lib/src/storage/client/iscsi.rs index 37a02323bd..2dd3795c32 100644 --- a/rust/agama-lib/src/storage/client/iscsi.rs +++ b/rust/agama-lib/src/storage/client/iscsi.rs @@ -2,7 +2,7 @@ use core::fmt; use std::collections::HashMap; use crate::{ - dbus::get_property, + dbus::{extract_id_from_path, get_property}, error::ServiceError, storage::proxies::{InitiatorProxy, NodeProxy}, }; @@ -185,7 +185,7 @@ impl<'a> ISCSIClient<'a> { let mut nodes: Vec = vec![]; for (path, ifaces) in managed_objects { if let Some(properties) = ifaces.get("org.opensuse.Agama.Storage1.ISCSI.Node") { - let id = extract_node_id(&path).unwrap_or(0); + let id = extract_id_from_path(&path).unwrap_or(0); match ISCSINode::try_from(properties) { Ok(mut node) => { node.id = id; @@ -253,11 +253,6 @@ impl<'a> ISCSIClient<'a> { } } -fn extract_node_id(path: &OwnedObjectPath) -> Option { - let id = path.split("/").nth(6)?; - id.parse::().ok() -} - #[derive(Serialize)] pub enum LoginResult { Success = 0, diff --git a/rust/agama-server/src/storage/web/iscsi/stream.rs b/rust/agama-server/src/storage/web/iscsi/stream.rs index e3b0f5c4cb..785233737e 100644 --- a/rust/agama-server/src/storage/web/iscsi/stream.rs +++ b/rust/agama-server/src/storage/web/iscsi/stream.rs @@ -71,14 +71,11 @@ impl ISCSINodeStream { values: &HashMap, ) -> Result<&'a ISCSINode, ServiceError> { let node = cache.find_or_create(&path); - if let Some((_, id)) = path.as_str().rsplit_once("/") { - node.id = id.parse().unwrap(); - } + node.id = extract_id_from_path(&path)?; property_from_dbus!(node, target, "Target", values, str); property_from_dbus!(node, address, "Address", values, str); property_from_dbus!(node, interface, "Interface", values, str); property_from_dbus!(node, startup, "Startup", values, str); - property_from_dbus!(node, id, "Id", values, u32); property_from_dbus!(node, port, "Port", values, u32); property_from_dbus!(node, connected, "Connected", values, bool); Ok(node) From 48c792163d9a27ec01ac5511bbff75dd514d1981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 17:05:27 +0100 Subject: [PATCH 16/24] rust: ISCSINodeStream reads the existing nodes --- rust/agama-lib/src/storage/client/iscsi.rs | 2 +- rust/agama-server/src/dbus.rs | 4 ++++ .../src/storage/web/iscsi/stream.rs | 22 +++++++++++++++---- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/rust/agama-lib/src/storage/client/iscsi.rs b/rust/agama-lib/src/storage/client/iscsi.rs index 2dd3795c32..6469a56426 100644 --- a/rust/agama-lib/src/storage/client/iscsi.rs +++ b/rust/agama-lib/src/storage/client/iscsi.rs @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use zbus::{ fdo::ObjectManagerProxy, - zvariant::{self, ObjectPath, OwnedObjectPath, OwnedValue, Value}, + zvariant::{self, ObjectPath, OwnedValue, Value}, Connection, }; diff --git a/rust/agama-server/src/dbus.rs b/rust/agama-server/src/dbus.rs index c1fb2c66f3..9143aa9ab1 100644 --- a/rust/agama-server/src/dbus.rs +++ b/rust/agama-server/src/dbus.rs @@ -204,6 +204,10 @@ impl ObjectsCache where T: Default, { + pub fn add(&mut self, path: OwnedObjectPath, object: T) { + _ = self.objects.insert(path, object) + } + pub fn find_or_create(&mut self, path: &OwnedObjectPath) -> &mut T { match self.objects.entry(path.clone()) { Entry::Vacant(entry) => entry.insert(T::default()), diff --git a/rust/agama-server/src/storage/web/iscsi/stream.rs b/rust/agama-server/src/storage/web/iscsi/stream.rs index 785233737e..eb67aeb8be 100644 --- a/rust/agama-server/src/storage/web/iscsi/stream.rs +++ b/rust/agama-server/src/storage/web/iscsi/stream.rs @@ -1,7 +1,10 @@ use std::{collections::HashMap, task::Poll}; use agama_lib::{ - dbus::get_optional_property, error::ServiceError, property_from_dbus, storage::ISCSINode, + dbus::{extract_id_from_path, get_optional_property}, + error::ServiceError, + property_from_dbus, + storage::{ISCSIClient, ISCSINode}, }; use futures_util::{ready, Stream}; use pin_project::pin_project; @@ -42,11 +45,14 @@ impl ISCSINodeStream { /// /// * `connection`: D-Bus connection to listen on. pub async fn new(dbus: &zbus::Connection) -> Result { + const MANAGER_PATH: &str = "/org/opensuse/Agama/Storage1"; + const NAMESPACE: &str = "/org/opensuse/Agama/Storage1/iscsi_nodes"; + let (tx, rx) = unbounded_channel(); let mut stream = DBusObjectChangesStream::new( &dbus, - &ObjectPath::from_str_unchecked("/org/opensuse/Agama/Storage1"), - &ObjectPath::from_str_unchecked("/org/opensuse/Agama/Storage1/iscsi_nodes"), + &ObjectPath::from_str_unchecked(MANAGER_PATH), + &ObjectPath::from_str_unchecked(NAMESPACE), "org.opensuse.Agama.Storage1.ISCSI.Node", ) .await?; @@ -58,9 +64,17 @@ impl ISCSINodeStream { }); let rx = UnboundedReceiverStream::new(rx); + // Populate the objects cache + let mut cache: ObjectsCache = Default::default(); + let client = ISCSIClient::new(dbus.clone()).await?; + for node in client.get_nodes().await? { + let path = ObjectPath::from_string_unchecked(format!("{}/{}", NAMESPACE, node.id)); + cache.add(path.into(), node); + } + Ok(Self { dbus: dbus.clone(), - cache: Default::default(), + cache, inner: rx, }) } From 1ff3f1dd69f525b0582f827a0e6f862857c48b42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 21:10:51 +0100 Subject: [PATCH 17/24] web: improve error handling on iSCSI login --- web/src/client/storage.js | 5 +++-- web/src/client/storage.test.js | 38 +++++++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index aea82d96d5..9fd24b4ed4 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -1431,8 +1431,9 @@ class ISCSIManager { const path = this.nodePath(node) + "/login"; const response = await this.client.post(path, options); if (!response.ok) { - console.error("Could not login into the iSCSI node", response); - return response.json(); + const reason = await response.json(); + console.warn("Could not login into the iSCSI node", reason); + return reason === "InvalidStartup" ? 1 : 2; } return 0; diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index 550989b4f4..7c155db033 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -2374,21 +2374,43 @@ describe("#iscsi", () => { }); describe("#login", () => { + let auth = { + username: "test", + password: "12345", + reverseUsername: "target", + reversePassword: "nonsecret", + startup: "automatic", + }; + it("performs an iSCSI login with the given options", async () => { - const auth = { - username: "test", - password: "12345", - reverseUsername: "target", - reversePassword: "nonsecret", - startup: "automatic", - }; - await client.iscsi.login({ id: "1" }, auth); + const result = await client.iscsi.login({ id: "1" }, auth); + expect(result).toEqual(0); expect(mockPostFn).toHaveBeenCalledWith( "/storage/iscsi/nodes/1/login", auth, ); }); + + it("returns 1 when the startup is invalid", async () => { + mockPostFn.mockImplementation(() => ( + { ok: false, json: mockJsonFn } + )); + mockJsonFn.mockResolvedValue("InvalidStartup"); + + const result = await client.iscsi.login({ id: "1" }, { ...auth, startup: "invalid" }); + expect(result).toEqual(1); + }); + + it("returns 2 in case of an error different from an invalid startup value", async () => { + mockPostFn.mockImplementation(() => ( + { ok: false, json: mockJsonFn } + )); + mockJsonFn.mockResolvedValue("Failed"); + + const result = await client.iscsi.login({ id: "1" }, { ...auth, startup: "invalid" }); + expect(result).toEqual(2); + }); }); describe("#logout", () => { From 5e39371ee62a71bc097767d721ddc75abb720156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 21:28:45 +0100 Subject: [PATCH 18/24] rust: remove unwrap calls from the initiator stream --- rust/agama-server/src/storage/web/iscsi.rs | 29 ++++++++++++++-------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/rust/agama-server/src/storage/web/iscsi.rs b/rust/agama-server/src/storage/web/iscsi.rs index 7aa891dfaf..83506dc84b 100644 --- a/rust/agama-server/src/storage/web/iscsi.rs +++ b/rust/agama-server/src/storage/web/iscsi.rs @@ -34,7 +34,10 @@ use zbus::fdo::{PropertiesChanged, PropertiesProxy}; /// Returns the stream of iSCSI-related events. /// -/// It relies on [ObjectsStream]. +/// The stream combines the following events: +/// +/// * Changes on the iSCSI nodes collection. +/// * Changes to the initiator (name or ibft). /// /// * `dbus`: D-Bus connection to use. pub async fn iscsi_stream(dbus: &zbus::Connection) -> Result { @@ -56,20 +59,24 @@ async fn initiator_stream( let stream = proxy .receive_properties_changed() .await? - .filter_map(|change| { - let Ok(args) = change.args() else { - return None; - }; - - let changes = to_owned_hash(args.changed_properties()); - let name = get_optional_property(&changes, "InitiatorName").unwrap(); - let ibft = get_optional_property(&changes, "IBFT").unwrap(); - - Some(Event::ISCSIInitiatorChanged { ibft, name }) + .filter_map(|change| match handle_initiator_change(change) { + Ok(event) => Some(event), + Err(error) => { + log::warn!("Could not read the initiator change: {}", error); + None + } }); Ok(stream) } +fn handle_initiator_change(change: PropertiesChanged) -> Result { + let args = change.args()?; + let changes = to_owned_hash(args.changed_properties()); + let name = get_optional_property(&changes, "InitiatorName")?; + let ibft = get_optional_property(&changes, "IBFT")?; + Ok(Event::ISCSIInitiatorChanged { ibft, name }) +} + #[derive(Clone)] struct ISCSIState<'a> { client: ISCSIClient<'a>, From 3d0273ca36fa620cce9c66d28e855192bb89ad2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 23:04:30 +0100 Subject: [PATCH 19/24] rust: add OpenAPI for the iSCSI API --- rust/agama-lib/src/storage.rs | 2 +- rust/agama-lib/src/storage/client/iscsi.rs | 21 ++- rust/agama-server/src/storage/web.rs | 2 +- rust/agama-server/src/storage/web/iscsi.rs | 174 ++++++++++++++++----- rust/agama-server/src/web/docs.rs | 14 ++ 5 files changed, 166 insertions(+), 47 deletions(-) diff --git a/rust/agama-lib/src/storage.rs b/rust/agama-lib/src/storage.rs index f6f7fae89f..66940a3599 100644 --- a/rust/agama-lib/src/storage.rs +++ b/rust/agama-lib/src/storage.rs @@ -7,7 +7,7 @@ mod settings; mod store; pub use client::{ - iscsi::{ISCSIClient, ISCSINode}, + iscsi::{ISCSIAuth, ISCSIClient, ISCSIInitiator, ISCSINode}, StorageClient, }; pub use settings::StorageSettings; diff --git a/rust/agama-lib/src/storage/client/iscsi.rs b/rust/agama-lib/src/storage/client/iscsi.rs index 6469a56426..2a02588c2a 100644 --- a/rust/agama-lib/src/storage/client/iscsi.rs +++ b/rust/agama-lib/src/storage/client/iscsi.rs @@ -14,13 +14,13 @@ use zbus::{ Connection, }; -#[derive(Serialize)] -pub struct Initiator { +#[derive(Serialize, utoipa::ToSchema)] +pub struct ISCSIInitiator { name: String, ibft: bool, } -#[derive(Clone, Debug, Default, Serialize)] +#[derive(Clone, Debug, Default, Serialize, utoipa::ToSchema)] /// ISCSI node pub struct ISCSINode { /// Artificial ID to match it against the D-Bus backend. @@ -58,11 +58,15 @@ impl TryFrom<&HashMap> for ISCSINode { } } -#[derive(Clone, Default, Serialize, Deserialize)] +#[derive(Clone, Default, Serialize, Deserialize, utoipa::ToSchema)] pub struct ISCSIAuth { + /// Username for authentication by target. pub username: Option, + /// Password for authentication by target. pub password: Option, + /// Username for authentication by initiator. pub reverse_username: Option, + /// Password for authentication by initiator. pub reverse_password: Option, } @@ -165,10 +169,10 @@ impl<'a> ISCSIClient<'a> { } /// Returns the initiator data. - pub async fn get_initiator(&self) -> Result { + pub async fn get_initiator(&self) -> Result { let ibft = self.initiator_proxy.ibft().await?; let name = self.initiator_proxy.initiator_name().await?; - Ok(Initiator { name, ibft }) + Ok(ISCSIInitiator { name, ibft }) } /// Sets the initiator name. @@ -253,10 +257,13 @@ impl<'a> ISCSIClient<'a> { } } -#[derive(Serialize)] +#[derive(Serialize, utoipa::ToSchema)] pub enum LoginResult { + /// Successful login. Success = 0, + /// Invalid startup value. InvalidStartup = 1, + /// Failed login. Failed = 2, } diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index 123190d08f..f129adb747 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -24,7 +24,7 @@ use axum::{ use serde::Serialize; use tokio_stream::{Stream, StreamExt}; -mod iscsi; +pub mod iscsi; use crate::{ error::Error, diff --git a/rust/agama-server/src/storage/web/iscsi.rs b/rust/agama-server/src/storage/web/iscsi.rs index 83506dc84b..e73f78683b 100644 --- a/rust/agama-server/src/storage/web/iscsi.rs +++ b/rust/agama-server/src/storage/web/iscsi.rs @@ -13,8 +13,7 @@ use agama_lib::{ dbus::{get_optional_property, to_owned_hash}, error::ServiceError, storage::{ - client::iscsi::{ISCSIAuth, ISCSINode, Initiator, LoginResult}, - proxies::InitiatorProxy, + client::iscsi::{ISCSIAuth, ISCSIInitiator, ISCSINode, LoginResult}, ISCSIClient, }, }; @@ -25,7 +24,7 @@ use axum::{ routing::{delete, get, post}, Json, Router, }; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; mod stream; use stream::ISCSINodeStream; @@ -101,16 +100,39 @@ pub async fn iscsi_service(dbus: &zbus::Connection) -> Result, Serv Ok(router) } -async fn initiator(State(state): State>) -> Result, Error> { +/// Returns the iSCSI initiator properties. +/// +/// The iSCSI properties include the name and whether iBFT is enabled. +#[utoipa::path( + get, + path="/initiator", + context_path="/api/storage/iscsi", + responses( + (status = OK, description = "iSCSI initiator properties.", body = ISCSIInitiator), + (status = BAD_REQUEST, description = "It could not read the iSCSI initiator properties."), + ) +)] +async fn initiator(State(state): State>) -> Result, Error> { let initiator = state.client.get_initiator().await?; Ok(Json(initiator)) } -#[derive(Deserialize)] -struct InitiatorParams { +#[derive(Deserialize, utoipa::ToSchema)] +pub struct InitiatorParams { + /// iSCSI initiator name. name: String, } +/// Updates the iSCSI initiator properties. +#[utoipa::path( + patch, + path="/initiator", + context_path="/api/storage/iscsi", + responses( + (status = NO_CONTENT, description = "The iSCSI initiator properties were succesfully updated."), + (status = BAD_REQUEST, description = "It could not update the iSCSI initiator properties."), + ) +)] async fn update_initiator( State(state): State>, Json(params): Json, @@ -119,39 +141,42 @@ async fn update_initiator( Ok(StatusCode::NO_CONTENT) } +/// Returns the list of known iSCSI nodes. +#[utoipa::path( + get, + path="/nodes", + context_path="/api/storage/iscsi", + responses( + (status = OK, description = "List of iSCSI nodes.", body = Vec), + (status = BAD_REQUEST, description = "It was not possible to get the list of iSCSI nodes."), + ) +)] async fn nodes(State(state): State>) -> Result>, Error> { let nodes = state.client.get_nodes().await?; Ok(Json(nodes)) } -#[derive(Deserialize)] -struct DiscoverParams { - address: String, - port: u32, - #[serde(default)] - options: ISCSIAuth, -} - -async fn discover( - State(state): State>, - Json(params): Json, -) -> Result { - let result = state - .client - .discover(¶ms.address, params.port, params.options) - .await?; - if result { - Ok(StatusCode::NO_CONTENT) - } else { - Ok(StatusCode::BAD_REQUEST) - } -} - -#[derive(Deserialize)] -struct NodeParams { +#[derive(Deserialize, utoipa::ToSchema)] +pub struct NodeParams { + /// Startup value. startup: String, } +/// Updates iSCSI node properties. +/// +/// At this point, only the startup option can be changed. +#[utoipa::path( + put, + path="/nodes/{id}", + context_path="/api/storage/iscsi", + params( + ("id" = u32, Path, description = "iSCSI artificial ID.") + ), + responses( + (status = NO_CONTENT, description = "The iSCSI node was updated.", body = NodeParams), + (status = BAD_REQUEST, description = "Could not update the iSCSI node."), + ) +)] async fn update_node( State(state): State>, Path(id): Path, @@ -161,6 +186,19 @@ async fn update_node( Ok(StatusCode::NO_CONTENT) } +/// Deletes the iSCSI node. +#[utoipa::path( + delete, + path="/nodes/{id}", + context_path="/api/storage/iscsi", + params( + ("id" = u32, Path, description = "iSCSI artificial ID.") + ), + responses( + (status = NO_CONTENT, description = "The iSCSI node was deleted."), + (status = BAD_REQUEST, description = "Could not delete the iSCSI node."), + ) +)] async fn delete_node( State(state): State>, Path(id): Path, @@ -169,18 +207,29 @@ async fn delete_node( Ok(StatusCode::NO_CONTENT) } -#[derive(Deserialize)] -struct LoginParams { +#[derive(Deserialize, utoipa::ToSchema)] +pub struct LoginParams { + /// Authentication options. #[serde(flatten)] auth: ISCSIAuth, + /// Startup value. startup: String, } -#[derive(Serialize)] -struct LoginError { - code: LoginResult, -} - +#[utoipa::path( + post, + path="/nodes/{id}/login", + context_path="/api/storage/iscsi", + params( + ("id" = u32, Path, description = "iSCSI artificial ID.") + ), + responses( + (status = NO_CONTENT, description = "The login request was successful."), + (status = BAD_REQUEST, description = "Could not reach the iSCSI server."), + (status = UNPROCESSABLE_ENTITY, description = "The login request failed.", + body = LoginResult), + ) +)] async fn login_node( State(state): State>, Path(id): Path, @@ -196,6 +245,19 @@ async fn login_node( } } +#[utoipa::path( + post, + path="/nodes/{id}/logout", + context_path="/api/storage/iscsi", + params( + ("id" = u32, Path, description = "iSCSI artificial ID.") + ), + responses( + (status = 204, description = "The logout request was successful."), + (status = 400, description = "Could not reach the iSCSI server."), + (status = 422, description = "The logout request failed."), + ) +)] async fn logout_node( State(state): State>, Path(id): Path, @@ -206,3 +268,39 @@ async fn logout_node( Ok(StatusCode::UNPROCESSABLE_ENTITY) } } + +#[derive(Deserialize, utoipa::ToSchema)] +pub struct DiscoverParams { + /// iSCSI server address. + address: String, + /// iSCSI service port. + port: u32, + /// Authentication options. + #[serde(default)] + options: ISCSIAuth, +} + +/// Performs an iSCSI discovery. +#[utoipa::path( + post, + path="/discover", + context_path="/api/storage/iscsi", + responses( + (status = 204, description = "The iSCSI discovery request was successful."), + (status = 400, description = "The iSCSI discovery request failed."), + ) +)] +async fn discover( + State(state): State>, + Json(params): Json, +) -> Result { + let result = state + .client + .discover(¶ms.address, params.port, params.options) + .await?; + if result { + Ok(StatusCode::NO_CONTENT) + } else { + Ok(StatusCode::BAD_REQUEST) + } +} diff --git a/rust/agama-server/src/web/docs.rs b/rust/agama-server/src/web/docs.rs index 8d625b0c7e..e6d7d98f66 100644 --- a/rust/agama-server/src/web/docs.rs +++ b/rust/agama-server/src/web/docs.rs @@ -25,6 +25,12 @@ use utoipa::OpenApi; crate::users::web::remove_first_user, crate::users::web::patch_root, super::http::ping, + crate::storage::web::iscsi::initiator, + crate::storage::web::iscsi::update_initiator, + crate::storage::web::iscsi::nodes, + crate::storage::web::iscsi::update_node, + crate::storage::web::iscsi::delete_node, + crate::storage::web::iscsi::discover ), components( schemas(agama_lib::product::Product), @@ -51,6 +57,14 @@ use utoipa::OpenApi; schemas(crate::users::web::RootConfig), schemas(crate::users::web::RootPatchSettings), schemas(super::http::PingResponse), + schemas(crate::storage::web::iscsi::InitiatorParams), + schemas(crate::storage::web::iscsi::DiscoverParams), + schemas(crate::storage::web::iscsi::NodeParams), + schemas(crate::storage::web::iscsi::LoginParams), + schemas(agama_lib::storage::client::iscsi::ISCSIInitiator), + schemas(agama_lib::storage::client::iscsi::ISCSINode), + schemas(agama_lib::storage::client::iscsi::ISCSIAuth), + schemas(agama_lib::storage::client::iscsi::LoginResult), ) )] pub struct ApiDoc; From 209eeb0b791fcef138370341750d2d2b02fc4131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 8 May 2024 23:21:51 +0100 Subject: [PATCH 20/24] doc: update the documentation of the D-Bus API --- doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml | 1 + doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml | 2 ++ .../org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml | 1 + doc/dbus/org.opensuse.Agama.Storage1.doc.xml | 1 + 4 files changed, 5 insertions(+) diff --git a/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml index a0e96ad060..4b4e839ef5 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml @@ -1,5 +1,6 @@ + diff --git a/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml b/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml index 43d07f0035..16909f5447 100644 --- a/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml +++ b/doc/dbus/org.opensuse.Agama.Storage1.ISCSI.Initiator.doc.xml @@ -2,6 +2,8 @@ "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd"> + + diff --git a/doc/dbus/org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml b/doc/dbus/org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml index ec60f49dcd..5d2d449095 100644 --- a/doc/dbus/org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml +++ b/doc/dbus/org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml @@ -1,6 +1,7 @@ +