registryCredentials in docker extension are ignored if the local ~/.docker/config.json has "credsStore": "pass"

Question

registryCredentials in docker extension are ignored if the local ~/.docker/config.json has "credsStore": "pass"

tkrah opened this issue 2 years ago · 8 comments

Have a credsStore configured, e.g. pass to manage your docker credentials. If the repository in question is configured there the build will succeed and use that credentials from the credStore helper.

However, if you do not have all registries configured there, only a subset, but your project uses the docker extension to configure the credentials to use for another registry, that is ignored and the build will fail.

Example build.gradle:

import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage
import com.bmuschko.gradle.docker.tasks.image.DockerPushImage
import com.bmuschko.gradle.docker.tasks.image.DockerTagImage

plugins {
    id 'base'
    id 'com.bmuschko.docker-remote-api'
}

def dockergroup = "my.test.group"
def dockerRegistry = "HERE GOES YOUR REGISTRY"

docker {
    registryCredentials {
        url = "https://$dockerRegistry/v2/"
        username = project.property("repository_username")
        password = project.property("repository_password")
    }

}

tasks.register('prepareDocker') {
    doFirst {
        def dockerStageDir = "build/docker"

        delete "${dockerStageDir}"

        copy {
            from(".")
            include 'Dockerfile'
            into dockerStageDir
        }
    }
}

tasks.register('buildDocker', DockerBuildImage) {
    dependsOn('prepareDocker')
    buildArgs.set(['REGISTRY': dockerRegistry])
    images.add("${dockergroup}/test-docker:$project.version")
}

Dockerfile:

ARG REGISTRY
FROM $REGISTRY/httpd:2.2
RUN echo "Test"

Run:

gradle buildDocker -Prepository_username=XXX -Prepository_password=YYY

Expected Behavior

The docs have this:

By default, existing credentials are read from $HOME/.docker/config.json and reused for authentication purposes. You can overwrite those credentials with the help of the registryCredentials closure.

The build should work if credentials are configured in the docker extension for the registry in question.

Current Behavior

> Task :buildDocker FAILED
Building image using context '/tmp/test-docker/build/docker'.
Using images 'my.test.group/test-docker:unspecified'.
Step 1/3 : ARG REGISTRY
Step 2/3 : FROM $REGISTRY/httpd:2.2

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':buildDocker'.
> Could not build image: Head "https://$REGISTRY/v2/httpd/manifests/2.2": unknown: Authentication is required

If you remove the:

"credsStore": "pass"

from the docker.json and re-run the gradle buildDocker command the build will succeed.

Context

Have a pass credsStore configured in the ~/.docker/config.json.

Steps to Reproduce (for bugs)

See above.

Your Environment

Plugin 9.3.1, Gradle 8.1, JDK 17.

Answer 1 · 2023-04-25T23:29:57.000Z

I was kind of convinced that our test cases catch this use case e.g. via DockerRemoteApiPluginFunctionalTest. Maybe this is a special case. Would you interested in looking into it?

Answer 2 · 2023-09-25T11:03:04.000Z

I'm trying to upgrade to a newer version of this plugin and I hit this issue as well.
This is my docker config

{
	"auths": {
		"<something>.dkr.ecr.<somewhere>.amazonaws.com": {}
	},
	"credsStore": "desktop",
	"currentContext": "desktop-linux"
}

The last working version (not showing this unintended behaviour) is 7.3.0
From 7.4.0 I'm getting the unauthorized: The client does not have permission for manifest error.

may be related to this change? https://github.com/bmuschko/gradle-docker-plugin/pull/1065/files

NOTE: I'm getting unauthorized from a different registry than the one that appears in the docker config file.

Answer 3 · 2023-09-25T21:03:30.000Z

Same comment as for the initial reporter: Can you please provide a test case that reproduces the issue? We even have unit tests for the implementation.

Answer 4 · 2023-09-25T21:59:59.000Z

@bmuschko the initial report does have a whole and complete recipe to trigger the bug (yourself) - what else do you expect / need here?

Answer 5 · 2023-09-25T22:03:11.000Z

I do expect a pull request containing one or many automated test cases that reproduce the issue as a basis for fixing it. Optimally, the pull request would also contain the fix.

Answer 6 · 2023-09-25T22:30:18.000Z

Unfortunately the developer guide of the project is not that helpful to get anyone not familiar with the current codebase started in providing such test case easily, so at least for me it's impossible to provide one, as I don't have the time on learning how to do that at the moment, maybe l8r.

