From 3021b7a4a0a920c26d87361b6cb8651c110bc4a1 Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Mon, 20 Jul 2015 14:53:52 -0700 Subject: [PATCH] Refactor api/client/build.go Separated preparation of context and Dockerfile for the various different methods of specifying them. Docker-DCO-1.1-Signed-off-by: Josh Hawn (github: jlhawn) --- api/client/build.go | 353 +++++++++++++++++++++++++++++--------------- 1 file changed, 231 insertions(+), 122 deletions(-) diff --git a/api/client/build.go b/api/client/build.go index a6eee12b04641..c9992be519d54 100644 --- a/api/client/build.go +++ b/api/client/build.go @@ -70,147 +70,97 @@ func (cli *DockerCli) CmdBuild(args ...string) error { cmd.Require(flag.Exact, 1) + // For trusted pull on "FROM " instruction. + addTrustedFlags(cmd, true) + cmd.ParseFlags(args, true) var ( - context archive.Archive + context io.ReadCloser isRemote bool err error ) _, err = exec.LookPath("git") hasGit := err == nil - if cmd.Arg(0) == "-" { - // As a special case, 'docker build -' will build from either an empty context with the - // contents of stdin as a Dockerfile, or a tar-ed context from stdin. - buf := bufio.NewReader(cli.in) - magic, err := buf.Peek(tarHeaderSize) - if err != nil && err != io.EOF { - return fmt.Errorf("failed to peek context header from STDIN: %v", err) - } - if !archive.IsArchive(magic) { - dockerfile, err := ioutil.ReadAll(buf) - if err != nil { - return fmt.Errorf("failed to read Dockerfile from STDIN: %v", err) - } - // -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) - } - } else if urlutil.IsURL(cmd.Arg(0)) && (!urlutil.IsGitURL(cmd.Arg(0)) || !hasGit) { - isRemote = true - } else { - root := cmd.Arg(0) - if urlutil.IsGitURL(root) { - root, err = utils.GitClone(root) - if err != nil { - return err - } - defer os.RemoveAll(root) - } - if _, err := os.Stat(root); err != nil { - return err - } + specifiedContext := cmd.Arg(0) - absRoot, err := filepath.Abs(root) - if err != nil { - return err - } + var ( + contextDir string + tempDir string + relDockerfile string + ) - filename := *dockerfileName // path to Dockerfile - - if *dockerfileName == "" { - // 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 - } - } - } + switch { + case specifiedContext == "-": + tempDir, relDockerfile, err = getContextFromReader(cli.in, *dockerfileName) + case urlutil.IsGitURL(specifiedContext) && hasGit: + tempDir, relDockerfile, err = getContextFromGitURL(specifiedContext, *dockerfileName) + case urlutil.IsURL(specifiedContext): + tempDir, relDockerfile, err = getContextFromURL(cli.out, specifiedContext, *dockerfileName) + default: + contextDir, relDockerfile, err = getContextFromLocalDir(specifiedContext, *dockerfileName) + } - origDockerfile := *dockerfileName // used for error msg - if filename, err = filepath.Abs(filename); err != nil { - return err - } + if err != nil { + return fmt.Errorf("unable to prepare context: %s", err) + } - // Verify that 'filename' is within the build context - filename, err = symlink.FollowSymlinkInScope(filename, absRoot) - if err != nil { - return fmt.Errorf("The Dockerfile (%s) must be within the build context (%s)", origDockerfile, root) - } + if tempDir != "" { + defer os.RemoveAll(tempDir) + contextDir = tempDir + } - // Now reset the dockerfileName to be relative to the build context - *dockerfileName, err = filepath.Rel(absRoot, filename) - if err != nil { - return err - } - // And canonicalize dockerfile name to a platform-independent one - *dockerfileName, err = archive.CanonicalTarNameForPath(*dockerfileName) - if err != nil { - return fmt.Errorf("Cannot canonicalize dockerfile path %s: %v", *dockerfileName, err) - } + // And canonicalize dockerfile name to a platform-independent one + relDockerfile, err = archive.CanonicalTarNameForPath(relDockerfile) + if err != nil { + return fmt.Errorf("cannot canonicalize dockerfile path %s: %v", relDockerfile, err) + } - if _, err = os.Lstat(filename); os.IsNotExist(err) { - return fmt.Errorf("Cannot locate Dockerfile: %s", origDockerfile) - } - var includes = []string{"."} + var includes = []string{"."} - excludes, err := utils.ReadDockerIgnore(path.Join(root, ".dockerignore")) - if err != nil { - return err - } + excludes, err := utils.ReadDockerIgnore(path.Join(contextDir, ".dockerignore")) + if err != nil { + return err + } - // If .dockerignore mentions .dockerignore or the Dockerfile - // then make sure we send both files over to the daemon - // because Dockerfile is, obviously, needed no matter what, and - // .dockerignore is needed to know if either one needs to be - // removed. The deamon will remove them for us, if needed, after it - // parses the Dockerfile. - keepThem1, _ := fileutils.Matches(".dockerignore", excludes) - keepThem2, _ := fileutils.Matches(*dockerfileName, excludes) - if keepThem1 || keepThem2 { - includes = append(includes, ".dockerignore", *dockerfileName) - } + if err := utils.ValidateContextDirectory(contextDir, excludes); err != nil { + return fmt.Errorf("Error checking context: '%s'.", err) + } - if err := utils.ValidateContextDirectory(root, excludes); err != nil { - return fmt.Errorf("Error checking context: '%s'.", err) - } - options := &archive.TarOptions{ - Compression: archive.Uncompressed, - ExcludePatterns: excludes, - IncludeFiles: includes, - } - context, err = archive.TarWithOptions(root, options) - if err != nil { - return err - } + // If .dockerignore mentions .dockerignore or the Dockerfile + // then make sure we send both files over to the daemon + // because Dockerfile is, obviously, needed no matter what, and + // .dockerignore is needed to know if either one needs to be + // removed. The deamon will remove them for us, if needed, after it + // parses the Dockerfile. Ignore errors here, as they will have been + // caught by ValidateContextDirectory above. + keepThem1, _ := fileutils.Matches(".dockerignore", excludes) + keepThem2, _ := fileutils.Matches(relDockerfile, excludes) + if keepThem1 || keepThem2 { + includes = append(includes, ".dockerignore", relDockerfile) + } + + if context, err = archive.TarWithOptions(contextDir, &archive.TarOptions{ + Compression: archive.Uncompressed, + ExcludePatterns: excludes, + IncludeFiles: includes, + }); err != nil { + return err } - var body io.Reader // Setup an upload progress bar // FIXME: ProgressReader shouldn't be this annoying to use - if context != nil { - sf := streamformatter.NewStreamFormatter() - body = progressreader.New(progressreader.Config{ - In: context, - Out: cli.out, - Formatter: sf, - NewLines: true, - ID: "", - Action: "Sending build context to Docker daemon", - }) - } + sf := streamformatter.NewStreamFormatter() + var body io.Reader = progressreader.New(progressreader.Config{ + In: context, + Out: cli.out, + Formatter: sf, + NewLines: true, + ID: "", + Action: "Sending build context to Docker daemon", + }) var memory int64 if *flMemoryString != "" { @@ -283,7 +233,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { v.Set("memswap", strconv.FormatInt(memorySwap, 10)) v.Set("cgroupparent", *flCgroupParent) - v.Set("dockerfile", *dockerfileName) + v.Set("dockerfile", relDockerfile) ulimitsVar := flUlimits.GetList() ulimitsJson, err := json.Marshal(ulimitsVar) @@ -298,10 +248,8 @@ func (cli *DockerCli) CmdBuild(args ...string) error { return err } headers.Add("X-Registry-Config", base64.URLEncoding.EncodeToString(buf)) + headers.Set("Content-Type", "application/tar") - if context != nil { - headers.Set("Content-Type", "application/tar") - } sopts := &streamOpts{ rawTerminal: true, in: body, @@ -330,3 +278,164 @@ func (cli *DockerCli) CmdBuild(args ...string) error { } return err } + +// getDockerfileRelPath uses the given context directory for a `docker build` +// and returns the absolute path to the context directory, the relative path of +// the dockerfile in that context directory, and a non-nil error on success. +func getDockerfileRelPath(givenContextDir, givenDockerfile string) (absContextDir, relDockerfile string, err error) { + if absContextDir, err = filepath.Abs(givenContextDir); err != nil { + return "", "", fmt.Errorf("unable to get absolute context directory: %v", err) + } + + absDockerfile := givenDockerfile + if absDockerfile == "" { + // No -f/--file was specified so use the default relative to the + // context directory. + absDockerfile = filepath.Join(absContextDir, api.DefaultDockerfileName) + + // Just to be nice ;-) look for 'dockerfile' too but only + // use it if we found it, otherwise ignore this check + if _, err = os.Lstat(absDockerfile); os.IsNotExist(err) { + altPath := filepath.Join(absContextDir, strings.ToLower(api.DefaultDockerfileName)) + if _, err = os.Lstat(altPath); err == nil { + absDockerfile = altPath + } + } + } + + // If not already an absolute path, the Dockerfile path should be joined to + // the base directory. + if !filepath.IsAbs(absDockerfile) { + absDockerfile = filepath.Join(absContextDir, absDockerfile) + } + + // Verify that 'filename' is within the build context + absDockerfile, err = symlink.FollowSymlinkInScope(absDockerfile, absContextDir) + if err != nil { + return "", "", fmt.Errorf("The Dockerfile (%s) must be within the build context (%s)", givenDockerfile, givenContextDir) + } + + if _, err := os.Lstat(absDockerfile); err != nil { + if os.IsNotExist(err) { + return "", "", fmt.Errorf("Cannot locate Dockerfile: absDockerfile: %q", absDockerfile) + } + return "", "", fmt.Errorf("unable to stat Dockerfile: %v", err) + } + + if relDockerfile, err = filepath.Rel(absContextDir, absDockerfile); err != nil { + return "", "", fmt.Errorf("unable to get relative Dockerfile path: %v", err) + } + + return absContextDir, relDockerfile, nil +} + +// writeToFile copies from the given reader and writes it to a file with the +// given filename. +func writeToFile(r io.Reader, filename string) error { + file, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.FileMode(0600)) + if err != nil { + return fmt.Errorf("unable to create file: %v", err) + } + defer file.Close() + + if _, err := io.Copy(file, r); err != nil { + return fmt.Errorf("unable to write file: %v", err) + } + + return nil +} + +// getContextFromReader will read the contents of the given reader as either a +// Dockerfile or tar archive to be extracted to a temporary directory used as +// the context directory. Returns the absolute path to the temporary context +// directory, the relative path of the dockerfile in that context directory, +// and a non-nil error on success. +func getContextFromReader(r io.Reader, dockerfileName string) (absContextDir, relDockerfile string, err error) { + buf := bufio.NewReader(r) + + magic, err := buf.Peek(tarHeaderSize) + if err != nil && err != io.EOF { + return "", "", fmt.Errorf("failed to peek context header from STDIN: %v", err) + } + + if absContextDir, err = ioutil.TempDir("", "docker-build-context-"); err != nil { + return "", "", fmt.Errorf("unbale to create temporary context directory: %v", err) + } + + defer func(d string) { + if err != nil { + os.RemoveAll(d) + } + }(absContextDir) + + if !archive.IsArchive(magic) { // Input should be read as a Dockerfile. + // -f option has no meaning when we're reading it from stdin, + // so just use our default Dockerfile name + relDockerfile = api.DefaultDockerfileName + + return absContextDir, relDockerfile, writeToFile(buf, filepath.Join(absContextDir, relDockerfile)) + } + + if err := archive.Untar(buf, absContextDir, nil); err != nil { + return "", "", fmt.Errorf("unable to extract stdin to temporary context direcotry: %v", err) + } + + return getDockerfileRelPath(absContextDir, dockerfileName) +} + +// getContextFromGitURL uses a Git URL as context for a `docker build`. The +// git repo is cloned into a temporary directory used as the context directory. +// Returns the absolute path to the temporary context directory, the relative +// path of the dockerfile in that context directory, and a non-nil error on +// success. +func getContextFromGitURL(gitURL, dockerfileName string) (absContextDir, relDockerfile string, err error) { + if absContextDir, err = utils.GitClone(gitURL); err != nil { + return "", "", fmt.Errorf("unable to 'git clone' to temporary context directory: %v", err) + } + + return getDockerfileRelPath(absContextDir, dockerfileName) +} + +// getContextFromURL uses a remote URL as context for a `docker build`. The +// remote resource is downloaded as either a Dockerfile or a context tar +// archive and stored in a temporary directory used as the context directory. +// Returns the absolute path to the temporary context directory, the relative +// path of the dockerfile in that context directory, and a non-nil error on +// success. +func getContextFromURL(out io.Writer, remoteURL, dockerfileName string) (absContextDir, relDockerfile string, err error) { + response, err := httputils.Download(remoteURL) + if err != nil { + return "", "", fmt.Errorf("unable to download remote context %s: %v", remoteURL, err) + } + defer response.Body.Close() + + // Pass the response body through a progress reader. + progReader := &progressreader.Config{ + In: response.Body, + Out: out, + Formatter: streamformatter.NewStreamFormatter(), + Size: int(response.ContentLength), + NewLines: true, + ID: "", + Action: fmt.Sprintf("Downloading build context from remote url: %s", remoteURL), + } + + return getContextFromReader(progReader, dockerfileName) +} + +// getContextFromLocalDir uses the given local directory as context for a +// `docker build`. Returns the absolute path to the local context directory, +// the relative path of the dockerfile in that context directory, and a non-nil +// error on success. +func getContextFromLocalDir(localDir, dockerfileName string) (absContextDir, relDockerfile string, err error) { + // When using a local context directory, when the Dockerfile is specified + // with the `-f/--file` option then it is considered relative to the + // current directory and not the context directory. + if dockerfileName != "" { + if dockerfileName, err = filepath.Abs(dockerfileName); err != nil { + return "", "", fmt.Errorf("unable to get absolute path to Dockerfile: %v", err) + } + } + + return getDockerfileRelPath(localDir, dockerfileName) +}