Use POSIX mode for large numbers in tar archives (#4384) by schmidt-galen-heb · Pull Request #4388 · testcontainers/testcontainers-java

schmidt-galen-heb · 2021-08-20T18:27:16Z

Resolves #4384

junseok-oh-0 · 2021-08-21T14:10:14Z

Looks like there is one more place that uses TarArchiveOutputStream, it's org.testcontainers.images.builder.ImageFromDockerfile:124. But I'm not sure how to test it or whether it should be done at this time.
@kiview What do you think?

kiview · 2021-08-23T06:17:32Z

Thanks for rasing this PR @schmidt-galen-heb and thanks for pointing out the other relevant part in the code @batt842.

Could any of you run this test in your environment, so we see if we need the fix at this location as well?

schmidt-galen-heb · 2021-08-23T15:03:42Z

Thanks for pointing that out. I ran the test with commons-compress 1.21 and it passed, however I still think that location needs to be updated as well.

In investigating that test, I was able to determine why the build started failing in 1.21:

From the original stacktrace:

Caused by: java.lang.IllegalArgumentException: user id '<redacted>' is too big ( > 2097151 ).
	at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.failForBigNumber(TarArchiveOutputStream.java:651)
	at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.failForBigNumber(TarArchiveOutputStream.java:639)
	at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.failForBigNumbers(TarArchiveOutputStream.java:630)
	at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.putArchiveEntry(TarArchiveOutputStream.java:377)
	at org.testcontainers.utility.MountableFile.recursiveTar(MountableFile.java:327)
	at org.testcontainers.utility.MountableFile.transferTo(MountableFile.java:304)
	at org.testcontainers.containers.ContainerState.copyFileToContainer(ContainerState.java:276)
	at org.testcontainers.containers.ContainerState.copyFileToContainer(ContainerState.java:253)
	at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:684)
	at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:414)
	... 39 more

MountableFile uses the TarArchiveEntry(File, String) constructor to create the archive entry. In 1.20, this did not populate the user ID property, however in 1.21, they added a call to readOsSpecificProperties(...), which does populate the user ID.

So although the test passes, if a user were to call ImageFromDockerfile.withFileFromTransferable(...) with a MountableFile, it would fail.

schmidt-galen-heb · 2021-08-30T20:18:17Z

@kiview Anything else you'd like from my side on this?

kiview · 2021-08-31T08:48:15Z

@schmidt-galen-heb
Thanks a lot, LGTM. Let the others have a look and then we merge 👍

rnorth · 2021-09-06T14:20:26Z

This looks fine, but I'm confused by this comment by @schmidt-galen-heb:

So although the test passes, if a user were to call ImageFromDockerfile.withFileFromTransferable(...) with a MountableFile, it would fail.

Are you saying that this problem still exists, or it did exist but is now fixed? 😄

schmidt-galen-heb · 2021-09-07T17:05:52Z

Are you saying that this problem still exists, or it did exist but is now fixed? 😄

Sorry, it did exist, but is fixed by the change in ImageFromDockerfile.java

rnorth · 2021-09-07T17:06:29Z

Are you saying that this problem still exists, or it did exist but is now fixed? 😄

Sorry, it did exist, but is fixed by the change in ImageFromDockerfile.java

Great! Thanks for clarifying.

jayleenli · 2021-10-21T20:41:43Z

Currently still having this issue with version 1.16.1, was the fix pushed to the next release?

kiview · 2021-10-22T07:39:04Z

Hey @jayleenli, sorry to hear that, the fix is included in 1.16.1.

@schmidt-galen-heb @batt842 Can you verify, that the bug as you observed it is fixed in 1.16.1?

schmidt-galen-heb · 2021-10-22T15:56:46Z

Hey @kiview, I can confirm that upgrading to 1.16.1 resolved the issue for me

junseok-oh-0 · 2021-10-28T13:17:54Z

@kiview It works for me too.
Thanks @schmidt-galen-heb

abialas · 2021-11-25T15:01:28Z

I still have this issue on version 1.16.2 and MacOS:

Caused by: org.testcontainers.containers.ContainerFetchException: Can't get Docker image: RemoteDockerImage(imageName=, imagePullPolicy=DefaultPullPolicy())
at org.testcontainers.containers.GenericContainer.getDockerImageName(GenericContainer.java:1326)
at org.testcontainers.containers.GenericContainer.logger(GenericContainer.java:643)
at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:326)
... 36 more
Caused by: java.lang.IllegalArgumentException: group id '34548085' is too big ( > 2097151 ). Use STAR or POSIX extensions to overcome this limit

murat3 · 2021-12-08T14:47:29Z

I can confirm this issue still exist in 1.16.2 on MacOS

