more generic and full zstd compression support#562
Merged
cyphar merged 9 commits intoopencontainers:mainfrom Apr 4, 2025
Merged
more generic and full zstd compression support#562cyphar merged 9 commits intoopencontainers:mainfrom
cyphar merged 9 commits intoopencontainers:mainfrom
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #562 +/- ##
==========================================
+ Coverage 72.43% 72.72% +0.28%
==========================================
Files 60 65 +5
Lines 4985 5089 +104
==========================================
+ Hits 3611 3701 +90
- Misses 1000 1010 +10
- Partials 374 378 +4
🚀 New features to boost your workflow:
|
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The non-distributable stuff has all been deprecated by image-spec, so there's no need to support it yet. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
add() was only ever used by Add() and the logic was already kind of poorly split between them, so just merge the implementations. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
tych0
previously approved these changes
Apr 3, 2025
Member
tych0
left a comment
There was a problem hiding this comment.
Overall looks pretty reasonable to me.
The previous implementation of the BytesRead() would store the value of bytes read in the compressor implementation singleton, meaning that if you were to compress multiple data streams you would end up with a meaningless value from BytesRead(). Not good. It also makes little sense to have the byte-counting logic embedded in the compression code -- there is only one user that cares about this and it's trivial to add a dummy io.Reader wrapper that counts the bytes read. Fixes: 8f65e8f ("mutate: add uncompressed blob size annotation") Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Until now we have had separate the compression and decompression logic and algorithm definitions. This has made it easy for folks to forget to add support for both (such as with zstd), and makes it more annoying to try to make the feature generic. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Rather than hard-coding a set of media-types, parse the "+foo" MIME type suffix to figure out what compression algorithm was used for the layer. This also enables zstd decompression support out of the box (since that is a registered algorithm now) and also improves our error messages -- we now provide a more helpful error to explain whether the issue is that the underlying MIME type of the layer is unsupported (think squashfs) or if it's just an issue with the compression algorithm. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
If a user doesn't specify what compression type they would like to (*Mutator).Add(), auto-select the compression algorithm by first trying to use whatever the last layer's compression algorithm was (if we support it) followed by a fallback to a default (gzip for now). It is preferable to try to re-use the previous compression algorithm so that users which operate on zstd-compressed images will produce zstd-compressed layers rather than going with gzip. For now, make this the default for the CLI as well. A future patch will add support for a --compress=... flag that will let you configure this behaviour and explicitly set a compression method. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The default option is --compress=auto, which will try to match the existing layer compression. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
It would be nice to have a test using an upstream zstd-compressed image, rather than only using stuff we generate, but the tests we already have for --compress=... should be more than enough to verify that we are dealing with properly mixed compression types. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This was referenced Apr 4, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make our compression handling more generic, and support zstd decompression.
Also add a
--compressflag that lets users specify compression for new layers. The default configuration is--compress=autowhich (in contrast to the previous hardcode-gzip behaviour) will try to use the same compression algorithm as in the last layer. Seeumoci-repack(1)for more information.Signed-off-by: Aleksa Sarai cyphar@cyphar.com