Additionally my last PR where I tried to contribute something was closed and discarded as you wanted to bring your own solution and you did not want to improve/discuss the PR any further, with the original issue still unresolved (you still can't configure a http request timeout value on the current codebase for each task and/or at extension level, right?) - so I am not that keen on going that mile again too at the moment, I hope that's understandable.

Nevertheless, let's hope someone else can help out here to get that one fixed or pushed forward.

Answer 7 · 2023-09-25T23:14:27.000Z

Unfortunately the developer guide of the project is not that helpful to get anyone not familiar with the current codebase started in providing such test case easily, so at least for me it's impossible to provide one, as I don't have the time on learning how to do that at the moment, maybe l8r.

It may be helpful to know what instructions you'd like to see here specifically. You can find some instruction here: https://bmuschko.github.io/gradle-docker-plugin/current/dev-guide/.

Additionally my last PR where I tried to contribute something was closed and discarded as you wanted to bring your own solution and you did not want to improve/discuss the PR any further, with the original issue still unresolved (you still can't configure a http request timeout value on the current codebase for each task and/or at extension level, right?) - so I am not that keen on going that mile again too at the moment, I hope that's understandable.

We rolled back the Docker Java library version which had reduced the timeout limits significantly AFAIR. Generally speaking, it's better to discuss a potential change first before putting in the work of writing a pull request. In this specific case, I decided to not to accept the pull request as internal implementation details would bleed out into the public API which had the potential of future maintenance headaches (which would cost the maintainers of the project time to deal with it).

Nevertheless, let's hope someone else can help out here to get that one fixed or pushed forward.

My comment of contributing a test case was aimed at the community as a whole, not you individually.

Answer 8 · 2023-09-28T12:14:21.000Z

I had time to at least debug things and could verify the assumption from above, it is this change:

https://github.com/bmuschko/gradle-docker-plugin/pull/1065/files => src/main/groovy/com/bmuschko/gradle/docker/internal/RegistryAuthLocator.groovy (provided for #985)

which make this thing break.
If I force the line:

allAuthConfigs.addConfig(additionalAuthConfig);

to be evaluated in the debugger the image can be build.

This one:

AuthConfigurations allAuthConfigs = lookupAllAuthConfigs();

has my configured registry A (from the json file via credential helper) which is correct. But my registry B which I need to use to run my current build command is configured with the DockerExtension of the build itself and is correctly put to the argument of that method into: additionalAuthConfig.

So this is a regression coming from that merged PR.

The test and fix assumes that the provided additionalAuthConfig is an empty default one, but this is not the case here. If there is a configured AuthConfig on e.g. the extension, this one gets ignored now.

Looking at:

    /**
     * Gets all authorization information
     * using $DOCKER_CONFIG/.docker/config.json file
     * If missing, an AuthConfigurations object containing only the passed registryCredentials is returned
     *
     * @param registryCredentials extension of type registryCredentials
     * @return AuthConfigurations object containing all authorization information (if available), and the registryCredentials
     */
    public AuthConfigurations lookupAllAuthConfigs(DockerRegistryCredentials registryCredentials) {
        return lookupAllAuthConfigs(createAuthConfig(registryCredentials));
    }

    /**
     * Gets all authorization information
     * using $DOCKER_CONFIG/.docker/config.json file
     * If missing, an AuthConfigurations object containing only the passed additionalAuthConfig is returned
     *
     * @param additionalAuthConfig An additional AuthConfig object to add to the discovered authorization information.
     * @return AuthConfigurations object containing all authorization information (if available), and the additionalAuthConfig
     */
    public AuthConfigurations lookupAllAuthConfigs(AuthConfig additionalAuthConfig) {
        AuthConfigurations allAuthConfigs = lookupAllAuthConfigs();
        if (allAuthConfigs.getConfigs().isEmpty()) {
            allAuthConfigs.addConfig(additionalAuthConfig);
        }
        return allAuthConfigs;
    }

both methods javadoc "contract" is also broken because of this, because the return says it would contain all authorization infos AND the provided registryCredentials - which is clearly not the case anymore.

So basically to fix this and retain the change we should adapt it to be able to detect an empty AuthConfig itself (no username / password) and if we got such an argument we don't add it - but in all other cases when there is a valid provided Config we should always add it, right @bmuschko ?