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

Missing check for null causes NullPointerException in PullImageResultCallback.checkForDockerSwarmResponse #1998

Open
TheHaf opened this issue Nov 11, 2022 · 9 comments

Comments

@TheHaf
Copy link

TheHaf commented Nov 11, 2022

The method checkForDockerSwarmResponse in the class PullImageResultCallback contains a call to item.getStatus() (where item is of type PullResponseItem) and immediately calls .matches(...) on the return value. However the method getStatus() is defined in the class with an annotation @CheckForNull indicating that the return value could be null. Because of that SonarLint marks the call as unsafe (rule java:S2259 - "Null pointers should not be dereferenced") and I was also able to trigger a NullPointerException:

09:48:54.334 [docker-java-stream--865997958] ERROR com.github.dockerjava.api.async.ResultCallbackTemplate - Error during callback
java.lang.NullPointerException: Cannot invoke "String.matches(String)" because "status" is null
	at com.github.dockerjava.api.command.PullImageResultCallback.checkForDockerSwarmResponse(PullImageResultCallback.java:51) ~[classes/:?]
	at com.github.dockerjava.api.command.PullImageResultCallback.onNext(PullImageResultCallback.java:37) ~[classes/:?]
	at org.testcontainers.images.LoggedPullImageResultCallback.onNext(LoggedPullImageResultCallback.java:48) ~[testcontainers-1.17.5.jar:?]
	at org.testcontainers.images.TimeLimitedLoggedPullImageResultCallback.onNext(TimeLimitedLoggedPullImageResultCallback.java:73) ~[testcontainers-1.17.5.jar:?]
	at org.testcontainers.images.TimeLimitedLoggedPullImageResultCallback.onNext(TimeLimitedLoggedPullImageResultCallback.java:24) ~[testcontainers-1.17.5.jar:?]
	at org.testcontainers.shaded.com.github.dockerjava.core.exec.AbstrAsyncDockerCmdExec$1.onNext(AbstrAsyncDockerCmdExec.java:41) ~[testcontainers-1.17.5.jar:?]
	at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder$JsonSink.accept(DefaultInvocationBuilder.java:315) ~[testcontainers-1.17.5.jar:?]
	at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder$JsonSink.accept(DefaultInvocationBuilder.java:298) ~[testcontainers-1.17.5.jar:?]
	at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder.lambda$executeAndStream$1(DefaultInvocationBuilder.java:275) ~[testcontainers-1.17.5.jar:?]
	at java.lang.Thread.run(Thread.java:833) ~[?:?]

I was able to trigger the NullPointerException using the TestContainers library to try to pull an image from a remote repository that needs authentication without being logged in to said repository.

