Skip to content

Polish #45291

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Polish #45291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ain/java/org/springframework/boot/buildpack/platform/docker/configuration/Credential.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class Credential extends MappedObject {

private final String secret;

private String serverUrl;
private final String serverUrl;

Credential(JsonNode node) {
super(node, MethodHandles.lookup());
Expand Down
23 changes: 15 additions & 8 deletions ...va/org/springframework/boot/buildpack/platform/docker/configuration/CredentialHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class CredentialHelper {

private static final String USR_LOCAL_BIN = "/usr/local/bin/";

Set<String> CREDENTIAL_NOT_FOUND_MESSAGES = Set.of("credentials not found in native keychain",
private static final Set<String> CREDENTIAL_NOT_FOUND_MESSAGES = Set.of("credentials not found in native keychain",
"no credentials server URL", "no credentials username");

private final String executable;
Expand Down Expand Up @@ -73,12 +73,12 @@ Credential get(String serverUrl) throws IOException {
}
}

private ProcessBuilder processBuilder(String string) {
private ProcessBuilder processBuilder(String action) {
ProcessBuilder processBuilder = new ProcessBuilder().redirectErrorStream(true);
if (Platform.isWindows()) {
processBuilder.command("cmd", "/c");
}
processBuilder.command(this.executable, string);
processBuilder.command(this.executable, action);
return processBuilder;
}

Expand All @@ -90,14 +90,21 @@ private Process start(ProcessBuilder processBuilder) throws IOException {
if (!Platform.isMac()) {
throw ex;
}
List<String> command = new ArrayList<>(processBuilder.command());
command.set(0, USR_LOCAL_BIN + command.get(0));
return processBuilder.command(command).start();
try {
List<String> command = new ArrayList<>(processBuilder.command());
command.set(0, USR_LOCAL_BIN + command.get(0));
return processBuilder.command(command).start();
}
catch (Exception suppressed) {
// Suppresses the exception and rethrows the original exception
ex.addSuppressed(suppressed);
throw ex;
}
}
}

private boolean isCredentialsNotFoundError(String message) {
return this.CREDENTIAL_NOT_FOUND_MESSAGES.contains(message.trim());
private static boolean isCredentialsNotFoundError(String message) {
return CREDENTIAL_NOT_FOUND_MESSAGES.contains(message.trim());
}

}
21 changes: 11 additions & 10 deletions ...ngframework/boot/buildpack/platform/docker/configuration/DockerConfigurationMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Base64;
import java.util.HexFormat;
import java.util.Map;
import java.util.function.Supplier;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
Expand All @@ -36,6 +37,7 @@
import org.springframework.boot.buildpack.platform.system.Environment;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
import org.springframework.util.function.SingletonSupplier;

/**
* Docker configuration stored in metadata files managed by the Docker CLI.
Expand Down Expand Up @@ -63,7 +65,8 @@ final class DockerConfigurationMetadata {

private static final String CONTEXT_FILE_NAME = "meta.json";

private static volatile DockerConfigurationMetadata systemEnvironmentConfigurationMetadata;
private static final Supplier<DockerConfigurationMetadata> systemEnvironmentConfigurationMetadata = SingletonSupplier
.of(() -> DockerConfigurationMetadata.create(Environment.SYSTEM));

private final String configLocation;

Expand All @@ -90,20 +93,18 @@ DockerContext forContext(String context) {
}

static DockerConfigurationMetadata from(Environment environment) {
DockerConfigurationMetadata dockerConfigurationMetadata = (environment == Environment.SYSTEM)
? DockerConfigurationMetadata.systemEnvironmentConfigurationMetadata : null;
if (dockerConfigurationMetadata != null) {
return dockerConfigurationMetadata;
if (environment == Environment.SYSTEM) {
return systemEnvironmentConfigurationMetadata.get();
}
return create(environment);
}

private static DockerConfigurationMetadata create(Environment environment) {
String configLocation = environment.get(DOCKER_CONFIG);
configLocation = (configLocation != null) ? configLocation : getUserHomeConfigLocation();
DockerConfig dockerConfig = createDockerConfig(configLocation);
DockerContext dockerContext = createDockerContext(configLocation, dockerConfig.getCurrentContext());
dockerConfigurationMetadata = new DockerConfigurationMetadata(configLocation, dockerConfig, dockerContext);
if (environment == Environment.SYSTEM) {
DockerConfigurationMetadata.systemEnvironmentConfigurationMetadata = dockerConfigurationMetadata;
}
return dockerConfigurationMetadata;
return new DockerConfigurationMetadata(configLocation, dockerConfig, dockerContext);
}

private static String getUserHomeConfigLocation() {
Expand Down
10 changes: 7 additions & 3 deletions ...gframework/boot/buildpack/platform/docker/configuration/DockerRegistryAuthentication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.function.BiConsumer;

import org.springframework.boot.buildpack.platform.docker.type.ImageReference;
import org.springframework.util.Assert;

/**
* Docker registry authentication configuration.
Expand Down Expand Up @@ -83,7 +84,8 @@ static DockerRegistryAuthentication user(String username, String password, Strin
* Factory method that returns a new {@link DockerRegistryAuthentication} instance
* that uses the standard docker JSON config (including support for credential
* helpers) to generate auth headers.
* @param fallback the fallback authentication to use if no suitable config is found
* @param fallback the fallback authentication to use if no suitable config is found,
* may be null {@code}
* @return a new {@link DockerRegistryAuthentication} instance
* @since 3.5.0
* @see #configuration(DockerRegistryAuthentication, BiConsumer)
Expand All @@ -96,15 +98,17 @@ static DockerRegistryAuthentication configuration(DockerRegistryAuthentication f
* Factory method that returns a new {@link DockerRegistryAuthentication} instance
* that uses the standard docker JSON config (including support for credential
* helpers) to generate auth headers.
* @param fallback the fallback authentication to use if no suitable config is found
* @param fallback the fallback authentication to use if no suitable config is found,
* may be {@code null}
* @param credentialHelperExceptionHandler callback that should handle credential
* helper exceptions
* helper exceptions, never {@code null}
* @return a new {@link DockerRegistryAuthentication} instance
* @since 3.5.0
* @see #configuration(DockerRegistryAuthentication, BiConsumer)
*/
static DockerRegistryAuthentication configuration(DockerRegistryAuthentication fallback,
BiConsumer<String, Exception> credentialHelperExceptionHandler) {
Assert.notNull(credentialHelperExceptionHandler, () -> "'credentialHelperExceptionHandler' must not be null");
return new DockerRegistryConfigAuthentication(fallback, credentialHelperExceptionHandler);
}

Expand Down
15 changes: 11 additions & 4 deletions ...g/springframework/boot/buildpack/platform/docker/configuration/CredentialHelperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,17 @@ void getWhenUnknownErrorThrowsException() {
}

@Test
void getWhenCommandDoesNotExistErrorThrowsException() {
String name = "docker-credential-%s".formatted(UUID.randomUUID().toString());
assertThatIOException().isThrownBy(() -> new CredentialHelper(name).get("invalid.example.com"))
.withMessageContaining(name);
void getWhenExecutableDoesNotExistErrorThrowsException() {
String executable = "docker-credential-%s".formatted(UUID.randomUUID().toString());
assertThatIOException().isThrownBy(() -> new CredentialHelper(executable).get("invalid.example.com"))
.withMessageContaining(executable)
.satisfies((ex) -> {
if (Platform.isMac()) {
assertThat(ex.getMessage()).doesNotContain("/usr/local/bin/");
assertThat(ex.getSuppressed()).allSatisfy((suppressed) -> assertThat(suppressed)
.hasMessageContaining("/usr/local/bin/" + executable));
}
});
}

}
2 changes: 1 addition & 1 deletion ...ocker/configuration/CredentialsTests.java → ...docker/configuration/CredentialTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
*
* @author Dmytro Nosan
*/
class CredentialsTests {
class CredentialTests {

@Test
@WithResource(name = "credentials.json", content = """
Expand Down