Caused by: java.lang.IllegalArgumentException: group id '5361194532 is too big ( > 2097151 ). Use STAR or POSIX extensions to overcome this limit
at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.failForBigNumber(TarArchiveOutputStream.java:651)
at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.failForBigNumberWithPosixMessage(TarArchiveOutputStream.java:644)
at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.failForBigNumbers(TarArchiveOutputStream.java:626)
at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.putArchiveEntry(TarArchiveOutputStream.java:377)
at org.testcontainers.shaded.com.github.dockerjava.core.util.CompressArchiveUtil.addFileToTar(CompressArchiveUtil.java:41)
at org.testcontainers.shaded.com.github.dockerjava.core.util.CompressArchiveUtil.archiveTARFiles(CompressArchiveUtil.java:106)
at org.testcontainers.shaded.com.github.dockerjava.core.dockerfile.Dockerfile$ScannedResult.buildDockerFolderTar(Dockerfile.java:134)
at org.testcontainers.shaded.com.github.dockerjava.core.dockerfile.Dockerfile$ScannedResult.buildDockerFolderTar(Dockerfile.java:124)
at org.testcontainers.shaded.com.github.dockerjava.core.command.BuildImageCmdImpl.withDockerfile(BuildImageCmdImpl.java:346)
at org.testcontainers.shaded.com.github.dockerjava.core.command.BuildImageCmdImpl.withDockerfile(BuildImageCmdImpl.java:23)
at org.testcontainers.images.builder.ImageFromDockerfile.lambda$configure$0(ImageFromDockerfile.java:154)
at java.util.Optional.ifPresent(Optional.java:159)
at org.testcontainers.images.builder.ImageFromDockerfile.configure(ImageFromDockerfile.java:153)
at org.microshed.testing.testcontainers.internal.ImageFromDockerfile.configure(ImageFromDockerfile.java:45)
at org.testcontainers.images.builder.ImageFromDockerfile.resolve(ImageFromDockerfile.java:109)
at org.testcontainers.images.builder.ImageFromDockerfile.resolve(ImageFromDockerfile.java:37)
at org.testcontainers.utility.LazyFuture.getResolvedValue(LazyFuture.java:17)
at org.testcontainers.utility.LazyFuture.get(LazyFuture.java:39)
at org.testcontainers.shaded.com.google.common.util.concurrent.Futures$3.get(Futures.java:1332)
at org.testcontainers.images.RemoteDockerImage.getImageName(RemoteDockerImage.java:115)
at org.testcontainers.images.RemoteDockerImage.resolve(RemoteDockerImage.java:64)
at org.testcontainers.images.RemoteDockerImage.resolve(RemoteDockerImage.java:28)
at org.testcontainers.utility.LazyFuture.getResolvedValue(LazyFuture.java:17)
at org.testcontainers.utility.LazyFuture.get(LazyFuture.java:39)
at org.testcontainers.containers.GenericContainer.getDockerImageName(GenericContainer.java:1324)

schmidt-galen-heb · 2021-12-08T16:16:50Z

From your stack trace, the issue is actually in docker-java and not testcontainers.

Specifically, the same changes from this PR will need to be applied to CompressArchiveUtil.java, and once released, testcontainers will need to be updated to the new version of docker-java

murat3 · 2021-12-08T18:56:46Z

Yes indeed. I created a new issue on docker-java:
docker-java/docker-java#1747

todor-kolev · 2022-01-16T12:55:20Z

Yes indeed. I created a new issue on docker-java: docker-java/docker-java#1747

Is there a workaround of any sort, do you know?

schmidt-galen-heb requested review from bsideup, kiview and rnorth as code owners August 20, 2021 18:27

schmidt-galen-heb mentioned this pull request Aug 20, 2021

user id 'xxx' is too big ( > 2097151 ) when using commons-compress 1.21 with large user ID #4384

Closed

Use POSIX mode for large numbers in tar archives (#4384)

66cb810

PR feedback

9646274

kiview approved these changes Aug 31, 2021

View reviewed changes

rnorth approved these changes Sep 7, 2021

View reviewed changes

junseok-oh approved these changes Sep 8, 2021

View reviewed changes

rnorth merged commit 8746f20 into testcontainers:master Sep 8, 2021

kbalston mentioned this pull request Sep 15, 2021

Support uid values larger than 2097151 (common for enterprise users) batect/batect#1027

Merged

kiview added the type/bug label Oct 13, 2021

kiview added this to the next milestone Oct 13, 2021

Uh oh!

Conversation

schmidt-galen-heb commented Aug 20, 2021

Uh oh!

junseok-oh-0 commented Aug 21, 2021

Uh oh!

kiview commented Aug 23, 2021

Uh oh!

schmidt-galen-heb commented Aug 23, 2021

Uh oh!

schmidt-galen-heb commented Aug 30, 2021

Uh oh!

kiview commented Aug 31, 2021

Uh oh!

rnorth commented Sep 6, 2021

Uh oh!

schmidt-galen-heb commented Sep 7, 2021

Uh oh!

rnorth commented Sep 7, 2021

Uh oh!

jayleenli commented Oct 21, 2021

Uh oh!

kiview commented Oct 22, 2021

Uh oh!

schmidt-galen-heb commented Oct 22, 2021

Uh oh!

junseok-oh-0 commented Oct 28, 2021

Uh oh!

abialas commented Nov 25, 2021

Uh oh!

murat3 commented Dec 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

schmidt-galen-heb commented Dec 8, 2021

Uh oh!

murat3 commented Dec 8, 2021

Uh oh!

todor-kolev commented Jan 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

murat3 commented Dec 8, 2021 •

edited

Loading