diff --git a/components/engine/builder/dockerfile/builder.go b/components/engine/builder/dockerfile/builder.go index c206aa90c2..ec067eaa2e 100644 --- a/components/engine/builder/dockerfile/builder.go +++ b/components/engine/builder/dockerfile/builder.go @@ -234,6 +234,12 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri var shortImgID string total := len(b.dockerfile.Children) + for _, n := range b.dockerfile.Children { + if err := b.checkDispatch(n, false); err != nil { + return "", err + } + } + for i, n := range b.dockerfile.Children { select { case <-b.clientCtx.Done(): @@ -243,6 +249,7 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri default: // Not cancelled yet, keep going... } + if err := b.dispatch(i, total, n); err != nil { if b.options.ForceRemove { b.clearTmp() @@ -322,6 +329,12 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con b.disableCommit = true total := len(ast.Children) + for _, n := range ast.Children { + if err := b.checkDispatch(n, false); err != nil { + return nil, err + } + } + for i, n := range ast.Children { if err := b.dispatch(i, total, n); err != nil { return nil, err diff --git a/components/engine/builder/dockerfile/evaluator.go b/components/engine/builder/dockerfile/evaluator.go index cdecf05a3b..304739aa82 100644 --- a/components/engine/builder/dockerfile/evaluator.go +++ b/components/engine/builder/dockerfile/evaluator.go @@ -201,3 +201,44 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error { return fmt.Errorf("Unknown instruction: %s", upperCasedCmd) } + +// checkDispatch does a simple check for syntax errors of the Dockerfile. +// Because some of the instructions can only be validated through runtime, +// arg, env, etc., this syntax check will not be complete and could not replace +// the runtime check. Instead, this function is only a helper that allows +// user to find out the obvious error in Dockerfile earlier on. +// onbuild bool: indicate if instruction XXX is part of `ONBUILD XXX` trigger +func (b *Builder) checkDispatch(ast *parser.Node, onbuild bool) error { + cmd := ast.Value + upperCasedCmd := strings.ToUpper(cmd) + + // To ensure the user is given a decent error message if the platform + // on which the daemon is running does not support a builder command. + if err := platformSupports(strings.ToLower(cmd)); err != nil { + return err + } + + // The instruction itself is ONBUILD, we will make sure it follows with at + // least one argument + if upperCasedCmd == "ONBUILD" { + if ast.Next == nil { + return fmt.Errorf("ONBUILD requires at least one argument") + } + } + + // The instruction is part of ONBUILD trigger (not the instruction itself) + if onbuild { + switch upperCasedCmd { + 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", upperCasedCmd) + } + } + + if _, ok := evaluateTable[cmd]; ok { + return nil + } + + return fmt.Errorf("Unknown instruction: %s", upperCasedCmd) +} diff --git a/components/engine/builder/dockerfile/internals.go b/components/engine/builder/dockerfile/internals.go index 54d3301f9a..5c137918c1 100644 --- a/components/engine/builder/dockerfile/internals.go +++ b/components/engine/builder/dockerfile/internals.go @@ -435,14 +435,12 @@ func (b *Builder) processImageFrom(img builder.Image) error { } total := len(ast.Children) - for i, n := range ast.Children { - switch strings.ToUpper(n.Value) { - 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", n.Value) + for _, n := range ast.Children { + if err := b.checkDispatch(n, true); err != nil { + return err } - + } + for i, n := range ast.Children { if err := b.dispatch(i, total, n); err != nil { return err } diff --git a/components/engine/cli/command/image/build.go b/components/engine/cli/command/image/build.go index 85f51f14c0..17be405bd5 100644 --- a/components/engine/cli/command/image/build.go +++ b/components/engine/cli/command/image/build.go @@ -293,6 +293,9 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { response, err := dockerCli.Client().ImageBuild(ctx, body, buildOptions) if err != nil { + if options.quiet { + fmt.Fprintf(dockerCli.Err(), "%s", progBuff) + } return err } defer response.Body.Close() diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index 28c9dc09e2..c96531e427 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -6901,3 +6901,21 @@ func (s *DockerSuite) TestBuildStepsWithProgress(c *check.C) { c.Assert(out, checker.Contains, fmt.Sprintf("Step %d/%d : RUN echo foo", i, 1+totalRun)) } } + +func (s *DockerSuite) TestBuildWithFailure(c *check.C) { + name := "testbuildwithfailure" + + // First test case can only detect `nobody` in runtime so all steps will show up + buildCmd := "FROM busybox\nRUN nobody" + _, stdout, _, err := buildImageWithStdoutStderr(name, buildCmd, false, "--force-rm", "--rm") + c.Assert(err, checker.NotNil) + c.Assert(stdout, checker.Contains, "Step 1/2 : FROM busybox") + c.Assert(stdout, checker.Contains, "Step 2/2 : RUN nobody") + + // Second test case `FFOM` should have been detected before build runs so no steps + buildCmd = "FFOM nobody\nRUN nobody" + _, stdout, _, err = buildImageWithStdoutStderr(name, buildCmd, false, "--force-rm", "--rm") + c.Assert(err, checker.NotNil) + c.Assert(stdout, checker.Not(checker.Contains), "Step 1/2 : FROM busybox") + c.Assert(stdout, checker.Not(checker.Contains), "Step 2/2 : RUN nobody") +}