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

Allow mounting to inherited fuse fd (part of #150) #537

Closed
wants to merge 1 commit into from

Conversation

vladem
Copy link
Contributor

@vladem vladem commented Sep 27, 2023

Description of change

  • This change allows passing /dev/fd/N as a mount_dir, which will make mountpoint to bypass mounting and serve fuse requests at the provided file descriptor;
  • This allows mountpoint being executed without CAP_SYS_ADMIN and with no_new_privs flag, which corresponds to k8s's allowPrivilegeEscalation=false and is enabled by default;
  • Similar feature is provided by mount.fuse3 command and by gcs-fuse;
  • Changes are mostly located in vendor/fuser, making fuser::mnt::Mount struct public is required for testing and may be avoided.

Relevant issues: #150

Does this change impact existing behavior?

This is not a breaking change.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

Signed-off-by: Vladislav Volodkin <[email protected]>
@vladem vladem had a problem deploying to PR integration tests September 27, 2023 12:02 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests September 27, 2023 12:02 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests September 27, 2023 12:02 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests September 27, 2023 12:02 — with GitHub Actions Failure
@vladem
Copy link
Contributor Author

vladem commented Sep 27, 2023

Hey, noticed a test fuse_tests::readdir_test::readdir_mock failing with error 'called `Result::unwrap()` on an `Err` value: Os { code: 22, kind: InvalidInput, message: "Invalid argument" }', mountpoint-s3/tests/fuse_tests/mod.rs:124:10. While it occurs in the altered part of code, not sure if it's caused by my changes, as I am not able to reproduce it locally:

$ cargo test --features fuse_tests -- readdir_mock --nocapture
<...>
running 1 test
test fuse_tests::readdir_test::readdir_mock ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 96 filtered out; finished in 0.11s
<...>

Any ideas of how to reproduce error happening in CI?

Also let me know your thoughts about altering fuser fork kept at vendor/, I expect it can be burden to merge from upstream afterwards, but also don't see a better way of achieving the result we need.

@antoniomdk
Copy link

Awesome work! this is a feature I'm really interested in using. Any chance this can get reviewed and merged?

@jamesbornholt
Copy link
Member

@antoniomdk we're still evaluating whether we actually need this or not. Could you tell us a little more about your use case?

@antoniomdk
Copy link

@jamesbornholt thanks for the heads up. We are using mountpoint-s3 to load ML training data efficiently in K8s, but it requires a sidecar container with CAP_SYS_ADMIN capabilities. Probably this PR would resolve that. The other alternative we are considering is using a device plugin to register the /dev/fuse device and make it available without elevated permissions.

@jamesbornholt
Copy link
Member

Gotcha. We're actively working on a CSI driver that wraps Mountpoint, which you can track the progress of here: #150. The sidecar container permissions issue is definitely on our radar there.

@vladem vladem closed this Nov 22, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 17, 2024
## Description of change

fuser v0.15.0 added support for creating a `Session` from existing FUSE
file descriptor (via `Session::from_fd`). This PR adds this support to
Mountpoint. It allows passing FUSE file descriptor as mount point in the
form of `/dev/fd/{fd}`.

An example usage of this feature can be seen with a helper Go script,
[mounthelper.go](https://github.com/awslabs/mountpoint-s3/blob/86bdefa5147a7edc533a6be5d2724fec74ba91fb/examples/fuse-fd-mount-point/mounthelper.go):

```bash
$ go build mounthelper.go
$ sudo /sbin/setcap 'cap_sys_admin=ep' ./mounthelper # `mount` syscall requires `CAP_SYS_ADMIN`, alternatively, `mounthelper` can be run as root
$ ./mounthelper -mountpoint /tmp/mountpoint -bucket bucketname
bucket bucketname is mounted at /dev/fd/3
2024/11/07 17:23:42 Filesystem mounted, waiting for ctrl+c signal to terminate 

$ # in a different terminal session
$ echo "Hello at `date`" > /tmp/mountpoint/helloworld
$ cat /tmp/mountpoint/helloworld
Hello at Thu Nov  7 17:32:33 UTC 2024
$ rm /tmp/mountpoint/helloworld
$ cat /tmp/mountpoint/helloworld
cat: /tmp/mountpoint/helloworld: No such file or directory
```

Relevant issues: This PR resurrects a previous PR to add this feature:
#537

## Does this change impact existing behavior?

Shouldn't affect any existing behavior as we had an “is directory?”
check for passed mount points before, and it shouldn't have been
possible to pass a file descriptor as a mount point prior to this
change.

## Does this change need a changelog entry in any of the crates?

Updated CHANGELOG for `mountpoint-s3`.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

---------

Signed-off-by: Burak Varli <[email protected]>
Signed-off-by: Burak Varlı <[email protected]>
Signed-off-by: Burak Varlı <[email protected]>
Signed-off-by: Daniel Carl Jones <[email protected]>
Signed-off-by: Daniel Carl Jones <[email protected]>
Co-authored-by: Daniel Carl Jones <[email protected]>
Co-authored-by: Daniel Carl Jones <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants