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

Copies of originally-compressed images from c/storage to uncompressed destinations don’t trigger MIME type updates #2182

Closed
mtrmac opened this issue Nov 10, 2023 · 1 comment · Fixed by #2273
Labels
kind/bug A defect in an existing functionality (or a PR fixing it)

Comments

@mtrmac
Copy link
Collaborator

mtrmac commented Nov 10, 2023

Originally reported in containers/podman#20611 (comment) : pull a zstd:chunked image, then podman save (to docker-archive) fails with

Error: creating an updated image manifest: Error during manifest conversion: "application/vnd.oci.image.layer.v1.tar+zstd": zstd compression is not supported for docker images

What happens here:

  • The original manifest is recoded, with a zstd MIME type
  • Because the data is actually stored uncompressed, LayerInfosForCopy returns an uncompressed MIME type
  • imageCopier.copyLayers does account for the LayerInfosForCopy changes by setting srcInfosUpdated = true. But that isn’t sufficient, because it is documented that ManifestUpdateOptions.LayerInfos ignores the MIME type, the edit must be represented by CompressionOperation+CompressionAlgorithm.
  • imageCopier.copyLayer has code to compensate for this, by setting CompressionAlgorithm based on MIME type. But it doesn’t set CompressionOperation: Decompress for uncompressed MIME types; and it only applies on the TryReusingBlob code path.
  • On the primary “full copy” code path, CompressionOperation+CompressionAlgorithm are set separately, based just on the characteristics of the incoming blob. And when the blob is uncompressed, and the desired state is also uncompressed, there is “nothing to do”, and the code sets CompressionOperation: PreserveOriginal, CompressionAlgorithm: nil, i.e. “do no edits”.
  • As a result, the original manifest with a zstd MIME type is not updated to correctly represent an uncompressed layer, and a conversion to schema2 then fails.

Ugh. This is going to be a mess to untangle.

  • Maybe it’s time to finally deprecate the edit fields of BlobInfo, and build some internal-only edit interface afresh? (A lot of work, and what would that look like?)
  • Or the approach from the top of copyLayer, and have copyBlobFromStream preserve the supplied operation if it makes no changes?
@mtrmac
Copy link
Collaborator Author

mtrmac commented Feb 5, 2024

This manifests in existing Skopeo tests TestCopyWithManifestListStorage, TestCopyWithManifestListStorageDigest, TestCopyWithManifestListStorageDigestMultipleArches, and TestCopyWithManifestListStorageMultiple: copying from containers-storage: to dir: creates uncompressed layers (with uncompressed sizes and digits) with original on-registry compressed MIME types. Compare failures in containers/skopeo#2214 ’s containers/skopeo@fe72637 vs. containers/skopeo#2213 ’s containers/skopeo@2fda8e1 .

So, containers/skopeo#2213 will update the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A defect in an existing functionality (or a PR fixing it)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant