virtual-fs: Hide package contents under bind‑mounts by filtering overlay secondaries - #6161
Conversation
|
|
||
| fn permission_error_or_not_found(&self, path: &Path) -> Result<(), FsError> { | ||
| for fs in self.secondaries.filesystems() { | ||
| for fs in self.secondaries_iter(path) { |
There was a problem hiding this comment.
I've tried to find a better way to avoid replacing every call to secondaries, but couldn't find it. Suggestion are welcome
| } | ||
|
|
||
| /// Get a reference to the secondary filesystems. | ||
| pub fn secondaries(&self) -> &S { |
There was a problem hiding this comment.
secondaries (and following secondaries_mut) are public and contains old behaviour without filtering. I didn't find the usage of it in wasmer though, only in tests, so not sure if that behaviour should be fixed here or not?
And I guess as it's public then changing function signature is not possible, so changing that to the iterator is not acceptable, so not sure how to proceed with those
| assert_eq!( | ||
| overlay | ||
| .primary() | ||
| .metadata(Path::new("/app/wp-content/themes")) |
There was a problem hiding this comment.
Test reproduces exact manual test that we've discovered
| ); | ||
| assert_eq!( | ||
| overlay_fs | ||
| .metadata("/app/wp-content/themes".as_ref()) |
There was a problem hiding this comment.
Same test, but on the overlay layer, like an integration
| // primary rather than rename it | ||
| if !had_at_least_one_success { | ||
| for fs in self.secondaries.filesystems() { | ||
| let secondaries: Vec<_> = self.secondaries_iter(&from).collect(); |
There was a problem hiding this comment.
What about directly iterating the self.secondaries_iter(&from) Iteration output? Any reason for the collect here?
| if had_at_least_one_success { | ||
| for fs in self.secondaries.filesystems() { | ||
| let secondaries: Vec<_> = self.secondaries_iter(&from).collect(); | ||
| for fs in secondaries { |
|
This is a tricky one, will need to dive in a bit more |
|
@theduke ping! |
f1b8d24 to
e35b484
Compare
theduke
left a comment
theduke
left a comment
There was a problem hiding this comment.
I believe this PR may now be redundant, after the recent changes made to package mounting and MountFs.
@Arshia001 , can you validate?
|
Closing, since this is not needed anymore with our new mounting structure. |
3 participants
Host bind‑mounts were incorrectly merged with package filesystem contents, causing package files (and even directories) to appear inside the mounted path and leak back to the host.
This PR makes bind‑mounted paths opaque to the overlay by filtering secondary filesystems under those prefixes, so the host mount replaces the package path (basically like in Docker) while keeping package mounts working for other paths.