From ce28eedf2a3c2b8aa58d705c8b73b598f7986caf Mon Sep 17 00:00:00 2001 From: Ana Hobden Date: Thu, 9 Feb 2023 10:34:34 -0800 Subject: [PATCH] Better support users/groups existing before install (#238) * Better support users/groups existing before install * Skip instead of complete if found * Prod CI * Add debuging messages * Mark completed instead of skipped --- src/action/base/create_directory.rs | 4 +- src/action/base/create_file.rs | 4 +- src/action/base/create_group.rs | 72 ++--- src/action/base/create_or_insert_into_file.rs | 4 +- src/action/base/create_user.rs | 257 +++++++++--------- src/action/common/create_users_and_groups.rs | 12 +- src/action/mod.rs | 10 +- src/action/stateful.rs | 21 ++ src/settings.rs | 12 +- 9 files changed, 220 insertions(+), 176 deletions(-) diff --git a/src/action/base/create_directory.rs b/src/action/base/create_directory.rs index 7e4bd42..8b24931 100644 --- a/src/action/base/create_directory.rs +++ b/src/action/base/create_directory.rs @@ -105,7 +105,7 @@ impl Action for CreateDirectory { let gid = if let Some(group) = group { Some( Group::from_name(group.as_str()) - .map_err(|e| ActionError::GroupId(group.clone(), e))? + .map_err(|e| ActionError::GettingGroupId(group.clone(), e))? .ok_or(ActionError::NoGroup(group.clone()))? .gid, ) @@ -115,7 +115,7 @@ impl Action for CreateDirectory { let uid = if let Some(user) = user { Some( User::from_name(user.as_str()) - .map_err(|e| ActionError::UserId(user.clone(), e))? + .map_err(|e| ActionError::GettingUserId(user.clone(), e))? .ok_or(ActionError::NoUser(user.clone()))? .uid, ) diff --git a/src/action/base/create_file.rs b/src/action/base/create_file.rs index 5bc433a..b5c950f 100644 --- a/src/action/base/create_file.rs +++ b/src/action/base/create_file.rs @@ -118,7 +118,7 @@ impl Action for CreateFile { let gid = if let Some(group) = group { Some( Group::from_name(group.as_str()) - .map_err(|e| ActionError::GroupId(group.clone(), e))? + .map_err(|e| ActionError::GettingGroupId(group.clone(), e))? .ok_or(ActionError::NoGroup(group.clone()))? .gid, ) @@ -128,7 +128,7 @@ impl Action for CreateFile { let uid = if let Some(user) = user { Some( User::from_name(user.as_str()) - .map_err(|e| ActionError::UserId(user.clone(), e))? + .map_err(|e| ActionError::GettingUserId(user.clone(), e))? .ok_or(ActionError::NoUser(user.clone()))? .uid, ) diff --git a/src/action/base/create_group.rs b/src/action/base/create_group.rs index 8e35feb..249220e 100644 --- a/src/action/base/create_group.rs +++ b/src/action/base/create_group.rs @@ -1,3 +1,4 @@ +use nix::unistd::Group; use tokio::process::Command; use tracing::{span, Span}; @@ -12,13 +13,32 @@ Create an operating system level user group #[derive(Debug, serde::Deserialize, serde::Serialize, Clone)] pub struct CreateGroup { name: String, - gid: usize, + gid: u32, } impl CreateGroup { #[tracing::instrument(level = "debug", skip_all)] - pub fn plan(name: String, gid: usize) -> StatefulAction { - Self { name, gid }.into() + pub fn plan(name: String, gid: u32) -> Result, ActionError> { + let this = Self { + name: name.clone(), + gid, + }; + // Ensure group does not exists + if let Some(group) = Group::from_name(name.as_str()) + .map_err(|e| ActionError::GettingGroupId(name.clone(), e))? + { + if group.gid.as_raw() != gid { + return Err(ActionError::GroupGidMismatch( + name.clone(), + group.gid.as_raw(), + gid, + )); + } + + tracing::debug!("Creating group `{}` already complete", this.name); + return Ok(StatefulAction::completed(this)); + } + Ok(StatefulAction::uncompleted(this)) } } @@ -59,36 +79,22 @@ impl Action for CreateGroup { patch: _, } | OperatingSystem::Darwin => { - if Command::new("/usr/bin/dscl") - .process_group(0) - .args([".", "-read", &format!("/Groups/{name}")]) - .stdin(std::process::Stdio::null()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::piped()) - .status() - .await - .map_err(ActionError::Command)? - .success() - { - () - } else { - execute_command( - Command::new("/usr/sbin/dseditgroup") - .process_group(0) - .args([ - "-o", - "create", - "-r", - "Nix build group for nix-daemon", - "-i", - &format!("{gid}"), - name.as_str(), - ]) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - } + execute_command( + Command::new("/usr/sbin/dseditgroup") + .process_group(0) + .args([ + "-o", + "create", + "-r", + "Nix build group for nix-daemon", + "-i", + &format!("{gid}"), + name.as_str(), + ]) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; }, _ => { execute_command( diff --git a/src/action/base/create_or_insert_into_file.rs b/src/action/base/create_or_insert_into_file.rs index 58dd601..4641453 100644 --- a/src/action/base/create_or_insert_into_file.rs +++ b/src/action/base/create_or_insert_into_file.rs @@ -158,7 +158,7 @@ impl Action for CreateOrInsertIntoFile { let gid = if let Some(group) = group { Some( Group::from_name(group.as_str()) - .map_err(|e| ActionError::GroupId(group.clone(), e))? + .map_err(|e| ActionError::GettingGroupId(group.clone(), e))? .ok_or(ActionError::NoGroup(group.clone()))? .gid, ) @@ -168,7 +168,7 @@ impl Action for CreateOrInsertIntoFile { let uid = if let Some(user) = user { Some( User::from_name(user.as_str()) - .map_err(|e| ActionError::UserId(user.clone(), e))? + .map_err(|e| ActionError::GettingUserId(user.clone(), e))? .ok_or(ActionError::NoUser(user.clone()))? .uid, ) diff --git a/src/action/base/create_user.rs b/src/action/base/create_user.rs index 5a246f0..989c893 100644 --- a/src/action/base/create_user.rs +++ b/src/action/base/create_user.rs @@ -1,3 +1,4 @@ +use nix::unistd::User; use tokio::process::Command; use tracing::{span, Span}; @@ -12,21 +13,50 @@ Create an operating system level user in the given group #[derive(Debug, serde::Deserialize, serde::Serialize, Clone)] pub struct CreateUser { name: String, - uid: usize, + uid: u32, groupname: String, - gid: usize, + gid: u32, } impl CreateUser { #[tracing::instrument(level = "debug", skip_all)] - pub fn plan(name: String, uid: usize, groupname: String, gid: usize) -> StatefulAction { - Self { - name, + pub fn plan( + name: String, + uid: u32, + groupname: String, + gid: u32, + ) -> Result, ActionError> { + let this = Self { + name: name.clone(), uid, groupname, gid, + }; + // Ensure user does not exists + if let Some(user) = User::from_name(name.as_str()) + .map_err(|e| ActionError::GettingUserId(name.clone(), e))? + { + if user.uid.as_raw() != uid { + return Err(ActionError::UserUidMismatch( + name.clone(), + user.uid.as_raw(), + uid, + )); + } + + if user.gid.as_raw() != gid { + return Err(ActionError::UserGidMismatch( + name.clone(), + user.gid.as_raw(), + gid, + )); + } + + tracing::debug!("Creating user `{}` already complete", this.name); + return Ok(StatefulAction::completed(this)); } - .into() + + Ok(StatefulAction::uncompleted(this)) } } @@ -77,122 +107,105 @@ impl Action for CreateUser { patch: _, } | OperatingSystem::Darwin => { - // TODO(@hoverbear): Make this actually work... - // Right now, our test machines do not have a secure token and cannot delete users. - - if Command::new("/usr/bin/dscl") - .process_group(0) - .args([".", "-read", &format!("/Users/{name}")]) - .stdin(std::process::Stdio::null()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::piped()) - .status() - .await - .map_err(ActionError::Command)? - .success() - { - () - } else { - execute_command( - Command::new("/usr/bin/dscl") - .process_group(0) - .args([".", "-create", &format!("/Users/{name}")]) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - execute_command( - Command::new("/usr/bin/dscl") - .process_group(0) - .args([ - ".", - "-create", - &format!("/Users/{name}"), - "UniqueID", - &format!("{uid}"), - ]) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - execute_command( - Command::new("/usr/bin/dscl") - .process_group(0) - .args([ - ".", - "-create", - &format!("/Users/{name}"), - "PrimaryGroupID", - &format!("{gid}"), - ]) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - execute_command( - Command::new("/usr/bin/dscl") - .process_group(0) - .args([ - ".", - "-create", - &format!("/Users/{name}"), - "NFSHomeDirectory", - "/var/empty", - ]) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - execute_command( - Command::new("/usr/bin/dscl") - .process_group(0) - .args([ - ".", - "-create", - &format!("/Users/{name}"), - "UserShell", - "/sbin/nologin", - ]) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - execute_command( - Command::new("/usr/bin/dscl") - .process_group(0) - .args([ - ".", - "-append", - &format!("/Groups/{groupname}"), - "GroupMembership", - ]) - .arg(&name) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - execute_command( - Command::new("/usr/bin/dscl") - .process_group(0) - .args([".", "-create", &format!("/Users/{name}"), "IsHidden", "1"]) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - execute_command( - Command::new("/usr/sbin/dseditgroup") - .process_group(0) - .args(["-o", "edit"]) - .arg("-a") - .arg(&name) - .arg("-t") - .arg(&name) - .arg(groupname) - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(|e| ActionError::Command(e))?; - } + execute_command( + Command::new("/usr/bin/dscl") + .process_group(0) + .args([".", "-create", &format!("/Users/{name}")]) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; + execute_command( + Command::new("/usr/bin/dscl") + .process_group(0) + .args([ + ".", + "-create", + &format!("/Users/{name}"), + "UniqueID", + &format!("{uid}"), + ]) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; + execute_command( + Command::new("/usr/bin/dscl") + .process_group(0) + .args([ + ".", + "-create", + &format!("/Users/{name}"), + "PrimaryGroupID", + &format!("{gid}"), + ]) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; + execute_command( + Command::new("/usr/bin/dscl") + .process_group(0) + .args([ + ".", + "-create", + &format!("/Users/{name}"), + "NFSHomeDirectory", + "/var/empty", + ]) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; + execute_command( + Command::new("/usr/bin/dscl") + .process_group(0) + .args([ + ".", + "-create", + &format!("/Users/{name}"), + "UserShell", + "/sbin/nologin", + ]) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; + execute_command( + Command::new("/usr/bin/dscl") + .process_group(0) + .args([ + ".", + "-append", + &format!("/Groups/{groupname}"), + "GroupMembership", + ]) + .arg(&name) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; + execute_command( + Command::new("/usr/bin/dscl") + .process_group(0) + .args([".", "-create", &format!("/Users/{name}"), "IsHidden", "1"]) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; + execute_command( + Command::new("/usr/sbin/dseditgroup") + .process_group(0) + .args(["-o", "edit"]) + .arg("-a") + .arg(&name) + .arg("-t") + .arg(&name) + .arg(groupname) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| ActionError::Command(e))?; }, _ => { execute_command( diff --git a/src/action/common/create_users_and_groups.rs b/src/action/common/create_users_and_groups.rs index dcf79f5..34f7cd8 100644 --- a/src/action/common/create_users_and_groups.rs +++ b/src/action/common/create_users_and_groups.rs @@ -10,11 +10,11 @@ use tracing::{span, Instrument, Span}; #[derive(Debug, serde::Deserialize, serde::Serialize, Clone)] pub struct CreateUsersAndGroups { - nix_build_user_count: usize, + nix_build_user_count: u32, nix_build_group_name: String, - nix_build_group_id: usize, + nix_build_group_id: u32, nix_build_user_prefix: String, - nix_build_user_id_base: usize, + nix_build_user_id_base: u32, create_group: StatefulAction, create_users: Vec>, } @@ -22,12 +22,10 @@ pub struct CreateUsersAndGroups { impl CreateUsersAndGroups { #[tracing::instrument(level = "debug", skip_all)] pub async fn plan(settings: CommonSettings) -> Result, ActionError> { - // TODO(@hoverbear): CHeck if it exist, error if so let create_group = CreateGroup::plan( settings.nix_build_group_name.clone(), settings.nix_build_group_id, - ); - // TODO(@hoverbear): CHeck if they exist, error if so + )?; let create_users = (0..settings.nix_build_user_count) .map(|count| { CreateUser::plan( @@ -37,7 +35,7 @@ impl CreateUsersAndGroups { settings.nix_build_group_id, ) }) - .collect(); + .collect::>()?; Ok(Self { nix_build_user_count: settings.nix_build_user_count, nix_build_group_name: settings.nix_build_group_name, diff --git a/src/action/mod.rs b/src/action/mod.rs index a05e8d3..0903349 100644 --- a/src/action/mod.rs +++ b/src/action/mod.rs @@ -303,11 +303,17 @@ pub enum ActionError { #[error("Truncating `{0}`")] Truncate(std::path::PathBuf, #[source] std::io::Error), #[error("Getting uid for user `{0}`")] - UserId(String, #[source] nix::errno::Errno), + GettingUserId(String, #[source] nix::errno::Errno), + #[error("User `{0}` existed but had a different uid ({1}) than planned ({2})")] + UserUidMismatch(String, u32, u32), + #[error("User `{0}` existed but had a different gid ({1}) than planned ({2})")] + UserGidMismatch(String, u32, u32), #[error("Getting user `{0}`")] NoUser(String), #[error("Getting gid for group `{0}`")] - GroupId(String, #[source] nix::errno::Errno), + GettingGroupId(String, #[source] nix::errno::Errno), + #[error("Group `{0}` existed but had a different gid ({1}) than planned ({2})")] + GroupGidMismatch(String, u32, u32), #[error("Getting group `{0}`")] NoGroup(String), #[error("Chowning path `{0}`")] diff --git a/src/action/stateful.rs b/src/action/stateful.rs index dc21f75..e2929cc 100644 --- a/src/action/stateful.rs +++ b/src/action/stateful.rs @@ -217,6 +217,27 @@ where }, } } + + pub fn completed(action: A) -> Self { + Self { + state: ActionState::Completed, + action, + } + } + + pub fn skipped(action: A) -> Self { + Self { + state: ActionState::Skipped, + action, + } + } + + pub fn uncompleted(action: A) -> Self { + Self { + state: ActionState::Uncompleted, + action, + } + } } /** The state of an [`Action`](crate::action::Action) diff --git a/src/settings.rs b/src/settings.rs index 44296bc..a160bdb 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -95,7 +95,7 @@ pub struct CommonSettings { global = true ) )] - pub(crate) nix_build_user_count: usize, + pub(crate) nix_build_user_count: u32, /// The Nix build group name #[cfg_attr( @@ -119,7 +119,7 @@ pub struct CommonSettings { global = true ) )] - pub(crate) nix_build_group_id: usize, + pub(crate) nix_build_group_id: u32, /// The Nix build user prefix (user numbers will be postfixed) #[cfg_attr( @@ -147,7 +147,7 @@ pub struct CommonSettings { all(target_os = "linux", feature = "cli"), clap(default_value_t = 30_000) )] - pub(crate) nix_build_user_id_base: usize, + pub(crate) nix_build_user_id_base: u32, /// The Nix package URL #[cfg_attr( @@ -361,7 +361,7 @@ async fn linux_detect_init() -> (InitSystem, bool) { // Builder Pattern impl CommonSettings { /// Number of build users to create - pub fn nix_build_user_count(&mut self, count: usize) -> &mut Self { + pub fn nix_build_user_count(&mut self, count: u32) -> &mut Self { self.nix_build_user_count = count; self } @@ -385,7 +385,7 @@ impl CommonSettings { } /// The Nix build group GID - pub fn nix_build_group_id(&mut self, count: usize) -> &mut Self { + pub fn nix_build_group_id(&mut self, count: u32) -> &mut Self { self.nix_build_group_id = count; self } @@ -397,7 +397,7 @@ impl CommonSettings { } /// The Nix build user base UID (ascending) - pub fn nix_build_user_id_base(&mut self, count: usize) -> &mut Self { + pub fn nix_build_user_id_base(&mut self, count: u32) -> &mut Self { self.nix_build_user_id_base = count; self }