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

Add support to enable/disable user creation. #47

Closed

Conversation

apiening
Copy link

As stated in #46 the role fails when the root-user is included in the users: array, since this user may be in use by the ansible_user:

weareinteractive.users : Adding user 'root' to the system
usermod: user root is currently used by process 1

However, it does make sense having the root user in the users: array sine other roles may pick it up from there to configure per user tasks.
This is true for gantsign.antigen for example that manages the setup of oh-my-zsh and related plugins.

I added a variable user_create to the users: array which is queried by a when filter for the Adding user task.
The default can be set with the globally scoped var users_user_create.

I have also added an example case to the README.md file.

@apiening
Copy link
Author

May I ask for some feedback on this PR?
Does the change make sense to you?
If so: Is there a chance for this to be merged?

@franklinkim
Copy link
Member

Hi, this role is supposed to manage all basic users. It's not designed to do anything with root or existing users. Taking out the creation of this role could result it unwanted side effects...

@apiening
Copy link
Author

@franklinkim sure, I agree on that. But I think you missed my point: My suggestion was not to manage root or existing users in fact it is the opposite: I suggested to skip the root user or configurable users in general and don't do anything on this users instead of failing.

However I noticed you commited a check to skip the root user, which is a solution for the described issue.

There would still be an issue with other non-root-users that may be included in the users var and should not be touched by this role. This PR would have solved this issue as well.

@franklinkim
Copy link
Member

Sorry, I don't quite get the whole use case....could you provide some code?

Maybe you just need to define and pass the users variable differently for multiple roles...

@apiening
Copy link
Author

If you take a look at the example for the role variables of the role gantsign.antigen for example:
https://github.com/gantsign/ansible_role_antigen#role-variables

It is possible to extend a var users with attributes for specific configuration for Oh My ZSH for example. There are other roles out there adding Vim-configuration, Mutt-Configuration or really any user specific setup based on a users-list.
The idea is to have one list with users and add the required attributes just for the users that should be configured by a particular role. For example: If the attributes prefixed with antigen_ are not set for a user in the list, this role would skip the setup for this user while other roles might still pick the user up to setup something for this user.

The role weareinteractive.users does not offer a way to skip a user that is in the users list as far as I can see.
So it is not possible to setup something for a user with gantsign.antigen without having weareinteractive.users to create / configure the user which might already exist and should not be touched.

I think I have explained it more complicated than necessary, sorry.
Do you understand my issue?

@franklinkim
Copy link
Member

I think I know now where you're going with this...

So first off, this role is much older than the antigen one and it was supposed to be a base for other roles to build on top of it like e.g. ansible-users-oh-my-zsh and was not intended to run along side other roles doing something else...

I guess you're just trying to use the ssh key part of this role ... in this case I would recommend to clone this role and adapt it to your specific needs. I would like to keep it as a base role and not extend it to be able to mix it in...

@j8r j8r mentioned this pull request Sep 17, 2020
franklinkim added a commit that referenced this pull request Nov 5, 2020
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.

2 participants