Add support for Docker's credential stores and helpers

[nosan]

I left TODO in DefaultDockerRegistryAuthentication for consideration.

I didn't have time to update the documentation.

See #44633

[nosan]

@philwebb
I'm not sure if you've started reviewing or working on this PR yet, but just as an FYI, I've pushed a few additional polish commits (I did not squash them).

[philwebb]

Thanks, I've started and I'm hoping to finish something before RC1 but it's going to be tight :)

[philwebb]

Thanks very very very much @nosan! This one was very timely and I'm super glad to get it into RC1.

I've did a little work on refactoring some of the platform code then I've rebased and split up your PR into more granular commits. Some of the PR was harder to apply due to my refactoring so I've combined a bit of your original PR with some updates that I made and made a single commit with us both as authors. I hope that's OK.

For the TODOs:

• I've kept the cache because I found with some manual testing I got an exception from the helper. I figured if the helper does a REST call and it's rate limited the cache would help. I don't think we need to worry about busting the cache because the build process is pretty short lived.

• I don't think ImageReference should provide the serverUrl. That feels like a helper concern.

• I aligned with Testcontainers on helper being empty

• For the errors, I added a callback so we can log properly with Maven

I've tested things manually on both Mac and Windows and things all seem to work very well.

Thanks again for the PR!

[nosan]

Thank you so much for taking the time to include this in 3.5.0! That is awesome!

I aligned with Testcontainers on the helper being empty.

I'm not entirely sure it should align with Testcontainers. I tested the Docker CLI with an empty helper, and it appears that an empty helper takes precedence over the credential store.

The algorithm should be as follows:

1. Check if a helper exists. (map.contains(...))
2. If it exists and has a value!="", use it.
3. If the value is empty, skip it and fall back to using static credentials.

I believe Spring Boot should follow the same behavior as the Docker CLI to avoid inconsistencies between the two. Users are more likely to compare Spring Boot's approach to the Docker CLI rather than Testcontainers. For this reason, I used the Docker CLI as the reference implementation.

[philwebb]

That would be this logic I guess in the updated code? Is the error in using the getCredsStore? We might have to treat it as a bug and fix it after RC1 at this point.

[nosan]

Is the error in using the getCredsStore

Nothing, Docker CLI treats it as a helper.

---

That would be this logic I guess in the updated code?

This should be:

String name = this.dockerConfig.getCredHelpers().getOrDefault(serverUrl, this.dockerConfig.getCredsStore());

to align with Go code from Docker CLI:

func getConfiguredCredentialStore(c *ConfigFile, registryHostname string) string {
	if c.CredentialHelpers != nil && registryHostname != "" {
		if helper, exists := c.CredentialHelpers[registryHostname]; exists {
			return helper
		}
	}
	return c.CredentialsStore
}

[philwebb]

Thanks, hopefully fixed in 79f7529

[nosan]

Thanks, @philwebb

//Builder.ImageFetcher
String authHeader = authHeader(this.registryAuthentication, reference);
			Assert.state(authHeader == null || reference.getDomain().equals(this.domain),
					() -> String.format("%s '%s' must be pulled from the '%s' authenticated registry",
							StringUtils.capitalize(type.getDescription()), reference, this.domain));
						

If someone tries to use a BUILDER image from a public registry and a RUNNER image from a private registry both with the default authentication, an exception is thrown because the domains are different. My understanding is that this check was originally added to ensure the credentials provided by the user are consistent for both the builder and runner images. With the new configuration strategy, this is no longer valid as the credentials can now differ.

[philwebb]

Yes, I just hit that one as well as a bunch of test failures. I really messed up the merge of this one 😭! I'm trying to fix it up now.

[nosan]

I really messed up the merge of this one

I don't think so! You have done an incredible piece of work in such a short time! That is amazing.

I should have provided more information about that PR to make it much easier to review,
as it contains quite a lot of changes and tricky moments. Next time, I will try to emphasise all the important moments.