New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to modify a user #434

Merged
merged 7 commits into from Oct 28, 2016

Conversation

Projects
None yet
3 participants
@arichardet
Copy link
Contributor

arichardet commented Oct 26, 2016

Add the ability to modify a user #278
Fixes #425

@rebeccaskinner

This comment has been minimized.

Copy link
Contributor

rebeccaskinner commented Oct 26, 2016

Could you add a samples/userModify.hcl (and a blackbox test if you think we need one)? I've noticed that we've had a lot of regressions introduced lately that we could have caught earlier if we'd had more cases being exercised by blackbox tests. Thanks!

@stevendborrelli stevendborrelli added this to the 0.4.0 milestone Oct 26, 2016

@rebeccaskinner

This comment has been minimized.

Copy link
Contributor

rebeccaskinner commented Oct 26, 2016

Using this module as an example:

user.user "created" {
  username = "test1"
  home_dir  = "/home/test1-home"
  state    = "present"
}

file.content "a" {
  destination = "/home/test1-home/foo"
  content = "foo"
  depends = ["user.user.created"]
}

user.user "modified" {
  username = "test1"
  home_dir   = "/home/test1"
  move_dir  = true
  groupname = "daemon"
  depends   = ["user.user.created","file.content.a"]
}

I found it a bit surprising that during initial user creation the home directory wasn't created, but then during user modification if the new home directory target existed it would give me an error.

It would be nice if it would create the home directory for you (unless you specify not to) and if it would not error when moving the home directory (maybe just give the user ownership over everything in that directory and merge it with the existing home directory).

If merging the directories isn't really feasible it would be nice to have a more explicit error message, the one I got was:

root/user.user.modified:
 Error: user modify: usermod: exit status 12
 Messages:
 Has Changes: yes
 Changes: No changes
@arichardet

This comment has been minimized.

Copy link
Contributor

arichardet commented Oct 27, 2016

I added fields for adding a user:

  • create_home creates the home directory
  • skel_dir contents are copied to the home directory if create_home is true
@arichardet

This comment has been minimized.

Copy link
Contributor

arichardet commented Oct 27, 2016

The error returned when modifying a user home_dir is "usermod: exit status 12", which indicates the home_dir specified for the modification already exists. This is not an error and the home directory name is modified to the new value indicated. Since we are unsure what other corner cases may be encountered with this status code, I am not in favor of suppressing the error.

@arichardet arichardet force-pushed the feature/user-user branch from 8d69c3c to dc8ba67 Oct 27, 2016

@rebeccaskinner
Copy link
Contributor

rebeccaskinner left a comment

LGTM

@arichardet arichardet merged commit 4dda174 into master Oct 28, 2016

2 checks passed

codeclimate Code Climate didn't find any new or fixed issues.
Details
wercker/build Wercker pipeline passed
Details

@arichardet arichardet deleted the feature/user-user branch Oct 28, 2016

BrianHicks pushed a commit that referenced this pull request Dec 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment