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

docker: Allow building for non-root user #9854

Merged
merged 6 commits into from
Nov 18, 2024

Conversation

the-sun-will-rise-tomorrow
Copy link
Contributor

@the-sun-will-rise-tomorrow the-sun-will-rise-tomorrow commented Jan 25, 2024

Motivation

Add options uid, gid, uname, and gname to docker.nix.

Setting these to e.g. 1000, 1000, "user", "user" will build an image which runs and allows using Nix as that user.

Context

Although (with NixOS/nixpkgs#281520) it's possible to simply do a chown ... /nix in fakeRootCommands, this is quite inefficient. A change only in ownership is not representable as a Docker layer, so the contents of the entire Nix store will end up being copied in the final layer. This is what NixOS/nixpkgs#282886 fixes and this PR builds on.

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@Ericson2314
Copy link
Member

I don't understand everything involved on the docker side, but the code change itself looks sensible good to me. Seems like a good companion to #789 too.

@thufschmitt thufschmitt marked this pull request as draft January 26, 2024 06:11
@thufschmitt
Copy link
Member

(marked as draft since it depends on open Nixpkgs PRs, so we don't want to accidentally merge it right now).

I'm a bit reluctant in general when it comes to extending the Docker setup because I'm not sure that we want to maintain a non-trivial Docker setup. But that change is great since it's – mostly – a thin layer on top of the Nixpkgs-side heavy lifting. So 👍

@roberth
Copy link
Member

roberth commented Feb 28, 2024

@cole-h

This comment was marked as resolved.

@dhess
Copy link

dhess commented Apr 6, 2024

Is this ready to merge now?

@the-sun-will-rise-tomorrow
Copy link
Contributor Author

Just noticed this was still missing, PR opened: NixOS/nixpkgs#302113

@the-sun-will-rise-tomorrow
Copy link
Contributor Author

Rebased; I think all dependencies are now satisfied.

@the-sun-will-rise-tomorrow the-sun-will-rise-tomorrow marked this pull request as ready for review June 8, 2024 16:31
@mmlb
Copy link

mmlb commented Oct 22, 2024

Looks like this has fallen through the cracks?

@dhess
Copy link

dhess commented Nov 10, 2024

Yes, can we get this merged, please?

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

This PR still needs:

  • tests: new test case in tests/nixos/?
  • documentation, doc/manual/source/installation/installing-docker.md

I apologize for the other maintainers who have merged insufficiently documented and untested code. That should not have happened.

Add options uid, gid, uname, and gname to docker.nix.

Setting these to e.g. 1000, 1000, "user", "user" will build an image
which runs and allows using Nix as that user.
@the-sun-will-rise-tomorrow
Copy link
Contributor Author

tests: new test case in tests/nixos/?

Added a very simple test. (I tried to put together a more complete test which does some store operations, but ran into this issue.)

documentation, doc/manual/source/installation/installing-docker.md

Thanks, added.

@the-sun-will-rise-tomorrow
Copy link
Contributor Author

the-sun-will-rise-tomorrow commented Nov 10, 2024

Added a very simple test. (I tried to put together a more complete test which does some store operations, but ran into this issue.)

I managed to get a more thorough test working with the help of a binary cache server machine, but I still can't figure out how to write a test that tries to actually build a derivation inside the container. We need to provide all dependencies offline (via the cache), but for some reason it's not happy with what's on the cache and tries to download some bootstrap tarballs, even if I try to warm up the cache by doing the same thing there first.

@roberth
Copy link
Member

roberth commented Nov 11, 2024

Nix's normal logging isn't the most helpful in this case because it gives you all the paths it needs, but if you pas -v or -vv it will tell you more about what it actually does. Probably the first path it queries is the one you need to provide (and repeat that solution strategy until it works).
What may also help is to build a simple derivation with builtins.derivation so that you control accurately what it depends on.
Otherwise you'll be copying Nixpkgs into the test derivation, slowing down the test.

I don't see the cache you're referring to, but I think you could bake the dependencies into the image.
For the expression file you put in the image, you could use a function storepath: builtins.appendContext storepath { ${storepath} = { path = true; } } to convert plain strings containing store paths into store paths that the Nix inside the image understands.

Feel free to push a work in progress commit so I can have a look.

@the-sun-will-rise-tomorrow
Copy link
Contributor Author

What may also help is to build a simple derivation with builtins.derivation so that you control accurately what it depends on.

Great idea, it worked, thanks!

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

This is great!

@roberth roberth enabled auto-merge November 15, 2024 10:49
We still have room to spare in vm_tests, as it's quicker than `nix flake check`
@dhess
Copy link

dhess commented Nov 15, 2024

Thanks for this, it'll be great to run Nix in containers as non-root.

If I understand correctly (from #5380 (comment)), pretty much the only thing that won't work is finding GC roots; is that right?

@the-sun-will-rise-tomorrow
Copy link
Contributor Author

If I understand correctly (from #5380 (comment)), pretty much the only thing that won't work is finding GC roots; is that right?

That would apply to a daemon (multi-user) setup. This container has a single-user installation, which makes sense for OCI containers. I think GC should work fine if everything in the container runs as the same user.

@roberth
Copy link
Member

roberth commented Nov 15, 2024

The container has a proper store db for the store object parts that aren't the path or data, like the references for GC. Single user setups are supported. The only problems I would expect is to perhaps corrupting the store by accident because it is less protected. Also the sandbox may or may not be possible to isolate properly, depending on the container runtime and settings.

@roberth roberth merged commit d8d5929 into NixOS:master Nov 18, 2024
11 checks passed
@dhess
Copy link

dhess commented Nov 18, 2024

Thanks all!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-11-18-nix-team-meeting-minutes-195/56229/1

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.

Support running Nix as a non-root user
8 participants