Skip to content

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Oct 20, 2022

This revisits fix provided regarding moby/moby#25122

According to https://www.rfc-editor.org/rfc/rfc9110.html#name-content-encoding

If the media type includes an inherent encoding, such as a data format that is always compressed, then that encoding would not be restated in Content-Encoding even if it happens to be the same algorithm as one of the content codings.

So, an image layer, served with media type application/vnd.oci.image.layer.v1.tar+gzip should not get any additional Content-Encoding header set until http server is misconfigured.

Using a preventive Accept-Encoding: identity allows to avoid such a misconfiguration to have consequences on image transfert, but on the other hand prevents adoption of any other transport optimization, while the default net/http golang transport transparently manages gzip and can at least compress json manifests.

This would also make it possible to publish images with uncompressed layers referenced in manifests, and get those served with gzip or a more efficient compression (zstd, or any future algorithm) using encoding negotiation between client and registry.

…ers"

This reverts commit 16f086a.

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

FWIW, we should consider this a bug; the original reason for adding this was effectively working around a mis-configured registry, and as a result we now have a client that is (partially) disfunctional. Perhaps worth backporting (as there's no release for v3 yet, so most consumers will still be using the v2 branch).

As a follow-up we should also consider implementing something like containerd/containerd#7563, but that's a larger change (and more a "feature").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants