Skip to content
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

Don't create accounts::home_dir resources #123

Merged
merged 1 commit into from
Jan 17, 2018

Conversation

arjenz
Copy link
Contributor

@arjenz arjenz commented Jan 12, 2018

Even with managehome set to false, accounts::home_dir resources need to be unique, implying all managed users should have a unique homedir. On default CentOS (and probably others) installs some system users share / as homedir.

Don't create a accounts::home_dir resource when managehome
is set to false.

@hunner
Copy link
Contributor

hunner commented Jan 12, 2018

@arjenz Hmm. It looks like if managehome => false then accounts::home_dir really does nothing other than raise a warning when ssh keys are passed (https://github.com/puppetlabs/puppetlabs-accounts/blob/1.2.1/manifests/home_dir.pp#L131-L133). So that means this change is appropriate and should simplify the home_dir define logic :D.

Could you remove the parameter and references to $managehome from accounts::home_dir and move that ssh key warning up to user.pp? That will also fix the failing tests. Thanks!

@arjenz arjenz force-pushed the managehome branch 2 times, most recently from c3c7d01 to fa204e8 Compare January 13, 2018 20:40
Don't create a `accounts::home_dir` resource when managehome
is set to false.

Move warning about not managing sshkeys outside of `accounts::home_dir`
@arjenz
Copy link
Contributor Author

arjenz commented Jan 13, 2018

@hunner Done!

@hunner hunner merged commit eb7275f into puppetlabs:master Jan 17, 2018
hunner added a commit that referenced this pull request Jan 17, 2018
Don't create accounts::home_dir resources
@hunner
Copy link
Contributor

hunner commented Jan 17, 2018

Thanks!

@arjenz arjenz deleted the managehome branch January 17, 2018 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants