feat: Can now save configuration, refactored repository into manager as it also executes commands to reload services and calling it a repository was found misleading by @stremblay"

harmony-rs/Cargo.lock

@@ -67,6 +67,21 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "android-tzdata"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0"
+
+[[package]]
+name = "android_system_properties"
+version = "0.1.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311"
+dependencies = [
+ "libc",
+]
+
 [[package]]
 name = "anstream"
 version = "0.6.15"
@@ -243,9 +258,9 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b"
 
 [[package]]
 name = "bytes"
-version = "1.7.1"
+version = "1.8.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8318a53db07bb3f8dca91a600466bdb3f2eaadeedfdbcf02e1accbad9271ba50"
+checksum = "9ac0150caa2ae65ca5bd83f25c7de183dea78d4d366469f148435e2acfbad0da"
 
 [[package]]
 name = "cbc"
@@ -282,6 +297,20 @@ dependencies = [
  "cpufeatures",
 ]
 
+[[package]]
+name = "chrono"
+version = "0.4.38"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a21f936df1771bf62b77f047b726c4625ff2e8aa607c01ec06e5a05bd8463401"
+dependencies = [
+ "android-tzdata",
+ "iana-time-zone",
+ "js-sys",
+ "num-traits",
+ "wasm-bindgen",
+ "windows-targets 0.52.6",
+]
+
 [[package]]
 name = "cidr"
 version = "0.2.3"
@@ -950,6 +979,29 @@ dependencies = [
  "tokio-native-tls",
 ]
 
+[[package]]
+name = "iana-time-zone"
+version = "0.1.61"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "235e081f3925a06703c2d0117ea8b91f042756fd6e7a6e5d901e8ca1a996b220"
+dependencies = [
+ "android_system_properties",
+ "core-foundation-sys",
+ "iana-time-zone-haiku",
+ "js-sys",
+ "wasm-bindgen",
+ "windows-core",
+]
+
+[[package]]
+name = "iana-time-zone-haiku"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f"
+dependencies = [
+ "cc",
+]
+
 [[package]]
 name = "idna"
 version = "0.5.0"
