diff options
author | Didier Wenzek <didier.wenzek@acidalie.com> | 2021-06-03 15:12:04 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-06-03 15:12:04 +0100 |
commit | e7e5320c9c19a2c3798c98e763054ec7770ef790 (patch) | |
tree | 5e6e474dddc486184295c17a449c0774139a6f5b | |
parent | ef3e46124da193b51e834977508f4d7506989b8e (diff) |
[CIT-394] fix build on windows (#267)
* [CIT-394] Create unix/windows module for flockfile
* [CIT-394] Extract the `users` module into the `common/tedge_users` crate
* [CIT-394] cargo fmt
* [CIT-394] Use `tedge_users` rather than `users`
Co-authored-by: Wenzek <diw@softwareag.com>
25 files changed, 239 insertions, 173 deletions
@@ -378,13 +378,13 @@ dependencies = [ "mockall", "mqtt_client", "tedge_config", + "tedge_users", "thin_edge_json", "thiserror", "tokio", "tokio-test", "tracing", "tracing-subscriber", - "users", ] [[package]] @@ -2135,12 +2135,12 @@ dependencies = [ "serde", "structopt", "tedge_config", + "tedge_users", "tempfile", "thiserror", "tokio", "toml", "url", - "users", "which", ] @@ -2174,12 +2174,20 @@ dependencies = [ "mqtt_client", "serde_json", "tedge_config", + "tedge_users", "thin_edge_json", "thiserror", "tokio", "tokio-test", "tracing", "tracing-subscriber", +] + +[[package]] +name = "tedge_users" +version = "0.1.0" +dependencies = [ + "thiserror", "users", ] @@ -5,6 +5,7 @@ members = [ "common/flockfile", "common/mqtt_client", "common/clock", + "common/tedge_users", "tedge", "tedge_config", "mapper/cumulocity/c8y_translator_lib", diff --git a/common/flockfile/Cargo.toml b/common/flockfile/Cargo.toml index e70f8a82..8f1a205f 100644 --- a/common/flockfile/Cargo.toml +++ b/common/flockfile/Cargo.toml @@ -5,9 +5,11 @@ authors = ["Software AG <thin-edge-team@softwareag.com>"] edition = "2018" [dependencies] +thiserror = "1.0" + +[target.'cfg(unix)'.dependencies] log = "0.4" nix = "0.20" -thiserror = "1.0" [dev-dependencies] assert_matches = "1.5" diff --git a/common/flockfile/src/lib.rs b/common/flockfile/src/lib.rs index ca290926..e536431e 100644 --- a/common/flockfile/src/lib.rs +++ b/common/flockfile/src/lib.rs @@ -1,135 +1,11 @@ -use log::{debug, warn}; -use nix::fcntl::{flock, FlockArg}; -use std::{ - fs::{self, File, OpenOptions}, - io, - os::unix::io::AsRawFd, - path::{Path, PathBuf}, -}; +#[cfg(unix)] +mod unix; -#[derive(thiserror::Error, Debug)] -pub enum FlockfileError { - #[error(transparent)] - IoError(#[from] std::io::Error), +#[cfg(unix)] +pub use unix::*; - #[error("Couldn't acquire file lock.")] - NixError(#[from] nix::Error), -} +#[cfg(not(unix))] +mod windows; -/// flockfile creates a lockfile in the filesystem under `/run/lock` and then creates a filelock using system fcntl with flock. -/// flockfile will automatically remove lockfile on application exit and the OS should cleanup the filelock afterwards. -/// If application exits unexpectedly the filelock will be dropped, but the lockfile will not be removed unless handled in signal handler. -#[derive(Debug)] -pub struct Flockfile { - handle: Option<File>, - pub path: PathBuf, -} - -impl Flockfile { - /// Create new lockfile in `/run/lock` with specific name: - /// - /// #Example - /// - /// let _lockfile = match flockfile::Flockfile::new_lock("app")).unwrap(); - /// - pub fn new_lock(lock_name: impl AsRef<Path>) -> Result<Flockfile, FlockfileError> { - let path = Path::new("/run/lock").join(lock_name); - - let file = OpenOptions::new() - .create(true) - .read(true) - .write(true) - .open(&path)?; - - let () = flock(file.as_raw_fd(), FlockArg::LockExclusiveNonblock)?; - - Ok(Flockfile { - handle: Some(file), - path, - }) - } - - /// Manually remove filelock and lockfile from the filesystem, this method doesn't have to be called explicitly, - /// however if access to the locked file is required this must be called. - pub fn unlock(mut self) -> Result<(), io::Error> { - self.handle.take().expect("handle dropped"); - fs::remove_file(&self.path)?; - Ok(()) - } -} - -impl Drop for Flockfile { - /// The Drop trait will be called always when the lock goes out of scope, however, - /// if the program exits unexpectedly and drop is not called the lock will be removed by the system. - fn drop(&mut self) { - if let Some(handle) = self.handle.take() { - drop(handle); - - // Even if the file is not removed this is not an issue, as OS will take care of the flock. - // Additionally if the file is created before an attempt to create the lock that won't be an issue as we rely on filesystem lock. - match fs::remove_file(&self.path) { - Ok(()) => debug!(r#"Lockfile deleted "{:?}""#, self.path), - Err(err) => warn!( - r#"Error while handling lockfile at "{:?}": {:?}"#, - self.path, err - ), - } - } - } -} - -impl AsRef<Path> for Flockfile { - fn as_ref(&self) -> &Path { - self.path.as_ref() - } -} - -#[cfg(test)] -mod tests { - - use super::*; - use assert_matches::*; - use std::{fs, io}; - use tempfile::NamedTempFile; - - #[test] - fn lock_access_remove() { - let path = NamedTempFile::new().unwrap().into_temp_path().to_owned(); - let lockfile = Flockfile::new_lock(&path).unwrap(); - - assert_eq!(lockfile.path, path); - - lockfile.unlock().unwrap(); - - assert_eq!( - fs::metadata(path).unwrap_err().kind(), - io::ErrorKind::NotFound - ); - } - - #[test] - fn lock_out_of_scope() { - let path = NamedTempFile::new().unwrap().into_temp_path().to_owned(); - { - let _lockfile = Flockfile::new_lock(&path).unwrap(); - // assert!(path.exists()); - assert!(fs::metadata(&path).is_ok()); - } - - assert_eq!( - fs::metadata(path).unwrap_err().kind(), - io::ErrorKind::NotFound - ); - } - - #[test] - fn lock_twice() { - let path = NamedTempFile::new().unwrap().into_temp_path().to_owned(); - let _lockfile = Flockfile::new_lock(&path).unwrap(); - - assert_matches!( - Flockfile::new_lock(&path).unwrap_err(), - FlockfileError::NixError(_) - ); - } -} +#[cfg(not(unix))] +pub use windows::*; diff --git a/common/flockfile/src/unix.rs b/common/flockfile/src/unix.rs new file mode 100644 index 00000000..ca290926 --- /dev/null +++ b/common/flockfile/src/unix.rs @@ -0,0 +1,135 @@ +use log::{debug, warn}; +use nix::fcntl::{flock, FlockArg}; +use std::{ + fs::{self, File, OpenOptions}, + io, + os::unix::io::AsRawFd, + path::{Path, PathBuf}, +}; + +#[derive(thiserror::Error, Debug)] +pub enum FlockfileError { + #[error(transparent)] + IoError(#[from] std::io::Error), + + #[error("Couldn't acquire file lock.")] + NixError(#[from] nix::Error), +} + +/// flockfile creates a lockfile in the filesystem under `/run/lock` and then creates a filelock using system fcntl with flock. +/// flockfile will automatically remove lockfile on application exit and the OS should cleanup the filelock afterwards. +/// If application exits unexpectedly the filelock will be dropped, but the lockfile will not be removed unless handled in signal handler. +#[derive(Debug)] +pub struct Flockfile { + handle: Option<File>, + pub path: PathBuf, +} + +impl Flockfile { + /// Create new lockfile in `/run/lock` with specific name: + /// + /// #Example + /// + /// let _lockfile = match flockfile::Flockfile::new_lock("app")).unwrap(); + /// + pub fn new_lock(lock_name: impl AsRef<Path>) -> Result<Flockfile, FlockfileError> { + let path = Path::new("/run/lock").join(lock_name); + + let file = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .open(&path)?; + + let () = flock(file.as_raw_fd(), FlockArg::LockExclusiveNonblock)?; + + Ok(Flockfile { + handle: Some(file), + path, + }) + } + + /// Manually remove filelock and lockfile from the filesystem, this method doesn't have to be called explicitly, + /// however if access to the locked file is required this must be called. + pub fn unlock(mut self) -> Result<(), io::Error> { + self.handle.take().expect("handle dropped"); + fs::remove_file(&self.path)?; + Ok(()) + } +} + +impl Drop for Flockfile { + /// The Drop trait will be called always when the lock goes out of scope, however, + /// if the program exits unexpectedly and drop is not called the lock will be removed by the system. + fn drop(&mut self) { + if let Some(handle) = self.handle.take() { + drop(handle); + + // Even if the file is not removed this is not an issue, as OS will take care of the flock. + // Additionally if the file is created before an attempt to create the lock that won't be an issue as we rely on filesystem lock. + match fs::remove_file(&self.path) { + Ok(()) => debug!(r#"Lockfile deleted "{:?}""#, self.path), + Err(err) => warn!( + r#"Error while handling lockfile at "{:?}": {:?}"#, + self.path, err + ), + } + } + } +} + +impl AsRef<Path> for Flockfile { + fn as_ref(&self) -> &Path { + self.path.as_ref() + } +} + +#[cfg(test)] +mod tests { + + use super::*; + use assert_matches::*; + use std::{fs, io}; + use tempfile::NamedTempFile; + + #[test] + fn lock_access_remove() { + let path = NamedTempFile::new().unwrap().into_temp_path().to_owned(); + let lockfile = Flockfile::new_lock(&path).unwrap(); + + assert_eq!(lockfile.path, path); + + lockfile.unlock().unwrap(); + + assert_eq!( + fs::metadata(path).unwrap_err().kind(), + io::ErrorKind::NotFound + ); + } + + #[test] + fn lock_out_of_scope() { + let path = NamedTempFile::new().unwrap().into_temp_path().to_owned(); + { + let _lockfile = Flockfile::new_lock(&path).unwrap(); + // assert!(path.exists()); + assert!(fs::metadata(&path).is_ok()); + } + + assert_eq!( + fs::metadata(path).unwrap_err().kind(), + io::ErrorKind::NotFound + ); + } + + #[test] + fn lock_twice() { + let path = NamedTempFile::new().unwrap().into_temp_path().to_owned(); + let _lockfile = Flockfile::new_lock(&path).unwrap(); + + assert_matches!( + Flockfile::new_lock(&path).unwrap_err(), + FlockfileError::NixError(_) + ); + } +} diff --git a/common/flockfile/src/windows.rs b/common/flockfile/src/windows.rs new file mode 100644 index 00000000..41542414 --- /dev/null +++ b/common/flockfile/src/windows.rs @@ -0,0 +1,32 @@ +use std::{ + io, + path::{Path, PathBuf}, +}; + +#[derive(thiserror::Error, Debug)] +pub enum FlockfileError { + #[error(transparent)] + IoError(#[from] std::io::Error), +} + +#[derive(Debug)] +pub struct Flockfile { + pub path: PathBuf, +} + +impl Flockfile { + pub fn new_lock(lock_name: impl AsRef<Path>) -> Result<Flockfile, FlockfileError> { + let path = Path::new("/no/lock/on/windows").join(lock_name); + Ok(Flockfile { path: path }) + } + + pub fn unlock(self) -> Result<(), io::Error> { + Ok(()) + } +} + +impl AsRef<Path> for Flockfile { + fn as_ref(&self) -> &Path { + self.path.as_ref() + } +} diff --git a/common/tedge_users/Cargo.toml b/common/tedge_users/Cargo.toml new file mode 100644 index 00000000..fed8d807 --- /dev/null +++ b/common/tedge_users/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "tedge_users" +version = "0.1.0" +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +thiserror = "1.0" + +[target.'cfg(unix)'.dependencies] +users = "0.11.0" diff --git a/tedge/src/utils/users/mod.rs b/common/tedge_users/src/lib.rs index 68ccc32e..68ccc32e 100644 --- a/tedge/src/utils/users/mod.rs +++ b/common/tedge_users/src/lib.rs diff --git a/tedge/src/utils/users/unix.rs b/common/tedge_users/src/unix.rs index 888c809a..6ac81194 100644 --- a/tedge/src/utils/users/unix.rs +++ b/common/tedge_users/src/unix.rs @@ -45,11 +45,14 @@ impl UserManager { /// Check if the process has been launched using `sudo` or not. /// + /// # Example + /// /// ``` + /// # use tedge_users::UserManager; /// let path = if UserManager::running_as_root() { - /// "/etc/mosquitto/mosquitto.conf" + /// "/etc/mosquitto/mosquitto.conf" /// } else { - /// home_dir(".tedge/mosquitto.conf") + /// ".tedge/mosquitto.conf" /// }; /// ``` pub fn running_as_root() -> bool { @@ -65,11 +68,12 @@ impl UserManager { /// # Example /// /// ``` + /// # use tedge_users::UserManager; /// let user_manager = UserManager::new(); - /// let _user_guard_1 = user_manager.become_user("user_1")?; + /// let _user_guard_1 = user_manager.become_user("user_1").expect("Fail to become user_1"); /// // Running as user1 /// { - /// let _user_guard_2 = user_manager.become_user("user_2")?; + /// let _user_guard_2 = user_manager.become_user("user_2").expect("Fail to become user_2"); /// // Running as user2 /// } /// // Running as user1 @@ -173,7 +177,12 @@ impl InnerUserManager { /// /// Such a guard implements the RAII pattern and provides no methods beyond `drop`. /// -/// fn create_certificate(&self, user_manager: &UserManager) -> Result<(), CertError> { +/// # Example +/// +/// ``` +/// # use tedge_users::UserManager; +/// # use tedge_users::UserSwitchError; +/// fn create_certificate(user_manager: &UserManager) -> Result<(), UserSwitchError> { /// let _user_guard = user_manager.become_user("mosquitto")?; /// // As long as the _user_guard is owned, the process run as mosquitto. /// @@ -181,6 +190,7 @@ impl InnerUserManager { /// /// Ok(()) /// } // Here, the _user_guard is dropped and the process switches back to the former user. +/// ``` pub struct UserGuard { user_manager: UserManager, } diff --git a/tedge/src/utils/users/windows.rs b/common/tedge_users/src/windows.rs index eedb2745..eedb2745 100644 --- a/tedge/src/utils/users/windows.rs +++ b/common/tedge_users/src/windows.rs diff --git a/mapper/collectd_mapper/Cargo.toml b/mapper/collectd_mapper/Cargo.toml index d1b16832..7e208a1d 100644 --- a/mapper/collectd_mapper/Cargo.toml +++ b/mapper/collectd_mapper/Cargo.toml @@ -32,7 +32,7 @@ tracing-subscriber = "0.2" mockall = "0.9" async-trait = "0.1" tedge_config = {path = "../../tedge_config" } -users = "0.11" +tedge_users = { path = "../../common/tedge_users" } [dev-dependencies] assert_matches = "1.4" diff --git a/mapper/collectd_mapper/src/main.rs b/mapper/collectd_mapper/src/main.rs index 109bec6d..02ac1a6f 100644 --- a/mapper/collectd_mapper/src/main.rs +++ b/mapper/collectd_mapper/src/main.rs @@ -49,7 +49,7 @@ fn config_repository() -> anyhow::Result<TEdgeConfigRepository> { } fn config_location() -> anyhow::Result<TEdgeConfigLocation> { - let tedge_config_location = if running_as_root() { + let tedge_config_location = if tedge_users::UserManager::running_as_root() { tedge_config::TEdgeConfigLocation::from_default_system_location() } else { tedge_config::TEdgeConfigLocation::from_users_home_location( @@ -59,11 +59,6 @@ fn config_location() -> anyhow::Result<TEdgeConfigLocation> { Ok(tedge_config_location) } -// Copied from tedge/src/utils/users/unix.rs. In the future, it would be good to separate it from tedge crate. -fn running_as_root() -> bool { - users::get_current_uid() == 0 -} - // Copied from tedge/src/utils/paths.rs. In the future, it would be good to separate it from tedge crate. fn home_dir() -> Option<PathBuf> { std::env::var_os("HOME") diff --git a/mapper/tedge_mapper/Cargo.toml b/mapper/tedge_mapper/Cargo.toml index 8d2d65ce..9957bfb3 100644 --- a/mapper/tedge_mapper/Cargo.toml +++ b/mapper/tedge_mapper/Cargo.toml @@ -33,7 +33,7 @@ tracing = { version = "0.1", features = ["attributes", "log"] } tracing-subscriber = "0.2" async-trait = "0.1" thiserror = "1.0" -users = "0.11" +tedge_users = { path = "../../common/tedge_users" } [dev-dependencies] assert-json-diff = "2.0" diff --git a/mapper/tedge_mapper/src/main.rs b/mapper/tedge_mapper/src/main.rs index 8519014d..f040facc 100644 --- a/mapper/tedge_mapper/src/main.rs +++ b/mapper/tedge_mapper/src/main.rs @@ -121,7 +121,7 @@ fn check_another_instance_is_running(app_name: &str) -> Result<Flockfile, Flockf } fn config_repository() -> Result<TEdgeConfigRepository, MapperError> { - let tedge_config_location = if running_as_root() { + let tedge_config_location = if tedge_users::UserManager::running_as_root() { tedge_config::TEdgeConfigLocation::from_default_system_location() } else { tedge_config::TEdgeConfigLocation::from_users_home_location( @@ -132,11 +132,6 @@ fn config_repository() -> Result<TEdgeConfigRepository, MapperError> { Ok(config_repository) } -// Copied from tedge/src/utils/users/unix.rs. In the future, it would be good to separate it from tedge crate. -fn running_as_root() -> bool { - users::get_current_uid() == 0 -} - // Copied from tedge/src/utils/paths.rs. In the future, it would be good to separate it from tedge crate. fn home_dir() -> Option<PathBuf> { std::env::var_os("HOME") diff --git a/tedge/Cargo.toml b/tedge/Cargo.toml index 2555c2aa..6d5109b5 100644 --- a/tedge/Cargo.toml +++ b/tedge/Cargo.toml @@ -28,9 +28,7 @@ toml = "0.5" url = "2.2" which = "4.0" tedge_config = { path = "../tedge_config" } - -[target.'cfg(unix)'.dependencies] -users = "0.11.0" +tedge_users = { path = "../common/tedge_users" } [dev-dependencies] assert_cmd = "1.0" diff --git a/tedge/src/cli/certificate/create.rs b/tedge/src/cli/certificate/create.rs index 74bd5794..8afd5581 100644 --- a/tedge/src/cli/certificate/create.rs +++ b/tedge/src/cli/certificate/create.rs @@ -1,5 +1,5 @@ use crate::command::{Command, ExecutionContext}; -use crate::utils::{paths, users}; +use crate::utils::paths; use certificate::{KeyCertPair, NewCertificateConfig}; use std::{ fs::{File, OpenOptions}, @@ -7,6 +7,7 @@ use std::{ path::Path, }; use tedge_config::*; +use tedge_users; use super::error::CertError; @@ -38,9 +39,9 @@ impl CreateCertCmd { fn create_test_certificate( &self, config: &NewCertificateConfig, - user_manager: &users::UserManager, + user_manager: &tedge_users::UserManager, ) -> Result<(), CertError> { - let _user_guard = user_manager.become_user(users::BROKER_USER)?; + let _user_guard = user_manager.become_user(tedge_users::BROKER_USER)?; paths::validate_parent_dir_exists(&self.cert_path).map_err(CertError::CertPathError)?; paths::validate_parent_dir_exists(&self.key_path).map_err(CertError::KeyPathError)?; @@ -84,9 +85,9 @@ fn create_new_file(path: impl AsRef<Path>) -> Result<File, CertError> { #[cfg(test)] mod tests { use super::*; - use crate::utils::users::UserManager; use assert_matches::assert_matches; use std::fs; + use tedge_users::UserManager; use tempfile::*; #[test] diff --git a/tedge/src/cli/certificate/error.rs b/tedge/src/cli/certificate/error.rs index 02635af7..3021d685 100644 --- a/tedge/src/cli/certificate/error.rs +++ b/tedge/src/cli/certificate/error.rs @@ -1,7 +1,8 @@ use reqwest::StatusCode; use tedge_config::FilePath; -use crate::utils::{paths::PathsError, users::UserSwitchError}; +use crate::utils::paths::PathsError; +use tedge_users::UserSwitchError; #[derive(thiserror::Error, Debug)] pub enum CertError { diff --git a/tedge/src/cli/certificate/remove.rs b/tedge/src/cli/certificate/remove.rs index f3abacac..ada5b233 100644 --- a/tedge/src/cli/certificate/remove.rs +++ b/tedge/src/cli/certificate/remove.rs @@ -1,7 +1,8 @@ use crate::command::{Command, ExecutionContext}; -use crate::utils::{paths::*, users::*}; +use crate::utils::paths::*; use tedge_config::*; +use tedge_users::UserManager; use super::error::CertError; @@ -27,7 +28,7 @@ impl Command for RemoveCertCmd { impl RemoveCertCmd { fn remove_certificate(&self, user_manager: &UserManager) -> Result<(), CertError> { - let _user_guard = user_manager.become_user(crate::utils::users::BROKER_USER)?; + let _user_guard = user_manager.become_user(tedge_users::BROKER_USER)?; std::fs::remove_file(&self.cert_path).or_else(ok_if_not_found)?; std::fs::remove_file(&self.key_path).or_else(ok_if_not_found)?; diff --git a/tedge/src/cli/connect/command.rs b/tedge/src/cli/connect/command.rs index 795ba2de..1201a9b7 100644 --- a/tedge/src/cli/connect/command.rs +++ b/tedge/src/cli/connect/command.rs @@ -4,12 +4,12 @@ use crate::services::{ self, mosquitto::MosquittoService, tedge_mapper::TedgeMapperService, SystemdService, }; use crate::utils::paths; -use crate::utils::users::UserManager; use crate::ConfigError; use mqtt_client::{Client, Message, MqttClient, Topic, TopicFilter}; use std::path::Path; use std::time::Duration; use tedge_config::*; +use tedge_users::UserManager; use tempfile::NamedTempFile; use tokio::time::timeout; use which::which; diff --git a/tedge/src/cli/disconnect/disconnect_bridge.rs b/tedge/src/cli/disconnect/disconnect_bridge.rs index 38c07cbc..d7c889de 100644 --- a/tedge/src/cli/disconnect/disconnect_bridge.rs +++ b/tedge/src/cli/disconnect/disconnect_bridge.rs @@ -3,7 +3,8 @@ use crate::command::*; use crate::services::{ mosquitto::MosquittoService, tedge_mapper::TedgeMapperService, SystemdService, }; -use crate::utils::{paths, users::*}; +use crate::utils::paths; +use tedge_users::*; use which::which; const TEDGE_BRIDGE_CONF_DIR_PATH: &str = "mosquitto-conf"; diff --git a/tedge/src/command.rs b/tedge/src/command.rs index 74189196..20964c0c 100644 --- a/tedge/src/command.rs +++ b/tedge/src/command.rs @@ -1,4 +1,4 @@ -use crate::utils::users::UserManager; +use tedge_users::UserManager; /// A trait to be implemented by all tedge sub-commands. /// diff --git a/tedge/src/main.rs b/tedge/src/main.rs index 901bd36c..34cb134d 100644 --- a/tedge/src/main.rs +++ b/tedge/src/main.rs @@ -17,11 +17,11 @@ use command::{BuildCommand, BuildContext, ExecutionContext}; fn main() -> anyhow::Result<()> { let context = ExecutionContext::new(); - let _user_guard = context.user_manager.become_user(utils::users::TEDGE_USER)?; + let _user_guard = context.user_manager.become_user(tedge_users::TEDGE_USER)?; let opt = cli::Opt::from_args(); |