search

containers / storage

Container Storage Library
Apache License 2.0
558 stars 239 forks source link

overlay: drop check for mount_program AND force_mask #1970

Closed giuseppe closed 3 months ago

giuseppe commented 3 months ago

Using a mount_program is not a necessary requirement for users creating a shared store, as the store can be consumed by other users.

Stop enforcing this rule.

rhatdan commented 3 months ago

LGTM @saschagrunert @mtrmac @nalind PTAL

mtrmac commented 3 months ago

Probably best if someone else reviews this…

Using a mount_program is not a necessary requirement for users creating a shared store, as the store can be consumed by other users.

I don’t understand how “shared store” and “mount_program” interact.

It seems to me that when creating a layer applying a diff, we can set the permissions ourselves while untarring. Fine.

What happens with read-write layers? AFAICS in that case we really need the filesystem driver (in-kernel or FUSE) to enforce the mask … but I also see nothing that passes the value to the mount operation.

So I don’t understand how any of this works even before this PR, and I’m just confused.

giuseppe commented 3 months ago

What happens with read-write layers? AFAICS in that case we really need the filesystem driver (in-kernel or FUSE) to enforce the mask … but I also see nothing that passes the value to the mount operation.

yes a mount_program is still needed if you plan to use that same store, the issue though is that we require a mount_program even if the root user is not going to use that store to run containers, but only to share images to different users. The store can be made available also on a network file system, so users are not necessarily on the same machine. So we either require fuse-overlayfs to be installed, or provide a workaround with a dummy mount_program, since it doesn't matter when we pull an image.

mtrmac commented 3 months ago

Would it make sense to refuse to create RW layers if force_mask is used without mount_program, then? (I don’t see how that actually works, I don’t know what I’m saying…)

giuseppe commented 3 months ago

Would it make sense to refuse to create RW layers if force_mask is used without mount_program, then? (I don’t see how that actually works, I don’t know what I’m saying…)

sure we can do that. Added a check and submitted a new version

cgwalters commented 3 months ago

Don't know this code really well, but looks sane to me /approve

openshift-ci[bot] commented 3 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/containers/storage/blob/main/OWNERS)~~ [giuseppe] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
mtrmac commented 3 months ago

/lgtm

as I threatened.