jib icon indicating copy to clipboard operation
jib copied to clipboard

Support base image layer compressed with zstd

Open rquinio opened this issue 1 year ago • 0 comments

This is a proposal to resolve #3714

Changes:

  • Use commons-compress automatic algorithm detection via magic bytes
  • Add optional dependency zstd-jni from commons-compress to support application/vnd.oci.image.layer.v1.tar+zstd layers

Before filing a pull request, make sure to do the following:

  • [x] Create a new issue at https://github.com/GoogleContainerTools/jib/issues/new/choose.
  • [ ] Ensure that your implementation plan is approved by the team.
  • [x] Verify that integration tests and unit tests are passing after the change.
  • [x] Address all checkstyle issues. Refer to the style guide.

This helps to reduce the chance of having a pull request rejected.

rquinio avatar Jul 30 '22 15:07 rquinio

No worries. Apache Commons compress does rely on optional dependency zstd-jni (https://commons.apache.org/proper/commons-compress/examples.html#Zstandard, https://github.com/apache/commons-compress/blob/aeb1defce5676b5c15a1dc377960fdcf119cddb4/pom.xml#L120 ), which is a binding around the reference C implementation.

There is also a pure Java port listed on https://facebook.github.io/zstd/#other-languages, but I don't know its status.

Checking the other implementations, moby and podman use the Golang port https://pkg.go.dev/github.com/klauspost/compress/zstd#section-readme (https://github.com/giuseppe/docker/blob/e187eb2bb5f0c3f899fe643e95d1af8c57e89a73/pkg/archive/archive.go#L25 and https://github.com/containers/podman/blob/67e7b2d6e3788c3560a852156a3505f0e2b6c8be/vendor/modules.txt#L485)

A short-term option could be to keep the transitive dependency to zstd-jni as optional in jib-core (i.e. test scope), and ask users to add it as a direct dependency if they have to deal with "application/vnd.oci.image.layer.v1.tar+zstd" layers, and re-assess when these layers become widely used...

rquinio avatar Aug 13 '22 11:08 rquinio

Hi,

re-assess when these layers become widely used

From what I understand Red Hat plans (in podman) are to enable it by default, when Docker 22.06 will be out (first non beta docker release that will support zstd layers) and spread enough among the different installations.

Cheers, Romain

Romain-Geissler-1A avatar Aug 13 '22 16:08 Romain-Geissler-1A

Ok, I've tried the Gradle feature, which I understand will translate to an optional dependency in Maven pom. Also I've added an FAQ entry for NoClassDefFoundError: com/github/luben/zstd/ZstdOutputStream so users know what to do in case that happens.

rquinio avatar Aug 20 '22 13:08 rquinio

To reach 80% on 6 lines changed I had to cover the 2 lines dealing with exception ^^ Should now be 100%

rquinio avatar Aug 24 '22 20:08 rquinio

Yeah, Sonar gets really weird on small changes.

elefeint avatar Aug 24 '22 20:08 elefeint