testcontainers / testcontainers-java

Testcontainers is a Java library that supports JUnit tests, providing lightweight, throwaway instances of common databases, Selenium web browsers, or anything else that can run in a Docker container.
https://testcontainers.org
MIT License
7.91k stars 1.62k forks source link

Creating images on the fly from a builder does not support authenticated pulls #5354

Open davidmc24 opened 2 years ago

davidmc24 commented 2 years ago

I'm attempting to use "Creating images on the fly" to build an image based on an existing image. While ImageFromDockerfile supports authenticated pre-pulls in many cases (based on parsing the Dockerfile), it currently doesn't appear to do so when withDockerfileFromBuilder is used. In this case, since the from I'm passing to the builder uses a repository that's authenticated with a credStore (Docker Desktop on Mac), I get an error com.github.dockerjava.api.exception.DockerClientException: Could not build image: Head "<PARENT_IMAGE_URL>": no basic auth credentials

https://www.testcontainers.org/features/creating_images/

eddumelendez commented 2 years ago

@davidmc24

You may have a ~/.docker/config.json like this:

{
        "auths": { },
        "credsStore": "desktop"
}

In order to download from a private registry you need to add the credentials here. By default docker use keychain in mac and in the flow you described the right credentials are not taken.

As a workaround you can do the following:

In your terminal run echo "username:password" | base64 and copy it in the ~/.docker/config.json

{
        "auths": {
                "<registry>": {"auth": "<base64-output>"}
        },
        "credsStore": "desktop"
}

<registry> can be ghcr.io for example.

davidmc24 commented 2 years ago

Thank you for your response.

That is an accurate representation of my config, and I had confirmed that adding basic auth credentials as you specified, but it is also insecure and should not be needed, as in general, TestContainers has features in place to use authenticated pulls to avoid needing to store credentials insecurely like that. This case, for example, is inconsistent with what the behavior would be if I had the exact same unchanged ~/.docker/config.json but used ImageFromDockerfile with a Dockerfile stored on disk instead of withDockerfileFromBuilder.

For those who may be interested my current (Spock test) workaround is to manually do a pull before creating the container.

        // Pre-pull the parent image.  TestContainers would normally do this automatically, but it apparently doesn't work with `withDockerfileFromBuilder` yet.
        new RemoteDockerImage(DockerImageName.parse(parentImage))
            .withImageNameSubstitutor(ImageNameSubstitutor.noop())
            .get()
        // Build a temporary image using
        def imageBuilder = new ImageFromDockerfile()
            .withDockerfileFromBuilder(builder -> builder
                .from(parentImage)
                // ... 
                .build()
            )
        new GenericContainer(imageBuilder)

Alternatively, if you're comfortable with exceeding access rights in Groovy tests, it's possible to do something like this (further demonstrating that this is built-in functionality that just isn't wired up in this case):

        def imageBuilder = new ImageFromDockerfile()
            .withDockerfileFromBuilder(builder -> builder
                .from(parentImage)
                // ...
                .build()
            )
        // Manually register a dependency on the parent image, so that it will be pre-pulled with proper authentication.
        // TestContainers would normally do this automatically, but it apparently doesn't work with `withDockerfileFromBuilder` yet.
        imageBuilder.dependencyImageNames = [parentImage]
        new GenericContainer(imageBuilder)
eddumelendez commented 2 years ago

I had confirmed that adding basic auth credentials as you specified, but it is also insecure and should not be needed, as in general, TestContainers has features in place to use authenticated pulls to avoid needing to store credentials insecurely like that.

Yes, and that's why I said As a workaround ... 😅

We will be looking at it.

Thanks for the report, @davidmc24 !

davidmc24 commented 2 years ago

@eddumelendez Ah, thanks for the clarification. I misunderstood.

eddumelendez commented 2 years ago

@davidmc24 we would like to promote the use of withFileFromString, withDockefile or any other method instead. is there anything that prevent to you to use those instead?

spmason commented 1 year ago

Just to add, auth doesn't seem to be working for dockerfiles taken from the classpath either (ie withFileFromClasspath("Dockerfile", <RESOURCE_NAME>) - on testcontainers v1.17.6)

For now I've worked around by extracting my Dockerfile to a temp directory then passing my path to withDockerfile, but it would be good if this worked as expected

kiview commented 1 year ago

Thanks for sharing the feedback @spmason, you are right, the reason is the special handling for the dockerfile field to pull the dependent image: https://github.com/testcontainers/testcontainers-java/blob/66bcd391df31c4afdac32ff325974fe0f62968c8/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java#L170-L179

This code won't get executed when using withFileFromClasspath. I would agree this not very user friendly. In the implementation, we have special semantic for the Dockerfile and when you call withFileFromClasspath("Dockerfile", <RESOURCE_NAME>), this file will not be considered specifically as the Dockerfile in the code.

Can you try the following workaround?

withDockerfile(Paths.get(MountableFile.forClasspathResource(<RESOURCE_NAME>).getResolvedPath()))
spmason commented 1 year ago

Thanks for the quick reply @kiview, that workaround works and is nicer than the code I'd written to deal with this

Would a reasonable fix for my particular issue be to add a new withDockerfileFromClassPath method to ClasspathTrait that basically just implements the workaround, or is there a fix in mind that would work for the original Builder issue as well?

kiview commented 1 year ago

@spmason As @eddumelendez mentioned, we are not particularly interested in further promoting the use of the builder in its current form, so I'd imagine adding such a method to ImageFromDockerfile or ClasspathTrait sounds like a good idea (not sure if it can be implemented in the trait though).

Would you like to look into contributing a PR?

spmason commented 1 year ago

Yes I’d be happy to open a PR. At the moment though the workaround is giving me some issues on our Linux build nodes (my initial testing was on Mac). I’ll need some time to dig into the problem

spmason commented 1 year ago

OK, the issue seems to be that my Dockerfile resource name is <folder>/Dockerfile, which gets extracted to a temporary folder under that sub-path, then the build can't seem to find the other files it refers to.

I've opened https://github.com/testcontainers/testcontainers-java/pull/6425 to shows what I mean