From 15924f238503ffe44e5fcb99415ff753a36e5971 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Tue, 17 Feb 2015 10:25:36 -0800 Subject: [PATCH] Support dockerfile and Dockerfile Closes #10807 Adds support for `dockerfile` ONLY when `Dockerfile` can't be found. If we're building from a Dockerfile via stdin/URL then always download it a `Dockerfile` and ignore the -f flag. Signed-off-by: Doug Davis --- api/client/commands.go | 17 ++- builder/evaluator.go | 21 +++- builder/job.go | 9 +- .../reference/api/docker_remote_api_v1.18.md | 7 +- docs/sources/reference/commandline/cli.md | 34 ++--- integration-cli/docker_api_containers_test.go | 100 +++++++++++++++ integration-cli/docker_cli_build_test.go | 119 ++++++++++++++++++ 7 files changed, 282 insertions(+), 25 deletions(-) diff --git a/api/client/commands.go b/api/client/commands.go index 64801fabea..4275bcc062 100644 --- a/api/client/commands.go +++ b/api/client/commands.go @@ -114,9 +114,10 @@ func (cli *DockerCli) CmdBuild(args ...string) error { if err != nil { return fmt.Errorf("failed to read Dockerfile from STDIN: %v", err) } - if *dockerfileName == "" { - *dockerfileName = api.DefaultDockerfileName - } + + // -f option has no meaning when we're reading it from stdin, + // so just use our default Dockerfile name + *dockerfileName = api.DefaultDockerfileName context, err = archive.Generate(*dockerfileName, string(dockerfile)) } else { context = ioutil.NopCloser(buf) @@ -156,6 +157,16 @@ func (cli *DockerCli) CmdBuild(args ...string) error { // No -f/--file was specified so use the default *dockerfileName = api.DefaultDockerfileName filename = filepath.Join(absRoot, *dockerfileName) + + // Just to be nice ;-) look for 'dockerfile' too but only + // use it if we found it, otherwise ignore this check + if _, err = os.Lstat(filename); os.IsNotExist(err) { + tmpFN := path.Join(absRoot, strings.ToLower(*dockerfileName)) + if _, err = os.Lstat(tmpFN); err == nil { + *dockerfileName = strings.ToLower(*dockerfileName) + filename = tmpFN + } + } } origDockerfile := *dockerfileName // used for error msg diff --git a/builder/evaluator.go b/builder/evaluator.go index 3ce5f0926f..6b8ad8c67c 100644 --- a/builder/evaluator.go +++ b/builder/evaluator.go @@ -28,6 +28,7 @@ import ( "strings" log "github.com/Sirupsen/logrus" + "github.com/docker/docker/api" "github.com/docker/docker/builder/command" "github.com/docker/docker/builder/parser" "github.com/docker/docker/daemon" @@ -146,7 +147,7 @@ func (b *Builder) Run(context io.Reader) (string, error) { } }() - if err := b.readDockerfile(b.dockerfileName); err != nil { + if err := b.readDockerfile(); err != nil { return "", err } @@ -177,7 +178,23 @@ func (b *Builder) Run(context io.Reader) (string, error) { // Reads a Dockerfile from the current context. It assumes that the // 'filename' is a relative path from the root of the context -func (b *Builder) readDockerfile(origFile string) error { +func (b *Builder) readDockerfile() error { + // If no -f was specified then look for 'Dockerfile'. If we can't find + // that then look for 'dockerfile'. If neither are found then default + // back to 'Dockerfile' and use that in the error message. + if b.dockerfileName == "" { + b.dockerfileName = api.DefaultDockerfileName + tmpFN := filepath.Join(b.contextPath, api.DefaultDockerfileName) + if _, err := os.Lstat(tmpFN); err != nil { + tmpFN = filepath.Join(b.contextPath, strings.ToLower(api.DefaultDockerfileName)) + if _, err := os.Lstat(tmpFN); err == nil { + b.dockerfileName = strings.ToLower(api.DefaultDockerfileName) + } + } + } + + origFile := b.dockerfileName + filename, err := symlink.FollowSymlinkInScope(filepath.Join(b.contextPath, origFile), b.contextPath) if err != nil { return fmt.Errorf("The Dockerfile (%s) must be within the build context", origFile) diff --git a/builder/job.go b/builder/job.go index 559335298e..fb629e1c20 100644 --- a/builder/job.go +++ b/builder/job.go @@ -78,10 +78,6 @@ func (b *BuilderJob) CmdBuild(job *engine.Job) engine.Status { } } - if dockerfileName == "" { - dockerfileName = api.DefaultDockerfileName - } - if remoteURL == "" { context = ioutil.NopCloser(job.Stdin) } else if urlutil.IsGitURL(remoteURL) { @@ -113,6 +109,11 @@ func (b *BuilderJob) CmdBuild(job *engine.Job) engine.Status { if err != nil { return job.Error(err) } + + // When we're downloading just a Dockerfile put it in + // the default name - don't allow the client to move/specify it + dockerfileName = api.DefaultDockerfileName + c, err := archive.Generate(dockerfileName, string(dockerFile)) if err != nil { return job.Error(err) diff --git a/docs/sources/reference/api/docker_remote_api_v1.18.md b/docs/sources/reference/api/docker_remote_api_v1.18.md index 72d184115b..0193ab8832 100644 --- a/docs/sources/reference/api/docker_remote_api_v1.18.md +++ b/docs/sources/reference/api/docker_remote_api_v1.18.md @@ -1073,10 +1073,13 @@ command*](/reference/builder/#dockerbuilder)). Query Parameters: -- **dockerfile** - path within the build context to the Dockerfile +- **dockerfile** - path within the build context to the Dockerfile. This is + ignored if `remote` is specified and points to an individual filename. - **t** – repository name (and optionally a tag) to be applied to the resulting image in case of success -- **remote** – git or HTTP/HTTPS URI build source +- **remote** – A Git repository URI or HTTP/HTTPS URI build source. If the + URI specifies a filename, the file's contents are placed into a file + called `Dockerfile`. - **q** – suppress verbose build output - **nocache** – do not use the cache when building the image - **pull** - attempt to pull the image even if an older image exists locally diff --git a/docs/sources/reference/commandline/cli.md b/docs/sources/reference/commandline/cli.md index 345d11fcc0..01b7864d86 100644 --- a/docs/sources/reference/commandline/cli.md +++ b/docs/sources/reference/commandline/cli.md @@ -506,21 +506,27 @@ is returned by the `docker attach` command to its caller too: --rm=true Remove intermediate containers after a successful build -t, --tag="" Repository name (and optionally a tag) for the image -Use this command to build Docker images from a Dockerfile and a -"context". +Builds Docker images from a Dockerfile and a "context". A build's context is +the files located in the specified `PATH` or `URL`. The build process can +refer to any of the files in the context. For example, your build can use +an [*ADD*](/reference/builder/#add) instruction to reference a file in the +context. -The files at `PATH` or `URL` are called the "context" of the build. The -build process may refer to any of the files in the context, for example -when using an [*ADD*](/reference/builder/#add) instruction. -When a single Dockerfile is given as `URL` or is piped through `STDIN` -(`docker build - < Dockerfile`), then no context is set. +The `URL` parameter can specify the location of a Git repository; in this +case, the repository is the context. The Git repository is recursively +cloned with its submodules. The system does a fresh `git clone -recursive` +in a temporary directory on your local host. Then, this clone is sent to +the Docker daemon as the context. Local clones give you the ability to +access private repositories using local user credentials, VPN's, and so forth. -When a Git repository is set as `URL`, then the repository is used as -the context. The Git repository is cloned with its submodules -(`git clone -recursive`). A fresh `git clone` occurs in a temporary directory -on your local host, and then this is sent to the Docker daemon as the -context. This way, your local user credentials and VPN's etc can be -used to access private repositories. +Instead of specifying a context, you can pass a single Dockerfile in the +`URL` or pipe the file in via `STDIN`. To pipe a Dockerfile from `STDIN`: + + docker build - < Dockerfile + +If you use STDIN or specify a `URL`, the system places the contents into a +file called `Dockerfile`, and any `-f`, `--file` option is ignored. In this +scenario, there is no context. If a file named `.dockerignore` exists in the root of `PATH` then it is interpreted as a newline-separated list of exclusion patterns. @@ -529,7 +535,7 @@ will be excluded from the context. Globbing is done using Go's [filepath.Match](http://golang.org/pkg/path/filepath#Match) rules. Please note that `.dockerignore` files in other subdirectories are -considered as normal files. Filepaths in .dockerignore are absolute with +considered as normal files. Filepaths in `.dockerignore` are absolute with the current directory as the root. Wildcards are allowed but the search is not recursive. diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 50c92c5149..2f4d207dcd 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -353,6 +353,106 @@ func TestBuildApiDockerfilePath(t *testing.T) { logDone("container REST API - check build w/bad Dockerfile path") } +func TestBuildApiDockerFileRemote(t *testing.T) { + server, err := fakeStorage(map[string]string{ + "testD": `FROM busybox +COPY * /tmp/ +RUN find /tmp/`, + }) + if err != nil { + t.Fatal(err) + } + defer server.Close() + + buf, err := sockRequestRaw("POST", "/build?dockerfile=baz&remote="+server.URL+"/testD", nil, "application/json") + if err != nil { + t.Fatalf("Build failed: %s", err) + } + + out := string(buf) + if !strings.Contains(out, "/tmp/Dockerfile") || + strings.Contains(out, "/tmp/baz") { + t.Fatalf("Incorrect output: %s", out) + } + + logDone("container REST API - check build with -f from remote") +} + +func TestBuildApiLowerDockerfile(t *testing.T) { + git, err := fakeGIT("repo", map[string]string{ + "dockerfile": `FROM busybox +RUN echo from dockerfile`, + }) + if err != nil { + t.Fatal(err) + } + defer git.Close() + + buf, err := sockRequestRaw("POST", "/build?remote="+git.RepoURL, nil, "application/json") + if err != nil { + t.Fatalf("Build failed: %s\n%q", err, buf) + } + + out := string(buf) + if !strings.Contains(out, "from dockerfile") { + t.Fatalf("Incorrect output: %s", out) + } + + logDone("container REST API - check build with lower dockerfile") +} + +func TestBuildApiBuildGitWithF(t *testing.T) { + git, err := fakeGIT("repo", map[string]string{ + "baz": `FROM busybox +RUN echo from baz`, + "Dockerfile": `FROM busybox +RUN echo from Dockerfile`, + }) + if err != nil { + t.Fatal(err) + } + defer git.Close() + + // Make sure it tries to 'dockerfile' query param value + buf, err := sockRequestRaw("POST", "/build?dockerfile=baz&remote="+git.RepoURL, nil, "application/json") + if err != nil { + t.Fatalf("Build failed: %s\n%q", err, buf) + } + + out := string(buf) + if !strings.Contains(out, "from baz") { + t.Fatalf("Incorrect output: %s", out) + } + + logDone("container REST API - check build from git w/F") +} + +func TestBuildApiDoubleDockerfile(t *testing.T) { + git, err := fakeGIT("repo", map[string]string{ + "Dockerfile": `FROM busybox +RUN echo from Dockerfile`, + "dockerfile": `FROM busybox +RUN echo from dockerfile`, + }) + if err != nil { + t.Fatal(err) + } + defer git.Close() + + // Make sure it tries to 'dockerfile' query param value + buf, err := sockRequestRaw("POST", "/build?remote="+git.RepoURL, nil, "application/json") + if err != nil { + t.Fatalf("Build failed: %s", err) + } + + out := string(buf) + if !strings.Contains(out, "from Dockerfile") { + t.Fatalf("Incorrect output: %s", out) + } + + logDone("container REST API - check build with two dockerfiles") +} + func TestBuildApiDockerfileSymlink(t *testing.T) { // Test to make sure we stop people from trying to leave the // build context when specifying a symlink as the path to the dockerfile diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 00183c0671..ac1ac14775 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -4699,6 +4699,125 @@ func TestBuildRenamedDockerfile(t *testing.T) { logDone("build - rename dockerfile") } +func TestBuildFromMixedcaseDockerfile(t *testing.T) { + defer deleteImages("test1") + + ctx, err := fakeContext(`FROM busybox + RUN echo from dockerfile`, + map[string]string{ + "dockerfile": "FROM busybox\nRUN echo from dockerfile", + }) + defer ctx.Close() + if err != nil { + t.Fatal(err) + } + + out, _, err := dockerCmdInDir(t, ctx.Dir, "build", "-t", "test1", ".") + if err != nil { + t.Fatalf("Failed to build: %s\n%s", out, err) + } + + if !strings.Contains(out, "from dockerfile") { + t.Fatalf("Missing proper output: %s", out) + } + + logDone("build - mixedcase Dockerfile") +} + +func TestBuildWithTwoDockerfiles(t *testing.T) { + defer deleteImages("test1") + + ctx, err := fakeContext(`FROM busybox +RUN echo from Dockerfile`, + map[string]string{ + "dockerfile": "FROM busybox\nRUN echo from dockerfile", + }) + defer ctx.Close() + if err != nil { + t.Fatal(err) + } + + out, _, err := dockerCmdInDir(t, ctx.Dir, "build", "-t", "test1", ".") + if err != nil { + t.Fatalf("Failed to build: %s\n%s", out, err) + } + + if !strings.Contains(out, "from Dockerfile") { + t.Fatalf("Missing proper output: %s", out) + } + + logDone("build - two Dockerfiles") +} + +func TestBuildFromURLWithF(t *testing.T) { + defer deleteImages("test1") + + server, err := fakeStorage(map[string]string{"baz": `FROM busybox +RUN echo from baz +COPY * /tmp/ +RUN find /tmp/`}) + if err != nil { + t.Fatal(err) + } + defer server.Close() + + ctx, err := fakeContext(`FROM busybox +RUN echo from Dockerfile`, + map[string]string{}) + defer ctx.Close() + if err != nil { + t.Fatal(err) + } + + // Make sure that -f is ignored and that we don't use the Dockerfile + // that's in the current dir + out, _, err := dockerCmdInDir(t, ctx.Dir, "build", "-f", "baz", "-t", "test1", server.URL+"/baz") + if err != nil { + t.Fatalf("Failed to build: %s\n%s", out, err) + } + + if !strings.Contains(out, "from baz") || + strings.Contains(out, "/tmp/baz") || + !strings.Contains(out, "/tmp/Dockerfile") { + t.Fatalf("Missing proper output: %s", out) + } + + logDone("build - from URL with -f") +} + +func TestBuildFromStdinWithF(t *testing.T) { + defer deleteImages("test1") + + ctx, err := fakeContext(`FROM busybox +RUN echo from Dockerfile`, + map[string]string{}) + defer ctx.Close() + if err != nil { + t.Fatal(err) + } + + // Make sure that -f is ignored and that we don't use the Dockerfile + // that's in the current dir + dockerCommand := exec.Command(dockerBinary, "build", "-f", "baz", "-t", "test1", "-") + dockerCommand.Dir = ctx.Dir + dockerCommand.Stdin = strings.NewReader(`FROM busybox +RUN echo from baz +COPY * /tmp/ +RUN find /tmp/`) + out, status, err := runCommandWithOutput(dockerCommand) + if err != nil || status != 0 { + t.Fatalf("Error building: %s", err) + } + + if !strings.Contains(out, "from baz") || + strings.Contains(out, "/tmp/baz") || + !strings.Contains(out, "/tmp/Dockerfile") { + t.Fatalf("Missing proper output: %s", out) + } + + logDone("build - from stdin with -f") +} + func TestBuildFromOfficialNames(t *testing.T) { name := "testbuildfromofficial" fromNames := []string{