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

users-groups: add skel support via environment.etc.skel #78648

Closed
wants to merge 2 commits into from

Conversation

mkg20001
Copy link
Member

@mkg20001 mkg20001 commented Jan 28, 2020

Motivation for this change

For projects like https://github.com/mercode-org/meros-nix it makes sense to have the ability to customize a few things here and there. Since /etc/skel is available everywhere else, why not on nixOS?

This adds multiple options:

  • environment.etc.skel: Provides the directory to copy from
  • users.users..copySkel: Boolean, default true if createHome true. Enables copying of /etc/skel for that user.
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@worldofpeace
Copy link
Contributor

I recall having to do some funny interesting things without skel in NixOS

But I'm not sure how I feel about it being here. I don't see much of managing anything in the users homedir in NixOS. I'm wondering why it was never included in the first place.

@mkg20001
Copy link
Member Author

I'm wondering why it was never included in the first place.

The other PR #41858 was rejected because nixOS shouldn't manage the homedir.

Note that the backbone of the idea for that PR was what home-manager does today.

This PR is entirely different and adds a way to initialize the homedir once and then leave it be, so no managing, making it more similar to services.postgresql.initialScript

@aanderse
Copy link
Member

cc @grahamc who has opinions on this sort of thing


nodes = {
machine = { config, lib, pkgs, ... }: {
users.users.testuser = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't copySkel be set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's implict if createHome is set, but I think it should be implicitly set if createHome && isNormalUser

This tries to replicate the behaviour of the adduser command which automatically copies skel unless otherwise specified AFAIK

@matthewbauer
Copy link
Member

matthewbauer commented Jan 29, 2020

Would it be possible to get this to use /etc/skel? For instance, I might have some configuration like:

environment.etc."skel/.bashrc".text = ''
  export NIX_PATH="..."
'';

And would like it to work equally well with "immutableUsers" and useradd and pam_mkhomedir.

@mkg20001
Copy link
Member Author

@matthewbauer Yes, the trouble is just resolving the links and then copying just the contents (does cp have an option for that?). Would it be enough to just use /etc/skel or do I need to reference the actual store path?

@mkg20001
Copy link
Member Author

mkg20001 commented Feb 11, 2020

@matthewbauer I've implemented it to use /etc/skel now (from system.build.etc)

@mkg20001 mkg20001 changed the title users-groups: add environment.skel option users-groups: add skel support via environment.etc.skel Feb 11, 2020
@worldofpeace
Copy link
Contributor

Everything else seems fine to me.

machine.wait_for_unit("default.target")

machine.succeed("test -d /home/testuser/new-directory")
machine.succeed("test -f /home/testuser/new-file")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a test asserting that the testuser can delete these things?

@edolstra
Copy link
Member

edolstra commented Mar 24, 2020

Why do we need /etc/skel? We could also just specify a directory in the Nix store to use as a skeleton. The use of /etc should generally be avoided for configuration files/data that are internal to one program (in this case, update-users-groups.pl). Also, using a global directory like /etc/skel precludes having different skeletons for different users, which could be avoided if we had an option like users.users.*.skeleton = ... derivation that creates some files ....

@edolstra
Copy link
Member

BTW uncommon abbreviations like "skel" should be avoided in favour of "skeleton" or "template" or whatever.

@worldofpeace
Copy link
Contributor

Why do we need /etc/skel? We could also just specify a directory in the Nix store to use as a skeleton.

I think it would be a good idea to have an improved feature, but I think the scope of the PR was to be /etc/skel support in NixOS.

BTW uncommon abbreviations like "skel" should be avoided in favour of "skeleton" or "template" or whatever.

I think the abbreviation is what it's widely referred to http://www.linfo.org/etc_skel.html.
Though I could agree that copySkel doesn't really stick out to what this thing does. And it still gets implicitly enabled with createHome.

@edolstra
Copy link
Member

I think it would be a good idea to have an improved feature, but I think the scope of the PR was to be /etc/skel support in NixOS.

I don't think we need /etc/skel support in NixOS. Rather, what we want is a way to initialize home directories. But we don't have to copy the non-declarative mechanisms of other Linux distributions for that.

@worldofpeace
Copy link
Contributor

I think it would be a good idea to have an improved feature, but I think the scope of the PR was to be /etc/skel support in NixOS.

I don't think we need /etc/skel support in NixOS. Rather, what we want is a way to initialize home directories. But we don't have to copy the non-declarative mechanisms of other Linux distributions for that.

I see, I think that sounds reasonable. And your edit for a per-user option intrigues me.

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work as is, but I retract my approval because @edolstra actually raised some good points, and I'd actually enjoy this feature more with them.

@worldofpeace
Copy link
Contributor

@edolstra I believe the scope of your request is to make it more declarative, but still just for initialization of a user's home directory with createHome? I'm mentioning this because I remembered home-manager's home.file https://github.com/rycee/home-manager/blob/master/modules/files.nix.

@mkg20001
Copy link
Member Author

@edolstra After a talk with @worldofpeace we realized #41858 seems more like you want this PR to be. But there's already home-manager's home.file and /etc/skel is just a simple standard, if users need more I feel like they're really better off just switching to home-manager.

What do you think?

@edolstra
Copy link
Member

Yeah, #41858 (or home-manager) seems like a better fit. The issue with /etc/skel is that it's the sort of standard that we don't really want to follow in NixOS because it's not very declarative (e.g. if users modify it outside of their NixOS configuration). Having said that, any option specifying the initial contents of a stateful entity is inherently a non-congruent form of configuration management, so it doesn't really matter that much...

@erdnaxe
Copy link
Contributor

erdnaxe commented Mar 29, 2020

It may be worth mentioning that the following actually works :

users.users.<user>.createHome = false;
security.pam.services.login.makeHomeDir = true;
security.pam.services.sddm.makeHomeDir = true;
security.pam.makeHomeDir.skelDirectory = "${./user-skeleton}";

This enables users to create NixOS live media without a /etc/skel directory.

@mkg20001
Copy link
Member Author

Don't see any chance of this getting merged, so see ya downstream fellas

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

Successfully merging this pull request may close these issues.

None yet

7 participants