Skip to content

Commit 75fe588

Browse files
committed
Merge pull request #45292 from nosan
* pr/45292: Align DockerRegistryConfigAuthentication with Docker CLI Closes gh-45292
2 parents 14f9a1e + 3f22d4b commit 75fe588

File tree

2 files changed

+31
-10
lines changed

2 files changed

+31
-10
lines changed

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/configuration/DockerRegistryConfigAuthentication.java

+7-9
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@
1616

1717
package org.springframework.boot.buildpack.platform.docker.configuration;
1818

19-
import java.io.IOException;
2019
import java.util.Map;
21-
import java.util.Map.Entry;
2220
import java.util.concurrent.ConcurrentHashMap;
2321
import java.util.function.BiConsumer;
2422
import java.util.function.Function;
@@ -55,7 +53,7 @@ class DockerRegistryConfigAuthentication implements DockerRegistryAuthentication
5553
DockerRegistryConfigAuthentication(DockerRegistryAuthentication fallback,
5654
BiConsumer<String, Exception> credentialHelperExceptionHandler) {
5755
this(fallback, credentialHelperExceptionHandler, Environment.SYSTEM,
58-
(helper) -> new CredentialHelper("docker-credential-" + helper.trim()));
56+
(helper) -> new CredentialHelper("docker-credential-" + helper));
5957
}
6058

6159
DockerRegistryConfigAuthentication(DockerRegistryAuthentication fallback,
@@ -106,14 +104,14 @@ private DockerRegistryAuthentication getAuthentication(Credential credentialsFro
106104
}
107105
String username = credentialsFromHelper.getUsername();
108106
String password = credentialsFromHelper.getSecret();
109-
String serverAddress = (credentialsFromHelper.getServerUrl() != null
110-
&& !credentialsFromHelper.getServerUrl().isEmpty()) ? credentialsFromHelper.getServerUrl() : serverUrl;
107+
String serverAddress = (StringUtils.hasLength(credentialsFromHelper.getServerUrl()))
108+
? credentialsFromHelper.getServerUrl() : serverUrl;
111109
String email = (authConfig != null) ? authConfig.getEmail() : null;
112110
return DockerRegistryAuthentication.user(username, password, serverAddress, email);
113111
}
114112

115113
private Credential getCredentialsFromHelper(String serverUrl) {
116-
return (StringUtils.hasText(serverUrl))
114+
return StringUtils.hasLength(serverUrl)
117115
? credentialFromHelperCache.computeIfAbsent(serverUrl, this::computeCredentialsFromHelper) : null;
118116
}
119117

@@ -123,7 +121,7 @@ private Credential computeCredentialsFromHelper(String serverUrl) {
123121
try {
124122
return credentialHelper.get(serverUrl);
125123
}
126-
catch (IOException ex) {
124+
catch (Exception ex) {
127125
String message = "Error retrieving credentials for '%s' due to: %s".formatted(serverUrl,
128126
ex.getMessage());
129127
this.credentialHelperExceptionHandler.accept(message, ex);
@@ -134,10 +132,10 @@ private Credential computeCredentialsFromHelper(String serverUrl) {
134132

135133
private CredentialHelper getCredentialHelper(String serverUrl) {
136134
String name = this.dockerConfig.getCredHelpers().getOrDefault(serverUrl, this.dockerConfig.getCredsStore());
137-
return (name != null) ? this.credentialHelperFactory.apply(name.trim()) : null;
135+
return (StringUtils.hasLength(name)) ? this.credentialHelperFactory.apply(name) : null;
138136
}
139137

140-
private Entry<String, Auth> getAuthConfigEntry(String serverUrl) {
138+
private Map.Entry<String, Auth> getAuthConfigEntry(String serverUrl) {
141139
for (Map.Entry<String, Auth> candidate : this.dockerConfig.getAuths().entrySet()) {
142140
if (candidate.getKey().equals(serverUrl) || candidate.getKey().endsWith("://" + serverUrl)) {
143141
return candidate;

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/configuration/DockerRegistryConfigAuthenticationTests.java

+24-1
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,11 @@
3737
import org.springframework.core.io.ClassPathResource;
3838

3939
import static org.assertj.core.api.Assertions.assertThat;
40+
import static org.mockito.ArgumentMatchers.any;
4041
import static org.mockito.BDDMockito.given;
42+
import static org.mockito.BDDMockito.then;
4143
import static org.mockito.Mockito.mock;
44+
import static org.mockito.Mockito.never;
4245

4346
/**
4447
* Tests for {@link DockerRegistryConfigAuthentication}.
@@ -53,7 +56,7 @@ class DockerRegistryConfigAuthenticationTests {
5356

5457
private final Map<String, Exception> helperExceptions = new LinkedHashMap<>();
5558

56-
private Map<String, CredentialHelper> credentialHelpers = new HashMap<>();
59+
private final Map<String, CredentialHelper> credentialHelpers = new HashMap<>();
5760

5861
@BeforeEach
5962
void cleanup() {
@@ -315,6 +318,7 @@ void getAuthHeaderWhenEmptyCredHelperReturnsFallbackAndDoesNotUseCredStore(@Reso
315318
throws Exception {
316319
this.environment.put("DOCKER_CONFIG", directory.toString());
317320
ImageReference imageReference = ImageReference.of("gcr.io/ubuntu:latest");
321+
CredentialHelper desktopHelper = mockHelper("desktop");
318322
String authHeader = getAuthHeader(imageReference, DockerRegistryAuthentication.EMPTY_USER);
319323
// The Docker CLI appears to prioritize the credential helper over the
320324
// credential store, even when the helper is empty.
@@ -323,6 +327,25 @@ void getAuthHeaderWhenEmptyCredHelperReturnsFallbackAndDoesNotUseCredStore(@Reso
323327
.containsEntry("username", "")
324328
.containsEntry("password", "")
325329
.containsEntry("email", "");
330+
then(desktopHelper).should(never()).get(any(String.class));
331+
}
332+
333+
@WithResource(name = "config.json", content = """
334+
{
335+
"credsStore": "desktop"
336+
}
337+
""")
338+
@Test
339+
void getAuthHeaderReturnsFallbackWhenImageReferenceNull(@ResourcesRoot Path directory) throws Exception {
340+
this.environment.put("DOCKER_CONFIG", directory.toString());
341+
CredentialHelper desktopHelper = mockHelper("desktop");
342+
String authHeader = getAuthHeader(null, DockerRegistryAuthentication.EMPTY_USER);
343+
assertThat(decode(authHeader)).hasSize(4)
344+
.containsEntry("serveraddress", "")
345+
.containsEntry("username", "")
346+
.containsEntry("password", "")
347+
.containsEntry("email", "");
348+
then(desktopHelper).should(never()).get(any(String.class));
326349
}
327350

328351
private String getAuthHeader(ImageReference imageReference) {

0 commit comments

Comments
 (0)