diff --git a/Cargo.lock b/Cargo.lock index 376c367825e..7a0fcfc556b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3492,6 +3492,7 @@ dependencies = [ "clap", "doku", "hyper", + "mockall", "mockito", "mqtt_tests", "nix", diff --git a/crates/common/tedge_utils/src/file.rs b/crates/common/tedge_utils/src/file.rs index 6636a11cd95..8d7a4f6b8bb 100644 --- a/crates/common/tedge_utils/src/file.rs +++ b/crates/common/tedge_utils/src/file.rs @@ -22,6 +22,9 @@ pub enum FileError { #[error("Failed to change owner: {name:?}.")] MetaDataError { name: String, from: std::io::Error }, + #[error("Failed to change permissions of file: {name:?}.")] + ChangeModeError { name: String, from: std::io::Error }, + #[error("User not found: {user:?}.")] UserNotFound { user: String }, @@ -65,13 +68,13 @@ pub enum FileError { pub fn create_directory>( dir: P, - permissions: PermissionEntry, + permissions: &PermissionEntry, ) -> Result<(), FileError> { permissions.create_directory(dir.as_ref()) } /// Create the directory owned by the user running this API with default directory permissions pub fn create_directory_with_defaults>(dir: P) -> Result<(), FileError> { - create_directory(dir, PermissionEntry::default()) + create_directory(dir, &PermissionEntry::default()) } pub fn create_directory_with_user_group( @@ -380,7 +383,7 @@ pub fn change_user_and_group(file: &Path, user: &str, group: &str) -> Result<(), let gid = get_metadata(Path::new(file))?.gid(); // if user and group are same as existing, then do not change - if (ud != uid) && (gd != gid) { + if (ud != uid) || (gd != gid) { chown(file, Some(Uid::from_raw(ud)), Some(Gid::from_raw(gd))).map_err(|e| { FileError::MetaDataError { name: file.display().to_string(), @@ -431,15 +434,22 @@ fn change_group(file: &Path, group: &str) -> Result<(), FileError> { } fn change_mode(file: &Path, mode: u32) -> Result<(), FileError> { - let mut perm = get_metadata(Path::new(file))?.permissions(); - perm.set_mode(mode); - - fs::set_permissions(file, perm).map_err(|e| FileError::MetaDataError { - name: file.display().to_string(), - from: e, - })?; + let mut permissions = get_metadata(Path::new(file))?.permissions(); - Ok(()) + if permissions.mode() & 0o777 != mode { + permissions.set_mode(mode); + debug!("Setting mode of {} to {mode:0o}", file.display()); + fs::set_permissions(file, permissions).map_err(|e| FileError::ChangeModeError { + name: file.display().to_string(), + from: e, + }) + } else { + debug!( + "Not changing mode of {} as it is already {mode:0o}", + file.display() + ); + Ok(()) + } } /// Return metadata when the given path exists and accessible by user diff --git a/crates/core/tedge/Cargo.toml b/crates/core/tedge/Cargo.toml index dc6ea842c2e..96a7c8df361 100644 --- a/crates/core/tedge/Cargo.toml +++ b/crates/core/tedge/Cargo.toml @@ -58,6 +58,7 @@ yansi = { workspace = true } [dev-dependencies] assert_cmd = { workspace = true } assert_matches = { workspace = true } +mockall = { workspace = true } mockito = { workspace = true } mqtt_tests = { workspace = true } pem = { workspace = true } diff --git a/crates/core/tedge/src/cli/init.rs b/crates/core/tedge/src/cli/init.rs index b0935093541..f5e4901c72f 100644 --- a/crates/core/tedge/src/cli/init.rs +++ b/crates/core/tedge/src/cli/init.rs @@ -4,11 +4,14 @@ use crate::Component; use anyhow::bail; use anyhow::Context; use clap::Subcommand; +use std::io::ErrorKind; use std::os::unix::fs::MetadataExt; use std::path::Path; +use std::path::PathBuf; use tedge_utils::file::change_user_and_group; use tedge_utils::file::create_directory; use tedge_utils::file::PermissionEntry; +use tracing::debug; #[derive(Debug)] pub struct TEdgeInitCmd { @@ -27,7 +30,9 @@ impl TEdgeInitCmd { context, } } +} +impl TEdgeInitCmd { fn initialize_tedge(&self) -> anyhow::Result<()> { let executable_name = std::env::current_exe().context("retrieving the current executable name")?; @@ -57,140 +62,40 @@ impl TEdgeInitCmd { .chain(["tedge-apt-plugin".to_owned()]) .collect(); + let target = Target { + path: match self.relative_links { + true => Path::new(executable_file_name), + false => &executable_name, + }, + uid: stat.uid(), + gid: stat.gid(), + }; + for component in &component_subcommands { - let link = executable_dir.join(component); - match std::fs::symlink_metadata(&link) { - Err(e) if e.kind() != std::io::ErrorKind::NotFound => bail!( - "couldn't read metadata for {}. do you need to run with sudo?", - link.display() - ), - meta => { - let file_exists = meta.is_ok(); - if file_exists { - nix::unistd::unlink(&link).with_context(|| { - format!("removing old version of {component} at {}", link.display()) - })?; - } - - let tedge = if self.relative_links { - Path::new(executable_file_name) - } else { - &*executable_name - }; - std::os::unix::fs::symlink(tedge, &link).with_context(|| { - format!("creating symlink for {component} to {}", tedge.display()) - })?; - - // Use -h over --no-dereference as the former is supported in more environments, - // busybox, bsd etc. - let res = std::process::Command::new("chown") - .arg("-h") - .arg(&format!("{}:{}", stat.uid(), stat.gid())) - .arg(&link) - .output() - .with_context(|| { - format!( - "executing chown to change ownership of symlink at {}", - link.display() - ) - })?; - anyhow::ensure!( - res.status.success(), - "failed to change ownership of symlink at {}\n\nSTDERR: {}", - link.display(), - String::from_utf8_lossy(&res.stderr), - ) - } - } + create_symlinks_for(component, target, executable_dir, &RealEnv)?; } let config_dir = self.context.config_location.tedge_config_root_path.clone(); - create_directory( - &config_dir, - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o775), - ), - )?; - - create_directory( - config_dir.join("mosquitto-conf"), - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o775), - ), - )?; - create_directory( - config_dir.join("operations"), - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o775), - ), - )?; - create_directory( - config_dir.join("operations").join("c8y"), - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o755), - ), - )?; - create_directory( - config_dir.join("plugins"), + let permissions = { PermissionEntry::new( Some(self.user.clone()), Some(self.group.clone()), Some(0o775), - ), - )?; - create_directory( - config_dir.join("sm-plugins"), - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o755), - ), - )?; - create_directory( - config_dir.join("device-certs"), - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o775), - ), - )?; + ) + }; + create_directory(&config_dir, &permissions)?; + create_directory(config_dir.join("mosquitto-conf"), &permissions)?; + create_directory(config_dir.join("operations"), &permissions)?; + create_directory(config_dir.join("operations").join("c8y"), &permissions)?; + create_directory(config_dir.join("plugins"), &permissions)?; + create_directory(config_dir.join("sm-plugins"), &permissions)?; + create_directory(config_dir.join("device-certs"), &permissions)?; + create_directory(config_dir.join(".tedge-mapper-c8y"), &permissions)?; let config = self.context.load_config()?; - create_directory( - config.logs.path.clone(), - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o775), - ), - )?; - - create_directory( - &config.data.path, - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o775), - ), - )?; - - create_directory( - config_dir.join(".tedge-mapper-c8y"), - PermissionEntry::new( - Some(self.user.clone()), - Some(self.group.clone()), - Some(0o775), - ), - )?; + create_directory(&config.logs.path, &permissions)?; + create_directory(&config.data.path, &permissions)?; let entity_store_file = config_dir .join(".tedge-mapper-c8y") @@ -214,3 +119,218 @@ impl Command for TEdgeInitCmd { .with_context(|| "Failed to initialize tedge. You have to run tedge with sudo.") } } + +#[cfg_attr(test, mockall::automock)] +trait FileSystem { + fn read_link(&self, link: &Path) -> std::io::Result { + match std::fs::read_link(link) { + // File exists, but it's not a symlink + Err(err) if err.kind() == ErrorKind::InvalidInput => Ok(link.to_owned()), + res => res, + } + } + + fn unlink(&self, link: &Path) -> nix::Result<()> { + nix::unistd::unlink(link) + } + + fn symlink(&self, original: &Path, link: &Path) -> std::io::Result<()> { + std::os::unix::fs::symlink(original, link) + } + + fn chown_symlink(&self, link: &Path, uid: u32, gid: u32) -> anyhow::Result<()> { + // Use -h over --no-dereference as the former is supported in more environments, + // busybox, bsd etc. + let res = std::process::Command::new("chown") + .arg("-h") + .arg(&format!("{uid}:{gid}")) + .arg(link) + .output() + .with_context(|| { + format!( + "executing chown to change ownership of symlink at {}", + link.display() + ) + })?; + anyhow::ensure!( + res.status.success(), + "failed to change ownership of symlink at {}\n\nSTDERR: {}", + link.display(), + String::from_utf8_lossy(&res.stderr), + ); + Ok(()) + } +} + +#[derive(Copy, Clone)] +struct Target<'a> { + path: &'a Path, + uid: u32, + gid: u32, +} + +fn create_symlinks_for( + component: &str, + tedge: Target<'_>, + executable_dir: &Path, + fs: &impl FileSystem, +) -> anyhow::Result<()> { + let link = executable_dir.join(component); + match fs.read_link(&link) { + Err(e) if e.kind() != ErrorKind::NotFound => bail!( + "couldn't read metadata for {}. do you need to run with sudo?", + link.display() + ), + existing_file => { + if let Ok(target) = existing_file { + // If the symlink already exists, don't modify it + if target == tedge.path { + debug!("Leaving symlink for {component} unchanged"); + return Ok(()); + } + + fs.unlink(&link).with_context(|| { + format!("removing old version of {component} at {}", link.display()) + })?; + } + + fs.symlink(tedge.path, &link).with_context(|| { + format!( + "creating symlink for {component} to {}", + tedge.path.display() + ) + })?; + + fs.chown_symlink(&link, tedge.uid, tedge.gid)?; + Ok(()) + } + } +} + +pub struct RealEnv; +impl FileSystem for RealEnv {} + +#[cfg(test)] +mod tests { + use super::*; + use mockall::predicate::*; + + mod create_symlinks_for { + use super::*; + + #[test] + fn replaces_binaries_with_symlinks() { + let mut fs = MockFileSystem::new(); + // Simulate a non-symlinked file - read link returns the input path + fs.expect_read_link().return_once(|input| Ok(input.into())); + fs.expect_unlink() + .with(eq(Path::new("/usr/bin/tedge-mapper"))) + .times(1) + .returning(|_| Ok(())); + fs.expect_symlink() + .with( + eq(Path::new("/usr/bin/tedge")), + eq(Path::new("/usr/bin/tedge-mapper")), + ) + .times(1) + .returning(|_, _| Ok(())); + fs.expect_chown_symlink() + .times(1) + .with(eq(Path::new("/usr/bin/tedge-mapper")), eq(987), eq(986)) + .returning(|_, _, _| Ok(())); + let target = Target { + path: Path::new("/usr/bin/tedge"), + uid: 987, + gid: 986, + }; + + create_symlinks_for("tedge-mapper", target, Path::new("/usr/bin"), &fs).unwrap() + } + + #[test] + fn creates_symlinks_if_they_dont_exist() { + let mut fs = MockFileSystem::new(); + // Simulate a non-symlinked file - read link returns the input path + fs.expect_read_link().return_once(|_| io_error::not_found()); + fs.expect_symlink() + .with( + eq(Path::new("/usr/bin/tedge")), + eq(Path::new("/usr/bin/tedge-mapper")), + ) + .times(1) + .returning(|_, _| Ok(())); + fs.expect_chown_symlink() + .times(1) + .with(eq(Path::new("/usr/bin/tedge-mapper")), eq(987), eq(986)) + .returning(|_, _, _| Ok(())); + let target = Target { + path: Path::new("/usr/bin/tedge"), + uid: 987, + gid: 986, + }; + + create_symlinks_for("tedge-mapper", target, Path::new("/usr/bin"), &fs).unwrap() + } + + #[test] + fn replaces_symlinks_if_they_differ_from_the_configured_target() { + let mut fs = MockFileSystem::new(); + // Simulate a non-symlinked file - read link returns the input path + fs.expect_read_link() + .return_once(|_| Ok("/usr/bin/tedge".into())); + fs.expect_unlink() + .with(eq(Path::new("/usr/bin/tedge-mapper"))) + .times(1) + .returning(|_| Ok(())); + fs.expect_symlink() + .with( + eq(Path::new("tedge")), + eq(Path::new("/usr/bin/tedge-mapper")), + ) + .times(1) + .returning(|_, _| Ok(())); + fs.expect_chown_symlink() + .times(1) + .with(eq(Path::new("/usr/bin/tedge-mapper")), eq(987), eq(986)) + .returning(|_, _, _| Ok(())); + let target = Target { + path: Path::new("tedge"), + uid: 987, + gid: 986, + }; + + create_symlinks_for("tedge-mapper", target, Path::new("/usr/bin"), &fs).unwrap() + } + + #[test] + fn leaves_up_to_date_symlinks_unchanged() { + let mut fs = MockFileSystem::new(); + // Simulate a non-symlinked file - read link returns the input path + fs.expect_read_link() + .return_once(|_| Ok("/usr/bin/tedge".into())); + let target = Target { + path: Path::new("/usr/bin/tedge"), + uid: 987, + gid: 986, + }; + + create_symlinks_for("tedge-mapper", target, Path::new("/usr/bin"), &fs).unwrap() + } + } + + #[derive(thiserror::Error, Debug)] + #[error("{0}")] + struct DummyError(&'static str); + + mod io_error { + use crate::cli::init::tests::DummyError; + use std::io::ErrorKind; + + pub fn not_found() -> std::io::Result { + Err(std::io::Error::new( + ErrorKind::NotFound, + Box::new(DummyError("File not found")), + )) + } + } +}