diff --git a/Cargo.lock b/Cargo.lock index a811bd3f0d0..526ed2bca3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3915,6 +3915,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", + "tokio-test", "tracing", "users", "whoami", diff --git a/crates/common/tedge_config/src/tedge_config_cli/tedge_config_location.rs b/crates/common/tedge_config/src/tedge_config_cli/tedge_config_location.rs index b7ea2b630e7..7cab37d0443 100644 --- a/crates/common/tedge_config/src/tedge_config_cli/tedge_config_location.rs +++ b/crates/common/tedge_config/src/tedge_config_cli/tedge_config_location.rs @@ -5,8 +5,6 @@ use camino::Utf8PathBuf; pub const DEFAULT_TEDGE_CONFIG_PATH: &str = "/etc/tedge"; const TEDGE_CONFIG_FILE: &str = "tedge.toml"; -const TEDGE_CONFIG_FILE_TMP: &str = "tedge.toml.tmp"; - /// Information about where `tedge.toml` is located. /// /// Broadly speaking, we distinguish two different locations: @@ -49,10 +47,6 @@ impl TEdgeConfigLocation { pub fn tedge_config_file_path(&self) -> &Utf8Path { &self.tedge_config_file_path } - - pub fn temporary_tedge_config_file_path(&self) -> Utf8PathBuf { - self.tedge_config_root_path.join(TEDGE_CONFIG_FILE_TMP) - } } #[test] diff --git a/crates/common/tedge_config/src/tedge_config_cli/tedge_config_repository.rs b/crates/common/tedge_config/src/tedge_config_cli/tedge_config_repository.rs index c4e59bab2fc..aa75221a4a5 100644 --- a/crates/common/tedge_config/src/tedge_config_cli/tedge_config_repository.rs +++ b/crates/common/tedge_config/src/tedge_config_cli/tedge_config_repository.rs @@ -92,7 +92,6 @@ impl TEdgeConfigRepository { } atomically_write_file_sync( - self.config_location.temporary_tedge_config_file_path(), self.config_location.tedge_config_file_path(), toml.as_bytes(), )?; diff --git a/crates/common/tedge_utils/Cargo.toml b/crates/common/tedge_utils/Cargo.toml index 07bc6372143..d0c5eacdb82 100644 --- a/crates/common/tedge_utils/Cargo.toml +++ b/crates/common/tedge_utils/Cargo.toml @@ -38,4 +38,5 @@ assert_matches = "1.5" maplit = "1.0" tedge_test_utils = { path = "../../tests/tedge_test_utils" } tokio = { version = "1.23", features = ["rt-multi-thread"] } +tokio-test = "0.4" whoami = "1.2.1" diff --git a/crates/common/tedge_utils/src/file.rs b/crates/common/tedge_utils/src/file.rs index 063a65601c0..e6c9f09895a 100644 --- a/crates/common/tedge_utils/src/file.rs +++ b/crates/common/tedge_utils/src/file.rs @@ -1,3 +1,4 @@ +use futures::TryFutureExt; use nix::unistd::*; use std::fs; use std::io; @@ -90,6 +91,13 @@ pub fn create_file_with_user_group( perm_entry.create_file(file.as_ref(), default_content) } +/// This function move file to destination path using rename. +/// If directories are located on a different mount point, copy and delete method will be used instead. +/// If the destination directory does not exist, it will create it, as well as all parent directories. +/// Function cannot move whole directories if copy and delete method is used. +/// This method returns +/// Ok() when file was moved successfully +/// Err(_) when the source path does not exists or function has no permission to move file pub async fn move_file( src_path: impl AsRef, dest_path: impl AsRef, @@ -97,6 +105,7 @@ pub async fn move_file( ) -> Result<(), FileError> { let src_path = src_path.as_ref(); let dest_path = dest_path.as_ref(); + if !dest_path.exists() { if let Some(dir_to) = dest_path.parent() { tokio::fs::create_dir_all(dir_to).await?; @@ -115,7 +124,14 @@ pub async fn move_file( false => None, }; - tokio::fs::rename(src_path, dest_path).await?; + // Copy source to destination using rename. If that one fails due to cross-filesystem, use copy and delete. + // As a ErrorKind::CrossesDevices is nightly feature we call copy and delete no matter what kind of error we get. + tokio::fs::rename(src_path, dest_path) + .or_else(|_| { + tokio::fs::copy(src_path, dest_path).and_then(|_| tokio::fs::remove_file(src_path)) + }) + .await?; + debug!("Moved file from {:?} to {:?}", src_path, dest_path); let file_permissions = if let Some(mode) = original_permission_mode { @@ -536,4 +552,22 @@ mod tests { let err = get_gid_by_name("test").unwrap_err(); assert!(err.to_string().contains("Group not found")); } + + #[tokio::test] + async fn move_file_to_different_filesystem() { + let file_dir = TempDir::new().unwrap(); + let file_path = file_dir.path().join("file"); + let user = whoami::username(); + create_file_with_user_group(&file_path, &user, &user, 0o775, Some("test")).unwrap(); + + let dest_dir = TempDir::new_in(".").unwrap(); + let dest_path = dest_dir.path().join("another-file"); + + move_file(file_path, &dest_path, PermissionEntry::default()) + .await + .unwrap(); + + let content = fs::read(&dest_path).unwrap(); + assert_eq!("test".as_bytes(), content); + } } diff --git a/crates/common/tedge_utils/src/fs.rs b/crates/common/tedge_utils/src/fs.rs index d2fa6f70536..a88d67cdaad 100644 --- a/crates/common/tedge_utils/src/fs.rs +++ b/crates/common/tedge_utils/src/fs.rs @@ -1,20 +1,20 @@ use std::fs as std_fs; use std::io::Write; use std::path::Path; +use std::path::PathBuf; use tokio::fs as tokio_fs; use tokio::io::AsyncWriteExt; /// Write file to filesystem atomically using std::fs synchronously. -pub fn atomically_write_file_sync( - tempfile: impl AsRef, - dest: impl AsRef, - content: &[u8], -) -> std::io::Result<()> { +pub fn atomically_write_file_sync(dest: impl AsRef, content: &[u8]) -> std::io::Result<()> { + let mut tempfile = PathBuf::from(dest.as_ref()); + tempfile.set_extension("tmp"); + let mut file = std_fs::OpenOptions::new() .write(true) .create_new(true) - .open(tempfile.as_ref())?; + .open(&tempfile)?; if let Err(err) = file.write_all(content) { let _ = std_fs::remove_file(tempfile); @@ -23,7 +23,7 @@ pub fn atomically_write_file_sync( file.flush()?; - if let Err(err) = std_fs::rename(tempfile.as_ref(), dest) { + if let Err(err) = std_fs::rename(&tempfile, dest) { let _ = std_fs::remove_file(tempfile); return Err(err); } @@ -33,14 +33,16 @@ pub fn atomically_write_file_sync( /// Write file to filesystem atomically using tokio::fs asynchronously. pub async fn atomically_write_file_async( - tempfile: impl AsRef, dest: impl AsRef, content: &[u8], ) -> std::io::Result<()> { + let mut tempfile = PathBuf::from(dest.as_ref()); + tempfile.set_extension("tmp"); + let mut file = tokio_fs::OpenOptions::new() .write(true) .create_new(true) - .open(tempfile.as_ref()) + .open(&tempfile) .await?; if let Err(err) = file.write_all(content).await { @@ -50,7 +52,7 @@ pub async fn atomically_write_file_async( file.flush().await?; - if let Err(err) = tokio_fs::rename(tempfile.as_ref(), dest).await { + if let Err(err) = tokio_fs::rename(&tempfile, dest).await { tokio_fs::remove_file(tempfile).await?; return Err(err); } @@ -73,7 +75,7 @@ mod tests { let content = "test_data"; - atomically_write_file_async(&temp_path, &destination_path, content.as_bytes()) + atomically_write_file_async(&destination_path, content.as_bytes()) .await .unwrap(); @@ -88,15 +90,12 @@ mod tests { #[test] fn atomically_write_file_file_sync() { let temp_dir = tempdir().unwrap(); - let temp_path = temp_dir.path().join("test1"); let destination_path = temp_dir.path().join("test2"); let content = "test_data"; - let () = - atomically_write_file_sync(&temp_path, &destination_path, content.as_bytes()).unwrap(); + let () = atomically_write_file_sync(&destination_path, content.as_bytes()).unwrap(); - std::fs::File::open(&temp_path).unwrap_err(); if let Ok(destination_content) = std::fs::read(&destination_path) { assert_eq!(destination_content, content.as_bytes()); } else { diff --git a/crates/core/tedge_agent/src/state.rs b/crates/core/tedge_agent/src/state.rs index 08f4ac8286f..f9fb3fef4c8 100644 --- a/crates/core/tedge_agent/src/state.rs +++ b/crates/core/tedge_agent/src/state.rs @@ -46,11 +46,7 @@ impl StateRepository for AgentStateRepository { fs::create_dir(&self.state_repo_root).await?; } - let mut temppath = self.state_repo_path.clone(); - temppath.set_extension("tmp"); - - let () = - atomically_write_file_async(temppath, &self.state_repo_path, toml.as_bytes()).await?; + let () = atomically_write_file_async(&self.state_repo_path, toml.as_bytes()).await?; Ok(()) } diff --git a/crates/extensions/c8y_config_manager/src/upload.rs b/crates/extensions/c8y_config_manager/src/upload.rs index 7d1420a28d8..6bc7844066b 100644 --- a/crates/extensions/c8y_config_manager/src/upload.rs +++ b/crates/extensions/c8y_config_manager/src/upload.rs @@ -35,6 +35,8 @@ use tedge_mqtt_ext::Topic; use tedge_timer_ext::SetTimeout; use tedge_utils::file::create_directory_with_user_group; use tedge_utils::file::create_file_with_user_group; +use tedge_utils::file::move_file; +use tedge_utils::file::PermissionEntry; pub struct ConfigUploadManager { config: ConfigManagerConfig, @@ -299,7 +301,7 @@ impl ConfigUploadManager { config_response.get_child_id() ); let path_to = Path::new(path_to); - std::fs::rename(path_from, path_to)?; + move_file(path_from, path_to, PermissionEntry::default()).await?; } // send 119 let child_plugin_config = PluginConfig::new(Path::new(&format!(