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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

the-sun-will-rise-tomorrow
Copy link

@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
Author

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

@the-sun-will-rise-tomorrow
Copy link
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
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
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
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!

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
7 participants