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

+ Revert workaround for checking docker version for checking whether to add `docker --config` flag or not. We need to always add config to builder whenever it involves pulling or pushing images. + Currently Docker CLI on Mac OS don't seem to respect `--config` flag. When DMP tries to override default Docker config directory by providing `--config` flag, Docker CLI is no longer able to recognize buildx options. This seems to happening for scenarios where docker-buildx is installed in `~/.docker/cli-plugins`, whenever `docker --config new/path/config` is provided docker CLI uses new config path (which does not contain buildx). + Add a workaround to copy `docker-buildx` binary to temporary config directory created for docker buildx build. This seems to make docker recognize buildx even after config override. Signed-off-by: Rohan Kumar <[email protected]>
fabric8io · Feb 11, 2024 · f58346e · f58346e
1 parent 21c0520
commit f58346e
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 237 deletions.
diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildXService.java b/src/main/java/io/fabric8/maven/docker/service/BuildXService.java
@@ -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;
@@ -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;
@@ -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;
@@ -71,13 +68,12 @@ protected <C> void useBuilder(ProjectPaths projectPaths, ImageConfiguration imag
         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");
@@ -89,29 +85,16 @@ protected <C> void useBuilder(ProjectPaths projectPaths, ImageConfiguration imag
         }
     }
 
-    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());
         }
-        return false;
     }
 
     protected void createConfigJson(Path configJson, AuthConfigList authConfig) throws MojoExecutionException {
@@ -335,27 +318,30 @@ private void pumpStream(InputStream is) {
         }
     }
 
-    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;
         }
+    }
 
-        @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;
         }
     }
 }
diff --git a/src/main/java/io/fabric8/maven/docker/util/EnvUtil.java b/src/main/java/io/fabric8/maven/docker/util/EnvUtil.java
@@ -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;

diff --git a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java
@@ -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;
 
@@ -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 {
 
@@ -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");
 

diff --git a/src/test/java/io/fabric8/maven/docker/PushMojoBuildXTest.java b/src/test/java/io/fabric8/maven/docker/PushMojoBuildXTest.java
@@ -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;
@@ -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;
@@ -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;
 
@@ -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;
 
@@ -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 {
@@ -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";
@@ -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");
@@ -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");
 
@@ -247,8 +169,4 @@ private ImageConfiguration createImageConfiguration() {
             .build())
         .build();
   }
-
-  private void givenDockerCLIVersion(String version) {
-    dockerClIVersion = version;
-  }
 }
diff --git a/src/test/java/io/fabric8/maven/docker/service/BuildXListWithConfigCommandTest.java b/src/test/java/io/fabric8/maven/docker/service/BuildXListWithConfigCommandTest.java
@@ -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());
+  }
+}