Fix #174: Build failure when trying to use Dockerfile for arguments i…

…n FROM port of fabric8io/docker-maven-plugin#1299
eclipse-jkube · Sep 11, 2020 · 724e8f2 · 724e8f2
1 parent 9ce411d
commit 724e8f2
Show file tree

Hide file tree

Showing 6 changed files with 145 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ Usage:
 ./scripts/extract-changelog-for-version.sh 1.3.37 5
 ```
 ### 1.0.1-SNAPSHOT
+* Fix #174: Build failure when trying to use Dockerfile for arguments in FROM, port of fabric8io/docker-maven-plugin#1299
 
 ### 1.0.0 (2020-09-09)
 * Fix #351: Fix AutoTLSEnricher - add annotation + volume config to resource

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;
@@ -56,23 +57,34 @@ private DockerFileUtil() {}
      */
     public static List<String> extractBaseImages(File dockerFile, Properties properties, String filter) throws IOException {
         List<String[]> fromLines = extractLines(dockerFile, "FROM", properties, filter);
+        Map<String, String> args = extractArgs(dockerFile, properties, filter);
         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
+     * @return HashMap of arguments or empty collection if none is found
+     */
+    public static Map<String, String> extractArgs(File dockerfile, Properties properties, String filter) throws IOException {
+        return extractArgsFromLines(extractLines(dockerfile, "ARG", properties, filter));
+    }
+
     /**
      * Extract all lines containing the given keyword
      *
@@ -130,6 +142,47 @@ private static Reader getFileReaderFromDir(File file) {
         }
     }
 
+    /**
+     * Helper method for extractArgs(exposed for test)
+     *
+     * @param argLines list of string arrays containing lines with words
+     * @return map of parsed arguments
+     */
+    protected static Map<String, String> extractArgsFromLines(List<String[]> argLines) {
+        Map<String, String> result = new HashMap<>();
+        for (String[] argLine : argLines) {
+            if (argLine.length > 1) {
+                updateMapWithArgValue(result, 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;
+    }
+
+    private static String resolveArgValueFromStrContainingArgKey(String argString, Map<String, String> args) {
+        if (argString.startsWith("$") && args.containsKey(argString.substring(1))) {
+            return args.get(argString.substring(1));
+        }
+        return null;
+    }
+
     public static JsonObject readDockerConfig() {
         String dockerConfig = System.getenv("DOCKER_CONFIG");
 
@@ -209,4 +262,28 @@ private static File getHomeDir() {
         return new File(homeDir);
     }
 
+    private static void updateMapWithArgValue(Map<String, String> result, 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.put(argString.split("\\s+")[0], "");
+        }
+    }
+
+    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;
@@ -158,6 +160,56 @@ 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)
+                .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"})).toString());
+        assertEquals("{user1=someuser, buildno=1}", DockerFileUtil.extractArgsFromLines(Arrays.asList(new String[]{"ARG", "user1=someuser"}, new String[]{"ARG", "buildno=1"})).toString());
+        assertEquals("{NPM_VERSION=latest, NODE_VERSION=latest}", DockerFileUtil.extractArgsFromLines(Arrays.asList(new String[]{"ARG","NODE_VERSION=\"latest\""}, new String[]{"ARG",  "NPM_VERSION=\"latest\""})).toString());
+        assertEquals("{NPM_VERSION=latest, NODE_VERSION=latest}", DockerFileUtil.extractArgsFromLines(Arrays.asList(new String[]{"ARG","NODE_VERSION='latest'"}, new String[]{"ARG",  "NPM_VERSION='latest'"})).toString());
+        assertEquals("{MESSAGE=argument with spaces}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[] {"ARG", "MESSAGE='argument with spaces'"})).toString());
+        assertEquals("{MESSAGE=argument with spaces}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[] {"ARG", "MESSAGE=\"argument with spaces\""})).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM"})).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM="})).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM:"})).toString());
+        assertEquals("{MESSAGE=argument:two}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=argument:two"})).toString());
+        assertEquals("{MESSAGE2=argument=two}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE2=argument=two"})).toString());
+        assertEquals("{VER=0.0.3}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER=0.0.3"})).toString());
+        assertEquals("{VER={0.0.3}}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER={0.0.3}"})).toString());
+        assertEquals("{VER=[0.0.3]}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER=[0.0.3]"})).toString());
+        assertEquals("{VER={5,6}}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER={5,6}"})).toString());
+        assertEquals("{VER={5,6}}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER={5,6}"})).toString());
+        assertEquals("{VER={}}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER={}"})).toString());
+        assertEquals("{VER=====}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "VER====="})).toString());
+        assertEquals("{MESSAGE=:message}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=:message"})).toString());
+        assertEquals("{MYAPP_IMAGE=myorg/myapp:latest}", DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MYAPP_IMAGE=myorg/myapp:latest"})).toString());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithSpacesFromDockerfile() {
+        DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MY_IMAGE image with spaces"}));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithTrailingArgumentFromDockerfile() {
+        DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=foo bar"}));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithArrayWithSpaceFromDockerfile() {
+        DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=[5, 6]"}));
+    }
+
     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/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

diff --git a/quickstarts/maven/docker-file-simple/pom.xml b/quickstarts/maven/docker-file-simple/pom.xml
@@ -38,7 +38,7 @@
 
     <properties>
         <java.version>11</java.version>
-        <jkube.version>${project.version}</jkube.version>
+        <jkube.version>1.0.1-SNAPSHOT</jkube.version>
         <jkube.enricher.jkube-service.port>8080</jkube.enricher.jkube-service.port>
         <jkube.enricher.jkube-service.type>NodePort</jkube.enricher.jkube-service.type>
     </properties>