Skip to content
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.

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

fix (buildx) : BuildX failing for Docker CLI on MacOS #1754

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 30 additions & 44 deletions src/main/java/io/fabric8/maven/docker/service/BuildXService.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import io.fabric8.maven.docker.util.ImageName;
import io.fabric8.maven.docker.util.Logger;
import io.fabric8.maven.docker.util.ProjectPaths;
import org.apache.commons.lang3.StringUtils;
import org.apache.maven.plugin.MojoExecutionException;

import java.io.BufferedReader;
Expand All @@ -27,6 +26,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -36,10 +36,7 @@
import java.util.Optional;
import java.util.concurrent.CompletableFuture;

import static io.fabric8.maven.docker.util.AuthConfigFactory.hasAuthForRegistryInDockerConfig;

public class BuildXService {
private static final int DOCKER_CLI_BUILDX_CONFIG_COMPATIBLE_MAJOR_VERSION = 23;
private static final String DOCKER = "docker";
private final DockerAccess dockerAccess;
private final DockerAssemblyManager dockerAssemblyManager;
Expand Down Expand Up @@ -71,13 +68,12 @@
BuildDirs buildDirs = new BuildDirs(projectPaths, imageConfig.getName());

Path configPath = getDockerStateDir(imageConfig.getBuildConfiguration(), buildDirs);
List<String> buildX = new ArrayList<>();
buildX.add(DOCKER);
if (isDockerCLINotLegacy() || shouldAddConfigInLegacyDockerCLI(authConfig, configuredRegistry)) {
buildX.add("--config");
buildX.add(configPath.toString());
List<String> buildX = new ArrayList<>(Arrays.asList(DOCKER, "--config", configPath.toString(), "buildx"));
if (!isDockerBuildXWorkingWithOverriddenConfig(configPath)) {
logger.debug("Detected current version of BuildX not working with --config override");
logger.debug("Copying BuildX binary to " + configPath);
copyBuildXToConfigPathIfBuildXBinaryInDefaultDockerConfig(configPath);
}
buildX.add("buildx");

String builderName = createBuilder(configPath, buildX, imageConfig, buildDirs);
Path configJson = configPath.resolve("config.json");
Expand All @@ -89,29 +85,16 @@
}
}

private boolean shouldAddConfigInLegacyDockerCLI(AuthConfigList authConfigList, String configuredRegistry) throws MojoExecutionException {
return authConfigList != null && !authConfigList.isEmpty() &&
!hasAuthForRegistryInDockerConfig(logger, configuredRegistry, authConfigList);
}

