Only use podman unshare if not root
#117
Conversation
src/oci/mod.rs
Outdated
| @@ -13,6 +13,7 @@ use oci_spec::image::{Descriptor, ImageConfiguration, ImageManifest, MediaType}; | |||
| use sha2::{Digest, Sha256}; | |||
| use tokio::{io::AsyncReadExt, sync::Semaphore}; | |||
|
|
|||
| use crate::util::is_podman_container; | |||
There was a problem hiding this comment.
I'd just inline that. Either as a function in the same file or just directly inline... I'd inline the constant, too :)
src/oci/mod.rs
Outdated
| @@ -69,7 +70,8 @@ type ContentAndVerity<ObjectID> = (Sha256Digest, ObjectID); | |||
| impl<ObjectID: FsVerityHashValue> ImageOp<ObjectID> { | |||
| async fn new(repo: &Arc<Repository<ObjectID>>, imgref: &str) -> Result<Self> { | |||
| // See https://github.com/containers/skopeo/issues/2563 | |||
| let skopeo_cmd = if imgref.starts_with("containers-storage:") { | |||
| // TODO: Also check here for Docker container... | |||
| let skopeo_cmd = if imgref.starts_with("containers-storage:") && !is_podman_container() { | |||
There was a problem hiding this comment.
I don't think this check is completely correct. It would break my usecase for sure.
Specifically: I'm running in a toolbox, as a non-root user, and when I want to pull skopeo containers-storage: URLs I want to use podman unshare to escape the container. It's a very common practice to have some kind of a flatpak-spawn --host-based script for that in toolbox-type environments...
Checking if we were root would get a bit closer to a solution here, I guess...
What happened to the idea of mounting the container's rootfs inside of itself using --mount= and building the image from that?
There was a problem hiding this comment.
Checking if we were root would get a bit closer to a solution here, I guess...
Probably, but I think what we actually need anyways is to override the proxy configuration entirely anyways - ref bootc-dev/bootc#1292 (comment) right?
What happened to the idea of mounting the container's rootfs inside of itself using --mount= and building the image from that?
A bit confused by that; / is already the merged rootfs, why would we mount it again?
But the real problem is that we don't want the merged root, we want the layers so we can store them separately and we want the manifest/config.
There was a problem hiding this comment.
Interesting. I didn't think about the config, but I guess that has some useful stuff in it.
Why do we want to store the layers separately?
There was a problem hiding this comment.
Why do we want to store the layers separately?
I think it's a fundamental advantage of OCI that it has the concept of layers, and we don't want to re-fetch layers that didn't change across updates. Using intelligently split layers is a huge network optimization.
There was a problem hiding this comment.
Sure, but by the time we go to install the image, we've already downloaded the entire thing, no?
There was a problem hiding this comment.
Yep absolutely, but we want to retain distinct knowledge of the layers so that the next bootc upgrade doesn't re-fetch all of the content again (xref bootc-dev/bootc#1197 ) and we only have a hope of doing that if we retain knowledge of the layer structure.
Calling `podman unshare` from inside a rootful container fails, which breaks image pulls in said case. We have sufficient privileges to pull from containers storage if we are in a rootful container thus don't need unshare Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
5d68bad to
e9031a8
Compare
podman unshare outside of a containerpodman unshare if not root
Updated to check for root instead of checking if we're inside a podman container