AnotherKamila / bsdops

DebOps-derived reusable, integrated Ansible configs for FreeBSD-based machines
BSD 2-Clause "Simplified" License
10 stars 1 forks source link

Group name `root` everywhere! How do we handle this? #3

Open AnotherKamila opened 5 years ago

AnotherKamila commented 5 years ago

DebOps sets almost all config files to be owned by group root, which does not exist on FreeBSD. We have two options:

I am leaning towards the first option from the usability viewpoint, but it is very annoying for development. Opinions?

drybjed commented 5 years ago

Presumably, you could drop the group parameter from the tasks completely in the cases where the roles are executed as root. In that case, Ansible should use the primary group of the account that creates the file, which on FreeBSD would be wheel. In cases where the group could be overwritten by a parameter with root as the default, setting the default to omit should do the same thing. This of course can be done with the owner parameter as well.

To be clear, currently DebOps roles are assumed to be executed as root by default, either directly or via sudo. In most of the cases enforcing owner and group in file-based tasks wouldn't have to be explicit since otherwise Ansible would use the default user and primary group anyway (root on Linux). Having them set explicitly can have a marginal security implications, but it seems that alternative usage, like on FreeBSD, suffers for it. Perhaps not setting owner and group explicitly could lead to wider DebOps adoption and easier remixes(?) like BSDOps... @ypid, any thoughts on that front?

AnotherKamila commented 5 years ago

That's interesting! Thanks for the information, I think I'll go with removing it then.

Would it perhaps be useful to do this in the upstream repo (perhaps in its own branch) instead of in mine, or should I keep the change in my repo for now?

@drybjed BTW, I really appreciate that you jumped in and helped me out. I am new to Ansible and DebOps, so it's really really helpful.

ypid commented 5 years ago

I also like the being explicit part about this. This is also what Ansible recommends in their docs to be explicit. DebOps uses Ansible as config management engine to ensure a desired state it active. So even if the group of a file/dir is changed manually it will be changed back on the next run to our desired state. This only works if they are given explicitly. Creation of file/dir is another story of course which is what @drybjed referred to. So I would not drop them in DebOps but it might be an option for a patch-set outside of the DebOps master branch. The mono repo of DebOps makes such patches easy to maintain and use. Still patches outside of the master branch should be the last option.

Edit: I just realized that this is done as a fork of DebOps basically. Still, I think it could be interesting to "upstream" (push some changes back to DebOps).

I am in favor of using variables for this. As DebOps does not support *BSD and maintaining a variable in every role has some overhead I would suggest we check for a variable optionally set by debops.core and default to root if not set.

drybjed commented 5 years ago

@AnotherKamila I'm open to making changes in upstream that could make this fork(?) easier to manage, although I would be cautious on what types of changes are done there. Ideally, the code shouldn't be less readable than it is now. For now, I'm watching closely the changes you are implementing to make the project work on FreeBSD, mostly to check where DebOps could be changed to be more portable, although my focus is on Debian. It will be interesting to see it from another angle though.

drybjed commented 5 years ago

@ypid I was thinking about detecting the root primary group in the debops.root_account local facts. Then it could be available to be used in the tasks with something like this:

- name: Create a file
  copy:
    content: 'File contents'
    dest: '/tmp/file'
    owner: 'root'
    group: '{{ ansible_local.root_account.group
               if (ansible_local|d() and ansible_local.root_account|d() and
                   ansible_local.root_account.group|d())
               else "root" }}'
    mode: '0644'

But then I came up with dropping the owner and group entirely, which would result in:

- name: Create a file
  copy:
    content: 'File contents'
    dest: '/tmp/file'
    mode: '0644'

An alternative would be to set the root group globally, but DebOps does not have that functionality apart from using debops.core role, which would result in a similar code as the debops.root_account version.

I guess it's a thing to carefully think about and research. One other interesting result of dropping the explicit owner/group could be usage of some roles on unprivileged accounts, depending on changes to the file paths.

AnotherKamila commented 5 years ago

@ypid Variables for this would make it easier for me. I would be very happy if that appeared in upstream, as that would allow me to not need to diverge. I am happy to help with the PR if you (plural) decide do do that.

@drybjed Caution is wise, as many people depend on DebOps. I will be very happy to see changes that will make projects like this easier, but I will of course understand when you decide something isn't worth it.

I have spent only two afternoons on Ansible and DebOps and this project, so I do not yet feel qualified to propose changes upstream. However, I will be very happy to make PRs once I get a better idea of what might be a good idea (and of course only if you actually want them).

In terms of whether this is a fork, I think so :D I don't expect you to import most of my changes back upstream (even though they should be non-breaking), because that would add development and testing overhead for you, so I have made peace with maintaining a fork.

drybjed commented 5 years ago

@AnotherKamila I wrote about this topic more on the DebOps mailing list, I think that you might want to take a look. :-)

evilham commented 5 years ago

Oh, how come I miss that? :-D Debops' should be one of the few MLs that reach my inbox, apparently I changed something. Will read carefully over the weekend, thank you!

MirLach commented 8 months ago

I don't know if it makes any sense to comment 5 years later, but it's worth noting that not explicitly specifying a group when creating a file on FreeBSD results in the created file not having the primary owner group, but the group that the parent directory has. So when the root user creates a file in a directory that has a group nobody, the newly created file will have a group nobody and not wheel.