-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
dockertools: fix buildLayeredImage nix-store permissions #94243
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the change!
A similar PR was submitted a few days ago too: #93811, but we were waiting for a test. Since this one has also tests, @adrian-gierakowski do you mind if we continue from this PR? The changes are almost identical save the name of the function and this one has a test case.
@utdemir sounds good to me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests have passed on my system, I think this PR is good to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one little detail: "priviliged" refers to containers that run with extra capabilities that could harm the host.
Otherwise looks good. Thank you for adding a test case!
@roberth Valid point! I applied your suggestion just now using the fancy github feature. :) I'll squash those extra commits into my test-commit in a sec. |
…h buildLayeredImage Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
e394a11
to
f5db415
Compare
@ofborg test docker-tools |
Ofborg is happy too! Thanks :) |
Motivation for this change
Fix bug in
dockerTools.buildLayeredImage
that causes non-root containers to be not runnable.Pardon my python, but I was unsure how - in the prettiest way - to hook into the existing logic around /nix/store dir-creation.
example docker error message:
The cause of the error is that
/nix
and/nix/store
has wrong permissions inside the final image, i.e.:The top-level directories lack world execute permissions, which, is ok as long we run the image as root, but fails when the image is run as an unprivileged user.
Steps to reproduce
docker load <$(nix-build . -A dockerTools.examples.bashLayeredWithUser)
docker run --rm -it -u somebody bash-layered-with-user /bin/echo "it works"
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)cc @LnL7 @utdemir