private boolean isDockerCLINotLegacy() {
DockerVersionExternalCommand dockerVersionExternalCommand = new DockerVersionExternalCommand(logger);
private void copyBuildXToConfigPathIfBuildXBinaryInDefaultDockerConfig(Path configPath) {
try {
dockerVersionExternalCommand.execute();
} catch (IOException e) {
logger.info("Failure in getting docker CLI version", e);
}
String version = dockerVersionExternalCommand.getVersion();
if (StringUtils.isNotBlank(version)) {
version = version.replaceAll("(^')|('$)", "");
String[] versionParts = version.split("\\.");
logger.info("Using Docker CLI " + version);
if (versionParts.length >= 3) {
int cliMajorVersion = Integer.parseInt(versionParts[0]);
return cliMajorVersion >= DOCKER_CLI_BUILDX_CONFIG_COMPATIBLE_MAJOR_VERSION;
File buildXInUserHomeDockerConfig = Paths.get(EnvUtil.getUserHome(), ".docker/cli-plugins/docker-buildx").toFile();
Files.createDirectory(configPath.resolve("cli-plugins"));
if (buildXInUserHomeDockerConfig.exists() && buildXInUserHomeDockerConfig.isFile()) {
Files.copy(buildXInUserHomeDockerConfig.toPath(), configPath.resolve("cli-plugins").resolve("docker-buildx"), StandardCopyOption.COPY_ATTRIBUTES);
}
} catch (IOException exception) {
logger.debug(exception.getMessage());

Check warning on line 96 in src/main/java/io/fabric8/maven/docker/service/BuildXService.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/io/fabric8/maven/docker/service/BuildXService.java#L95-L96

Added lines #L95 - L96 were not covered by tests
}
return false;
}

protected void createConfigJson(Path configJson, AuthConfigList authConfig) throws MojoExecutionException {
Expand Down Expand Up @@ -335,27 +318,30 @@
}
}

public static class DockerVersionExternalCommand extends ExternalCommand {
private final StringBuilder outputBuilder;
public DockerVersionExternalCommand(Logger logger) {
super(logger);
outputBuilder = new StringBuilder();
private boolean isDockerBuildXWorkingWithOverriddenConfig(Path configPath) {
BuildXListWithConfigCommand buildXList = new BuildXListWithConfigCommand(logger, configPath);
try {
buildXList.execute();
return buildXList.isSuccessFul();
} catch (IOException e) {
return false;

Check warning on line 327 in src/main/java/io/fabric8/maven/docker/service/BuildXService.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/io/fabric8/maven/docker/service/BuildXService.java#L326-L327

Added lines #L326 - L327 were not covered by tests
}
}

@Override
protected String[] getArgs() {
return new String[] {DOCKER, "version", "--format", "'{{.Client.Version}}'"};
static class BuildXListWithConfigCommand extends ExternalCommand {
private final Path configPath;
public BuildXListWithConfigCommand(Logger logger, Path configPath) {
super(logger);
this.configPath = configPath;
}

@Override
protected void processLine(String line) {
if (StringUtils.isNotBlank(line)) {
outputBuilder.append(line);
}
protected String[] getArgs() {
return new String[] { DOCKER, "--config", configPath.toString(), "buildx", "ls"};
}

public String getVersion() {
return outputBuilder.toString();
public boolean isSuccessFul() {
return getStatusCode() == 0;
}
}
}
1 change: 0 additions & 1 deletion src/main/java/io/fabric8/maven/docker/util/EnvUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import java.util.concurrent.TimeUnit;
Expand Down
19 changes: 3 additions & 16 deletions src/test/java/io/fabric8/maven/docker/BuildMojoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
import io.fabric8.maven.docker.service.BuildXService;
import io.fabric8.maven.docker.service.ImagePullManager;
import org.apache.maven.plugin.MojoExecutionException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockedConstruction;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;

Expand Down Expand Up @@ -54,22 +51,11 @@ class BuildMojoTest extends MojoTestBase {

@Mock
private BuildXService.Exec exec;
private MockedConstruction<BuildXService.DockerVersionExternalCommand> dockerVersionExternalCommandMockedConstruction;

private static String getOsDependentBuild(Path buildPath, String docker) {
return buildPath.resolve(docker).toString().replace('/', File.separatorChar);
}

@BeforeEach
void setUp() {
dockerVersionExternalCommandMockedConstruction = Mockito.mockConstruction(BuildXService.DockerVersionExternalCommand.class);
}

@AfterEach
void tearDown() {
dockerVersionExternalCommandMockedConstruction.close();
}

@Test
void skipWhenPom() throws IOException, MojoExecutionException {

Expand Down Expand Up @@ -305,18 +291,19 @@ private void verifyBuild(int wantedNumberOfInvocations) throws DockerAccessExcep
private void thenBuildxRun(String relativeConfigFile, String contextDir,
boolean nativePlatformIncluded, String attestation) throws MojoExecutionException {
Path buildPath = projectBaseDirectory.toPath().resolve("target/docker/example/latest");
String config = getOsDependentBuild(buildPath, "docker");
String configFile = relativeConfigFile != null ? getOsDependentBuild(projectBaseDirectory.toPath(), relativeConfigFile) : null;

List<String> cmds =
BuildXService.append(new ArrayList<>(), "docker", "buildx",
BuildXService.append(new ArrayList<>(), "docker", "--config", config, "buildx",
"create", "--driver", "docker-container", "--name", "maven" , "--node", "maven0");
if (configFile != null) {
BuildXService.append(cmds, "--config", configFile.replace('/', File.separatorChar));
}
Mockito.verify(exec).process(cmds);

if (nativePlatformIncluded) {
List<String> buildXLine = BuildXService.append(new ArrayList<>(), "docker", "buildx",
List<String> buildXLine = BuildXService.append(new ArrayList<>(), "docker", "--config", config, "buildx",
"build", "--progress=plain", "--builder", "maven",
"--platform", NATIVE_PLATFORM, "--tag", "example:latest", "--build-arg", "foo=bar");

Expand Down
92 changes: 5 additions & 87 deletions src/test/java/io/fabric8/maven/docker/PushMojoBuildXTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.fabric8.maven.docker;

import io.fabric8.maven.docker.access.AuthConfig;
import io.fabric8.maven.docker.access.DockerAccess;
import io.fabric8.maven.docker.config.BuildImageConfiguration;
import io.fabric8.maven.docker.config.BuildXConfiguration;
Expand All @@ -10,7 +9,6 @@
import io.fabric8.maven.docker.service.DockerAccessFactory;
import io.fabric8.maven.docker.service.ServiceHubFactory;
import io.fabric8.maven.docker.util.AuthConfigFactory;
import io.fabric8.maven.docker.util.DockerFileUtil;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.MojoFailureException;
import org.apache.maven.project.MavenProject;
Expand All @@ -23,10 +21,7 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.MockedConstruction;
import org.mockito.MockedStatic;
import org.sonatype.plexus.components.sec.dispatcher.SecDispatcher;
import org.sonatype.plexus.components.sec.dispatcher.SecDispatcherException;

Expand All @@ -46,7 +41,6 @@
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockConstruction;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand All @@ -59,8 +53,6 @@ class PushMojoBuildXTest {
private File expectedDockerStateConfigDir;
private Settings mockedMavenSettings;
private MockedConstruction<BuildXService.DefaultExec> defaultExecMockedConstruction;
private MockedConstruction<BuildXService.DockerVersionExternalCommand> dockerVersionExternalCommandMockedConstruction;
private String dockerClIVersion;

@BeforeEach
void setup() throws MojoExecutionException, MojoFailureException, IOException, ComponentLookupException, SecDispatcherException {
Expand Down Expand Up @@ -88,9 +80,6 @@ void setup() throws MojoExecutionException, MojoFailureException, IOException, C
when(mockedSecDispatcher.decrypt(anyString())).thenReturn("testpassword");
Map<String, Object> pluginContext = new HashMap<>();
defaultExecMockedConstruction = mockConstruction(BuildXService.DefaultExec.class);
dockerVersionExternalCommandMockedConstruction = mockConstruction(BuildXService.DockerVersionExternalCommand.class, (mock, ctx) -> {
when(mock.getVersion()).thenReturn(dockerClIVersion);
});
this.pushMojo = new PushMojo();
this.pushMojo.setPluginContext(pluginContext);
pushMojo.verbose = "true";
Expand All @@ -107,78 +96,12 @@ void setup() throws MojoExecutionException, MojoFailureException, IOException, C
@AfterEach
void tearDown() {
defaultExecMockedConstruction.close();
dockerVersionExternalCommandMockedConstruction.close();
}

@Test
@DisplayName("legacy docker CLI, docker:push buildx, no auth, then don't add --config option to buildx")
void execute_whenNoAuthConfig_thenRunBuildXCommandWithNoConfig() throws MojoExecutionException, MojoFailureException {
@DisplayName("docker:push buildx, auth from ~/.m2/settings.xml, then add --config option to buildx")
void execute_whenAuthConfigFromMavenSettings_thenAddConfigToDockerBuildXCommand() throws MojoExecutionException, MojoFailureException {
// Given
givenDockerCLIVersion("20.0.14");

// When
pushMojo.execute();

// Then
verifyNoDockerConfigAddedToBuildX();
}

@Test
@DisplayName("docker CLI 23.0.6, docker:push buildx, no auth, then add --config option to buildx")
void execute_whenNoAuthConfigAndDockerCLIPost23_thenRunBuildXCommandWithAddedConfig() throws MojoExecutionException, MojoFailureException {
// Given
givenDockerCLIVersion("23.0.6+azure-2");

// When
pushMojo.execute();

// Then
verifyDockerConfigOptionAddedToBuildX();
}

@Test
@DisplayName("legacy docker CLI, docker:push buildx, auth from ~/.docker/config.json, then don't add --config option to buildx")
void execute_whenLegacyDockerCLIAndAuthConfigFromLocalDockerConfig_thenDoNotAddConfigToDockerBuildXCommand() throws MojoExecutionException, MojoFailureException {
try (MockedStatic<DockerFileUtil> dockerFileUtilMockedStatic = mockStatic(DockerFileUtil.class)) {
// Given
givenDockerCLIVersion("20.0.14");
AuthConfig authConfig = new AuthConfig("testuser", "testpassword", null, null, null);
authConfig.setRegistry("test.example.org");
dockerFileUtilMockedStatic.when(DockerFileUtil::readDockerConfig)
.thenReturn(authConfig.toJsonObject());

// When
pushMojo.execute();

// Then
verifyNoDockerConfigAddedToBuildX();
}
}

@Test
@DisplayName("23.0.6+azure-2 docker CLI, docker:push buildx, auth from ~/.docker/config.json, then don't add --config option to buildx")
void execute_whenAuthConfigFromLocalDockerConfig_thenAddConfigToDockerBuildXCommand() throws MojoExecutionException, MojoFailureException {
try (MockedStatic<DockerFileUtil> dockerFileUtilMockedStatic = mockStatic(DockerFileUtil.class)) {
// Given
givenDockerCLIVersion("'23.0.6+azure-2'");
AuthConfig authConfig = new AuthConfig("testuser", "testpassword", null, null, null);
authConfig.setRegistry("test.example.org");
dockerFileUtilMockedStatic.when(DockerFileUtil::readDockerConfig)
.thenReturn(authConfig.toJsonObject());

// When
pushMojo.execute();

// Then
verifyDockerConfigOptionAddedToBuildX();
}
}

@ParameterizedTest(name = "docker CLI {0} docker:push buildx, auth from ~/.m2/settings.xml, then add --config option to buildx")
@ValueSource(strings = {"20.0.14", "'23.0.6+azure-2'"})
void execute_whenAuthConfigFromMavenSettings_thenAddConfigToDockerBuildXCommand(String dockerClIVersion) throws MojoExecutionException, MojoFailureException {
// Given
givenDockerCLIVersion(dockerClIVersion);
Server server = new Server();
server.setId("test.example.org");
server.setUsername("testuser");
Expand All @@ -192,12 +115,11 @@ void execute_whenAuthConfigFromMavenSettings_thenAddConfigToDockerBuildXCommand(
verifyDockerConfigOptionAddedToBuildX();
}

@ParameterizedTest(name = "docker CLI {0} docker:push buildx, auth from properties, then add --config option to buildx")
@ValueSource(strings = {"20.0.14", "'23.0.6+azure-2'"})
void execute_whenAuthConfigFromProperties_thenAddConfigOptionToBuildXCommand(String dockerClIVersion) throws MojoExecutionException, MojoFailureException {
@Test
@DisplayName("docker:push buildx, auth from properties, then add --config option to buildx")
void execute_whenAuthConfigFromProperties_thenAddConfigOptionToBuildXCommand() throws MojoExecutionException, MojoFailureException {
try {
// Given
givenDockerCLIVersion(dockerClIVersion);
System.setProperty("docker.push.username", "testuser");
System.setProperty("docker.push.password", "testpassword");

Expand Down Expand Up @@ -247,8 +169,4 @@ private ImageConfiguration createImageConfiguration() {
.build())
.build();
}

private void givenDockerCLIVersion(String version) {
dockerClIVersion = version;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package io.fabric8.maven.docker.service;

import io.fabric8.maven.docker.util.Logger;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.api.Test;

import java.io.File;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;

class BuildXListWithConfigCommandTest {
@TempDir
private File temporaryFolder;

private BuildXService.BuildXListWithConfigCommand buildXListWithConfigCommand;

@BeforeEach
void setUp() {
Logger logger = mock(Logger.class);
buildXListWithConfigCommand = new BuildXService.BuildXListWithConfigCommand(logger, temporaryFolder.toPath());
}

@Test
void getArgs() {
assertArrayEquals(new String[] {"docker", "--config", temporaryFolder.getAbsolutePath(), "buildx", "ls"}, buildXListWithConfigCommand.getArgs());
}

@Test
void isSuccessFul() {
// Given
assertTrue(buildXListWithConfigCommand.isSuccessFul());
}
}
Loading
Loading