From 7303467c771cc98f0efcdf8cbb2e0538ad496b72 Mon Sep 17 00:00:00 2001 From: cyphar Date: Tue, 17 Jun 2014 00:08:02 +1000 Subject: [PATCH 1/2] server: buildfile: ensure that ONBUILD triggers aren't committed This patch fixes the bug where ONBUILD triggers are committed each build step created during the ONBUILD trigger execution, since the triggers are only wiped *after* all ONBUILD trigger steps have been committed. This was fixed by simply copying the ONBUILD triggers and wiping the config *before* committing anything. Docker-DCO-1.1-Signed-off-by: Aleksa Sarai (github: cyphar) --- server/buildfile.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/buildfile.go b/server/buildfile.go index 26fc49890..5b94c9423 100644 --- a/server/buildfile.go +++ b/server/buildfile.go @@ -123,7 +123,12 @@ func (b *buildFile) CmdFrom(name string) error { if nTriggers := len(b.config.OnBuild); nTriggers != 0 { fmt.Fprintf(b.errStream, "# Executing %d build triggers\n", nTriggers) } - for n, step := range b.config.OnBuild { + + // Copy the ONBUILD triggers, and remove them from the config, since the config will be commited. + onBuildTriggers := b.config.OnBuild + b.config.OnBuild = []string{} + + for n, step := range onBuildTriggers { splitStep := strings.Split(step, " ") stepInstruction := strings.ToUpper(strings.Trim(splitStep[0], " ")) switch stepInstruction { @@ -136,7 +141,6 @@ func (b *buildFile) CmdFrom(name string) error { return err } } - b.config.OnBuild = []string{} return nil } From a57298791c882a2a065989b3386b5b14084e8639 Mon Sep 17 00:00:00 2001 From: cyphar Date: Tue, 17 Jun 2014 16:04:25 +1000 Subject: [PATCH 2/2] integration-cli: add build test for NOCACHE This patch adds CLI integration tests to ensure that NOCACHE instructions in Dockerfiles only apply to direct children of the original image. Docker-DCO-1.1-Signed-off-by: Aleksa Sarai (github: cyphar) --- integration-cli/docker_cli_build_test.go | 63 ++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 083708832..9a360c196 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -588,6 +588,7 @@ func TestBuildRm(t *testing.T) { logDone("build - ensure --rm doesn't leave containers behind and that --rm=true is the default") logDone("build - ensure --rm=false overrides the default") } + func TestBuildWithVolumes(t *testing.T) { name := "testbuildvolumes" expected := "map[/test1:map[] /test2:map[]]" @@ -766,6 +767,68 @@ func TestBuildEntrypoint(t *testing.T) { logDone("build - entrypoint") } +// #6445 ensure ONBUILD triggers aren't committed to grandchildren +func TestBuildOnBuildLimitedInheritence(t *testing.T) { + name1 := "testonbuildtrigger1" + dockerfile1 := ` + FROM busybox + RUN echo "GRANDPARENT" + ONBUILD RUN echo "ONBUILD PARENT" + ` + ctx1, err := fakeContext(dockerfile1, nil) + if err != nil { + t.Fatal(err) + } + + buildCmd := exec.Command(dockerBinary, "build", "-t", name1, ".") + buildCmd.Dir = ctx1.Dir + out1, _, err := runCommandWithOutput(buildCmd) + errorOut(err, t, fmt.Sprintf("build failed to complete: %v %v", out1, err)) + defer deleteImages(name1) + + name2 := "testonbuildtrigger2" + dockerfile2 := ` + FROM testonbuildtrigger1 + ` + ctx2, err := fakeContext(dockerfile2, nil) + if err != nil { + t.Fatal(err) + } + + buildCmd = exec.Command(dockerBinary, "build", "-t", name2, ".") + buildCmd.Dir = ctx2.Dir + out2, _, err := runCommandWithOutput(buildCmd) + errorOut(err, t, fmt.Sprintf("build failed to complete: %v %v", out2, err)) + defer deleteImages(name2) + + name3 := "testonbuildtrigger3" + dockerfile3 := ` + FROM testonbuildtrigger2 + ` + ctx3, err := fakeContext(dockerfile3, nil) + if err != nil { + t.Fatal(err) + } + + buildCmd = exec.Command(dockerBinary, "build", "-t", name3, ".") + buildCmd.Dir = ctx3.Dir + out3, _, err := runCommandWithOutput(buildCmd) + errorOut(err, t, fmt.Sprintf("build failed to complete: %v %v", out3, err)) + defer deleteImages(name3) + + // ONBUILD should be run in second build. + if !strings.Contains(out2, "ONBUILD PARENT") { + t.Fatalf("ONBUILD instruction did not run in child of ONBUILD parent") + } + + // ONBUILD should *not* be run in third build. + if strings.Contains(out3, "ONBUILD PARENT") { + t.Fatalf("ONBUILD instruction ran in grandchild of ONBUILD parent") + } + + logDone("build - onbuild") +} + func TestBuildWithCache(t *testing.T) { name := "testbuildwithcache" defer deleteImages(name)