diff --git a/builder/dockerfile/evaluator_test.go b/builder/dockerfile/evaluator_test.go index c1c9425087..db52c9c90b 100644 --- a/builder/dockerfile/evaluator_test.go +++ b/builder/dockerfile/evaluator_test.go @@ -3,6 +3,7 @@ package dockerfile import ( "io/ioutil" "os" + "path/filepath" "strings" "testing" @@ -16,6 +17,7 @@ import ( type dispatchTestCase struct { name, dockerfile, expectedError string + files map[string]string } func init() { @@ -34,21 +36,97 @@ func initDispatchTestCases() []dispatchTestCase { name: "ONBUILD forbidden FROM", dockerfile: "ONBUILD FROM scratch", expectedError: "FROM isn't allowed as an ONBUILD trigger", + files: nil, }, { name: "ONBUILD forbidden MAINTAINER", dockerfile: "ONBUILD MAINTAINER docker.io", expectedError: "MAINTAINER isn't allowed as an ONBUILD trigger", + files: nil, }, { name: "ARG two arguments", dockerfile: "ARG foo bar", expectedError: "ARG requires exactly one argument definition", + files: nil, }, { name: "MAINTAINER unknown flag", dockerfile: "MAINTAINER --boo joe@example.com", expectedError: "Unknown flag: boo", + files: nil, + }, + { + name: "ADD multiple files to file", + dockerfile: "ADD file1.txt file2.txt test", + expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /", + files: map[string]string{"file1.txt": "test1", "file2.txt": "test2"}, + }, + { + name: "JSON ADD multiple files to file", + dockerfile: `ADD ["file1.txt", "file2.txt", "test"]`, + expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /", + files: map[string]string{"file1.txt": "test1", "file2.txt": "test2"}, + }, + { + name: "Wiildcard ADD multiple files to file", + dockerfile: "ADD file*.txt test", + expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /", + files: map[string]string{"file1.txt": "test1", "file2.txt": "test2"}, + }, + { + name: "Wiildcard JSON ADD multiple files to file", + dockerfile: `ADD ["file*.txt", "test"]`, + expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /", + files: map[string]string{"file1.txt": "test1", "file2.txt": "test2"}, + }, + { + name: "COPY multiple files to file", + dockerfile: "COPY file1.txt file2.txt test", + expectedError: "When using COPY with more than one source file, the destination must be a directory and end with a /", + files: map[string]string{"file1.txt": "test1", "file2.txt": "test2"}, + }, + { + name: "JSON COPY multiple files to file", + dockerfile: `COPY ["file1.txt", "file2.txt", "test"]`, + expectedError: "When using COPY with more than one source file, the destination must be a directory and end with a /", + files: map[string]string{"file1.txt": "test1", "file2.txt": "test2"}, + }, + { + name: "ADD multiple files to file with whitespace", + dockerfile: `ADD [ "test file1.txt", "test file2.txt", "test" ]`, + expectedError: "When using ADD with more than one source file, the destination must be a directory and end with a /", + files: map[string]string{"test file1.txt": "test1", "test file2.txt": "test2"}, + }, + { + name: "COPY multiple files to file with whitespace", + dockerfile: `COPY [ "test file1.txt", "test file2.txt", "test" ]`, + expectedError: "When using COPY with more than one source file, the destination must be a directory and end with a /", + files: map[string]string{"test file1.txt": "test1", "test file2.txt": "test2"}, + }, + { + name: "COPY wildcard no files", + dockerfile: `COPY file*.txt /tmp/`, + expectedError: "No source files were specified", + files: nil, + }, + { + name: "COPY url", + dockerfile: `COPY https://index.docker.io/robots.txt /`, + expectedError: "Source can't be a URL for COPY", + files: nil, + }, + { + name: "Chaining ONBUILD", + dockerfile: `ONBUILD ONBUILD RUN touch foobar`, + expectedError: "Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed", + files: nil, + }, + { + name: "Invalid instruction", + dockerfile: `foo bar`, + expectedError: "Unknown instruction: FOO", + files: nil, }} return dispatchTestCases @@ -66,6 +144,10 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) { contextDir, cleanup := createTestTempDir(t, "", "builder-dockerfile-test") defer cleanup() + for filename, content := range testCase.files { + createTestTempFile(t, contextDir, filename, content, 0777) + } + tarStream, err := archive.Tar(contextDir, archive.Uncompressed) if err != nil { @@ -132,3 +214,16 @@ func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) { } } } + +// createTestTempFile creates a temporary file within dir with specific contents and permissions. +// When an error occurs, it terminates the test +func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.FileMode) string { + filePath := filepath.Join(dir, filename) + err := ioutil.WriteFile(filePath, []byte(contents), perm) + + if err != nil { + t.Fatalf("Error when creating %s file: %s", filename, err) + } + + return filePath +} diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 6cfb5374de..2f26265618 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -618,25 +618,8 @@ func (b *Builder) readDockerfile() error { } } - f, err := b.context.Open(b.options.Dockerfile) - if err != nil { - if os.IsNotExist(err) { - return fmt.Errorf("Cannot locate specified Dockerfile: %s", b.options.Dockerfile) - } - return err - } - if f, ok := f.(*os.File); ok { - // ignoring error because Open already succeeded - fi, err := f.Stat() - if err != nil { - return fmt.Errorf("Unexpected error reading Dockerfile: %v", err) - } - if fi.Size() == 0 { - return fmt.Errorf("The Dockerfile (%s) cannot be empty", b.options.Dockerfile) - } - } - b.dockerfile, err = parser.Parse(f) - f.Close() + err := b.parseDockerfile() + if err != nil { return err } @@ -655,6 +638,33 @@ func (b *Builder) readDockerfile() error { return nil } +func (b *Builder) parseDockerfile() error { + f, err := b.context.Open(b.options.Dockerfile) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("Cannot locate specified Dockerfile: %s", b.options.Dockerfile) + } + return err + } + defer f.Close() + if f, ok := f.(*os.File); ok { + // ignoring error because Open already succeeded + fi, err := f.Stat() + if err != nil { + return fmt.Errorf("Unexpected error reading Dockerfile: %v", err) + } + if fi.Size() == 0 { + return fmt.Errorf("The Dockerfile (%s) cannot be empty", b.options.Dockerfile) + } + } + b.dockerfile, err = parser.Parse(f) + if err != nil { + return err + } + + return nil +} + // determine if build arg is part of built-in args or user // defined args in Dockerfile at any point in time. func (b *Builder) isBuildArgAllowed(arg string) bool { diff --git a/builder/dockerfile/internals_test.go b/builder/dockerfile/internals_test.go new file mode 100644 index 0000000000..8cd723f514 --- /dev/null +++ b/builder/dockerfile/internals_test.go @@ -0,0 +1,55 @@ +package dockerfile + +import ( + "strings" + "testing" + + "github.com/docker/docker/builder" + "github.com/docker/docker/pkg/archive" + "github.com/docker/engine-api/types" +) + +func TestEmptyDockerfile(t *testing.T) { + contextDir, cleanup := createTestTempDir(t, "", "builder-dockerfile-test") + defer cleanup() + + createTestTempFile(t, contextDir, builder.DefaultDockerfileName, "", 0777) + + tarStream, err := archive.Tar(contextDir, archive.Uncompressed) + + if err != nil { + t.Fatalf("Error when creating tar stream: %s", err) + } + + defer func() { + if err = tarStream.Close(); err != nil { + t.Fatalf("Error when closing tar stream: %s", err) + } + }() + + context, err := builder.MakeTarSumContext(tarStream) + + if err != nil { + t.Fatalf("Error when creating tar context: %s", err) + } + + defer func() { + if err = context.Close(); err != nil { + t.Fatalf("Error when closing tar context: %s", err) + } + }() + + options := &types.ImageBuildOptions{} + + b := &Builder{options: options, context: context} + + err = b.readDockerfile() + + if err == nil { + t.Fatalf("No error when executing test for empty Dockerfile") + } + + if !strings.Contains(err.Error(), "The Dockerfile (Dockerfile) cannot be empty") { + t.Fatalf("Wrong error message. Should be \"%s\". Got \"%s\"", "The Dockerfile (Dockerfile) cannot be empty", err.Error()) + } +} diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index f48366c19f..2c81e18d42 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -862,138 +862,6 @@ RUN [ $(ls -l / | grep new_dir | awk '{print $3":"$4}') = 'root:root' ]`, true); } } -func (s *DockerSuite) TestBuildAddMultipleFilesToFile(c *check.C) { - name := "testaddmultiplefilestofile" - - ctx, err := fakeContext(`FROM `+minimalBaseImage()+` - ADD file1.txt file2.txt test - `, - map[string]string{ - "file1.txt": "test1", - "file2.txt": "test1", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - expected := "When using ADD with more than one source file, the destination must be a directory and end with a /" - if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) { - c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err) - } - -} - -func (s *DockerSuite) TestBuildJSONAddMultipleFilesToFile(c *check.C) { - name := "testjsonaddmultiplefilestofile" - - ctx, err := fakeContext(`FROM `+minimalBaseImage()+` - ADD ["file1.txt", "file2.txt", "test"] - `, - map[string]string{ - "file1.txt": "test1", - "file2.txt": "test1", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - expected := "When using ADD with more than one source file, the destination must be a directory and end with a /" - if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) { - c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err) - } - -} - -func (s *DockerSuite) TestBuildAddMultipleFilesToFileWild(c *check.C) { - name := "testaddmultiplefilestofilewild" - - ctx, err := fakeContext(`FROM `+minimalBaseImage()+` - ADD file*.txt test - `, - map[string]string{ - "file1.txt": "test1", - "file2.txt": "test1", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - expected := "When using ADD with more than one source file, the destination must be a directory and end with a /" - if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) { - c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err) - } - -} - -func (s *DockerSuite) TestBuildJSONAddMultipleFilesToFileWild(c *check.C) { - name := "testjsonaddmultiplefilestofilewild" - - ctx, err := fakeContext(`FROM `+minimalBaseImage()+` - ADD ["file*.txt", "test"] - `, - map[string]string{ - "file1.txt": "test1", - "file2.txt": "test1", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - expected := "When using ADD with more than one source file, the destination must be a directory and end with a /" - if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) { - c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err) - } - -} - -func (s *DockerSuite) TestBuildCopyMultipleFilesToFile(c *check.C) { - name := "testcopymultiplefilestofile" - - ctx, err := fakeContext(`FROM `+minimalBaseImage()+` - COPY file1.txt file2.txt test - `, - map[string]string{ - "file1.txt": "test1", - "file2.txt": "test1", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - expected := "When using COPY with more than one source file, the destination must be a directory and end with a /" - if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) { - c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err) - } - -} - -func (s *DockerSuite) TestBuildJSONCopyMultipleFilesToFile(c *check.C) { - name := "testjsoncopymultiplefilestofile" - - ctx, err := fakeContext(`FROM `+minimalBaseImage()+` - COPY ["file1.txt", "file2.txt", "test"] - `, - map[string]string{ - "file1.txt": "test1", - "file2.txt": "test1", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - expected := "When using COPY with more than one source file, the destination must be a directory and end with a /" - if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) { - c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err) - } - -} - func (s *DockerSuite) TestBuildAddFileWithWhitespace(c *check.C) { testRequires(c, DaemonIsLinux) // Not currently passing on Windows name := "testaddfilewithwhitespace" @@ -1066,48 +934,6 @@ RUN [ $(cat "/test dir/test_file6") = 'test6' ]`, } } -func (s *DockerSuite) TestBuildAddMultipleFilesToFileWithWhitespace(c *check.C) { - name := "testaddmultiplefilestofilewithwhitespace" - ctx, err := fakeContext(`FROM busybox - ADD [ "test file1", "test file2", "test" ] - `, - map[string]string{ - "test file1": "test1", - "test file2": "test2", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - expected := "When using ADD with more than one source file, the destination must be a directory and end with a /" - if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) { - c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err) - } - -} - -func (s *DockerSuite) TestBuildCopyMultipleFilesToFileWithWhitespace(c *check.C) { - name := "testcopymultiplefilestofilewithwhitespace" - ctx, err := fakeContext(`FROM busybox - COPY [ "test file1", "test file2", "test" ] - `, - map[string]string{ - "test file1": "test1", - "test file2": "test2", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - expected := "When using COPY with more than one source file, the destination must be a directory and end with a /" - if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) { - c.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err) - } - -} - func (s *DockerSuite) TestBuildCopyWildcard(c *check.C) { testRequires(c, DaemonIsLinux) // Windows doesn't have httpserver image yet name := "testcopywildcard" @@ -1159,26 +985,6 @@ func (s *DockerSuite) TestBuildCopyWildcard(c *check.C) { } -func (s *DockerSuite) TestBuildCopyWildcardNoFind(c *check.C) { - name := "testcopywildcardnofind" - ctx, err := fakeContext(`FROM busybox - COPY file*.txt /tmp/ - `, nil) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - _, err = buildImageFromContext(name, ctx, true) - if err == nil { - c.Fatal("should have failed to find a file") - } - if !strings.Contains(err.Error(), "No source files were specified") { - c.Fatalf("Wrong error %v, must be about no source files", err) - } - -} - func (s *DockerSuite) TestBuildCopyWildcardInName(c *check.C) { name := "testcopywildcardinname" ctx, err := fakeContext(`FROM busybox @@ -1580,17 +1386,6 @@ COPY . /`, } } -func (s *DockerSuite) TestBuildCopyDisallowRemote(c *check.C) { - name := "testcopydisallowremote" - - _, out, err := buildImageWithOut(name, `FROM `+minimalBaseImage()+` -COPY https://index.docker.io/robots.txt /`, - true) - if err == nil || !strings.Contains(out, "Source can't be a URL for COPY") { - c.Fatalf("Error should be about disallowed remote source, got err: %s, out: %q", err, out) - } -} - func (s *DockerSuite) TestBuildAddBadLinks(c *check.C) { testRequires(c, DaemonIsLinux) // Not currently working on Windows @@ -3289,18 +3084,6 @@ func (s *DockerSuite) TestBuildFails(c *check.C) { } } -func (s *DockerSuite) TestBuildFailsDockerfileEmpty(c *check.C) { - name := "testbuildfails" - _, err := buildImage(name, ``, true) - if err != nil { - if !strings.Contains(err.Error(), "The Dockerfile (Dockerfile) cannot be empty") { - c.Fatalf("Wrong error %v, must be about empty Dockerfile", err) - } - } else { - c.Fatal("Error must not be nil") - } -} - func (s *DockerSuite) TestBuildOnBuild(c *check.C) { name := "testbuildonbuild" _, err := buildImage(name, @@ -3319,21 +3102,6 @@ func (s *DockerSuite) TestBuildOnBuild(c *check.C) { } } -func (s *DockerSuite) TestBuildOnBuildForbiddenChained(c *check.C) { - name := "testbuildonbuildforbiddenchained" - _, err := buildImage(name, - `FROM busybox - ONBUILD ONBUILD RUN touch foobar`, - true) - if err != nil { - if !strings.Contains(err.Error(), "Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed") { - c.Fatalf("Wrong error %v, must be about chaining ONBUILD", err) - } - } else { - c.Fatal("Error must not be nil") - } -} - // gh #2446 func (s *DockerSuite) TestBuildAddToSymlinkDest(c *check.C) { testRequires(c, DaemonIsLinux) @@ -4564,16 +4332,6 @@ func (s *DockerSuite) TestBuildCmdJSONNoShDashC(c *check.C) { } -func (s *DockerSuite) TestBuildErrorInvalidInstruction(c *check.C) { - name := "testbuildignoreinvalidinstruction" - - out, _, err := buildImageWithOut(name, "FROM busybox\nfoo bar", true) - if err == nil { - c.Fatalf("Should have failed: %s", out) - } - -} - func (s *DockerSuite) TestBuildEntrypointInheritance(c *check.C) { if _, err := buildImage("parent", `