Skip to content

Commit

Permalink
cmd/initContainer: Simplify code by removing a function parameter
Browse files Browse the repository at this point in the history
Until now, configureUsers() was pushing the burden of deciding whether
to add a new user or modify an existing one on the callers, even though
it can trivially decide itself.  Involving the caller loosens the
encapsulation of the user configuration logic by spreading it across
configureUsers() and it's caller, and adds an extra function parameter
that needs to be carefully set and is vulnerable to programmer errors.

Fallout from 9ea6fe5

containers#1356
  • Loading branch information
debarshiray committed Aug 22, 2023
1 parent 219f5b4 commit 6bd7c87
Showing 1 changed file with 24 additions and 38 deletions.
62 changes: 24 additions & 38 deletions src/cmd/initContainer.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,24 +236,12 @@ func initContainer(cmd *cobra.Command, args []string) error {
}
}

if _, err := user.Lookup(initContainerFlags.user); err != nil {
if err := configureUsers(initContainerFlags.uid,
initContainerFlags.user,
initContainerFlags.home,
initContainerFlags.shell,
initContainerFlags.homeLink,
false); err != nil {
return err
}
} else {
if err := configureUsers(initContainerFlags.uid,
initContainerFlags.user,
initContainerFlags.home,
initContainerFlags.shell,
initContainerFlags.homeLink,
true); err != nil {
return err
}
if err := configureUsers(initContainerFlags.uid,
initContainerFlags.user,
initContainerFlags.home,
initContainerFlags.shell,
initContainerFlags.homeLink); err != nil {
return err
}

if utils.PathExists("/etc/krb5.conf.d") && !utils.PathExists("/etc/krb5.conf.d/kcm_default_ccache") {
Expand Down Expand Up @@ -386,9 +374,7 @@ func initContainerHelp(cmd *cobra.Command, args []string) {
}
}

func configureUsers(targetUserUid int,
targetUser, targetUserHome, targetUserShell string,
homeLink, targetUserExists bool) error {
func configureUsers(targetUserUid int, targetUser, targetUserHome, targetUserShell string, homeLink bool) error {
if homeLink {
if err := redirectPath("/home", "/var/home", true); err != nil {
return err
Expand All @@ -400,45 +386,45 @@ func configureUsers(targetUserUid int,
return fmt.Errorf("failed to get group for sudo: %w", err)
}

if targetUserExists {
logrus.Debugf("Modifying user %s with UID %d:", targetUser, targetUserUid)
if _, err := user.Lookup(targetUser); err != nil {
logrus.Debugf("Adding user %s with UID %d:", targetUser, targetUserUid)

usermodArgs := []string{
"--append",
useraddArgs := []string{
"--groups", sudoGroup,
"--home", targetUserHome,
"--home-dir", targetUserHome,
"--no-create-home",
"--shell", targetUserShell,
"--uid", fmt.Sprint(targetUserUid),
targetUser,
}

logrus.Debug("usermod")
for _, arg := range usermodArgs {
logrus.Debug("useradd")
for _, arg := range useraddArgs {
logrus.Debugf("%s", arg)
}

if err := shell.Run("usermod", nil, nil, nil, usermodArgs...); err != nil {
return fmt.Errorf("failed to modify user %s with UID %d: %w", targetUser, targetUserUid, err)
if err := shell.Run("useradd", nil, nil, nil, useraddArgs...); err != nil {
return fmt.Errorf("failed to add user %s with UID %d: %w", targetUser, targetUserUid, err)
}
} else {
logrus.Debugf("Adding user %s with UID %d:", targetUser, targetUserUid)
logrus.Debugf("Modifying user %s with UID %d:", targetUser, targetUserUid)

useraddArgs := []string{
usermodArgs := []string{
"--append",
"--groups", sudoGroup,
"--home-dir", targetUserHome,
"--no-create-home",
"--home", targetUserHome,
"--shell", targetUserShell,
"--uid", fmt.Sprint(targetUserUid),
targetUser,
}

logrus.Debug("useradd")
for _, arg := range useraddArgs {
logrus.Debug("usermod")
for _, arg := range usermodArgs {
logrus.Debugf("%s", arg)
}

if err := shell.Run("useradd", nil, nil, nil, useraddArgs...); err != nil {
return fmt.Errorf("failed to add user %s with UID %d: %w", targetUser, targetUserUid, err)
if err := shell.Run("usermod", nil, nil, nil, usermodArgs...); err != nil {
return fmt.Errorf("failed to modify user %s with UID %d: %w", targetUser, targetUserUid, err)
}
}

Expand Down

0 comments on commit 6bd7c87

Please sign in to comment.