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

Fix adding symlink to the sandbox paths #10456

Merged
merged 3 commits into from
Apr 11, 2024
Merged

Fix adding symlink to the sandbox paths #10456

merged 3 commits into from
Apr 11, 2024

Conversation

thufschmitt
Copy link
Member

Bind-mounting symlinks is apparently not possible, which is why the
thing was failing.

Fortunately, symlinks are small, so we can fallback to copy them at no cost.

Fix #9579

Priorities and Process

Add 👍 to pull requests you find important.

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

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Apr 10, 2024
Théophane Hufschmitt and others added 3 commits April 10, 2024 15:17
…o a symlink out of the store

Bind-mounting symlinks is apparently not possible, which is why the
thing was failing.

Fortunately, symlinks are small, so we can fallback to copy them at no cost.

Fix #9579

Co-authored-by: Artturin <[email protected]>
Doesn't change much, but brings a bit more consistency to the code
@thufschmitt
Copy link
Member Author

(cc @Artturin who wrote the initial version in #9723)

@thufschmitt thufschmitt merged commit da1e977 into master Apr 11, 2024
17 checks passed
@thufschmitt thufschmitt deleted the fixpermdeniedbind branch April 11, 2024 11:41
@thufschmitt thufschmitt added the backport 2.18-maintenance Automatically creates a PR against the branch label Apr 11, 2024
Copy link

Backport failed for 2.18-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.18-maintenance
git worktree add -d .worktree/backport-10456-to-2.18-maintenance origin/2.18-maintenance
cd .worktree/backport-10456-to-2.18-maintenance
git switch --create backport-10456-to-2.18-maintenance
git cherry-pick -x 872d93eb13f22e8705e03903b65c7eba8b26a99b 913db9f7385b8717d9eaf6269e9f319e78e4c564 ae4737294e91ab93526612b17950e1bc4f0b47f0

@thufschmitt thufschmitt added backport 2.19-maintenance Automatically creates a PR against the branch and removed backport 2.18-maintenance Automatically creates a PR against the branch labels Apr 11, 2024
Copy link

Successfully created backport PR for 2.19-maintenance:

@thufschmitt thufschmitt added the backport 2.21-maintenance Automatically creates a PR against the branch label Apr 11, 2024

This comment was marked as resolved.

Copy link

Successfully created backport PR for 2.21-maintenance:

@thufschmitt thufschmitt added the backport 2.20-maintenance Automatically creates a PR against the branch label Apr 11, 2024

This comment was marked as resolved.

Copy link

Successfully created backport PR for 2.20-maintenance:

Copy link

Git push to origin failed for 2.21-maintenance with exitcode 1

@Ericson2314
Copy link
Member

Thanks @thufschmitt for picking this up, I am sorry I let the last one stagnate.

thufschmitt pushed a commit to tweag/nix that referenced this pull request Apr 12, 2024
NixOS#10456 fixed the addition of symlink
store paths to the sandbox, but also made it so that the hardcoded
sandbox paths (like `/etc/hosts`) were now bind-mounted without
following the possible symlinks. This made these files unreadable if
there were symlinks (because the sandbox would now contain a symlink to
an unreachable file rather than the underlying file).
In particular, this broke FOD derivations on NixOS as `/etc/hosts` is a
symlink there.

Fix that by canonicalizing all these hardcoded sandbox paths before
adding them to the sandbox.
@dasJ
Copy link
Member

dasJ commented Jul 1, 2024

This breaks on NixOS when having static resolv.conf. Nix now copies the /etc/resolv.conf symlink (pointing to /etc/static) to the sandbox, resulting in a dangling symlink and breaking all DNS resolution inside the sandbox.

2.19.4:
image

2.19.5:
image

tebowy pushed a commit to tebowy/nix that referenced this pull request Jul 11, 2024
Fix adding symlink to the sandbox paths

(cherry-picked from commit da1e977)

Change-Id: I221c85a38180800ec6552d2e86a88df48398fad8
tebowy pushed a commit to tebowy/nix that referenced this pull request Jul 11, 2024
grahamc added a commit to grahamc/system-configurations that referenced this pull request Aug 31, 2024
The upstream issue appears to be fixed by NixOS/nix#10456.
roberth pushed a commit that referenced this pull request Oct 14, 2024
#10456 fixed the addition of symlink
store paths to the sandbox, but also made it so that the hardcoded
sandbox paths (like `/etc/hosts`) were now bind-mounted without
following the possible symlinks. This made these files unreadable if
there were symlinks (because the sandbox would now contain a symlink to
an unreachable file rather than the underlying file).
In particular, this broke FOD derivations on NixOS as `/etc/hosts` is a
symlink there.

Fix that by canonicalizing all these hardcoded sandbox paths before
adding them to the sandbox.

(cherry picked from commit acbb152)
roberth pushed a commit that referenced this pull request Oct 14, 2024
#10456 fixed the addition of symlink
store paths to the sandbox, but also made it so that the hardcoded
sandbox paths (like `/etc/hosts`) were now bind-mounted without
following the possible symlinks. This made these files unreadable if
there were symlinks (because the sandbox would now contain a symlink to
an unreachable file rather than the underlying file).
In particular, this broke FOD derivations on NixOS as `/etc/hosts` is a
symlink there.

Fix that by canonicalizing all these hardcoded sandbox paths before
adding them to the sandbox.

(cherry picked from commit acbb152)
mergify bot pushed a commit that referenced this pull request Oct 29, 2024
#10456 fixed the addition of symlink
store paths to the sandbox, but also made it so that the hardcoded
sandbox paths (like `/etc/hosts`) were now bind-mounted without
following the possible symlinks. This made these files unreadable if
there were symlinks (because the sandbox would now contain a symlink to
an unreachable file rather than the underlying file).
In particular, this broke FOD derivations on NixOS as `/etc/hosts` is a
symlink there.

Fix that by canonicalizing all these hardcoded sandbox paths before
adding them to the sandbox.

(cherry picked from commit acbb152)
(cherry picked from commit 1cc79f1)

# Conflicts:
#	tests/functional/linux-sandbox.sh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.19-maintenance Automatically creates a PR against the branch backport 2.20-maintenance Automatically creates a PR against the branch backport 2.21-maintenance Automatically creates a PR against the branch with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Permission denied error when building symlink derivation
4 participants