@@ -1246,6 +1298,7 @@ name = "opnsense-config"
 version = "0.1.0"
 dependencies = [
  "async-trait",
+ "chrono",
  "env_logger",
  "log",
  "opnsense-config-xml",
@@ -2440,6 +2493,15 @@ version = "0.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"
 
+[[package]]
+name = "windows-core"
+version = "0.52.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9"
+dependencies = [
+ "windows-targets 0.52.6",
+]
+
 [[package]]
 name = "windows-sys"
 version = "0.48.0"

harmony-rs/opnsense-config/Cargo.toml

@@ -15,6 +15,7 @@ thiserror = "1.0"
 async-trait = { workspace = true }
 tokio = { workspace = true }
 opnsense-config-xml = { path = "../opnsense-config-xml" }
+chrono = "0.4.38"
 
 [dev-dependencies]
 pretty_assertions = "1.4.1"

harmony-rs/opnsense-config/src/config/config.rs

@@ -2,17 +2,17 @@ use crate::{error::Error, modules::dhcp::DhcpConfig};
 use log::trace;
 use opnsense_config_xml::OPNsense;
 
-use super::ConfigRepository;
+use super::ConfigManager;
 
 #[derive(Debug)]
 pub struct Config {
     opnsense: OPNsense,
-    repository: Box<dyn ConfigRepository + Send + Sync>,
+    repository: Box<dyn ConfigManager + Send + Sync>,
 }
 
 impl Config {
-    pub async fn new(repository: Box<dyn ConfigRepository + Send + Sync>) -> Result<Self, Error> {
-        let xml = repository.load().await?;
+    pub async fn new(repository: Box<dyn ConfigManager + Send + Sync>) -> Result<Self, Error> {
+        let xml = repository.load_as_str().await?;
         trace!("xml {}", xml);
 
         let opnsense = OPNsense::from(xml);
@@ -27,14 +27,14 @@ impl Config {
         DhcpConfig::new(&mut self.opnsense)
     }
 
-    pub async fn save(&self) -> Result<(), Error> {
-        self.repository.save(&self.opnsense.to_xml()).await
+    pub async fn apply(&self) -> Result<(), Error> {
+        self.repository.apply_new_config(&self.opnsense.to_xml()).await
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::config::LocalFileConfigRepository;
+    use crate::config::LocalFileConfigManager;
     use crate::modules::dhcp::DhcpConfig;
     use std::fs;
     use std::net::Ipv4Addr;
@@ -55,8 +55,8 @@ mod tests {
 
             let config_file_path = test_file_path.to_str().unwrap().to_string();
             println!("File path {config_file_path}");
-            let repository = Box::new(LocalFileConfigRepository::new(config_file_path));
-            let config_file_str = repository.load().await.unwrap();
+            let repository = Box::new(LocalFileConfigManager::new(config_file_path));
+            let config_file_str = repository.load_as_str().await.unwrap();
             let config = Config::new(repository)
                 .await
                 .expect("Failed to load config");
@@ -82,7 +82,7 @@ mod tests {
 
         let config_file_path = test_file_path.to_str().unwrap().to_string();
         println!("File path {config_file_path}");
-        let repository = Box::new(LocalFileConfigRepository::new(config_file_path));
+        let repository = Box::new(LocalFileConfigManager::new(config_file_path));
         let mut config = Config::new(repository)
             .await
             .expect("Failed to load config");
@@ -107,8 +107,8 @@ mod tests {
 
         let config_file_path = test_file_path.to_str().unwrap().to_string();
         println!("File path {config_file_path}");
-        let repository = Box::new(LocalFileConfigRepository::new(config_file_path));
-        let expected_config_file_str = repository.load().await.unwrap();
+        let repository = Box::new(LocalFileConfigManager::new(config_file_path));
+        let expected_config_file_str = repository.load_as_str().await.unwrap();
         assert_eq!(expected_config_file_str, serialized);
     }
 }

harmony-rs/opnsense-config/src/config/manager/local_file.rs

@@ -0,0 +1,26 @@
+use crate::config::manager::ConfigManager;
+use crate::error::Error;
+use async_trait::async_trait;
+use std::fs;
+
+#[derive(Debug)]
+pub struct LocalFileConfigManager {
+    file_path: String,
+}
+
+impl LocalFileConfigManager {
+    pub fn new(file_path: String) -> Self {
+        Self { file_path }
+    }
+}
+
+#[async_trait]
+impl ConfigManager for LocalFileConfigManager {
+    async fn load_as_str(&self) -> Result<String, Error> {
+        Ok(fs::read_to_string(&self.file_path)?)
+    }
+
+    async fn apply_new_config(&self, content: &str) -> Result<(), Error> {
+        Ok(fs::write(&self.file_path, content)?)
+    }
+}

harmony-rs/opnsense-config/src/config/manager/mod.rs

@@ -0,0 +1,13 @@
+mod ssh;
+mod local_file;
+use async_trait::async_trait;
+pub use ssh::*;
+pub use local_file::*;
+
+use crate::Error;
+
+#[async_trait]
+pub trait ConfigManager: std::fmt::Debug {
+    async fn load_as_str(&self) -> Result<String, Error>;
+    async fn apply_new_config(&self, content: &str) -> Result<(), Error>;
+}

harmony-rs/opnsense-config/src/config/manager/ssh.rs

@@ -0,0 +1,206 @@
+use crate::config::manager::ConfigManager;
+use crate::error::Error;
+use async_trait::async_trait;
+use log::{debug, info};
+use russh::{
+    client::{Config as SshConfig, Handler, Msg},
+    Channel,
+};
+use russh_keys::key::{self, KeyPair};
+use russh_sftp::client::SftpSession;
+use std::{
+    net::Ipv4Addr,
+    sync::Arc,
+    time::{SystemTime, UNIX_EPOCH},
+};
+use tokio::io::AsyncWriteExt;
+
+struct Client {}
+
+#[async_trait]
+impl Handler for Client {
+    type Error = Error;
+
+    async fn check_server_key(
+        &mut self,
+        _server_public_key: &key::PublicKey,
+    ) -> Result<bool, Self::Error> {
+        Ok(true)
+    }
+}
+
+#[derive(Debug)]
+pub enum SshCredentials {
+    SshKey { username: String, key: Arc<KeyPair> },
+    Password { username: String, password: String },
+}
+
+#[derive(Debug)]
+pub struct SshConfigManager {
+    ssh_config: Arc<SshConfig>,
+    credentials: SshCredentials,
+    host: (Ipv4Addr, u16),
+}
+
+impl SshConfigManager {
+    pub fn new(
+        host: (Ipv4Addr, u16),
+        credentials: SshCredentials,
+        ssh_config: Arc<SshConfig>,
+    ) -> Self {
+        Self {
+            ssh_config,
+            credentials,
+            host,
+        }
+    }
+}
+
+impl SshConfigManager {
+    async fn get_ssh_channel(&self) -> Result<Channel<Msg>, Error> {
+        let mut ssh = russh::client::connect(self.ssh_config.clone(), self.host, Client {}).await?;
+
+        match &self.credentials {
+            SshCredentials::SshKey { username, key } => {
+                ssh.authenticate_publickey(username, key.clone()).await?;
+            }
+            SshCredentials::Password { username, password } => {
+                ssh.authenticate_password(username, password).await?;
+            }
+        }
+
+        Ok(ssh.channel_open_session().await?)
+    }
+
+    async fn write_content_to_temp_file(&self, content: &str) -> Result<String, Error> {
+        let temp_filename = format!(
+            "/tmp/opnsense-config-tmp-config_{}",
+            SystemTime::now()
+                .duration_since(UNIX_EPOCH)
+                .unwrap()
+                .as_millis()
+        );
+        let channel = self.get_ssh_channel().await?;
+        channel
+            .request_subsystem(true, "sftp")
+            .await
+            .expect("Should request sftp subsystem");
+        let sftp = SftpSession::new(channel.into_stream())
+            .await
+            .expect("Should acquire sftp subsystem");
+
+        let mut file = sftp.create(&temp_filename).await.unwrap();
+        file.write_all(content.as_bytes()).await?;
+
+        Ok(temp_filename)
+    }
+    async fn backup_config_remote(&self) -> Result<String, Error> {
+        let backup_filename = format!("config_{}.xml", chrono::Local::now().format("%Y%m%d%H%M%S"));
+
+        self.run_command(&format!("cp /conf/config.xml /tmp/{}", backup_filename))
+            .await
+    }
+
+    async fn move_to_live_config(&self, new_config_path: &str) -> Result<String, Error> {
+        info!("Overwriting OPNSense /conf/config.xml with {new_config_path}");
+        self.run_command(&format!("mv {new_config_path} /conf/config.xml"))
+            .await
+    }
+
+    async fn reload_all_services(&self) -> Result<String, Error> {
+        info!("Reloading all opnsense services");
+        self.run_command(&format!("configctl service reload all"))
+            .await
+    }
+
+    async fn run_command(&self, command: &str) -> Result<String, Error> {
+        debug!("Running ssh command {command}");
+        let mut channel = self.get_ssh_channel().await?;
+        channel.exec(true, command).await?;
+        wait_for_completion(&mut channel).await
+    }
+}
+
+#[async_trait]
+impl ConfigManager for SshConfigManager {
+    async fn load_as_str(&self) -> Result<String, Error> {
+        let mut channel = self.get_ssh_channel().await?;
+
+        channel.exec(true, "cat /conf/config.xml").await?;
+        let mut output: Vec<u8> = vec![];
+        loop {
+            let Some(msg) = channel.wait().await else {
+                break;
+            };
+
+            info!("got msg {:?}", msg);
+            match msg {
+                russh::ChannelMsg::Data { ref data } => {
+                    output.append(&mut data.to_vec());
+                }
+                russh::ChannelMsg::ExitStatus { .. } => {}
+                russh::ChannelMsg::WindowAdjusted { .. } => {}
+                russh::ChannelMsg::Success { .. } => {}
+                russh::ChannelMsg::Eof { .. } => {}
+                _ => todo!(),
+            }
+        }
+        Ok(String::from_utf8(output).expect("Valid utf-8 bytes"))
+    }
+
+    async fn apply_new_config(&self, content: &str) -> Result<(), Error> {
+        let temp_filename = self.write_content_to_temp_file(content).await?;
+        self.backup_config_remote().await?;
+        self.move_to_live_config(&temp_filename).await?;
+        self.reload_all_services().await?;
+
+        // TODO
+        // 1. use russh to copy the current opnsense /conf/config.xml to the backup folder with a
+        // proper name
+        //
+        // 2. use russh scp functionality to copy the content directly into a file
+        // if necessary, save it first to a local file with minimal permissions in
+        // /tmp/{randomname}
+        //
+        // 3. Use opnsense cli to validate the config file
+        //
+        // 4. Reload all opnsense services using opnsense cli (still through russh commands)
+        //
+        todo!("apply_new_config not fully implemented yet")
+    }
+}
+
+async fn wait_for_completion(channel: &mut Channel<Msg>) -> Result<String, Error> {
+    let mut output = Vec::new();
+
+    loop {
+        let Some(msg) = channel.wait().await else {
+            break;
+        };
+
+        match msg {
+            russh::ChannelMsg::ExtendedData { ref data, .. }
+            | russh::ChannelMsg::Data { ref data } => {
+                output.append(&mut data.to_vec());
+            }
+            russh::ChannelMsg::ExitStatus { exit_status } => {
+                if exit_status != 0 {
+                    return Err(Error::Command(format!(
+                        "Command failed with exit status {exit_status}, output {}",
+                        String::from_utf8(output).unwrap_or_default()
+                    )));
+                }
+            }
+            russh::ChannelMsg::Success { .. }
+            | russh::ChannelMsg::WindowAdjusted { .. }
+            | russh::ChannelMsg::Eof { .. } => {}
+            _ => {
+                return Err(Error::Unexpected(format!(
+                    "Russh got unexpected msg {msg:?}"
+                )))
+            }
+        }
+    }
+
+    Ok(String::from_utf8(output).unwrap_or_default())
+}

harmony-rs/opnsense-config/src/config/mod.rs

@@ -1,4 +1,4 @@
 mod config;
-mod repository;
-pub use repository::*;
+mod manager;
+pub use manager::*;
 pub use config::*;

harmony-rs/opnsense-config/src/config/repository.rs

@@ -1,151 +0,0 @@
-use crate::error::Error;
-use async_trait::async_trait;
-use log::info;
-use russh::{
-    client::{Config as SshConfig, Handler, Msg},
-    Channel,
-};
-use russh_keys::key::{self, KeyPair};
-use std::{fs, net::Ipv4Addr, sync::Arc};
-
-#[async_trait]
-pub trait ConfigRepository: std::fmt::Debug {
-    async fn load(&self) -> Result<String, Error>;
-    async fn save(&self, content: &str) -> Result<(), Error>;
-}
-
-struct Client {}
-
-#[async_trait]
-impl Handler for Client {
-    type Error = Error;
-
-    async fn check_server_key(
-        &mut self,
-        _server_public_key: &key::PublicKey,
-    ) -> Result<bool, Self::Error> {
-        Ok(true)
-    }
-}
-
-#[derive(Debug)]
-pub enum SshCredentials {
-    SshKey { username: String, key: Arc<KeyPair> },
-    Password { username: String, password: String },
-}
-
-#[derive(Debug)]
-pub struct SshConfigRepository {
-    ssh_config: Arc<SshConfig>,
-    credentials: SshCredentials,
-    host: (Ipv4Addr, u16),
-}
-
-impl SshConfigRepository {
-    pub fn new(
-        host: (Ipv4Addr, u16),
-        credentials: SshCredentials,
-        ssh_config: Arc<SshConfig>,
-    ) -> Self {
-        Self {
-            ssh_config,
-            credentials,
-            host,
-        }
-    }
-}
-
-impl SshConfigRepository {
-    async fn get_ssh_channel(&self) -> Result<Channel<Msg>, Error> {
-        let mut ssh = russh::client::connect(self.ssh_config.clone(), self.host, Client {}).await?;
-
-        match &self.credentials {
-            SshCredentials::SshKey { username, key } => {
-                ssh.authenticate_publickey(username, key.clone()).await?;
-            }
-            SshCredentials::Password { username, password } => {
-                ssh.authenticate_password(username, password).await?;
-            }
-        }
-
-        Ok(ssh.channel_open_session().await?)
-    }
-}
-
-#[async_trait]
-impl ConfigRepository for SshConfigRepository {
-    async fn load(&self) -> Result<String, Error> {
-        let mut channel = self.get_ssh_channel().await?;
-
-        channel.exec(true, "cat /conf/config.xml").await?;
-        let mut output: Vec<u8> = vec![];
-        loop {
-            let Some(msg) = channel.wait().await else {
-                break;
-            };
-
-            info!("got msg {:?}", msg);
-            match msg {
-                russh::ChannelMsg::Data { ref data } => {
-                    output.append(&mut data.to_vec());
-                }
-                russh::ChannelMsg::ExitStatus { .. } => {}
-                russh::ChannelMsg::WindowAdjusted { .. } => {}
-                russh::ChannelMsg::Success { .. } => {}
-                russh::ChannelMsg::Eof { .. } => {}
-                _ => todo!(),
-            }
-        }
-        Ok(String::from_utf8(output).expect("Valid utf-8 bytes"))
-    }
-
-    async fn save(&self, content: &str) -> Result<(), Error> {
-        todo!("Backup, Validate, Reload config file");
-        let mut channel = self.get_ssh_channel().await?;
-
-        let command = format!(
-            "echo '{}' > /conf/config.xml",
-            content.replace("'", "'\"'\"'")
-        );
-        channel.exec(true, command.as_bytes()).await?;
-
-        loop {
-            let Some(msg) = channel.wait().await else {
-                break;
-            };
-
-            match msg {
-                russh::ChannelMsg::ExitStatus { exit_status } => {
-                    if exit_status != 0 {
-                        return Err(Error::Ssh(russh::Error::Disconnect));
-                    }
-                }
-                _ => {}
-            }
-        }
-
-        Ok(())
-    }
-}
-
-#[derive(Debug)]
-pub struct LocalFileConfigRepository {
-    file_path: String,
-}
-
-impl LocalFileConfigRepository {
-    pub fn new(file_path: String) -> Self {
-        Self { file_path }
-    }
-}
-
-#[async_trait]
-impl ConfigRepository for LocalFileConfigRepository {
-    async fn load(&self) -> Result<String, Error> {
-        Ok(fs::read_to_string(&self.file_path)?)
-    }
-
-    async fn save(&self, content: &str) -> Result<(), Error> {
-        Ok(fs::write(&self.file_path, content)?)
-    }
-}

harmony-rs/opnsense-config/src/error.rs

@@ -6,8 +6,12 @@ pub enum Error {
     Xml(String),
     #[error("SSH error: {0}")]
     Ssh(#[from] russh::Error),
+    #[error("Command failed : {0}")]
+    Command(String),
     #[error("I/O error: {0}")]
     Io(#[from] std::io::Error),
     #[error("Config error: {0}")]
     Config(String),
+    #[error("Unexpected error: {0}")]
+    Unexpected(String),
 }

harmony-rs/opnsense-config/src/lib.rs

@@ -6,7 +6,7 @@ pub use config::Config;
 pub use error::Error;
 #[cfg(test)]
 mod test {
-    use config::SshConfigRepository;
+    use config::SshConfigManager;
     use russh::client;
     use std::{net::Ipv4Addr, sync::Arc, time::Duration};
 
@@ -28,18 +28,18 @@ mod test {
         };
 
         let repo =
-            SshConfigRepository::new((Ipv4Addr::new(192, 168, 5, 229), 22), credentials, config);
+            SshConfigManager::new((Ipv4Addr::new(192, 168, 5, 229), 22), credentials, config);
+
         let mut config = Config::new(Box::new(repo)).await.unwrap();
         config
             .dhcp()
             .add_static_mapping(
-                "test_mac",
-                Ipv4Addr::new(192, 168, 168, 168),
+                "11:22:33:44:55:66",
+                Ipv4Addr::new(10, 100, 8, 200),
                 "test_hostname",
             )
             .unwrap();
 
-        todo!();
-        // opnsense.apply_changes().await;
+        config.apply().await.unwrap();
     }
 }