core,scripts: When no cachedir is enabled in unified-core, disable FUSE #1591
Conversation
|
Part of #1332 |
| * anyways. | ||
| */ | ||
| g_autoptr(GKeyFile) config = ostree_repo_copy_config (self->pkgcache_repo); | ||
| g_key_file_set_boolean (config, "rpmostree", "enable-fuse", FALSE); |
There was a problem hiding this comment.
Can't we just tell the core directly not to use FUSE instead of adding the magical bit to the repo config? Feels a bit underhanded.
src/libpriv/rpmostree-core.c
Outdated
| @@ -524,6 +524,11 @@ rpmostree_context_set_repos (RpmOstreeContext *self, | |||
| { | |||
| g_set_object (&self->ostreerepo, base_repo); | |||
| g_set_object (&self->pkgcache_repo, pkgcache_repo); | |||
|
|
|||
| self->enable_fuse = | |||
There was a problem hiding this comment.
Minor/bikeshed: would enable_rofiles be a nicer name for this? Makes the correspondence to RPMOSTREE_BWRAP_MUTATE_ROFILES more obvious.
cgwalters
force-pushed
the
no-cachedir-no-fuse
branch
from
92bdb4c to
36d0c78
Compare
|
OK, this looks good! One concern: do we need to worry about files which map to the same file object but are transformed into different objects by scripts? I'm especially thinking about degenerate cases like empty files. |
|
Hmm, one potential way to handle this in libostree would be a flag to force copy if |
|
OK, we relied on fuse though to do the |
I don't understand this at all - since we're not using a devino cache...we'll just end up comitting whatever's in the target right? If the repo is corrupted, oh well, we're throwing it away anyways. |
|
Oh...like some case where files are hardlinked by libostree but aren't ordinarily hardlinks, and scripts mutating one don't expect to mutate all of them? Right...whee, yeah we could teach libostree about that. But, I don't think it should block this, if we hit those cases I figure we can work around it. |
Exactly! Fair enough, I'm fine for deferring this. |
|
This now blocks on #1592 |
This is prep for running inside (unprivileged) Kube containers as they exist today: #1329 Sadly FUSE today uses a suid binary that ends up wanting CAP_SYS_ADMIN. I think there's some work on FUSE-in-containers but I'm not sure of the current status. What rofiles-fuse here is doing here is protecting is the hardlinked repo imports. But if `--cachedir` isn't specified, that repository gets thrown away anyways. So there's no real value to using FUSE here. Also since nothing is cached, disable the devino cache. Down the line ideally we gain the capability to detect if either unprivileged overlayfs/FUSE are available. Then if `--cachedir` is specified we can make things work. Closes: #1591 Approved by: jlebon
cgwalters
changed the title
|
@rh-atomic-bot force |
|
💔 Test failed - status-atomicjenkins |
|
☔ The latest upstream changes (presumably 426e16e) made this pull request unmergeable. Please resolve the merge conflicts. |
cgwalters
force-pushed
the
no-cachedir-no-fuse
branch
from
36d0c78 to
62046c2
Compare
cgwalters
changed the title
|
Rebased 🏄♂️ and lifting WIP. This is just another step in the puzzle though. |
Hmm, same pkgset as the currently passing master at least as far as SELinux-related things are concerned. Seems like not using |
|
While I was working on the code I happened to see this bit: Which is a clearly topical comment here; annotate points to: I also noticed, comparing the logs of good vs bad: good: bad: |
|
Hmm, that error also echoes memories of #1002. Hmm, actually it might very well be the same issue:
This would be the case for the two lock files: #999 (comment) |
|
OK so your comment originally was very prescient 😄 I'll see about adding a libostree API to not hardlink empty files. That should solve most pathological cases. |
cgwalters
force-pushed
the
no-cachedir-no-fuse
branch
from
1585b04 to
e8e0c48
Compare
|
But yeah ugh: |
|
bot, retest this please |
|
☔ The latest upstream changes (presumably 096f8de) made this pull request unmergeable. Please resolve the merge conflicts. |
cgwalters
force-pushed
the
no-cachedir-no-fuse
branch
2 times, most recently
from
a089958 to
a6adb0e
Compare
|
This one needs ostreedev/ostree#1753 too. |
|
bot, retest this please |
1 similar comment
|
bot, retest this please |
|
OK, now passing tests ✔️. |
These shouldn't be in the package; the fact that they're empty files causes libostree to hardlink them which breaks things. See also coreos#1002
This is basically overriding what happens with `bare-user` mode OSTree repositories. I put a lot of thought into avoiding creating suid files with that mode. But today this creates a situation where if we don't have a devino cache, the file will lose its suid bits. In the end, since we're using the "inaccessible directory" pattern anyways for rpm-ostree on the host, we don't need to really worry about transient suid binaries. And similarly when we're run inside an existing container, that's also fine.
This is prep for running inside (unprivileged) Kube containers as they exist today: coreos#1329 Sadly FUSE today uses a suid binary that ends up wanting CAP_SYS_ADMIN. I think there's some work on FUSE-in-containers but I'm not sure of the current status. What rofiles-fuse here is doing here is protecting is the hardlinked repo imports. But if `--cachedir` isn't specified, that repository gets thrown away anyways. So there's no real value to using FUSE here. Also since nothing is cached, disable the devino cache. We also make use of --force-copy-zerosized that just landed in libostree: ostreedev/ostree#1752 Down the line ideally we gain the capability to detect if either unprivileged overlayfs/FUSE are available. Then if `--cachedir` is specified we can make things work.
cgwalters
force-pushed
the
no-cachedir-no-fuse
branch
from
3de3f84 to
3490ce7
Compare
This is basically overriding what happens with `bare-user` mode OSTree repositories. I put a lot of thought into avoiding creating suid files with that mode. But today this creates a situation where if we don't have a devino cache, the file will lose its suid bits. In the end, since we're using the "inaccessible directory" pattern anyways for rpm-ostree on the host, we don't need to really worry about transient suid binaries. And similarly when we're run inside an existing container, that's also fine. Closes: #1591 Approved by: jlebon
This is prep for running inside (unprivileged) Kube containers as they exist today: #1329 Sadly FUSE today uses a suid binary that ends up wanting CAP_SYS_ADMIN. I think there's some work on FUSE-in-containers but I'm not sure of the current status. What rofiles-fuse here is doing here is protecting is the hardlinked repo imports. But if `--cachedir` isn't specified, that repository gets thrown away anyways. So there's no real value to using FUSE here. Also since nothing is cached, disable the devino cache. We also make use of --force-copy-zerosized that just landed in libostree: ostreedev/ostree#1752 Down the line ideally we gain the capability to detect if either unprivileged overlayfs/FUSE are available. Then if `--cachedir` is specified we can make things work. Closes: #1591 Approved by: jlebon
|
@rh-atomic-bot retry |
This is basically overriding what happens with `bare-user` mode OSTree repositories. I put a lot of thought into avoiding creating suid files with that mode. But today this creates a situation where if we don't have a devino cache, the file will lose its suid bits. In the end, since we're using the "inaccessible directory" pattern anyways for rpm-ostree on the host, we don't need to really worry about transient suid binaries. And similarly when we're run inside an existing container, that's also fine. Closes: #1591 Approved by: jlebon
This is prep for running inside (unprivileged) Kube containers as they exist today: #1329 Sadly FUSE today uses a suid binary that ends up wanting CAP_SYS_ADMIN. I think there's some work on FUSE-in-containers but I'm not sure of the current status. What rofiles-fuse here is doing here is protecting is the hardlinked repo imports. But if `--cachedir` isn't specified, that repository gets thrown away anyways. So there's no real value to using FUSE here. Also since nothing is cached, disable the devino cache. We also make use of --force-copy-zerosized that just landed in libostree: ostreedev/ostree#1752 Down the line ideally we gain the capability to detect if either unprivileged overlayfs/FUSE are available. Then if `--cachedir` is specified we can make things work. Closes: #1591 Approved by: jlebon
|
☀️ Test successful - status-atomicjenkins |
This is prep for running inside (unprivileged) Kube containers
as they exist today: #1329
Sadly FUSE today uses a suid binary that ends up wanting CAP_SYS_ADMIN.
I think there's some work on FUSE-in-containers but I'm not sure of
the current status.
What rofiles-fuse here is doing here is protecting is the hardlinked
repo imports. But if
--cachedirisn't specified, that repositorygets thrown away anyways. So there's no real value to using FUSE
here.
Also since nothing is cached, disable the devino cache.
Down the line ideally we gain the capability to detect if either
unprivileged overlayfs/FUSE are available. Then if
--cachediris specified we can make things work.