diff --git a/cli/command/image/build.go b/cli/command/image/build.go index 88f9405957..1da0b31c55 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -67,6 +67,18 @@ type buildOptions struct { target string } +// dockerfileFromStdin returns true when the user specified that the Dockerfile +// should be read from stdin instead of a file +func (o buildOptions) dockerfileFromStdin() bool { + return o.dockerfileName == "-" +} + +// contextFromStdin returns true when the user specified that the build context +// should be read from stdin +func (o buildOptions) contextFromStdin() bool { + return o.context == "-" +} + // NewBuildCommand creates a new `docker build` command func NewBuildCommand(dockerCli *command.DockerCli) *cobra.Command { ulimits := make(map[string]*units.Ulimit) @@ -155,6 +167,13 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { buildBuff io.Writer ) + if options.dockerfileFromStdin() { + if options.contextFromStdin() { + return errors.New("invalid argument: can't use stdin for both build context and dockerfile") + } + dockerfileCtx = dockerCli.In() + } + specifiedContext := options.context progBuff = dockerCli.Out() buildBuff = dockerCli.Out() @@ -163,15 +182,8 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { buildBuff = bytes.NewBuffer(nil) } - if options.dockerfileName == "-" { - if specifiedContext == "-" { - return errors.New("invalid argument: can't use stdin for both build context and dockerfile") - } - dockerfileCtx = dockerCli.In() - } - switch { - case specifiedContext == "-": + case options.contextFromStdin(): buildCtx, relDockerfile, err = build.GetContextFromReader(dockerCli.In(), options.dockerfileName) case isLocalDir(specifiedContext): contextDir, relDockerfile, err = build.GetContextFromLocalDir(specifiedContext, options.dockerfileName) @@ -196,12 +208,6 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { } if buildCtx == nil { - // And canonicalize dockerfile name to a platform-independent one - relDockerfile, err = archive.CanonicalTarNameForPath(relDockerfile) - if err != nil { - return errors.Errorf("cannot canonicalize dockerfile path %s: %v", relDockerfile, err) - } - f, err := os.Open(filepath.Join(contextDir, ".dockerignore")) if err != nil && !os.IsNotExist(err) { return err @@ -220,6 +226,12 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { return errors.Errorf("Error checking context: '%s'.", err) } + // And canonicalize dockerfile name to a platform-independent one + relDockerfile, err = archive.CanonicalTarNameForPath(relDockerfile) + if err != nil { + return errors.Errorf("cannot canonicalize dockerfile path %s: %v", relDockerfile, 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 @@ -231,7 +243,7 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { if keep, _ := fileutils.Matches(".dockerignore", excludes); keep { excludes = append(excludes, "!.dockerignore") } - if keep, _ := fileutils.Matches(relDockerfile, excludes); keep && dockerfileCtx == nil { + if keep, _ := fileutils.Matches(relDockerfile, excludes); keep && !options.dockerfileFromStdin() { excludes = append(excludes, "!"+relDockerfile) } diff --git a/cli/command/image/build/context.go b/cli/command/image/build/context.go index 348c721931..af1892b96c 100644 --- a/cli/command/image/build/context.go +++ b/cli/command/image/build/context.go @@ -134,15 +134,21 @@ func GetContextFromReader(r io.ReadCloser, dockerfileName string) (out io.ReadCl // 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) { +func GetContextFromGitURL(gitURL, dockerfileName string) (string, string, error) { if _, err := exec.LookPath("git"); err != nil { - return "", "", errors.Errorf("unable to find 'git': %v", err) + return "", "", errors.Wrapf(err, "unable to find 'git'") } - if absContextDir, err = gitutils.Clone(gitURL); err != nil { - return "", "", errors.Errorf("unable to 'git clone' to temporary context directory: %v", err) + absContextDir, err := gitutils.Clone(gitURL) + if err != nil { + return "", "", errors.Wrapf(err, "unable to 'git clone' to temporary context directory") } - return getDockerfileRelPath(absContextDir, dockerfileName) + absContextDir, err = resolveAndValidateContextPath(absContextDir) + if err != nil { + return "", "", err + } + relDockerfile, err := getDockerfileRelPath(absContextDir, dockerfileName) + return absContextDir, relDockerfile, err } // GetContextFromURL uses a remote URL as context for a `docker build`. The @@ -166,8 +172,13 @@ func GetContextFromURL(out io.Writer, remoteURL, dockerfileName string) (io.Read // `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 +func GetContextFromLocalDir(localDir, dockerfileName string) (string, string, error) { + localDir, err := resolveAndValidateContextPath(localDir) + if err != nil { + return "", "", err + } + + // When using a local context directory, and 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 != "" && dockerfileName != "-" { @@ -176,15 +187,16 @@ func GetContextFromLocalDir(localDir, dockerfileName string) (absContextDir, rel } } - return getDockerfileRelPath(localDir, dockerfileName) + relDockerfile, err := getDockerfileRelPath(localDir, dockerfileName) + return localDir, relDockerfile, 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 "", "", errors.Errorf("unable to get absolute context directory of given context directory %q: %v", givenContextDir, err) +// resolveAndValidateContextPath uses the given context directory for a `docker build` +// and returns the absolute path to the context directory. +func resolveAndValidateContextPath(givenContextDir string) (string, error) { + absContextDir, err := filepath.Abs(givenContextDir) + if err != nil { + return "", errors.Errorf("unable to get absolute context directory of given context directory %q: %v", givenContextDir, err) } // The context dir might be a symbolic link, so follow it to the actual @@ -197,17 +209,28 @@ func getDockerfileRelPath(givenContextDir, givenDockerfile string) (absContextDi if !isUNC(absContextDir) { absContextDir, err = filepath.EvalSymlinks(absContextDir) if err != nil { - return "", "", errors.Errorf("unable to evaluate symlinks in context path: %v", err) + return "", errors.Errorf("unable to evaluate symlinks in context path: %v", err) } } stat, err := os.Lstat(absContextDir) if err != nil { - return "", "", errors.Errorf("unable to stat context directory %q: %v", absContextDir, err) + return "", errors.Errorf("unable to stat context directory %q: %v", absContextDir, err) } if !stat.IsDir() { - return "", "", errors.Errorf("context must be a directory: %s", absContextDir) + return "", errors.Errorf("context must be a directory: %s", absContextDir) + } + return absContextDir, err +} + +// getDockerfileRelPath returns the dockerfile path relative to the context +// directory +func getDockerfileRelPath(absContextDir, givenDockerfile string) (string, error) { + var err error + + if givenDockerfile == "-" { + return givenDockerfile, nil } absDockerfile := givenDockerfile @@ -224,8 +247,6 @@ func getDockerfileRelPath(givenContextDir, givenDockerfile string) (absContextDi absDockerfile = altPath } } - } else if absDockerfile == "-" { - absDockerfile = filepath.Join(absContextDir, DefaultDockerfileName) } // If not already an absolute path, the Dockerfile path should be joined to @@ -240,32 +261,31 @@ func getDockerfileRelPath(givenContextDir, givenDockerfile string) (absContextDi // an issue in golang. On Windows, EvalSymLinks does not work on UNC file // paths (those starting with \\). This hack means that when using links // on UNC paths, they will not be followed. - if givenDockerfile != "-" { - if !isUNC(absDockerfile) { - absDockerfile, err = filepath.EvalSymlinks(absDockerfile) - if err != nil { - return "", "", errors.Errorf("unable to evaluate symlinks in Dockerfile path: %v", err) + if !isUNC(absDockerfile) { + absDockerfile, err = filepath.EvalSymlinks(absDockerfile) + if err != nil { + return "", errors.Errorf("unable to evaluate symlinks in Dockerfile path: %v", err) - } - } - - if _, err := os.Lstat(absDockerfile); err != nil { - if os.IsNotExist(err) { - return "", "", errors.Errorf("Cannot locate Dockerfile: %q", absDockerfile) - } - return "", "", errors.Errorf("unable to stat Dockerfile: %v", err) } } - if relDockerfile, err = filepath.Rel(absContextDir, absDockerfile); err != nil { - return "", "", errors.Errorf("unable to get relative Dockerfile path: %v", err) + if _, err := os.Lstat(absDockerfile); err != nil { + if os.IsNotExist(err) { + return "", errors.Errorf("Cannot locate Dockerfile: %q", absDockerfile) + } + return "", errors.Errorf("unable to stat Dockerfile: %v", err) + } + + relDockerfile, err := filepath.Rel(absContextDir, absDockerfile) + if err != nil { + return "", errors.Errorf("unable to get relative Dockerfile path: %v", err) } if strings.HasPrefix(relDockerfile, ".."+string(filepath.Separator)) { - return "", "", errors.Errorf("The Dockerfile (%s) must be within the build context (%s)", givenDockerfile, givenContextDir) + return "", errors.Errorf("the Dockerfile (%s) must be within the build context", givenDockerfile) } - return absContextDir, relDockerfile, nil + return relDockerfile, nil } // isUNC returns true if the path is UNC (one starting \\). It always returns diff --git a/cli/command/image/build/context_test.go b/cli/command/image/build/context_test.go index afa04a4fcd..3cab228a77 100644 --- a/cli/command/image/build/context_test.go +++ b/cli/command/image/build/context_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/testutil/assert" ) const dockerfileContents = "FROM busybox" @@ -45,19 +46,8 @@ func TestGetContextFromLocalDirNoDockerfile(t *testing.T) { contextDir, cleanup := createTestTempDir(t, "", "builder-context-test") defer cleanup() - absContextDir, relDockerfile, err := GetContextFromLocalDir(contextDir, "") - - if err == nil { - t.Fatalf("Error should not be nil") - } - - if absContextDir != "" { - t.Fatalf("Absolute directory path should be empty, got: %s", absContextDir) - } - - if relDockerfile != "" { - t.Fatalf("Relative path to Dockerfile should be empty, got: %s", relDockerfile) - } + _, _, err := GetContextFromLocalDir(contextDir, "") + assert.Error(t, err, "Dockerfile") } func TestGetContextFromLocalDirNotExistingDir(t *testing.T) { @@ -87,19 +77,8 @@ func TestGetContextFromLocalDirNotExistingDockerfile(t *testing.T) { fakePath := filepath.Join(contextDir, "fake") - absContextDir, relDockerfile, err := GetContextFromLocalDir(contextDir, fakePath) - - if err == nil { - t.Fatalf("Error should not be nil") - } - - if absContextDir != "" { - t.Fatalf("Absolute directory path should be empty, got: %s", absContextDir) - } - - if relDockerfile != "" { - t.Fatalf("Relative path to Dockerfile should be empty, got: %s", relDockerfile) - } + _, _, err := GetContextFromLocalDir(contextDir, fakePath) + assert.Error(t, err, "fake") } func TestGetContextFromLocalDirWithNoDirectory(t *testing.T) { @@ -314,35 +293,13 @@ func TestValidateContextDirectoryWithOneFileExcludes(t *testing.T) { // When an error occurs, it terminates the test. func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) { path, err := ioutil.TempDir(dir, prefix) - - if err != nil { - t.Fatalf("Error when creating directory %s with prefix %s: %s", dir, prefix, err) - } + assert.NilError(t, err) return path, func() { - err = os.RemoveAll(path) - - if err != nil { - t.Fatalf("Error when removing directory %s: %s", path, err) - } + assert.NilError(t, os.RemoveAll(path)) } } -// createTestTempSubdir creates a temporary directory for testing. -// It returns the created path but doesn't provide a cleanup function, -// so createTestTempSubdir should be used only for creating temporary subdirectories -// whose parent directories are properly cleaned up. -// When an error occurs, it terminates the test. -func createTestTempSubdir(t *testing.T, dir, prefix string) string { - path, err := ioutil.TempDir(dir, prefix) - - if err != nil { - t.Fatalf("Error when creating directory %s with prefix %s: %s", dir, prefix, err) - } - - return path -} - // 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 {