From 98e914841b8a967eeaaa58cbed6c96581af2f55f Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Fri, 11 Sep 2020 20:30:40 +0530 Subject: [PATCH] Fix #174: Build failure when trying to use Dockerfile for arguments in FROM Port of fabric8io/docker-maven-plugin#1299 --- CHANGELOG.md | 1 + .../kit/build/api/helper/DockerFileUtil.java | 116 ++++++++++++++++-- .../build/api/helper/DockerFileUtilTest.java | 67 +++++++++- .../helper/Dockerfile_multi_stage_with_args | 4 + .../build/service/docker/BuildService.java | 2 +- .../openshift/OpenshiftBuildService.java | 2 +- .../maven/docker-file-simple/Dockerfile | 3 +- 7 files changed, 178 insertions(+), 17 deletions(-) create mode 100644 jkube-kit/build/api/src/test/resources/org/eclipse/jkube/kit/build/api/helper/Dockerfile_multi_stage_with_args diff --git a/CHANGELOG.md b/CHANGELOG.md index 9637a84453..6c27958eb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Usage: * Fix: Valid content type for REST docker API requests with body * Fix #448: Service.spec.type added from config if existing Service fragment doesn't specify it * 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 index 0166148eaf..bec84672e3 100644 --- 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 extractBaseImages(File dockerFile, Properties properties, String filter) throws IOException { + public static List extractBaseImages(File dockerFile, Properties properties, String filter, Map argsFromBuildConfig) throws IOException { List fromLines = extractLines(dockerFile, "FROM", properties, filter); + Map args = extractArgs(dockerFile, properties, filter, argsFromBuildConfig); Set result = new LinkedHashSet<>(); Set 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 extractArgs(File dockerfile, Properties properties, String filter, Map 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 extractArgsFromLines(List argLines, Map argsFromBuildConfig) { + Map 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 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 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 readKubeConfig() { : getFileReaderFromDir(new File(kubeConfig)); if (reader != null) { Yaml ret = new Yaml(); - return (Map) 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 result, Map 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 fetchArgsFromBuildConfiguration(String argString, Map args) { + Map 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/jkube-kit/build/api/src/test/java/org/eclipse/jkube/kit/build/api/helper/DockerFileUtilTest.java b/jkube-kit/build/api/src/test/java/org/eclipse/jkube/kit/build/api/helper/DockerFileUtilTest.java index 54e217fc4b..c35245d765 100644 --- a/jkube-kit/build/api/src/test/java/org/eclipse/jkube/kit/build/api/helper/DockerFileUtilTest.java +++ b/jkube-kit/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 fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER).iterator(); + Iterator 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 fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER).iterator(); + Iterator 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 fromClauses = DockerFileUtil.extractBaseImages(toTest, new Properties(), BuildConfiguration.DEFAULT_FILTER).iterator(); + Iterator 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 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/jkube-kit/build/api/src/test/resources/org/eclipse/jkube/kit/build/api/helper/Dockerfile_multi_stage_with_args b/jkube-kit/build/api/src/test/resources/org/eclipse/jkube/kit/build/api/helper/Dockerfile_multi_stage_with_args new file mode 100644 index 0000000000..920c8226ad --- /dev/null +++ b/jkube-kit/build/api/src/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 \ No newline at end of file diff --git a/jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/BuildService.java b/jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/BuildService.java index 25461ff022..6f265212ec 100644 --- a/jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/BuildService.java +++ b/jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/BuildService.java @@ -282,7 +282,7 @@ private List 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/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildService.java b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildService.java index 16fda4e294..488f261b89 100644 --- a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildService.java +++ b/jkube-kit/config/service/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 index ca9fef6d61..a2f7247441 100644 --- 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