Skip to content

Commit

Permalink
Fix #1372: Plugin now resolves ARGs provided in BuildImageConfigurati…
Browse files Browse the repository at this point in the history 
…on (#1373)

Refactored `DockerFileUtil.extractBaseImages` to accept a HashMap of
Build args which would be passed from BuildService in order to resolve
ARG values which are provided from plugin configuration

Co-authored-by: Roland Huß <roland@ro14nd.de>
  • Loading branch information
@rohanKanojia @rhuss
rohanKanojia and rhuss authored Sep 27, 2020
1 parent 6177b15 commit dae2839
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 45 deletions.
3 changes: 2 additions & 1 deletion doc/changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* **0.34-SNAPSHOT**
- Fix NPE with "skipPush" and no build configuration given (#1381)
- upgrade to jib-core 0.15.0 (#1378)

- Plugin now resolves ARG provided in BuildImageConfiguration (#1373)

* **0.34.0** (2020-09-13)
- Support `ARG` in `FROM` ([#859](https://github.com/fabric8io/docker-maven-plugin/issues/859))
- Handle authentication tokens returned from credential helpers ([#1348](https://github.com/fabric8io/docker-maven-plugin/issues/1348))
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/io/fabric8/maven/docker/service/BuildService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,8 @@ private List<String> extractBaseFromDockerfile(BuildImageConfiguration buildConf
File fullDockerFilePath = buildConfig.getAbsoluteDockerFilePath(buildContext.getMojoParameters());
fromImage = DockerFileUtil.extractBaseImages(
fullDockerFilePath,
DockerFileUtil.createInterpolator(buildContext.getMojoParameters(), buildConfig.getFilter()));
DockerFileUtil.createInterpolator(buildContext.getMojoParameters(), 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.
Expand Down
40 changes: 25 additions & 15 deletions src/main/java/io/fabric8/maven/docker/util/DockerFileUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ private DockerFileUtil() {}
* @param interpolator interpolator for replacing properties
* @return LinkedList of base images name or empty collection if none is found.
*/
public static List<String> extractBaseImages(File dockerFile, FixedStringSearchInterpolator interpolator) throws IOException {
public static List<String> extractBaseImages(File dockerFile, FixedStringSearchInterpolator interpolator, Map<String, String> argsFromBuildConfig) throws IOException {
List<String[]> fromLines = extractLines(dockerFile, "FROM", interpolator);
Map<String, String> args = extractArgs(dockerFile, interpolator);
Map<String, String> args = extractArgs(dockerFile, argsFromBuildConfig, interpolator);
Set<String> result = new LinkedHashSet<>();
Set<String> fromAlias = new HashSet<>();
for (String[] fromLine : fromLines) {
Expand All @@ -80,8 +80,8 @@ public static List<String> extractBaseImages(File dockerFile, FixedStringSearchI
* @param interpolator interpolator for replacement
* @return HashMap of arguments or empty collection if none is found
*/
public static Map<String, String> extractArgs(File dockerfile, FixedStringSearchInterpolator interpolator) throws IOException {
return extractArgsFromLines(extractLines(dockerfile, "ARG", interpolator));
public static Map<String, String> extractArgs(File dockerfile, Map<String, String> argsFromBuildConfig, FixedStringSearchInterpolator interpolator) throws IOException {
return extractArgsFromLines(extractLines(dockerfile, "ARG", interpolator), argsFromBuildConfig);
}

/**
Expand Down Expand Up @@ -155,16 +155,25 @@ public static FixedStringSearchInterpolator createInterpolator(MojoParameters pa
* @param argLines list of string arrays containing lines with words
* @return map of parsed arguments
*/
protected static Map<String, String> extractArgsFromLines(List<String[]> argLines) {
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, argLine[1]);
updateMapWithArgValue(result, argsFromBuildConfig, argLine[1]);
}
}
return result;
}

static String resolveArgValueFromStrContainingArgKey(String argString, Map<String, String> args) {
if (argString.startsWith("$") && args.containsKey(argString.substring(1))) {
return args.get(argString.substring(1));
} else if (argString.startsWith("${") && argString.endsWith("}") && args.containsKey(argString.substring(2, argString.length() - 1))) {
return args.get(argString.substring(2, argString.length() - 1));
}
return null;
}

private static String resolveImageTagFromArgs(String imageTagString, Map<String, String> args) {
if (imageTagString.startsWith("$")) { // FROM $IMAGE
String resolvedVal = resolveArgValueFromStrContainingArgKey(imageTagString, args);
Expand All @@ -183,13 +192,6 @@ private static String resolveImageTagFromArgs(String imageTagString, Map<String,
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) {
try {
Expand Down Expand Up @@ -248,7 +250,7 @@ private static File getHomeDir() {
return new File(homeDir);
}

private static void updateMapWithArgValue(Map<String, String> result, String argString) {
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);
Expand All @@ -261,8 +263,16 @@ private static void updateMapWithArgValue(Map<String, String> result, String arg
result.put(argStringParts[0], argStringValue);
} else {
validateArgValue(argString);
result.put(argString.split("\\s+")[0], "");
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) {
Expand Down
63 changes: 35 additions & 28 deletions src/test/java/io/fabric8/maven/docker/util/DockerFileUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ public class DockerFileUtilTest {
public void testSimple() throws Exception {
File toTest = copyToTempDir("Dockerfile_from_simple");
assertEquals("fabric8/s2i-java", DockerFileUtil.extractBaseImages(
toTest, FixedStringSearchInterpolator.create()).get(0));
toTest, FixedStringSearchInterpolator.create(), Collections.emptyMap()).get(0));
}

@Test
public void testMultiStage() throws Exception {
File toTest = copyToTempDir("Dockerfile_multi_stage");
Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(
toTest, FixedStringSearchInterpolator.create()).iterator();
toTest, FixedStringSearchInterpolator.create(), Collections.emptyMap()).iterator();

assertEquals("fabric8/s2i-java", fromClauses.next());
assertEquals("fabric8/s1i-java", fromClauses.next());
Expand All @@ -61,7 +61,7 @@ public void testMultiStage() throws Exception {
public void testMultiStageNamed() throws Exception {
File toTest = copyToTempDir("Dockerfile_multi_stage_named_build_stages");
Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(
toTest, FixedStringSearchInterpolator.create()).iterator();
toTest, FixedStringSearchInterpolator.create(), Collections.emptyMap()).iterator();

assertEquals("fabric8/s2i-java", fromClauses.next());
assertFalse(fromClauses.hasNext());
Expand All @@ -71,7 +71,7 @@ public void testMultiStageNamed() throws Exception {
public void testMultiStageWithArgs() throws Exception {
File toTest = copyToTempDir("Dockerfile_multi_stage_with_args");
Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(
toTest, FixedStringSearchInterpolator.create()).iterator();
toTest, FixedStringSearchInterpolator.create(), Collections.emptyMap()).iterator();

assertEquals("fabric8/s2i-java:latest", fromClauses.next());
assertEquals("busybox:latest", fromClauses.next());
Expand All @@ -80,54 +80,61 @@ public void testMultiStageWithArgs() throws Exception {

@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());
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"}));
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"}));
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]"}));
DockerFileUtil.extractArgsFromLines(Collections.singletonList(new String[]{"ARG", "MESSAGE=[5, 6]"}), Collections.emptyMap());
}

@Test
public void testMultiStageNamedWithDuplicates() throws Exception {
File toTest = copyToTempDir("Dockerfile_multi_stage_named_redundant_build_stages");
Iterator<String> fromClauses = DockerFileUtil.extractBaseImages(
toTest, FixedStringSearchInterpolator.create()).iterator();
toTest, FixedStringSearchInterpolator.create(), Collections.emptyMap()).iterator();

assertEquals("centos", fromClauses.next());
assertFalse(fromClauses.hasNext());

}

@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 copyToTempDir(String resource) throws IOException {
File dir = Files.createTempDirectory("d-m-p").toFile();
File ret = new File(dir, "Dockerfile");
Expand Down

0 comments on commit dae2839

Please sign in to comment.