Merge pull request #4260 from unclejack/onbuild_fixes

forbid chained onbuild, from & maintainer triggers

buildfile.go

@@ -117,6 +117,14 @@ func (b *buildFile) CmdFrom(name string) error {
 		fmt.Fprintf(b.errStream, "# Executing %d build triggers\n", nTriggers)
 	}
 	for n, step := range b.config.OnBuild {
+		splitStep := strings.Split(step, " ")
+		stepInstruction := strings.ToUpper(strings.Trim(splitStep[0], " "))
+		switch stepInstruction {
+		case "ONBUILD":
+			return fmt.Errorf("Source image contains forbidden chained `ONBUILD ONBUILD` trigger: %s", step)
+		case "MAINTAINER", "FROM":
+			return fmt.Errorf("Source image contains forbidden %s trigger: %s", stepInstruction, step)
+		}
 		if err := b.BuildStep(fmt.Sprintf("onbuild-%d", n), step); err != nil {
 			return err
 		}
@@ -128,6 +136,14 @@ func (b *buildFile) CmdFrom(name string) error {
 // The ONBUILD command declares a build instruction to be executed in any future build
 // using the current image as a base.
 func (b *buildFile) CmdOnbuild(trigger string) error {
+	splitTrigger := strings.Split(trigger, " ")
+	triggerInstruction := strings.ToUpper(strings.Trim(splitTrigger[0], " "))
+	switch triggerInstruction {
+	case "ONBUILD":
+		return fmt.Errorf("Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed")
+	case "MAINTAINER", "FROM":
+		return fmt.Errorf("%s isn't allowed as an ONBUILD trigger", triggerInstruction)
+	}
 	b.config.OnBuild = append(b.config.OnBuild, trigger)
 	return b.commit("", b.config.Cmd, fmt.Sprintf("ONBUILD %s", trigger))
 }

docs/sources/reference/builder.rst

@@ -466,6 +466,8 @@ For example you might add something like this:
     ONBUILD RUN /usr/local/bin/python-build --dir /app/src
     [...]
 
+.. warning:: Chaining ONBUILD instructions using `ONBUILD ONBUILD` isn't allowed.
+.. warning:: ONBUILD may not trigger FROM or MAINTAINER instructions.
 
 .. _dockerfile_examples:
 

integration/buildfile_test.go

@@ -924,3 +924,45 @@ func TestBuildOnBuildTrigger(t *testing.T) {
 	}
 	// FIXME: test that the 'foobar' file was created in the final build.
 }
+
+func TestBuildOnBuildForbiddenChainedTrigger(t *testing.T) {
+	_, err := buildImage(testContextTemplate{`
+	from {IMAGE}
+	onbuild onbuild run echo test
+	`,
+		nil, nil,
+	},
+		t, nil, true,
+	)
+	if err == nil {
+		t.Fatal("Error should not be nil")
+	}
+}
+
+func TestBuildOnBuildForbiddenFromTrigger(t *testing.T) {
+	_, err := buildImage(testContextTemplate{`
+	from {IMAGE}
+	onbuild from {IMAGE}
+	`,
+		nil, nil,
+	},
+		t, nil, true,
+	)
+	if err == nil {
+		t.Fatal("Error should not be nil")
+	}
+}
+
+func TestBuildOnBuildForbiddenMaintainerTrigger(t *testing.T) {
+	_, err := buildImage(testContextTemplate{`
+	from {IMAGE}
+	onbuild maintainer test
+	`,
+		nil, nil,
+	},
+		t, nil, true,
+	)
+	if err == nil {
+		t.Fatal("Error should not be nil")
+	}
+}