Fix #859: ARG not support in FROM

fabric8io · May 2, 2020 · e84d0c8 · e84d0c8
1 parent e4548d2
commit e84d0c8
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 14 deletions.
diff --git a/doc/changelog.md b/doc/changelog.md
@@ -1,5 +1,8 @@
 # ChangeLog
 
+* **0.33-SNAPSHOT**
+  - Support `ARG` in `FROM` (#859)
+
 * **0.33.0** (2020-01-21)
   - Update to jnr-unixsocket 0.25 to solve concurrency issues ([#552](https://github.com/fabric8io/docker-maven-plugin/issues/552))
   - Udate ECR AuthorizationToken URL to new endpoint ([#1317](https://github.com/fabric8io/docker-maven-plugin/issues/1317))

diff --git a/samples/zero-config/Dockerfile b/samples/zero-config/Dockerfile
@@ -1,7 +1,7 @@
 FROM openjdk:jre
 
-ARG version=1-SNAPSHOT
-ARG jar_file=target/zero-config-$version.jar
+ARG jar_file=target/dmp-sample-zero-config.jar
 
+ARG FOO=${}
 ADD $jar_file /tmp/zero-config.jar
 CMD java -cp /tmp/zero-config.jar HelloWorld
diff --git a/samples/zero-config/pom.xml b/samples/zero-config/pom.xml
@@ -13,15 +13,18 @@
 
 
   <groupId>io.fabric8.dmp.samples</groupId>
-  <artifactId>demp-sample-zero-config</artifactId>
+  <artifactId>dmp-sample-zero-config</artifactId>
   <packaging>jar</packaging>
   <version>0.33-SNAPSHOT</version>
 
   <properties>
     <jar_file>${project.build.directory}/${project.build.finalName}.jar</jar_file>
+    <maven.compiler.source>1.8</maven.compiler.source>
+    <maven.compiler.target>1.8</maven.compiler.target>
   </properties>
 
   <build>
+    <finalName>dmp-sample-zero-config</finalName>
     <plugins>
       <plugin>
         <groupId>io.fabric8</groupId>

diff --git a/src/main/java/io/fabric8/maven/docker/util/DockerFileUtil.java b/src/main/java/io/fabric8/maven/docker/util/DockerFileUtil.java
@@ -17,14 +17,14 @@
 
 import java.io.*;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.Collectors;
 
 import com.google.gson.Gson;
 import com.google.gson.JsonObject;
@@ -55,21 +55,38 @@ private DockerFileUtil() {}
      */
     public static List<String> extractBaseImages(File dockerFile, FixedStringSearchInterpolator interpolator) throws IOException {
         List<String[]> fromLines = extractLines(dockerFile, "FROM", interpolator);
+        Map<String, String> args = extractArgs(extractLines(dockerFile, "ARG", interpolator));
         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]);
+                    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(fromLine[1]);
+                        result.add(resolveImageTagFromArgs(fromLine[1], args));
                     }
-                    fromAlias.add(fromLine[3]);
+                    fromAlias.add(resolveImageTagFromArgs(fromLine[3], args));
                 }
             }
         }
-        return result.stream().collect(Collectors.toList());
+        return new ArrayList<>(result);
+    }
+
+    /**
+     * Extract Args from dockerfile. All lines containing ARG is taken.
+     *
+     * @param argLines list of lines containing arguments
+     * @return HashMap of arguments or empty collection if none is found
+     */
+    public static Map<String, String> extractArgs(List<String[]> argLines) {
+        Map<String, String> result = new HashMap<>();
+        for (String[] argLine : argLines) {
+            if (argLine.length > 1) {
+                updateMapWithArgValue(result, argLine[1]);
+            }
+        }
+        return result;
     }
 
     /**
@@ -137,6 +154,30 @@ public static FixedStringSearchInterpolator createInterpolator(MojoParameters pa
                 .withExpressionMarkers(delimiters[0], delimiters[1]);
     }
 
+    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;
+    }
 
     private static Reader getFileReaderFromDir(File file) {
         if (file.exists() && file.length() != 0) {
@@ -196,4 +237,27 @@ 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/src/test/java/io/fabric8/maven/docker/util/DockerFileUtilTest.java b/src/test/java/io/fabric8/maven/docker/util/DockerFileUtilTest.java
@@ -29,11 +29,9 @@
 import org.codehaus.plexus.util.IOUtil;
 import org.junit.Test;
 
-import mockit.Mock;
-import mockit.MockUp;
-
 import static io.fabric8.maven.docker.util.PathTestUtil.createTmpFile;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 
 /**
  * @author roland
@@ -56,7 +54,7 @@ public void testMultiStage() throws Exception {
 
         assertEquals("fabric8/s2i-java", fromClauses.next());
         assertEquals("fabric8/s1i-java", fromClauses.next());
-        assertEquals(false, fromClauses.hasNext());
+        assertFalse(fromClauses.hasNext());
     }
 
     @Test
@@ -66,7 +64,57 @@ public void testMultiStageNamed() throws Exception {
                 toTest, FixedStringSearchInterpolator.create()).iterator();
 
         assertEquals("fabric8/s2i-java", fromClauses.next());
-        assertEquals(false, fromClauses.hasNext());
+        assertFalse(fromClauses.hasNext());
+    }
+
+    @Test
+    public void testMultiStageWithArgs() throws Exception {
+        File toTest = copyToTempDir("Dockerfile_multi_stage_with_args");
+        Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(
+                toTest, FixedStringSearchInterpolator.create()).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.extractArgs(Arrays.asList(new String[]{"ARG", "VERSION:latest"}, new String[] {"ARG", "FULL_IMAGE=busybox:latest"})).toString());
+        assertEquals("{user1=someuser, buildno=1}", DockerFileUtil.extractArgs(Arrays.asList(new String[]{"ARG", "user1=someuser"}, new String[]{"ARG", "buildno=1"})).toString());
+        assertEquals("{NPM_VERSION=latest, NODE_VERSION=latest}", DockerFileUtil.extractArgs(Arrays.asList(new String[]{"ARG","NODE_VERSION=\"latest\""}, new String[]{"ARG",  "NPM_VERSION=\"latest\""})).toString());
+        assertEquals("{NPM_VERSION=latest, NODE_VERSION=latest}", DockerFileUtil.extractArgs(Arrays.asList(new String[]{"ARG","NODE_VERSION='latest'"}, new String[]{"ARG",  "NPM_VERSION='latest'"})).toString());
+        assertEquals("{MESSAGE=argument with spaces}", DockerFileUtil.extractArgs(Collections.singletonList(new String[] {"ARG", "MESSAGE='argument with spaces'"})).toString());
+        assertEquals("{MESSAGE=argument with spaces}", DockerFileUtil.extractArgs(Collections.singletonList(new String[] {"ARG", "MESSAGE=\"argument with spaces\""})).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM"})).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM="})).toString());
+        assertEquals("{TARGETPLATFORM=}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "TARGETPLATFORM:"})).toString());
+        assertEquals("{MESSAGE=argument:two}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "MESSAGE=argument:two"})).toString());
+        assertEquals("{MESSAGE2=argument=two}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "MESSAGE2=argument=two"})).toString());
+        assertEquals("{VER=0.0.3}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "VER=0.0.3"})).toString());
+        assertEquals("{VER={0.0.3}}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "VER={0.0.3}"})).toString());
+        assertEquals("{VER=[0.0.3]}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "VER=[0.0.3]"})).toString());
+        assertEquals("{VER={5,6}}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "VER={5,6}"})).toString());
+        assertEquals("{VER={5,6}}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "VER={5,6}"})).toString());
+        assertEquals("{VER={}}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "VER={}"})).toString());
+        assertEquals("{VER=====}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "VER====="})).toString());
+        assertEquals("{MESSAGE=:message}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "MESSAGE=:message"})).toString());
+        assertEquals("{MYAPP_IMAGE=myorg/myapp:latest}", DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "MYAPP_IMAGE=myorg/myapp:latest"})).toString());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithSpacesFromDockerfile() {
+        DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "MY_IMAGE image with spaces"}));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithTrailingArgumentFromDockerfile() {
+        DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "MESSAGE=foo bar"}));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidArgWithArrayWithSpaceFromDockerfile() {
+        DockerFileUtil.extractArgs(Collections.singletonList(new String[]{"ARG", "MESSAGE=[5, 6]"}));
     }
 
     @Test
@@ -76,7 +124,7 @@ public void testMultiStageNamedWithDuplicates() throws Exception {
                 toTest, FixedStringSearchInterpolator.create()).iterator();
 
         assertEquals("centos", fromClauses.next());
-        assertEquals(false, fromClauses.hasNext());
+        assertFalse(fromClauses.hasNext());
 
     }
 

diff --git a/src/test/resources/io/fabric8/maven/docker/util/Dockerfile_multi_stage_with_args b/src/test/resources/io/fabric8/maven/docker/util/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