oci: import images from containers-storage via skopeo's varlink socket by giuseppe · Pull Request #417 · composefs/composefs-rs

giuseppe · 2026-10-19T15:05:41Z

Add a way to import images from containers-storage without the data being copied through an intermediate OCI layout. The importer connects to a skopeo producer over a varlink socket and asks it for each layer.

The producer answers with a splitfdstream. File contents that are already on disk arrive as file descriptors, so reflinks or hardlinks can be used instead of re-reading and re-writing the data.

Needs: podman-container-tools/container-libs#651

cgwalters · 2026-10-19T15:35:34Z

        .partition(|(k, _)| k.starts_with(BOOT_IMAGE_REF_KEY))
 }

+const DOCKER_CONFIG_MEDIA_TYPE: &str = "application/vnd.docker.container.image.v1+json";


I had actually kind of wanted to take a hard stance that composefs only supports OCI.

We have an existing issue for this.

I understand the rationale behind wanting to support v2s2, but it has larger ramifications...right now we have many places which claim to return OCI types but with this will suddenly return v2s2...or fail right?

I am fine to drop this commit

cgwalters · 2026-10-19T15:38:18Z

                length,
                filename,
            } => {
-                let name = std::str::from_utf8(filename).with_context(|| {


This and some other changes look like they could be factored out into a clean "prep changes PR"?

Copilot

🟡 Changes recommended

Import compatibility, storage selection, config retrieval, and subprocess lifecycle regressions remain unresolved.

6 open issues

What changed in this PR

Adds containers-storage imports through skopeo’s varlink socket, avoiding intermediate OCI layouts and enabling file-content reuse.

Changes:

Adds proxy sessions and index-based layer requests.
Reuses verified layer transfers and supports non-UTF-8 filenames.
Updates image-type handling, logging, and integration tests.

File	Description
crates/composefs-oci/src/varlink_types.rs	Adds layer-index requests and errors.
crates/composefs-oci/src/skopeo_varlink.rs	Implements skopeo proxy imports.
crates/composefs-oci/src/oci_image.rs	Recognizes Docker container configs.
crates/composefs-oci/src/lib.rs	Routes local imports through skopeo.
crates/composefs-oci/src/layer_sync.rs	Supports byte-oriented filenames and transfer diagnostics.
crates/composefs-oci/src/cstor.rs	Shares transfer logic and verifies layer digests.
crates/composefs-oci/Cargo.toml	Adds version parsing and logging support.
crates/composefs-integration-tests/src/tests/cstor.rs	Updates references and import-stat assertions.
crates/composefs-integration-tests/src/lib.rs	Adds storage-reference formatting helper.

_{Review effort: Balanced}

+    tracing::debug!("skopeo proxy protocol version {version}");
+
+    // OpenImage
+    let (img_val, _, _) = proxy_call(&sock, "OpenImage", vec![imgref.into()])?;


+        "GetBlob",
+        vec![
+            img_id.into(),
+            config_descriptor.digest().to_string().into(),
+            config_descriptor.size().into(),


giuseppe · 2026-10-19T15:57:09Z

moved some patches here: #418

and dropped composefs-oci: accept the Docker v2 config media type

giuseppe · 2026-10-19T19:45:13Z

rebased

giuseppe · 2026-10-21T20:18:17Z

Importing from containers-storage no longer hardlinks the files. The old in-process importer could hardlink them into the repository, but that is not something that can be supported from containers-storage because a hardlink changes the inode's st_nlink and that affects containers.

cgwalters · 2026-10-21T21:43:41Z

 impl ImportContext {
+    /// Never hardlink source files into the repository; reflink or copy only.
+    ///
+    /// Use this when the source files belong to someone else.  Hardlinking


Hmm perhaps this should really be a feature flag advertised by the source?

let's leave this for later to figure out, but with containers-storage that is never safe (unless it is composefs)

Copilot

🟡 Changes recommended

Import compatibility regressions and missing layer-count validation remain unresolved.

6 open issues

4 resolved since last review

_{Review effort: Balanced}

+    let (mut reader, driver) = proxy
+        .get_blob(&img, config_descriptor.digest(), config_descriptor.size())
+        .await
+        .context("fetching config blob")?;


+    let diff_ids =
+        crate::extract_layer_ids(&manifest, &config).context("extracting layer IDs from config")?;


+    if let Some(opts) = storage_opts {
+        cmd.env("STORAGE_OPTS", opts);
+    }


cgwalters · 2026-10-22T15:54:30Z

+
+dest=${1:?usage: build-skopeo.sh DESTDIR}
+
+skopeo_repo=${SKOPEO_REPO:-https://github.com/containers/skopeo}


How about just fetching from a COPR or so?

Or alternatively of course, having skopeo ship binaries as part of its releases...which I know came up in the past, but seems more tractable now

That seems fine for a one-off test but how are we going to maintain that?

that = copr? or binaries in GH releases?

If this is about copr see https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/package/skopeo/ were you aware of that? I mean in general we should definitely be doing integration testing across the board on git main; it seems less work and more sustainable to keep the copr going which is already building from main right?

Oh is the issue here we haven't yet done the vendor bump for skopeo? But we can just...do that rihgt? Or is it blocking on having testing here?

copr and no I was not aware of that, thanks. I have opened a new PR for container-libs with some small followup fixes. I thought about doing the revendoring after it is merged, but I can do it twice if it is going to take too long

do we want to block on this or is it fine to move forward for now and I'll rework as soon as we have the fix in skopeo?

Not a blocker from my PoV

cgwalters

The bootc unified storage codepath relies on this functionality, but as of right now I think we effectively hard require the new skopeo for it to work, correct? So this will become a compatibility hazard where updating composefs-rs in bootc will break people who don't also have an updated skopeo.

I think it makes things messier but we should probably be conservative here and keep the direct import code around, and fall back if skopeo is too old?

giuseppe · 2026-10-27T15:22:17Z

ok fair, I'll drop the last patch and adjust the caller. Should I also drop the tests patch to not build skopeo?

ensure_object_from_file falls back from a reflink to hardlinking the source file into the repository, which is only right when the source is an object of another composefs repository. Hardlinking a file that belongs to someone else adds a name to its inode, so its link count changes, and enabling fs-verity on it makes it read-only for good. For files from containers-storage that is visible inside the containers using the image, so it is not safe. Add ImportContext::forbid_hardlinks() for callers whose sources are not theirs: the import then reflinks or copies, and errors in zerocopy mode if a reflink is not possible. The default is unchanged. Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>

privileged_test_cstor_idempotent_import asserted that the first import copied objects, but on a reflink-capable filesystem it copies none. Count every mechanism instead. Drop privileged_cstor_import_ext4_hardlink. It required the objects of a containers-storage import to be hardlinked into the repository, which enables fs-verity on, and shares the inode with, files that containers-storage still owns and may modify. That is not safe, and with the files now coming from skopeo's process the hardlinks are not possible anyway. Reflinks are safe, and the XFS test keeps covering them. Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>

import_layer_via_transfer drained the splitdirfdstream without checking what it reassembled against the diff ID it had asked for, and neither does the producer: containers-storage, reached through skopeo, only passes along what it has stored. Nothing tied the imported layer to the image's config. Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>

giuseppe · 2026-10-27T15:53:20Z

added the fallback

cgwalters · 2026-10-27T17:25:41Z

Should I also drop the tests patch to not build skopeo?

We need both to test both cases right? Basically in the GHA workflow we have one variant which has the newer skopeo, one which has the stock?

Add an import path that goes through skopeo's `experimental-image-proxy` and its `OpenVarlinkSocket` method to pull container images out of containers-storage. This replaces the direct containers-storage access in `pull()` for `containers-storage:` references, so the layer bytes no longer have to be re-read and re-hashed on this side. The flow: 1. Start `skopeo experimental-image-proxy` through containers-image-proxy (0.11.2 has ImageProxy::open_varlink_socket(), which this uses). Rootless, it runs under `podman unshare`. An explicit storage root and additional image stores are passed to it. 2. `OpenImage`, then the manifest and the config from the proxy. These come from the proxy rather than over varlink because it sits on containers/image: it knows which instance of a manifest list applies, rejects schema1, and normalises docker schema2 to OCI for us. The config has to be the blob the manifest points at, byte for byte; for a converted schema2 image that is the proxy's converted config, which is checked against the manifest's digest. 3. `OpenVarlinkSocket` with the opened image, to get the `org.composefs.Oci` socket serving that image's layers. 4. Transfer each layer via `GetLayer` / `import_layer_via_transfer`, and reject a config that lists a different number of layers than the manifest. 5. Finalize the OCI image in the composefs repository. Layers are asked for by their position in the image, using a new `layer_index` locator in GetLayerParams and a NoSuchLayerIndex error. Looking them up by the diff IDs in the config would only be right if containers-storage enforced those when the image was pulled, which it does not always do; otherwise a config could name the diff ID of a layer of an unrelated local image, and verifying the stream against the config would accept it. The layer is still verified against its diff ID as it is drained, which now ties the config to the layers that were actually pulled. The producer is told whether this side can bypass file permissions, so that it does not hand over descriptors we cannot open. The files come from containers-storage, so they are not hardlinked into the repository: that would change their link count, which the containers using the image can see. They are reflinked or copied. The reference is handed to the proxy verbatim, which is a behaviour change: the old path resolved the image itself, so it accepted an image ID written the way podman prints it, `sha256:<hex>`. The containers-storage transport does not - it reads that as the name `sha256` tagged `<hex>`, and `@sha256:<hex>` as a digest - so an ID now has to be written `@<id>`. Keeping the old spelling working would mean hardcoding "an image ID is the hex of a sha256 digest", which is exactly the assumption that is on its way out. The integration tests convert at the point they read podman's iidfile instead. Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>

The containers-storage import goes through skopeo's image proxy and its OpenVarlinkSocket method (protocol >= 0.2.9). The skopeo that CentOS ships speaks 0.2.6 / 0.2.8, so every privileged cstor test failed with "skopeo proxy version ... too old". Build skopeo, with the container-libs branch that adds the method, in a separate stage of the test Containerfile and install it over the distribution's. The cstor tests that only need user namespaces also run directly on hosts where they work, such as CI runners, and there they would find the host's skopeo. Have them check that the skopeo on PATH has the method and otherwise re-exec in the VM, like they already do without user namespaces. Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>

giuseppe · 2026-10-28T09:38:09Z

both versions pass now

cgwalters requested a balanced review from Copilot October 19, 2026 15:33

Copilot started reviewing on behalf of cgwalters October 19, 2026 15:34

View session

cgwalters requested changes Oct 19, 2026

View reviewed changes

Copilot AI reviewed Oct 19, 2026

View reviewed changes

giuseppe force-pushed the splitfdstream-from-skopeo branch from f3db15f to 028dbeb Compare October 19, 2026 15:55

giuseppe mentioned this pull request Oct 19, 2026

composefs-oci: logging and FileBackedData prep for the skopeo import #418

Merged

giuseppe force-pushed the splitfdstream-from-skopeo branch 2 times, most recently from 90aeb88 to 5d6194e Compare October 19, 2026 19:44

giuseppe force-pushed the splitfdstream-from-skopeo branch 7 times, most recently from 1267e59 to 37355bd Compare October 21, 2026 20:17

giuseppe force-pushed the splitfdstream-from-skopeo branch from 37355bd to 073321b Compare October 22, 2026 13:29

cgwalters requested a balanced review from Copilot October 22, 2026 14:32

cgwalters reviewed Oct 22, 2026

View reviewed changes

Copilot started reviewing on behalf of cgwalters October 22, 2026 14:33

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

giuseppe force-pushed the splitfdstream-from-skopeo branch from 073321b to fc825f8 Compare October 22, 2026 15:31

cgwalters reviewed Oct 22, 2026

View reviewed changes

giuseppe force-pushed the splitfdstream-from-skopeo branch from fc825f8 to 64276dc Compare October 26, 2026 09:01

cgwalters reviewed Oct 27, 2026

View reviewed changes

giuseppe added 3 commits October 27, 2026 16:34

giuseppe force-pushed the splitfdstream-from-skopeo branch from 64276dc to 6842474 Compare October 27, 2026 15:53

giuseppe force-pushed the splitfdstream-from-skopeo branch from 6842474 to 90e38da Compare October 27, 2026 17:37

giuseppe added 2 commits October 28, 2026 09:52

giuseppe force-pushed the splitfdstream-from-skopeo branch from 90e38da to 8f57ca1 Compare October 28, 2026 08:52

		let diff_ids =
		crate::extract_layer_ids(&manifest, &config).context("extracting layer IDs from config")?;


		dest=${1:?usage: build-skopeo.sh DESTDIR}

		skopeo_repo=${SKOPEO_REPO:-https://github.com/containers/skopeo}

Conversation

giuseppe commented Oct 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

giuseppe commented Oct 19, 2026

Uh oh!

giuseppe commented Oct 19, 2026

Uh oh!

giuseppe commented Oct 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

giuseppe commented Oct 27, 2026

Uh oh!

giuseppe commented Oct 27, 2026

Uh oh!

cgwalters commented Oct 27, 2026

Uh oh!

giuseppe commented Oct 28, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants