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

@@ -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,
             )

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,
             )

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> {
+    pub fn plan(name: String, gid: u32) -> Result<StatefulAction<Self>, ActionError> {
-        Self { name, gid }.into()
+        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,19 +79,6 @@ 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)
@@ -88,7 +95,6 @@ impl Action for CreateGroup {
                 )
                 .await
                 .map_err(|e| ActionError::Command(e))?;
-                }
             },
             _ => {
                 execute_command(

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,
             )

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> {
+    pub fn plan(
-        Self {
+        name: String,
-            name,
+        uid: u32,
+        groupname: String,
+        gid: u32,
+    ) -> Result<StatefulAction<Self>, 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,
+                ));
             }
-        .into()
+
+            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));
+        }
+
+        Ok(StatefulAction::uncompleted(this))
     }
 }
 
@@ -77,22 +107,6 @@ 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)
@@ -192,7 +206,6 @@ impl Action for CreateUser {
                 )
                 .await
                 .map_err(|e| ActionError::Command(e))?;
-                }
             },
             _ => {
                 execute_command(

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<CreateGroup>,
     create_users: Vec<StatefulAction<CreateUser>>,
 }
@@ -22,12 +22,10 @@ pub struct CreateUsersAndGroups {
 impl CreateUsersAndGroups {
     #[tracing::instrument(level = "debug", skip_all)]
     pub async fn plan(settings: CommonSettings) -> Result<StatefulAction<Self>, 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::<Result<_, _>>()?;
         Ok(Self {
             nix_build_user_count: settings.nix_build_user_count,
             nix_build_group_name: settings.nix_build_group_name,

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}`")]

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)

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
     }