-
-
Notifications
You must be signed in to change notification settings - Fork 15.1k
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 streamLayeredImage for symlinks #108416
dockerTools: Fix streamLayeredImage for symlinks #108416
Conversation
When archiving `/nix/store/foo` and `foo` is itself a symlink, we must not traverse the symlink target, but archive the `foo` symlink itself
@GrahamcOfBorg test docker-tools |
Could you turn your example from the issue into a regression test? The fix seems to make sense, but I'd appreciate a review by @utdemir. |
@GrahamcOfBorg test docker-tools |
This exercises layer creation in face of store path symlinks, ensuring they are not dereferenced, which can lead to broken layer tarballs
a16c85c
to
ffe5ff6
Compare
@GrahamcOfBorg test docker-tools |
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.
It does look good to me, given the tests pass. And I appreciate the test case.
Nice, thanks for this fix! 🏆 |
Thank you! |
When archiving
/nix/store/foo
andfoo
is itself a symlink, we mustnot traverse the symlink target, but archive the
foo
symlink itselfI think this is sufficient to fix #107021 but I don't know a lot about the streaming mechanics here.
Motivation for this change
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)