Skip to content

Support ONBUILD instruction in Dockerfiles #393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcuslinke merged 2 commits into from
Dec 8, 2015
Merged

Support ONBUILD instruction in Dockerfiles #393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be493f0
Added test to demonstrate #219
Dec 7, 2015
9653b70
Fixes #219
Dec 7, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 9 additions & 71 deletions src/main/java/com/github/dockerjava/core/dockerfile/Dockerfile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import com.github.dockerjava.core.util.FilePathUtil;
import com.github.dockerjava.core.GoLangFileMatch;
import com.github.dockerjava.core.exception.GoLangFileMatchException;
import com.github.dockerjava.core.GoLangMatchFileFilter;
import com.google.common.base.Function;
import com.google.common.base.Objects;
import com.google.common.base.Optional;
Expand All @@ -21,9 +20,7 @@
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;

/**
Expand Down Expand Up @@ -115,8 +112,6 @@ public class ScannedResult {

final List<String> ignores;

final Map<String, String> environmentMap = new HashMap<String, String>();

final List<File> filesToAdd = new ArrayList<File>();

public InputStream buildDockerFolderTar() {
Expand Down Expand Up @@ -162,7 +157,8 @@ public void close() throws IOException {

@Override
public String toString() {
return Objects.toStringHelper(this).add("ignores", ignores).add("environmentMap", environmentMap)
return Objects.toStringHelper(this)
.add("ignores", ignores)
.add("filesToAdd", filesToAdd).toString();
}

Expand All @@ -172,18 +168,17 @@ public ScannedResult() throws IOException {

String matchingIgnorePattern = effectiveMatchingIgnorePattern(dockerFile);

if (matchingIgnorePattern == null) {
filesToAdd.add(dockerFile);
} else {
if (matchingIgnorePattern != null) {
throw new DockerClientException(String.format(
"Dockerfile is excluded by pattern '%s' in .dockerignore file", matchingIgnorePattern));
}

for (DockerfileStatement statement : getStatements()) {
if (statement instanceof DockerfileStatement.Env) {
processEnvStatement((DockerfileStatement.Env) statement);
} else if (statement instanceof DockerfileStatement.Add) {
processAddStatement((DockerfileStatement.Add) statement);
Collection<File> filesInBuildContext = FileUtils.listFiles(getDockerFolder(), TrueFileFilter.INSTANCE,
TrueFileFilter.INSTANCE);

for (File f : filesInBuildContext) {
if (effectiveMatchingIgnorePattern(f) == null) {
filesToAdd.add(f);
}
}
}
Expand Down Expand Up @@ -238,62 +233,5 @@ private String effectiveMatchingIgnorePattern(File file) {

return lastMatchingPattern;
}

private void processAddStatement(DockerfileStatement.Add add) throws IOException {

add = add.transform(environmentMap);

for (String resource : add.getFileResources()) {

File dockerFolder = getDockerFolder();

File src = new File(resource);
if (!src.isAbsolute()) {
src = new File(dockerFolder, resource);
} else {
throw new DockerClientException(String.format("Source file %s must be relative to %s", src,
dockerFolder));
}

if (src.exists()) {
src = src.getCanonicalFile();
if (src.isDirectory()) {
Collection<File> files = FileUtils.listFiles(src, new GoLangMatchFileFilter(src, ignores),
TrueFileFilter.INSTANCE);
filesToAdd.addAll(files);
} else if (effectiveMatchingIgnorePattern(src) == null) {
filesToAdd.add(src);
} else {
throw new DockerClientException(String.format(
"Source file %s is excluded by .dockerignore file", src));
}
} else {
filesToAdd.addAll(resolveWildcards(src, ignores));
}
}
}

private Collection<File> resolveWildcards(File file, List<String> ignores) {
List<File> filesToAdd = new ArrayList<File>();

File parent = file.getParentFile();
if (parent != null) {
if (parent.isDirectory()) {
Collection<File> files = FileUtils.listFiles(parent, new GoLangMatchFileFilter(parent, ignores),
TrueFileFilter.INSTANCE);
filesToAdd.addAll(files);
} else {
filesToAdd.addAll(resolveWildcards(parent, ignores));
}
} else {
throw new DockerClientException(String.format("Source file %s doesn't exist", file));
}

return filesToAdd;
}

private void processEnvStatement(DockerfileStatement.Env env) {
environmentMap.put(env.variable, env.value);
}
}
}
13 changes: 13 additions & 0 deletions src/test/java/com/github/dockerjava/core/command/BuildImageCmdImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,19 @@ public void testDockerBuilderFromTar() throws Exception {
assertThat(response, containsString("Successfully executed testrun.sh"));
}

@Test
public void testDockerBuildWithOnBuild() throws Exception {
File baseDir = new File(Thread.currentThread().getContextClassLoader().getResource("testAddOnBuild/onbuild").getFile());
dockerClient.buildImageCmd(baseDir)
.withNoCache(true)
.withTag("docker-java-onbuild")
.exec(new BuildImageResultCallback())
.awaitImageId();
baseDir = new File(Thread.currentThread().getContextClassLoader().getResource("testAddOnBuild/test").getFile());
String response = dockerfileBuild(baseDir);
assertThat(response, containsString("Successfully executed testrun.sh"));
}

@Test
public void testDockerBuilderAddUrl() throws Exception {
File baseDir = new File(Thread.currentThread().getContextClassLoader().getResource("testAddUrl").getFile());
Expand Down
8 changes: 8 additions & 0 deletions src/test/resources/testAddOnBuild/onbuild/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
FROM ubuntu:latest

# Copy testrun.sh files into the container

ONBUILD ADD ./testrun.sh /tmp/
ONBUILD RUN cp /tmp/testrun.sh /usr/local/bin/ && chmod +x /usr/local/bin/testrun.sh

CMD ["testrun.sh"]
1 change: 1 addition & 0 deletions src/test/resources/testAddOnBuild/test/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FROM docker-java-onbuild
3 changes: 3 additions & 0 deletions src/test/resources/testAddOnBuild/test/testrun.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/sh

echo "Successfully executed testrun.sh"
2 changes: 1 addition & 1 deletion src/test/resources/testDockerignore/.dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
b
*/b
Copy link
Copy Markdown
Contributor

@marcuslinke marcuslinke Dec 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orzeh Could you explain this change please?

Copy link
Copy Markdown
Contributor Author

@orzeh orzeh Dec 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to .dockerignore doc:

the root of the context is considered to be both the working and the root directory

then pattern b excludes all files and directories that are named b in root directory of build context.
However, in this test the file named b is under directory a, so it should not be ignored. After my changes this tests fails.

Pattern */b works as expected (and described in the doc see */temp* example)

Copy link
Copy Markdown
Contributor

@marcuslinke marcuslinke Dec 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, now I got it. Thanks for explanation.