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

Related to fabric8io#1709 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]>
rohanKanojia · Feb 4, 2024 · bfdcd95 · bfdcd95
1 parent e8207b9
commit bfdcd95
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 18 deletions.
diff --git a/it/multi-assembly/pom.xml b/it/multi-assembly/pom.xml
@@ -97,6 +97,7 @@
               <run>
                 <wait>
                   <log>Hello from Spring Boot!</log>
+                  <time>30000</time>
                 </wait>
               </run>
             </image>

diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildXService.java b/src/main/java/io/fabric8/maven/docker/service/BuildXService.java
@@ -27,6 +27,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;
@@ -73,6 +74,10 @@ protected <C> void useBuilder(ProjectPaths projectPaths, ImageConfiguration imag
         Path configPath = getDockerStateDir(imageConfig.getBuildConfiguration(),  buildDirs);
         List<String> buildX = new ArrayList<>();
         buildX.add(DOCKER);
+        if (EnvUtil.isMac() && !isDockerBuildXWorkingWithOverriddenConfig(configPath)) {
+            logger.info("Copying docker-buildx binary to temporary config so that it doesn't collide with user docker config");
+            copyBuildXToConfigPathIfBuildXBinaryInDefaultDockerConfig(configPath);
+        }
         if (isDockerCLINotLegacy() || shouldAddConfigInLegacyDockerCLI(authConfig, configuredRegistry)) {
             buildX.add("--config");
             buildX.add(configPath.toString());
@@ -89,6 +94,18 @@ protected <C> void useBuilder(ProjectPaths projectPaths, ImageConfiguration imag
         }
     }
 
+    private void copyBuildXToConfigPathIfBuildXBinaryInDefaultDockerConfig(Path configPath) {
+        try {
+            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.warn(exception.getMessage());
+        }
+    }
+
     private boolean shouldAddConfigInLegacyDockerCLI(AuthConfigList authConfigList, String configuredRegistry) throws MojoExecutionException {
         return authConfigList != null && !authConfigList.isEmpty() &&
             !hasAuthForRegistryInDockerConfig(logger, configuredRegistry, authConfigList);
@@ -335,6 +352,33 @@ private void pumpStream(InputStream is) {
         }
     }
 
+    private boolean isDockerBuildXWorkingWithOverriddenConfig(Path configPath) {
+        DockerBuildXListWithConfigCommand buildXList = new DockerBuildXListWithConfigCommand(logger, configPath);
+        try {
+            buildXList.execute();
+            return buildXList.isSuccessFul();
+        } catch (IOException e) {
+          return false;
+        }
+    }
+
+    private static class DockerBuildXListWithConfigCommand extends ExternalCommand {
+        private final Path configPath;
+        public DockerBuildXListWithConfigCommand(Logger logger, Path configPath) {
+            super(logger);
+            this.configPath = configPath;
+        }
+
+        @Override
+        protected String[] getArgs() {
+            return new String[] {DOCKER, "--config", configPath.toString(), "buildx", "ls"};
+        }
+
+        public boolean isSuccessFul() {
+            return getStatusCode() == 0;
+        }
+    }
+
     public static class DockerVersionExternalCommand extends ExternalCommand {
         private final StringBuilder outputBuilder;
         public DockerVersionExternalCommand(Logger logger) {

diff --git a/src/main/java/io/fabric8/maven/docker/util/EnvUtil.java b/src/main/java/io/fabric8/maven/docker/util/EnvUtil.java
@@ -463,6 +463,10 @@ public static boolean isWindows() {
         return System.getProperty("os.name").toLowerCase().contains("windows");
     }
 
+    public static boolean isMac() {
+        return System.getProperty("os.name").toLowerCase().contains("mac");
+    }
+
     public static boolean isMaven350OrLater(MavenSession mavenSession) {
         // Maven enforcer and help:evaluate goals both use mavenSession.getSystemProperties(),
         // and it turns out that System.getProperty("maven.version") does not return the value.

diff --git a/src/test/java/io/fabric8/maven/docker/util/EnvUtilTest.java b/src/test/java/io/fabric8/maven/docker/util/EnvUtilTest.java
@@ -13,6 +13,8 @@
 import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
 /**
  * @author roland
  * @since 14.10.14
@@ -30,17 +32,17 @@ void splitPath() {
         };
         for (String[] strings : expected) {
             String[] got = it.next();
-            Assertions.assertEquals(2, got.length);
-            Assertions.assertEquals(strings[0], got[0]);
-            Assertions.assertEquals(strings[1], got[1]);
+            assertEquals(2, got.length);
+            assertEquals(strings[0], got[0]);
+            assertEquals(strings[1], got[1]);
         }
         Assertions.assertFalse(it.hasNext());
     }
 
     @Test
     void removeEmptyEntries() {
-        Assertions.assertEquals(ImmutableList.of("ein"), EnvUtil.removeEmptyEntries(Arrays.asList(null, "", " ein", " ")));
-        Assertions.assertEquals(ImmutableList.of(), EnvUtil.removeEmptyEntries(null));
+        assertEquals(ImmutableList.of("ein"), EnvUtil.removeEmptyEntries(Arrays.asList(null, "", " ein", " ")));
+        assertEquals(ImmutableList.of(), EnvUtil.removeEmptyEntries(null));
     }
 
     @Test
@@ -79,9 +81,9 @@ void splitAtCommasNullInList() {
     @MethodSource("parametersForSplitOnSpace")
     void splitOnSpace(String input, String[] expected) {
         String[] result = EnvUtil.splitOnSpaceWithEscape(input);
-        Assertions.assertEquals(expected.length, result.length);
+        assertEquals(expected.length, result.length);
         for (int j = 0; j < expected.length; j++) {
-            Assertions.assertEquals(expected[j], result[j]);
+            assertEquals(expected[j], result[j]);
         }
     }
 
@@ -101,9 +103,9 @@ void extractMapFromProperties() {
             "bla." + EnvUtil.PROPERTY_COMBINE_POLICY_SUFFIX, "ignored-since-it-is-reserved",
             "blub.not", "aMap");
         Map<String, String> result = EnvUtil.extractFromPropertiesAsMap("bla", props);
-        Assertions.assertEquals(2, result.size());
-        Assertions.assertEquals("world", result.get("hello"));
-        Assertions.assertEquals("morlock", result.get("max"));
+        assertEquals(2, result.size());
+        assertEquals("world", result.get("hello"));
+        assertEquals("morlock", result.get("max"));
     }
 
     @Test
@@ -115,7 +117,7 @@ void extractListFromProperties() {
             "bla." + EnvUtil.PROPERTY_COMBINE_POLICY_SUFFIX, "ignored-since-it-is-reserved",
             "blub.1", "unknown");
         List<String> result = EnvUtil.extractFromPropertiesAsList("bla", props);
-        Assertions.assertEquals(Arrays.asList("hello", "world", "last"), result);
+        assertEquals(Arrays.asList("hello", "world", "last"), result);
     }
 
     @Test
@@ -149,14 +151,14 @@ void extractListOfPropertiesFromProperties() {
         List<Properties> result = EnvUtil.extractFromPropertiesAsListOfProperties("bla", testProperties);
 
         Assertions.assertNotNull(result);
-        Assertions.assertEquals(5, result.size());
+        assertEquals(5, result.size());
         Assertions.assertArrayEquals(expectedListOfProperties, result.toArray());
     }
 
     @ParameterizedTest(name = "{displayName}: expression {0} => variable {1}")
     @MethodSource("parametersForMavenPropertyExtract")
     void mavenPropertyExtract(String expression, String varName) {
-        Assertions.assertEquals(varName, EnvUtil.extractMavenPropertyName(expression));
+        assertEquals(varName, EnvUtil.extractMavenPropertyName(expression));
     }
 
     private static Stream<Arguments> parametersForMavenPropertyExtract() {
@@ -172,8 +174,8 @@ private static Stream<Arguments> parametersForMavenPropertyExtract() {
     @ParameterizedTest(name = "{displayName}: max({0},{1}) = {2}, {0} >= {1} ? {3}")
     @MethodSource("parametersForVersionChecks")
     void versionChecks(String versionA, String versionB, String largerVersion, boolean isGreaterOrEquals) {
-        Assertions.assertEquals(largerVersion, EnvUtil.extractLargerVersion(versionA, versionB));
-        Assertions.assertEquals(isGreaterOrEquals, EnvUtil.greaterOrEqualsVersion(versionA, versionB));
+        assertEquals(largerVersion, EnvUtil.extractLargerVersion(versionA, versionB));
+        assertEquals(isGreaterOrEquals, EnvUtil.greaterOrEqualsVersion(versionA, versionB));
     }
 
     private static Stream<Arguments> parametersForVersionChecks() {
@@ -207,16 +209,36 @@ private Properties getTestProperties(String... vals) {
 
     })
     void ensureRegistryHttpUrl(String input, String expected) {
-        Assertions.assertEquals( expected, EnvUtil.ensureRegistryHttpUrl(input), ">> " + input);
+        assertEquals( expected, EnvUtil.ensureRegistryHttpUrl(input), ">> " + input);
     }
 
     @Test
     void resolveHomeReferenceNoHome() {
-        Assertions.assertEquals("~chas/relative", EnvUtil.resolveHomeReference("~chas/relative"));
+        assertEquals("~chas/relative", EnvUtil.resolveHomeReference("~chas/relative"));
     }
 
     @Test
     void resolveHomeReference() {
-        Assertions.assertEquals(Paths.get(EnvUtil.getUserHome(), "relative").toString(), EnvUtil.resolveHomeReference("~/relative"));
+        assertEquals(Paths.get(EnvUtil.getUserHome(), "relative").toString(), EnvUtil.resolveHomeReference("~/relative"));
+    }
+
+    @ParameterizedTest
+    @CsvSource(value = {
+        "Mac OS,true",
+        "Windows 11,false",
+        "Linux,false"
+    })
+    void isMac(String osNameOverrideValue, boolean expectedResult) {
+        String actualOsName = System.getProperty("os.name");
+        try {
+            // Given
+            System.setProperty("os.name", osNameOverrideValue);
+            // When
+            boolean result = EnvUtil.isMac();
+            // Then
+            assertEquals(expectedResult, result);
+        } finally {
+            System.setProperty("os.name", actualOsName);
+        }
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -97,6 +97,7 @@ @@
                   <run>
                     <wait>
                       <log>Hello from Spring Boot!</log>
+                      <time>30000</time>
                     </wait>
                   </run>
                 </image>
@@ Expand Down @@