Merge pull request #864 from ajeddeloh/chroot

Clean up user/group handling

internal/distro/distro.go

@@ -33,9 +33,7 @@ var (
 	systemConfigDir = "/usr/lib/ignition"
 
 	// Helper programs
-	chrootCmd   = "chroot"
 	groupaddCmd = "groupadd"
-	idCmd       = "id"
 	mdadmCmd    = "mdadm"
 	mountCmd    = "mount"
 	sgdiskCmd   = "sgdisk"
@@ -71,9 +69,7 @@ func DiskByPartUUIDDir() string { return diskByPartUUIDDir }
 func KernelCmdlinePath() string { return kernelCmdlinePath }
 func SystemConfigDir() string   { return fromEnv("SYSTEM_CONFIG_DIR", systemConfigDir) }
 
-func ChrootCmd() string     { return chrootCmd }
 func GroupaddCmd() string   { return groupaddCmd }
-func IdCmd() string         { return idCmd }
 func MdadmCmd() string      { return mdadmCmd }
 func MountCmd() string      { return mountCmd }
 func SgdiskCmd() string     { return sgdiskCmd }

internal/exec/util/passwd.go

@@ -26,9 +26,22 @@ import (
 	"github.com/coreos/ignition/v2/config/v3_1_experimental/types"
 	"github.com/coreos/ignition/v2/internal/as_user"
 	"github.com/coreos/ignition/v2/internal/distro"
-	"github.com/coreos/ignition/v2/internal/log"
 )
 
+func appendIfTrue(args []string, test *bool, newargs string) []string {
+	if test != nil && *test {
+		return append(args, newargs)
+	}
+	return args
+}
+
+func appendIfStringSet(args []string, arg string, str *string) []string {
+	if str != nil && *str != "" {
+		return append(args, arg, *str)
+	}
+	return args
+}
+
 // EnsureUser ensures that the user exists as described. If the user does not
 // yet exist, they will be created, otherwise the existing user will be
 // modified.
@@ -49,27 +62,17 @@ func (u Util) EnsureUser(c types.PasswdUser) error {
 	} else {
 		cmd = distro.UseraddCmd()
 
-		if c.HomeDir != nil && *c.HomeDir != "" {
-			args = append(args, "--home-dir", *c.HomeDir)
-		}
+		args = appendIfStringSet(args, "--home-dir", c.HomeDir)
 
 		if c.NoCreateHome != nil && *c.NoCreateHome {
 			args = append(args, "--no-create-home")
 		} else {
 			args = append(args, "--create-home")
 		}
 
-		if c.NoUserGroup != nil && *c.NoUserGroup {
-			args = append(args, "--no-user-group")
-		}
-
-		if c.System != nil && *c.System {
-			args = append(args, "--system")
-		}
-
-		if c.NoLogInit != nil && *c.NoLogInit {
-			args = append(args, "--no-log-init")
-		}
+		args = appendIfTrue(args, c.NoUserGroup, "--no-user-group")
+		args = appendIfTrue(args, c.System, "--system")
+		args = appendIfTrue(args, c.NoLogInit, "--no-log-init")
 	}
 
 	if c.PasswordHash != nil {
@@ -89,21 +92,14 @@ func (u Util) EnsureUser(c types.PasswdUser) error {
 			strconv.FormatUint(uint64(*c.UID), 10))
 	}
 
-	if c.Gecos != nil && *c.Gecos != "" {
-		args = append(args, "--comment", *c.Gecos)
-	}
-
-	if c.PrimaryGroup != nil && *c.PrimaryGroup != "" {
-		args = append(args, "--gid", *c.PrimaryGroup)
-	}
+	args = appendIfStringSet(args, "--comment", c.Gecos)
+	args = appendIfStringSet(args, "--gid", c.PrimaryGroup)
 
 	if len(c.Groups) > 0 {
 		args = append(args, "--groups", strings.Join(translateV2_1PasswdUserGroupSliceToStringSlice(c.Groups), ","))
 	}
 
-	if c.Shell != nil && *c.Shell != "" {
-		args = append(args, "--shell", *c.Shell)
-	}
+	args = appendIfStringSet(args, "--shell", c.Shell)
 
 	args = append(args, c.Name)
 
@@ -114,18 +110,11 @@ func (u Util) EnsureUser(c types.PasswdUser) error {
 
 // CheckIfUserExists will return Info log when user is empty
 func (u Util) CheckIfUserExists(c types.PasswdUser) (bool, error) {
-	code := -1
-	cmd := exec.Command(distro.ChrootCmd(), u.DestDir, distro.IdCmd(), c.Name)
-	stdout, err := cmd.CombinedOutput()
+	_, err := u.userLookup(c.Name)
+	if _, ok := err.(user.UnknownUserError); ok {
+		return false, nil
+	}
 	if err != nil {
-		if exitErr, ok := err.(*exec.ExitError); ok {
-			code = exitErr.Sys().(syscall.WaitStatus).ExitStatus()
-		}
-		if code == 1 {
-			u.Info("checking if user \"%s\" exists: %s", c.Name, fmt.Errorf("[Attention] %v: Cmd: %s Stdout: %s", err, log.QuotedCmd(cmd), stdout))
-			return false, nil
-		}
-		u.Logger.Info("error encountered (%T): %v", err, err)
 		return false, err
 	}
 	return true, nil
@@ -240,9 +229,7 @@ func (u Util) CreateGroup(g types.PasswdGroup) error {
 		args = append(args, "--password", "*")
 	}
 
-	if g.System != nil && *g.System {
-		args = append(args, "--system")
-	}
+	args = appendIfTrue(args, g.System, "--system")
 
 	args = append(args, g.Name)
 

internal/exec/util/user_group_lookup.c

@@ -62,10 +62,16 @@ static int user_lookup_fn(lookup_ctxt_t *ctxt) {
 		goto out_err;
 	}
 
-	if(getpwnam_r(ctxt->name, &p, buf, sizeof(buf), &pptr) != 0 || !pptr) {
+	if(getpwnam_r(ctxt->name, &p, buf, sizeof(buf), &pptr) != 0) {
 		goto out_err;
 	}
 
+	if (!pptr) {
+		// successfully found nothing
+		ctxt->res->name = NULL;
+		return 0;
+	}
+
 	if(!(ctxt->res->name = strdup(p.pw_name))) {
 		goto out_err;
 	}
@@ -98,10 +104,16 @@ static int group_lookup_fn(lookup_ctxt_t *ctxt) {
 		goto out_err;
 	}
 
-	if(getgrnam_r(ctxt->name, &g, buf, sizeof(buf), &gptr) != 0 || !gptr) {
+	if(getgrnam_r(ctxt->name, &g, buf, sizeof(buf), &gptr) != 0) {
 		goto out_err;
 	}
 
+	if (!gptr) {
+		// successfully found nothing
+		ctxt->res->name = NULL;
+		return 0;
+	}
+
 	if(!(ctxt->res->name = strdup(g.gr_name))) {
 		goto out_err;
 	}

internal/exec/util/user_group_lookup.go

@@ -37,7 +37,7 @@ func (u Util) userLookup(name string) (*user.User, error) {
 	}
 
 	if res.name == nil {
-		return nil, fmt.Errorf("user %q not found", name)
+		return nil, user.UnknownUserError(fmt.Sprintf("user %q not found", name))
 	}
 
 	homedir, err := u.JoinPath(C.GoString(res.home))
@@ -67,7 +67,7 @@ func (u Util) groupLookup(name string) (*user.Group, error) {
 	}
 
 	if res.name == nil {
-		return nil, fmt.Errorf("user %q not found", name)
+		return nil, user.UnknownGroupError(fmt.Sprintf("group %q not found", name))
 	}
 
 	grp := &user.Group{