My suggestion would be to change the first line in the method checkForDockerSwarmResponse to the following:

        String status = item.getStatus();
        if (status != null && status.matches("Pulling\\s.+\\.{3}$")) {

I can open a pull request for that change if that's ok.

@kiview
Copy link
Contributor

kiview commented Nov 25, 2022

Hi @TheHaf, since you mentioned this issue in the Podman issue I was wondering, where you able to replicate this using Docker as well?

@TheHaf
Copy link
Author

TheHaf commented Nov 28, 2022

I don't have docker installed on my work machine due to company policy, but I was able to test the behavior on our CI platform where docker is still available. 😸
It seems that using Docker, another code path is taken and ultimately the onError method of the ResultCallbackTemplate is being called according to the resulting error message:

09:42:06.507 [docker-java-stream--1741773083] ERROR com.github.dockerjava.api.async.ResultCallbackTemplate - Error during callback
com.github.dockerjava.api.exception.InternalServerErrorException: Status 500: {"message":"Head \"https://<redacted>/<path>/<image>/manifests/<version>\": no basic auth credentials"}
	at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder.execute(DefaultInvocationBuilder.java:247) ~[testcontainers-1.17.5.jar:?]
	at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder.lambda$executeAndStream$1(DefaultInvocationBuilder.java:269) ~[testcontainers-1.17.5.jar:?]
	at java.lang.Thread.run(Thread.java:829) ~[?:?]

I don't know what triggers this call of onError though without digging further.

So concerning the handling of auth problems, podman seems to behave differnet from Docker. I would see that as a separate issue though (and maybe on the side of podman once we know what triggers the different code path), as the kernel of this issue here is that the contract of a method (using @CheckForNull) is not being abided by the caller.

@AshwinPrabhuB
Copy link

I have the same error as well

Caused by: java.lang.NullPointerException: Cannot invoke "String.matches(String)" because the return value of "com.github.dockerjava.api.model.PullResponseItem.getStatus()" is null
at com.github.dockerjava.api.command.PullImageResultCallback.checkForDockerSwarmResponse(PullImageResultCallback.java:48)
at com.github.dockerjava.api.command.PullImageResultCallback.onNext(PullImageResultCallback.java:35)
at org.testcontainers.images.LoggedPullImageResultCallback.onNext(LoggedPullImageResultCallback.java:48)
at org.testcontainers.images.TimeLimitedLoggedPullImageResultCallback.onNext(TimeLimitedLoggedPullImageResultCallback.java:73)
at org.testcontainers.images.TimeLimitedLoggedPullImageResultCallback.onNext(TimeLimitedLoggedPullImageResultCallback.java:24)
at org.testcontainers.shaded.com.github.dockerjava.core.exec.AbstrAsyncDockerCmdExec$1.onNext(AbstrAsyncDockerCmdExec.java:41)
at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder$JsonSink.accept(DefaultInvocationBuilder.java:315)
at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder$JsonSink.accept(DefaultInvocationBuilder.java:298)
at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder.lambda$executeAndStream$1(DefaultInvocationBuilder.java:275)
at java.base/java.lang.Thread.run(Thread.java:833)

@kiview
Copy link
Contributor

kiview commented Jan 4, 2023

@AshwinPrabhuB Can you please share if you are using Docker, or an alternative container solution?

@AshwinPrabhuB
Copy link

AshwinPrabhuB commented Jan 4, 2023

This is observed only when using podman and if the local does not have the image. If I pull the image myself through podman pull command-line, the test container can run with the local copy. I am using Default pull policy, hence it does not bother downloading the remote image and the issue is avoided. This is a good enough workaround for personal workstations, but cannot guarantee the VMs used in CI pipelines will always carry the image locally.

I have not seen this problem with Docker and Rancher. This is Podman specific confirmed.

I landed here from a reference in test-containers issue: testcontainers/testcontainers-java#2088

@AshwinPrabhuB
Copy link

AshwinPrabhuB commented Jan 6, 2023

A good workaround for this issue for those using podman is to prefetch the image to local before test-containers bootstrap. This bug does not manifest if the image is available locally and default pull policy.

If you are using maven to orchestrate your test routines in CI pipeline, a good workaround is to use the maven exec plugin and perform a docker pull of the image from the remote in the "generate-test-resources" phase. This ensures that when the test-containers code executes in the "test" phase, the image is already available locally and this bug is worked around.

Ex:

          <plugin>
                <artifactId>exec-maven-plugin</artifactId>
                <version>${version.plugin.mojo}</version>
                <groupId>org.codehaus.mojo</groupId>
                <executions>
                    <execution>
                        <id>image-prefetch</id>
                        <phase>generate-test-resources</phase>
                        <goals>
                            <goal>exec</goal>
                        </goals>
                        <configuration>
                            <executable>bash</executable>
                            <commandlineArgs>${main.basedir}/pull.sh</commandlineArgs>
                        </configuration>
                    </execution>
                </executions>
            </plugin>

and in pull.sh, pull the image, say Redis like so:

docker pull docker.io/library/redis:latest --storage-opt ignore_chown_errors=true

To make this complete, you will also require to set up 2 environment variables DOCKER_HOST and TESTCONTAINERS_RYUK_DISABLED (this is Podman specific). The latter is needed to suppress Ryuk as it is currently not supported in Podman. You would have to stop the container after the test run through test code (shutdown hooks if using Java)

      <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <configuration>
                    <environmentVariables>
                        <!-- Required for executing test-containers with podman -->
                        <!-- The variable points to location of podman socket file that is created by the daemon executed via "podman system service" or "systemctl -user enable -now podman.socket" -->
                        <!-- Check the socket file exists: ls -la /run/user/$UID/podman/podman.sock -->
                        <DOCKER_HOST>unix://${env.XDG_RUNTIME_DIR}/podman/podman.sock</DOCKER_HOST>
                        <TESTCONTAINERS_RYUK_DISABLED>true</TESTCONTAINERS_RYUK_DISABLED>
                    </environmentVariables>
                    <systemPropertyVariables>
                        <java.net.useSystemProxies>true</java.net.useSystemProxies>
                    </systemPropertyVariables>
                 </configuration>
            </plugin>

The above works for maven setups, you may translate this solution to suit your build systems until this bug is fixed.

@TheHaf
Copy link
Author

TheHaf commented Mar 14, 2023

@AshwinPrabhuB Pulling the images beforehand is a good workaround, so thumbs up.

In our setup we are now caching all container images in a Nexus registry, so we set the environment variable TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX for Testcontainers in our Maven pom, forcing Testcontainers to provide a full name to the docker-java api. This also acts as a kind of workaround.

Back to the issue at hand, I tried to take a look at the official Docker API but I couldn't find anything about this getStatus() call. Maybe I was looking in the wrong place. Can anyone point me towards the documentation for that specific API call?

@kiview
Copy link
Contributor

kiview commented Apr 13, 2023

Back to the issue at hand, I tried to take a look at the official Docker API but I couldn't find anything about this getStatus() call. Maybe I was looking in the wrong place. Can anyone point me towards the documentation for that specific API call?

@TheHaf I did the same and also could not find it in the reference documentation. Sometimes with Docker, you can only find this information in their source code.

@avdv
Copy link

avdv commented Jun 12, 2023

Hi.

I ran into the same error, and I am also using podman with its docker socket. It seems if there is any error (auth required but failed / manifest not found in the registry) the status will be null, e.g.:

item = {PullResponseItem@44958} "ResponseItem(stream=null, status=null, progressDetail=ResponseItem.ProgressDetail(current=null, total=null, start=null), progress=null, id=null, from=null, time=null, errorDetail=ResponseItem.ErrorDetail(code=null, message=initializing source docker://xyz.jfrog.io/abc:0.1.2: reading manifest 0.1.2 in xyz.jfrog.io/abc: manifest unknown: The named manifest is not known to the registry.), error=initializing source docker://xyz.jfrog.io/abc:0.1.2: reading manifest 0.1.2 in xyz.jfrog.io/abc: manifest unknown: The named manifest is not known to the registry., aux=null)"
 stream = null
 status = null
 progressDetail = {ResponseItem$ProgressDetail@44960} "ResponseItem.ProgressDetail(current=null, total=null, start=null)"
 progress = null
 id = null
 from = null
 time = null
 errorDetail = {ResponseItem$ErrorDetail@44961} "ResponseItem.ErrorDetail(code=null, message=initializing source docker://xyz.jfrog.io/abc:0.1.2: reading manifest 0.1.2 in xyz.jfrog.io/abc: manifest unknown: The named manifest is not known to the registry.)"
 error = "initializing source docker://xyz.jfrog.io/abc:0.1.2: reading manifest 0.1.2 in xyz.jfrog.io/abc: manifest unknown: The named manifest is not known to the registry."
 aux = null

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

No branches or pull requests

4 participants