Fix eclipse-jkube#174: Build failure when trying to use Dockerfile fo…

…r arguments in FROM Port of fabric8io/docker-maven-plugin#1299
rohanKanojia · Oct 30, 2020 · 7355a25 · 7355a25
1 parent 3005d57
commit 7355a25
Show file tree

Hide file tree

Showing 7 changed files with 179 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,8 @@ Usage:
 * Fix #448: Service.spec.type added from config if existing Service fragment doesn't specify it
 * Fix #451: Docker push works with Podman REST API
 * Fix: Removed misleading watch-postGoal warning + fixed rolling update in DockerImageWatcher
+* Fix: Docker push works with Podman REST API
+* Fix #174: Build failure when trying to use Dockerfile for arguments in FROM, port of fabric8io/docker-maven-plugin#1299
 
 ### 1.0.1 (2020-10-05)
 * Fix #381: Remove root as default user in AssemblyConfigurationUtils#getAssemblyConfigurationOrCreateDefault

diff --git a/jkube-kit/build/api/src/main/java/org/eclipse/jkube/kit/build/api/helper/DockerFileUtil.java b/jkube-kit/build/api/src/main/java/org/eclipse/jkube/kit/build/api/helper/DockerFileUtil.java
@@ -27,6 +27,7 @@
 import java.io.IOException;
 import java.io.Reader;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -42,6 +43,7 @@
  * @since 21/01/16
  */
 public class DockerFileUtil {
+    private static final String ARG_PATTERN_REGEX = "\\$(?:\\{(.*)\\}|(.*))";
 
     private DockerFileUtil() {}
 
@@ -51,28 +53,42 @@ private DockerFileUtil() {}
      *
      * @param dockerFile file from where to extract the base image.
      * @param properties properties to interpolate in the provided dockerFile.
+     * @param filter string representing filter parameters for properties in dockerfile
+     * @param argsFromBuildConfig Docker Build args received from Build Configuration
      * @return LinkedList of base images name or empty collection if none is found.
      * @throws IOException if there's a problem while performin IO operations.
      */
-    public static List<String> extractBaseImages(File dockerFile, Properties properties, String filter) throws IOException {
+    public static List<String> extractBaseImages(File dockerFile, Properties properties, String filter, Map<String, String> argsFromBuildConfig) throws IOException {
         List<String[]> fromLines = extractLines(dockerFile, "FROM", properties, filter);
+        Map<String, String> args = extractArgs(dockerFile, properties, filter, argsFromBuildConfig);
         Set<String> result = new LinkedHashSet<>();
         Set<String> fromAlias = new HashSet<>();
         for (String[] fromLine :  fromLines) {
-            if (fromLine.length > 1) {
-                if (fromLine.length == 2) { // FROM image:tag use case
-                    result.add(fromLine[1]);
-                } else if (fromLine.length == 4) { // FROM image:tag AS alias use case
-                    if (!fromAlias.contains(fromLine[1])) {
-                        result.add(fromLine[1]);
-                    }
-                    fromAlias.add(fromLine[3]);
+            if (fromLine.length == 2) { // FROM image:tag use case
+                result.add(resolveImageTagFromArgs(fromLine[1], args));
+            } else if (fromLine.length == 4) { // FROM image:tag AS alias use case
+                if (!fromAlias.contains(fromLine[1])) {
+                    result.add(resolveImageTagFromArgs(fromLine[1], args));
                 }
+                fromAlias.add(resolveImageTagFromArgs(fromLine[3], args));
             }
         }
         return new ArrayList<>(result);
     }
 
+    /**
+     * Extract Args from dockerfile. All lines containing ARG is taken.
+     *
+     * @param dockerfile Docker File
+     * @param properties properties
+     * @param filter interpolation filter
+     * @param argsFromBuildConfig Docker build args from Build Configuration
+     * @return HashMap of arguments or empty collection if none is found
+     */
+    public static Map<String, String> extractArgs(File dockerfile, Properties properties, String filter, Map<String, String> argsFromBuildConfig) throws IOException {
+        return extractArgsFromLines(extractLines(dockerfile, "ARG", properties, filter), argsFromBuildConfig);
+    }
+
     /**
      * Extract all lines containing the given keyword
      *
@@ -130,6 +146,54 @@ private static Reader getFileReaderFromDir(File file) {
         }
     }
 
+    /**
+     * Helper method for extractArgs(exposed for test)
+     *
+     * @param argLines list of string arrays containing lines with words
+     * @param argsFromBuildConfig Docker build args from Build Configuration
+     * @return map of parsed arguments
+     */
+    protected static Map<String, String> extractArgsFromLines(List<String[]> argLines, Map<String, String> argsFromBuildConfig) {
+        Map<String, String> result = new HashMap<>();
+        for (String[] argLine : argLines) {
+            if (argLine.length > 1) {
+                updateMapWithArgValue(result, argsFromBuildConfig, argLine[1]);
+            }
+        }
+        return result;
+    }
+
+    private static String resolveImageTagFromArgs(String imageTagString, Map<String, String> args) {
+        if (imageTagString.startsWith("$")) { // FROM $IMAGE
+            String resolvedVal = resolveArgValueFromStrContainingArgKey(imageTagString, args);
+            if (resolvedVal != null) {
+                return resolvedVal;
+            }
+        } else { // FROM image:$TAG_ARG
+            String[] imageTagArr = imageTagString.split(":");
+            if (imageTagArr.length > 1) {
+                String tag = resolveArgValueFromStrContainingArgKey(imageTagArr[1], args);
+                if (tag != null) {
+                    return imageTagArr[0] + ":" + tag;
+                }
+            }
+        }
+        return imageTagString;
+    }
+
+    static String resolveArgValueFromStrContainingArgKey(String argString, Map<String, String> args) {
+        final Pattern argPattern = Pattern.compile(ARG_PATTERN_REGEX);
+        Matcher matcher = argPattern.matcher(argString);
+        if (matcher.matches()) {
+            if (matcher.group(1) != null) {
+                return args.get(matcher.group(1));
+            } else if (matcher.group(2) != null) {
+                return args.get(matcher.group(2));
+            }
+        }
+        return null;
+    }
+
     public static JsonObject readDockerConfig() {
         String dockerConfig = System.getenv("DOCKER_CONFIG");
 
@@ -162,7 +226,7 @@ public static Map<String,?> readKubeConfig() {
                 : getFileReaderFromDir(new File(kubeConfig));
         if (reader != null) {
             Yaml ret = new Yaml();
-            return (Map<String, ?>) ret.load(reader);
+            return ret.load(reader);
         }
         return null;
     }
@@ -209,4 +273,36 @@ private static File getHomeDir() {
         return new File(homeDir);
     }
 
+    private static void updateMapWithArgValue(Map<String, String> result, Map<String, String> args, String argString) {
+        if (argString.contains("=") || argString.contains(":")) {
+            String[] argStringParts = argString.split("[=:]");
+            String argStringValue = argString.substring(argStringParts[0].length() + 1);
+            if (argStringValue.startsWith("\"") || argStringValue.startsWith("'")) {
+                // Replaces surrounding quotes
+                argStringValue = argStringValue.replaceAll("^[\"']+|[\"']$", "");
+            } else {
+                validateArgValue(argStringValue);
+            }
+            result.put(argStringParts[0], argStringValue);
+        } else {
+            validateArgValue(argString);
+            result.putAll(fetchArgsFromBuildConfiguration(argString, args));
+        }
+    }
+
+    private static Map<String, String> fetchArgsFromBuildConfiguration(String argString, Map<String, String> args) {
+        Map<String, String> argFromBuildConfig = new HashMap<>();
+        if (args != null) {
+            argFromBuildConfig.put(argString, args.getOrDefault(argString, ""));
+        }
+        return argFromBuildConfig;
+    }
+
+    private static void validateArgValue(String argStringParam) {
+        String[] argStringParts = argStringParam.split("\\s+");
+        if (argStringParts.length > 1) {
+            throw new IllegalArgumentException("Dockerfile parse error: ARG requires exactly one argument. Provided : " + argStringParam);
+        }
+    }
+
 }
diff --git a/...it/build/api/src/test/java/org/eclipse/jkube/kit/build/api/helper/DockerFileUtilTest.java b/...it/build/api/src/test/java/org/eclipse/jkube/kit/build/api/helper/DockerFileUtilTest.java
@@ -23,6 +23,8 @@
 import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -41,13 +43,13 @@ public class DockerFileUtilTest {
     @Test
     public void testSimple() throws Exception {
         File toTest = copyToTempDir("Dockerfile_from_simple");
-        assertEquals("fabric8/s2i-java", DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER).get(0));
+        assertEquals("fabric8/s2i-java", DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER, Collections.emptyMap()).get(0));
     }
 
     @Test
     public void testMultiStage() throws Exception {
         File toTest = copyToTempDir("Dockerfile_multi_stage");
-        Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER).iterator();
+        Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER, Collections.emptyMap()).iterator();
 
         assertEquals("fabric8/s2i-java", fromClauses.next());
         assertEquals("fabric8/s1i-java", fromClauses.next());
@@ -57,7 +59,7 @@ public void testMultiStage() throws Exception {
     @Test
     public void testMultiStageNamed() throws Exception {
         File toTest = copyToTempDir("Dockerfile_multi_stage_named_build_stages");
-        Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER).iterator();
+        Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER, Collections.emptyMap()).iterator();
 
         assertEquals("fabric8/s2i-java", fromClauses.next());
         assertFalse(fromClauses.hasNext());
@@ -66,7 +68,7 @@ public void testMultiStageNamed() throws Exception {
     @Test
     public void testMultiStageNamedWithDuplicates() throws Exception {
         File toTest = copyToTempDir("Dockerfile_multi_stage_named_redundant_build_stages");
-        Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER).iterator();
+        Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER, Collections.emptyMap()).iterator();
 
         assertEquals("centos", fromClauses.next());
         assertFalse(fromClauses.hasNext());
@@ -158,6 +160,63 @@ public void testCustomInterpolation() throws IOException {
         }
     }
 
+    @Test
+    public void testMultiStageWithArgs() throws Exception {
+        File toTest = copyToTempDir("Dockerfile_multi_stage_with_args");
+        Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER, Collections.emptyMap())
+                .iterator();
+
+        assertEquals("fabric8/s2i-java:latest", fromClauses.next());
+        assertEquals("busybox:latest", fromClauses.next());
+        assertFalse(fromClauses.hasNext());
+    }
+
+    @Test
+    public void testExtractArgsFromDockerfile() {
+        assertEquals("{VERSION=latest, FULL_IMAGE=busybox:latest}", DockerFileUtil.extractArgsFromLines(Arrays.asList(new String[]{"ARG", "VERSION:latest"}, new String[] {"ARG", "FULL_IMAGE=busybox:latest"}), Collections.emptyMap()).toString());
+        assertEquals("{user1=someuser, buildno=1}", DockerFileUtil.extractArgsFromLines(Arrays.asList(new String[]{"ARG", "user1=someuser"}, new String[]{"ARG", "buildno=1"}), Collections.emptyMap()).toString());
+        assertEquals("{NPM_VERSION=latest, NODE_VERSION=latest}", DockerFileUtil.extractArgsFromLines(Arrays.asList(new String[]{"ARG","NODE_VERSION=\"latest\""}, new String[]{"ARG",  "NPM_VERSION=\"latest\""}), Collections.emptyMap()).toString());
+        assertEquals("{NPM_VERSION=latest, NODE_VERSION=latest}", DockerFileUtil.extractArgsFromLines(Arrays.asList(new String[]{"ARG","NODE_VERSION='latest'"}, new String[]{"ARG",  "NPM_VERSION='latest'"}), Collections.emptyMap()).toString());
+        assertEquals("{MESSAGE=argument with spaces}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[] {"ARG", "MESSAGE='argument with spaces'"}), Collections.emptyMap()).toString());
+        assertEquals("{MESSAGE=argument with spaces}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[] {"ARG", "MESSAGE=\"argument with spaces\""}), Collections.emptyMap()).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM"}), Collections.emptyMap()).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM="}), Collections.emptyMap()).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM:"}), Collections.emptyMap()).toString());
+        assertEquals("{MESSAGE=argument:two}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=argument:two"}), Collections.emptyMap()).toString());
+        assertEquals("{MESSAGE2=argument=two}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE2=argument=two"}), Collections.emptyMap()).toString());
+        assertEquals("{VER=0.0.3}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER=0.0.3"}), Collections.emptyMap()).toString());
+        assertEquals("{VER={0.0.3}}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER={0.0.3}"}), Collections.emptyMap()).toString());
+        assertEquals("{VER=[0.0.3]}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER=[0.0.3]"}), Collections.emptyMap()).toString());
+        assertEquals("{VER={5,6}}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER={5,6}"}), Collections.emptyMap()).toString());
+        assertEquals("{VER={5,6}}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER={5,6}"}), Collections.emptyMap()).toString());
+        assertEquals("{VER={}}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER={}"}), Collections.emptyMap()).toString());
+        assertEquals("{VER=====}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER====="}), Collections.emptyMap()).toString());
+        assertEquals("{MESSAGE=:message}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=:message"}), Collections.emptyMap()).toString());
+        assertEquals("{MYAPP_IMAGE=myorg/myapp:latest}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MYAPP_IMAGE=myorg/myapp:latest"}), Collections.emptyMap()).toString());
+        assertEquals("{busyboxVersion=latest}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "busyboxVersion"}), Collections.singletonMap("busyboxVersion", "latest")).toString());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithSpacesFromDockerfile() {
+        DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MY_IMAGE image with spaces"}), Collections.emptyMap());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithTrailingArgumentFromDockerfile() {
+        DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=foo bar"}), Collections.emptyMap());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithArrayWithSpaceFromDockerfile() {
+        DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=[5, 6]"}), Collections.emptyMap());
+    }
+
+    @Test
+    public void testResolveArgValueFromStrContainingArgKey() {
+        assertEquals("latest", DockerFileUtil.resolveArgValueFromStrContainingArgKey("$VERSION", Collections.singletonMap("VERSION", "latest")));
+        assertEquals("test", DockerFileUtil.resolveArgValueFromStrContainingArgKey("${project.scope}", Collections.singletonMap("project.scope", "test")));
+    }
+
     private File getDockerfilePath(String dir) {
         ClassLoader classLoader = getClass().getClassLoader();
         return new File(Objects.requireNonNull(classLoader.getResource(

diff --git a/...rc/test/resources/org/eclipse/jkube/kit/build/api/helper/Dockerfile_multi_stage_with_args b/...rc/test/resources/org/eclipse/jkube/kit/build/api/helper/Dockerfile_multi_stage_with_args
@@ -0,0 +1,4 @@
+ARG VERSION:latest
+FROM fabric8/s2i-java:$VERSION AS BUILD
+ARG FULL_IMAGE=busybox:latest
+FROM $FULL_IMAGE AS DEPLOYABLE
diff --git a/...service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/BuildService.java b/...service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/BuildService.java
@@ -282,7 +282,7 @@ private List<String> extractBaseFromDockerfile(BuildConfiguration buildConfig, J
             File fullDockerFilePath = buildConfig.getAbsoluteDockerFilePath(configuration.getSourceDirectory(),
                 Optional.ofNullable(configuration.getProject().getBaseDirectory()).map(File::toString) .orElse(null));
 
-            fromImage = DockerFileUtil.extractBaseImages(fullDockerFilePath, configuration.getProperties(), buildConfig.getFilter());
+            fromImage = DockerFileUtil.extractBaseImages(fullDockerFilePath, configuration.getProperties(), buildConfig.getFilter(), buildConfig.getArgs());
         } catch (IOException e) {
             // Cant extract base image, so we wont try an auto pull. An error will occur later anyway when
             // building the image, so we are passive here.

diff --git a/...e/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildService.java b/...e/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildService.java
@@ -488,7 +488,7 @@ private String extractBaseFromDockerfile(BuildConfiguration buildConfig, JKubeCo
             File fullDockerFilePath = buildConfig.getAbsoluteDockerFilePath(configuration.getSourceDirectory(),
                 Optional.ofNullable(configuration.getProject().getBaseDirectory()).map(File::toString) .orElse(null));
             fromImage = DockerFileUtil.extractBaseImages(
-                    fullDockerFilePath, configuration.getProperties(), buildConfig.getFilter()).stream().findFirst().orElse(null);
+                    fullDockerFilePath, configuration.getProperties(), buildConfig.getFilter(), buildConfig.getArgs()).stream().findFirst().orElse(null);
         } catch (IOException e) {
             // Cant extract base image, so we wont try an auto pull. An error will occur later anyway when
             // building the image, so we are passive here.

diff --git a/quickstarts/maven/docker-file-simple/Dockerfile b/quickstarts/maven/docker-file-simple/Dockerfile
@@ -12,7 +12,8 @@
 #   Red Hat, Inc. - initial API and implementation
 #
 
-FROM openjdk:latest
+ARG VERSION=latest
+FROM openjdk:$VERSION
 COPY maven/target/docker-file-simple.jar /deployments/docker-file-simple.jar
 # Copying a file inside project root directory
 COPY maven/static-dir-in-project-root/my-file.txt /deployments/my-file.txt