From 120b6fec98fa5cb61eb262c5c0c749edac8349f7 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 4 Jul 2017 14:38:20 -0400 Subject: [PATCH 1/6] Replace replaceDockerfileTarWrapper with archive.ReplaceFileTarWrapper The bulk of this function was already implemented in a re-usable way. Also cleanup unnecessary args that should be part of the closure Signed-off-by: Daniel Nephin --- cli/command/image/build.go | 76 +++++++++----------------------------- 1 file changed, 17 insertions(+), 59 deletions(-) diff --git a/cli/command/image/build.go b/cli/command/image/build.go index 637068a80f4b..501a5ba360cb 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -276,17 +276,17 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { var resolvedTags []*resolvedTag if command.IsTrusted() { - translator := func(ctx context.Context, ref reference.NamedTagged) (reference.Canonical, error) { + translator := func(ref reference.NamedTagged) (reference.Canonical, error) { return TrustedReference(ctx, dockerCli, ref, nil) } // if there is a tar wrapper, the dockerfile needs to be replaced inside it if buildCtx != nil { // Wrap the tar archive to replace the Dockerfile entry with the rewritten // Dockerfile which uses trusted pulls. - buildCtx = replaceDockerfileTarWrapper(ctx, buildCtx, relDockerfile, translator, &resolvedTags) + buildCtx, resolvedTags = rewriteDockerfileForContentTrust(buildCtx, relDockerfile, translator) } else if dockerfileCtx != nil { // if there was not archive context still do the possible replacements in Dockerfile - newDockerfile, _, err := rewriteDockerfileFrom(ctx, dockerfileCtx, translator) + newDockerfile, _, err := rewriteDockerfileFrom(dockerfileCtx, translator) if err != nil { return err } @@ -468,7 +468,7 @@ func isLocalDir(c string) bool { return err == nil } -type translatorFunc func(context.Context, reference.NamedTagged) (reference.Canonical, error) +type translatorFunc func(reference.NamedTagged) (reference.Canonical, error) // validateTag checks if the given image name can be resolved. func validateTag(rawRepo string) (string, error) { @@ -493,7 +493,7 @@ type resolvedTag struct { // "FROM " instructions to a digest reference. `translator` is a // function that takes a repository name and tag reference and returns a // trusted digest reference. -func rewriteDockerfileFrom(ctx context.Context, dockerfile io.Reader, translator translatorFunc) (newDockerfile []byte, resolvedTags []*resolvedTag, err error) { +func rewriteDockerfileFrom(dockerfile io.Reader, translator translatorFunc) (newDockerfile []byte, resolvedTags []*resolvedTag, err error) { scanner := bufio.NewScanner(dockerfile) buf := bytes.NewBuffer(nil) @@ -511,7 +511,7 @@ func rewriteDockerfileFrom(ctx context.Context, dockerfile io.Reader, translator } ref = reference.TagNameOnly(ref) if ref, ok := ref.(reference.NamedTagged); ok && command.IsTrusted() { - trustedRef, err := translator(ctx, ref) + trustedRef, err := translator(ref) if err != nil { return nil, nil, err } @@ -533,57 +533,15 @@ func rewriteDockerfileFrom(ctx context.Context, dockerfile io.Reader, translator return buf.Bytes(), resolvedTags, scanner.Err() } -// replaceDockerfileTarWrapper wraps the given input tar archive stream and -// replaces the entry with the given Dockerfile name with the contents of the -// new Dockerfile. Returns a new tar archive stream with the replaced -// Dockerfile. -func replaceDockerfileTarWrapper(ctx context.Context, inputTarStream io.ReadCloser, dockerfileName string, translator translatorFunc, resolvedTags *[]*resolvedTag) io.ReadCloser { - pipeReader, pipeWriter := io.Pipe() - go func() { - tarReader := tar.NewReader(inputTarStream) - tarWriter := tar.NewWriter(pipeWriter) - - defer inputTarStream.Close() - - for { - hdr, err := tarReader.Next() - if err == io.EOF { - // Signals end of archive. - tarWriter.Close() - pipeWriter.Close() - return - } - if err != nil { - pipeWriter.CloseWithError(err) - return - } - - content := io.Reader(tarReader) - if hdr.Name == dockerfileName { - // This entry is the Dockerfile. Since the tar archive was - // generated from a directory on the local filesystem, the - // Dockerfile will only appear once in the archive. - var newDockerfile []byte - newDockerfile, *resolvedTags, err = rewriteDockerfileFrom(ctx, content, translator) - if err != nil { - pipeWriter.CloseWithError(err) - return - } - hdr.Size = int64(len(newDockerfile)) - content = bytes.NewBuffer(newDockerfile) - } - - if err := tarWriter.WriteHeader(hdr); err != nil { - pipeWriter.CloseWithError(err) - return - } - - if _, err := io.Copy(tarWriter, content); err != nil { - pipeWriter.CloseWithError(err) - return - } - } - }() - - return pipeReader +func rewriteDockerfileForContentTrust(buildCtx io.ReadCloser, dockerfileName string, translator translatorFunc) (io.ReadCloser, []*resolvedTag) { + var resolvedTags []*resolvedTag + return archive.ReplaceFileTarWrapper(buildCtx, map[string]archive.TarModifierFunc{ + dockerfileName: func(_ string, hdr *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + var newDockerfile []byte + var err error + newDockerfile, resolvedTags, err = rewriteDockerfileFrom(content, translator) + hdr.Size = int64(len(newDockerfile)) + return hdr, newDockerfile, err + }, + }), resolvedTags } From 4025b6f0bad875c42fa156650cdcd84295a0a5d7 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 4 Jul 2017 15:11:29 -0400 Subject: [PATCH 2/6] Factor out a buildBuffer from runBuild() This struct handles branching logging for quiet output mode. Signed-off-by: Daniel Nephin --- cli/command/image/build.go | 138 ++++++++++++++++++++++++------------- 1 file changed, 92 insertions(+), 46 deletions(-) diff --git a/cli/command/image/build.go b/cli/command/image/build.go index 501a5ba360cb..2656e65c8d2c 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -174,8 +174,6 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { contextDir string tempDir string relDockerfile string - progBuff io.Writer - buildBuff io.Writer remote string ) @@ -187,12 +185,6 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { } specifiedContext := options.context - progBuff = dockerCli.Out() - buildBuff = dockerCli.Out() - if options.quiet { - progBuff = bytes.NewBuffer(nil) - buildBuff = bytes.NewBuffer(nil) - } if options.imageIDFile != "" { // Avoid leaving a stale file if we eventually fail if err := os.Remove(options.imageIDFile); err != nil && !os.IsNotExist(err) { @@ -200,6 +192,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { } } + buildBuffer := newBuildBuffer(dockerCli.Out(), dockerCli.Err(), options.quiet) switch { case options.contextFromStdin(): // buildCtx is tar archive. if stdin was dockerfile then it is wrapped @@ -209,15 +202,13 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { case urlutil.IsGitURL(specifiedContext): tempDir, relDockerfile, err = build.GetContextFromGitURL(specifiedContext, options.dockerfileName) case urlutil.IsURL(specifiedContext): - buildCtx, relDockerfile, err = build.GetContextFromURL(progBuff, specifiedContext, options.dockerfileName) + buildCtx, relDockerfile, err = build.GetContextFromURL(buildBuffer.progress, specifiedContext, options.dockerfileName) default: return errors.Errorf("unable to prepare context: path %q not found", specifiedContext) } if err != nil { - if options.quiet && urlutil.IsURL(specifiedContext) { - fmt.Fprintln(dockerCli.Err(), progBuff) - } + buildBuffer.PrintProgressOnError() return errors.Errorf("unable to prepare context: %s", err) } @@ -302,7 +293,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { } // Setup an upload progress bar - progressOutput := streamformatter.NewProgressOutput(progBuff) + progressOutput := streamformatter.NewProgressOutput(buildBuffer.progress) if !dockerCli.Out().IsTerminal() { progressOutput = &lastProgressOutput{output: progressOutput} } @@ -332,14 +323,15 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { return err } - buf := newBufferedWriter(syncDone, buildBuff) + buf := newBufferedWriter(syncDone, buildBuffer.output) defer func() { select { case <-buf.flushed: case <-ctx.Done(): } }() - buildBuff = buf + // TODO: this overrides setting a buffer on quiet, is that correct? + buildBuffer.SetOutput(buf) remote = clientSessionRemote body = buildCtx @@ -392,37 +384,15 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { response, err := dockerCli.Client().ImageBuild(ctx, body, buildOptions) if err != nil { - if options.quiet { - fmt.Fprintf(dockerCli.Err(), "%s", progBuff) - } + buildBuffer.PrintErrorIfQuiet() cancel() return err } defer response.Body.Close() - imageID := "" - aux := func(auxJSON *json.RawMessage) { - var result types.BuildResult - if err := json.Unmarshal(*auxJSON, &result); err != nil { - fmt.Fprintf(dockerCli.Err(), "Failed to parse aux message: %s", err) - } else { - imageID = result.ID - } - } - - err = jsonmessage.DisplayJSONMessagesStream(response.Body, buildBuff, dockerCli.Out().FD(), dockerCli.Out().IsTerminal(), aux) + imageID, err := streamResponse(dockerCli, response, buildBuffer) if err != nil { - if jerr, ok := err.(*jsonmessage.JSONError); ok { - // If no error code is set, default to 1 - if jerr.Code == 0 { - jerr.Code = 1 - } - if options.quiet { - fmt.Fprintf(dockerCli.Err(), "%s%s", progBuff, buildBuff) - } - return cli.StatusError{Status: jerr.Message, StatusCode: jerr.Code} - } - return err + return nil } // Windows: show error message about modified file permissions if the @@ -435,12 +405,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { "files and directories.") } - // Everything worked so if -q was provided the output from the daemon - // should be just the image ID and we'll print that to stdout. - if options.quiet { - imageID = fmt.Sprintf("%s", buildBuff) - fmt.Fprintf(dockerCli.Out(), imageID) - } + buildBuffer.PrintImageIDIfQuiet() if options.imageIDFile != "" { if imageID == "" { @@ -545,3 +510,84 @@ func rewriteDockerfileForContentTrust(buildCtx io.ReadCloser, dockerfileName str }, }), resolvedTags } + +type buildBuffer struct { + quiet bool + stderr io.Writer + stdout io.Writer + output io.Writer + progress io.Writer +} + +func newBuildBuffer(out io.Writer, stderr io.Writer, quiet bool) *buildBuffer { + stdout := out + progress := out + if quiet { + out = bytes.NewBuffer(nil) + progress = bytes.NewBuffer(nil) + } + return &buildBuffer{ + output: out, + progress: progress, + stdout: stdout, + quiet: quiet, + stderr: stderr, + } +} + +// PrintErrorIfQuiet to the stderr stream. If quiet is not set then the error +// was printed elsewhere, so do nothing. +func (bb *buildBuffer) PrintErrorIfQuiet() { + if bb.quiet { + fmt.Fprintf(bb.stderr, "%s%s", bb.progress, bb.output) + } +} + +// PrintImageIDIfQuiet to stdout. If quiet is not set then the image ID is +// printed elsewhere, so do nothing. +func (bb *buildBuffer) PrintImageIDIfQuiet() { + if bb.quiet { + fmt.Fprintf(bb.stdout, "%s", bb.output) + } +} + +// PrintProgressOnError to stderr if quiet is set. If quiet is not set then +// the progress is printed elsewhere, so do nothing. +func (bb *buildBuffer) PrintProgressOnError() { + if bb.quiet { + fmt.Fprintln(bb.stderr, bb.progress) + } +} + +// SetOutput to a new Writer. If quiet is set then do nothing. +func (bb *buildBuffer) SetOutput(out io.Writer) { + if !bb.quiet { + bb.output = out + } +} + +func streamResponse(dockerCli command.Cli, response types.ImageBuildResponse, buffer *buildBuffer) (string, error) { + var imageID string + aux := func(auxJSON *json.RawMessage) { + var result types.BuildResult + if err := json.Unmarshal(*auxJSON, &result); err != nil { + fmt.Fprintf(dockerCli.Err(), "Failed to parse aux message: %s", err) + } else { + imageID = result.ID + } + } + + err := jsonmessage.DisplayJSONMessagesStream(response.Body, buffer.output, dockerCli.Out().FD(), dockerCli.Out().IsTerminal(), aux) + if err != nil { + if jerr, ok := err.(*jsonmessage.JSONError); ok { + // If no error code is set, default to 1 + if jerr.Code == 0 { + jerr.Code = 1 + } + buffer.PrintErrorIfQuiet() + return "", cli.StatusError{Status: jerr.Message, StatusCode: jerr.Code} + } + return "", err + } + return imageID, nil +} From b490e2d2740808de184874699b62791e89801536 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 4 Jul 2017 18:27:22 -0400 Subject: [PATCH 3/6] Extract and IDFile struct for handling build image ID file. Signed-off-by: Daniel Nephin --- cli/command/image/build.go | 45 +++++++++++++++---------------- cli/command/image/build/idfile.go | 40 +++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 cli/command/image/build/idfile.go diff --git a/cli/command/image/build.go b/cli/command/image/build.go index 2656e65c8d2c..0950689fd57e 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -184,14 +184,13 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { dockerfileCtx = dockerCli.In() } - specifiedContext := options.context - if options.imageIDFile != "" { - // Avoid leaving a stale file if we eventually fail - if err := os.Remove(options.imageIDFile); err != nil && !os.IsNotExist(err) { - return errors.Wrap(err, "Removing image ID file") - } + idFile := build.NewIDFile(options.imageIDFile) + // Avoid leaving a stale file if we eventually fail + if err := idFile.Remove(); err != nil { + return err } + specifiedContext := options.context buildBuffer := newBuildBuffer(dockerCli.Out(), dockerCli.Err(), options.quiet) switch { case options.contextFromStdin(): @@ -395,26 +394,12 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { return nil } - // Windows: show error message about modified file permissions if the - // daemon isn't running Windows. - if response.OSType != "windows" && runtime.GOOS == "windows" && !options.quiet { - fmt.Fprintln(dockerCli.Out(), "SECURITY WARNING: You are building a Docker "+ - "image from Windows against a non-Windows Docker host. All files and "+ - "directories added to build context will have '-rwxr-xr-x' permissions. "+ - "It is recommended to double check and reset permissions for sensitive "+ - "files and directories.") - } - + warnBuildWindowsToLinux(dockerCli.Out(), response.OSType, options.quiet) buildBuffer.PrintImageIDIfQuiet() - - if options.imageIDFile != "" { - if imageID == "" { - return errors.Errorf("Server did not provide an image ID. Cannot write %s", options.imageIDFile) - } - if err := ioutil.WriteFile(options.imageIDFile, []byte(imageID), 0666); err != nil { - return err - } + if err := idFile.Save(imageID); err != nil { + return err } + if command.IsTrusted() { // Since the build was successful, now we must tag any of the resolved // images from the above Dockerfile rewrite. @@ -591,3 +576,15 @@ func streamResponse(dockerCli command.Cli, response types.ImageBuildResponse, bu } return imageID, nil } + +func warnBuildWindowsToLinux(out io.Writer, osType string, quiet bool) { + // Windows: show error message about modified file permissions if the + // daemon isn't running Windows. + if osType != "windows" && runtime.GOOS == "windows" && !quiet { + fmt.Fprintln(out, "SECURITY WARNING: You are building a Docker "+ + "image from Windows against a non-Windows Docker host. All files and "+ + "directories added to build context will have '-rwxr-xr-x' permissions. "+ + "It is recommended to double check and reset permissions for sensitive "+ + "files and directories.") + } +} diff --git a/cli/command/image/build/idfile.go b/cli/command/image/build/idfile.go new file mode 100644 index 000000000000..e9f9b40cfaab --- /dev/null +++ b/cli/command/image/build/idfile.go @@ -0,0 +1,40 @@ +package build + +import ( + "io/ioutil" + "os" + + "github.com/pkg/errors" +) + +// IDFile manages a file which stores the imageID created by a build +type IDFile struct { + filename string +} + +// NewIDFile returns a new IDFile +func NewIDFile(filename string) *IDFile { + return &IDFile{filename: filename} +} + +// Remove the file +func (f *IDFile) Remove() error { + if f.filename == "" { + return nil + } + if err := os.Remove(f.filename); err != nil && !os.IsNotExist(err) { + return errors.Wrap(err, "failed to remove image ID file") + } + return nil +} + +// Save the imageID to the file +func (f *IDFile) Save(imageID string) error { + if f.filename == "" { + return nil + } + if imageID == "" { + return errors.Errorf("server did not provide an image ID. Cannot write %s", f.filename) + } + return ioutil.WriteFile(f.filename, []byte(imageID), 0666) +} From 874fbbcceebe44c850393cfab41ff5be414e6c91 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 4 Jul 2017 19:54:45 -0400 Subject: [PATCH 4/6] refactoring buildCtx and dockerfileCtx setup by extarcting a new buildInput struct Signed-off-by: Daniel Nephin --- cli/command/image/build.go | 237 ++++++++--------------------- cli/command/image/build_context.go | 167 ++++++++++++++++++++ cli/command/image/build_session.go | 38 +++-- 3 files changed, 262 insertions(+), 180 deletions(-) create mode 100644 cli/command/image/build_context.go diff --git a/cli/command/image/build.go b/cli/command/image/build.go index 0950689fd57e..1d57209a883e 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "io" - "io/ioutil" "os" "regexp" "runtime" @@ -21,14 +20,11 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/pkg/archive" - "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/streamformatter" - "github.com/docker/docker/pkg/urlutil" units "github.com/docker/go-units" - "github.com/pkg/errors" - "github.com/sirupsen/logrus" + "github.com/moby/buildkit/session" "github.com/spf13/cobra" "golang.org/x/net/context" ) @@ -165,175 +161,62 @@ func (out *lastProgressOutput) WriteProgress(prog progress.Progress) error { return out.output.WriteProgress(prog) } -// nolint: gocyclo func runBuild(dockerCli command.Cli, options buildOptions) error { - var ( - buildCtx io.ReadCloser - dockerfileCtx io.ReadCloser - err error - contextDir string - tempDir string - relDockerfile string - remote string - ) - - if options.dockerfileFromStdin() { - if options.contextFromStdin() { - return errors.New("invalid argument: can't use stdin for both build context and dockerfile") - } - dockerfileCtx = dockerCli.In() - } - idFile := build.NewIDFile(options.imageIDFile) // Avoid leaving a stale file if we eventually fail if err := idFile.Remove(); err != nil { return err } - specifiedContext := options.context buildBuffer := newBuildBuffer(dockerCli.Out(), dockerCli.Err(), options.quiet) - switch { - case options.contextFromStdin(): - // buildCtx is tar archive. if stdin was dockerfile then it is wrapped - buildCtx, relDockerfile, err = build.GetContextFromReader(dockerCli.In(), options.dockerfileName) - case isLocalDir(specifiedContext): - contextDir, relDockerfile, err = build.GetContextFromLocalDir(specifiedContext, options.dockerfileName) - case urlutil.IsGitURL(specifiedContext): - tempDir, relDockerfile, err = build.GetContextFromGitURL(specifiedContext, options.dockerfileName) - case urlutil.IsURL(specifiedContext): - buildCtx, relDockerfile, err = build.GetContextFromURL(buildBuffer.progress, specifiedContext, options.dockerfileName) - default: - return errors.Errorf("unable to prepare context: path %q not found", specifiedContext) - } - + buildInput, err := setupContextAndDockerfile(dockerCli, buildBuffer, options) if err != nil { - buildBuffer.PrintProgressOnError() - return errors.Errorf("unable to prepare context: %s", err) - } - - if tempDir != "" { - defer os.RemoveAll(tempDir) - contextDir = tempDir - } - - // read from a directory into tar archive - if buildCtx == nil && !options.stream { - excludes, err := build.ReadDockerignore(contextDir) - if err != nil { - return err - } - - if err := build.ValidateContextDirectory(contextDir, excludes); err != nil { - 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) - } - - excludes = build.TrimBuildFilesFromExcludes(excludes, relDockerfile, options.dockerfileFromStdin()) - buildCtx, err = archive.TarWithOptions(contextDir, &archive.TarOptions{ - ExcludePatterns: excludes, - ChownOpts: &idtools.IDPair{UID: 0, GID: 0}, - }) - if err != nil { - return err - } - } - - // replace Dockerfile if it was added from stdin and there is archive context - if dockerfileCtx != nil && buildCtx != nil { - buildCtx, relDockerfile, err = build.AddDockerfileToBuildContext(dockerfileCtx, buildCtx) - if err != nil { - return err - } - } - - // if streaming and dockerfile was not from stdin then read from file - // to the same reader that is usually stdin - if options.stream && dockerfileCtx == nil { - dockerfileCtx, err = os.Open(relDockerfile) - if err != nil { - return errors.Wrapf(err, "failed to open %s", relDockerfile) - } - defer dockerfileCtx.Close() + return err } + defer buildInput.cleanup() ctx, cancel := context.WithCancel(context.Background()) defer cancel() - var resolvedTags []*resolvedTag - if command.IsTrusted() { - translator := func(ref reference.NamedTagged) (reference.Canonical, error) { - return TrustedReference(ctx, dockerCli, ref, nil) - } - // if there is a tar wrapper, the dockerfile needs to be replaced inside it - if buildCtx != nil { - // Wrap the tar archive to replace the Dockerfile entry with the rewritten - // Dockerfile which uses trusted pulls. - buildCtx, resolvedTags = rewriteDockerfileForContentTrust(buildCtx, relDockerfile, translator) - } else if dockerfileCtx != nil { - // if there was not archive context still do the possible replacements in Dockerfile - newDockerfile, _, err := rewriteDockerfileFrom(dockerfileCtx, translator) - if err != nil { - return err - } - dockerfileCtx = ioutil.NopCloser(bytes.NewBuffer(newDockerfile)) - } + resolvedTags, err := updateBuildInputForContentTrust(ctx, dockerCli, buildInput) + if err != nil { + return err } if options.compress { - buildCtx, err = build.Compress(buildCtx) + buildInput.buildCtx, err = build.Compress(buildInput.buildCtx) if err != nil { return err } } - // Setup an upload progress bar - progressOutput := streamformatter.NewProgressOutput(buildBuffer.progress) - if !dockerCli.Out().IsTerminal() { - progressOutput = &lastProgressOutput{output: progressOutput} - } - - // if up to this point nothing has set the context then we must have another - // way for sending it(streaming) and set the context to the Dockerfile - if dockerfileCtx != nil && buildCtx == nil { - buildCtx = dockerfileCtx - } - - s, err := trySession(dockerCli, contextDir) - if err != nil { - return err + var contextSession *session.Session + if options.stream { + contextSession, err = newBuildContextSession(dockerCli, buildInput.contextDir) + if err != nil { + return err + } } + progressOutput := newProgaressOutput(dockerCli.Out(), buildBuffer.progress) var body io.Reader - if buildCtx != nil && !options.stream { - body = progress.NewProgressReader(buildCtx, progressOutput, 0, "", "Sending build context to Docker daemon") - } - - // add context stream to the session - if options.stream && s != nil { - syncDone := make(chan error) // used to signal first progress reporting completed. - // progress would also send errors but don't need it here as errors - // are handled by session.Run() and ImageBuild() - if err := addDirToSession(s, contextDir, progressOutput, syncDone); err != nil { + var remote string + if contextSession != nil { + buf, err := setupContextStream(progressOutput, contextSession, buildBuffer, buildInput.contextDir) + if err != nil { return err } - - buf := newBufferedWriter(syncDone, buildBuffer.output) + // TODO: move this to a method on bufferedWriter defer func() { select { case <-buf.flushed: case <-ctx.Done(): } }() - // TODO: this overrides setting a buffer on quiet, is that correct? - buildBuffer.SetOutput(buf) - remote = clientSessionRemote - body = buildCtx + body = buildInput.dockerfileCtx + } else { + body = setupRequestBody(progressOutput, buildInput.buildCtx) } configFile := dockerCli.ConfigFile() @@ -354,7 +237,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { CPUQuota: options.cpuQuota, CPUPeriod: options.cpuPeriod, CgroupParent: options.cgroupParent, - Dockerfile: relDockerfile, + Dockerfile: buildInput.dockerfilePath, ShmSize: options.shmSize.Value(), Ulimits: options.ulimits.GetList(), BuildArgs: configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll()), @@ -370,16 +253,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { Platform: options.platform, } - if s != nil { - go func() { - logrus.Debugf("running session: %v", s.ID()) - if err := s.Run(ctx, dockerCli.Client().DialSession); err != nil { - logrus.Error(err) - cancel() // cancel progress context - } - }() - buildOptions.SessionID = s.ID() - } + buildOptions.SessionID = startSession(ctx, contextSession, dockerCli.Client(), cancel) response, err := dockerCli.Client().ImageBuild(ctx, body, buildOptions) if err != nil { @@ -395,22 +269,14 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { } warnBuildWindowsToLinux(dockerCli.Out(), response.OSType, options.quiet) + buildBuffer.PrintImageIDIfQuiet() + if err := idFile.Save(imageID); err != nil { return err } - if command.IsTrusted() { - // Since the build was successful, now we must tag any of the resolved - // images from the above Dockerfile rewrite. - for _, resolved := range resolvedTags { - if err := TagTrusted(ctx, dockerCli, resolved.digestRef, resolved.tagRef); err != nil { - return err - } - } - } - - return nil + return tagTrustedImagesFromBuild(ctx, dockerCli, resolvedTags) } func isLocalDir(c string) bool { @@ -418,7 +284,31 @@ func isLocalDir(c string) bool { return err == nil } -type translatorFunc func(reference.NamedTagged) (reference.Canonical, error) +func setupRequestBody(progressOutput progress.Output, buildCtx io.ReadCloser) io.Reader { + return progress.NewProgressReader(buildCtx, progressOutput, 0, "", "Sending build context to Docker daemon") +} + +func setupContextStream(progressOutput progress.Output, contextSession *session.Session, buildBuffer *buildBuffer, contextDir string) (*bufferedWriter, error) { + syncDone := make(chan error) // used to signal first progress reporting completed. + // progress would also send errors but don't need it here as errors + // are handled by session.Run() and ImageBuild() + if err := addDirToSession(contextSession, contextDir, progressOutput, syncDone); err != nil { + return nil, err + } + + buf := newBufferedWriter(syncDone, buildBuffer.output) + buildBuffer.SetOutput(buf) + return buf, nil +} + +// Setup an upload progress bar +func newProgaressOutput(out *command.OutStream, progressWriter io.Writer) progress.Output { + progressOutput := streamformatter.NewProgressOutput(progressWriter) + if !out.IsTerminal() { + progressOutput = &lastProgressOutput{output: progressOutput} + } + return progressOutput +} // validateTag checks if the given image name can be resolved. func validateTag(rawRepo string) (string, error) { @@ -432,13 +322,6 @@ func validateTag(rawRepo string) (string, error) { var dockerfileFromLinePattern = regexp.MustCompile(`(?i)^[\s]*FROM[ \f\r\t\v]+(?P[^ \f\r\t\v\n#]+)`) -// resolvedTag records the repository, tag, and resolved digest reference -// from a Dockerfile rewrite. -type resolvedTag struct { - digestRef reference.Canonical - tagRef reference.NamedTagged -} - // rewriteDockerfileFrom rewrites the given Dockerfile by resolving images in // "FROM " instructions to a digest reference. `translator` is a // function that takes a repository name and tag reference and returns a @@ -588,3 +471,17 @@ func warnBuildWindowsToLinux(out io.Writer, osType string, quiet bool) { "files and directories.") } } + +func tagTrustedImagesFromBuild(ctx context.Context, dockerCli command.Cli, resolvedTags []*resolvedTag) error { + if !command.IsTrusted() { + return nil + } + // Since the build was successful, now we must tag any of the resolved + // images from the above Dockerfile rewrite. + for _, resolved := range resolvedTags { + if err := TagTrusted(ctx, dockerCli, resolved.digestRef, resolved.tagRef); err != nil { + return err + } + } + return nil +} diff --git a/cli/command/image/build_context.go b/cli/command/image/build_context.go new file mode 100644 index 000000000000..741a07c8ac8f --- /dev/null +++ b/cli/command/image/build_context.go @@ -0,0 +1,167 @@ +package image + +import ( + "bytes" + "io" + "io/ioutil" + "os" + + "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/command/image/build" + "github.com/docker/distribution/reference" + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/idtools" + "github.com/docker/docker/pkg/urlutil" + "github.com/pkg/errors" + "golang.org/x/net/context" +) + +type buildInput struct { + buildCtx io.ReadCloser + dockerfileCtx io.ReadCloser + dockerfilePath string + contextDir string + cleanups []func() +} + +func (bi *buildInput) cleanup() { + for _, fnc := range bi.cleanups { + fnc() + } +} + +func (bi *buildInput) addCleanup(fnc func()) { + bi.cleanups = append([]func(){fnc}, bi.cleanups...) +} + +func setupContextAndDockerfile(dockerCli command.Cli, buildBuffer *buildBuffer, options buildOptions) (*buildInput, error) { + result := &buildInput{} + + if options.dockerfileFromStdin() { + if options.contextFromStdin() { + return result, errors.New("invalid argument: can't use stdin for both build context and dockerfile") + } + result.dockerfileCtx = dockerCli.In() + } + + var err error + specifiedContext := options.context + switch { + case options.contextFromStdin(): + // buildCtx is tar archive. if stdin was dockerfile then it is wrapped + result.buildCtx, result.dockerfilePath, err = build.GetContextFromReader(dockerCli.In(), options.dockerfileName) + case isLocalDir(specifiedContext): + result.contextDir, result.dockerfilePath, err = build.GetContextFromLocalDir(specifiedContext, options.dockerfileName) + case urlutil.IsGitURL(specifiedContext): + result.contextDir, result.dockerfilePath, err = build.GetContextFromGitURL(specifiedContext, options.dockerfileName) + if result.contextDir != "" { + result.addCleanup(func() { os.RemoveAll(result.contextDir) }) + } + case urlutil.IsURL(specifiedContext): + result.buildCtx, result.dockerfilePath, err = build.GetContextFromURL(buildBuffer.progress, specifiedContext, options.dockerfileName) + default: + return result, errors.Errorf("unable to prepare context: path %q not found", specifiedContext) + } + + if err != nil { + buildBuffer.PrintProgressOnError() + return result, errors.Errorf("unable to prepare context: %s", err) + } + + if options.stream { + return createStreamBuildInput(result, specifiedContext) + } + + // Context is a git url, or a local dir + if result.buildCtx == nil { + result.buildCtx, err = createBuildContextFromLocalDir(result.contextDir, result.dockerfilePath, options.dockerfileFromStdin()) + if err != nil { + return result, err + } + } + + if result.dockerfileCtx != nil { + result.buildCtx, result.dockerfilePath, err = build.AddDockerfileToBuildContext(result.dockerfileCtx, result.buildCtx) + if err != nil { + return result, err + } + } + + return result, err +} + +func createBuildContextFromLocalDir(contextDir string, dockerfilePath string, dockerfileFromStdin bool) (io.ReadCloser, error) { + excludes, err := build.ReadDockerignore(contextDir) + if err != nil { + return nil, err + } + + if err := build.ValidateContextDirectory(contextDir, excludes); err != nil { + return nil, errors.Errorf("error checking context: '%s'.", err) + } + + // And canonicalize dockerfile name to a platform-independent one + dockerfilePath, err = archive.CanonicalTarNameForPath(dockerfilePath) + if err != nil { + return nil, errors.Errorf("cannot canonicalize dockerfile path %s: %v", dockerfilePath, err) + } + + excludes = build.TrimBuildFilesFromExcludes(excludes, dockerfilePath, dockerfileFromStdin) + return archive.TarWithOptions(contextDir, &archive.TarOptions{ + ExcludePatterns: excludes, + ChownOpts: &idtools.IDPair{UID: 0, GID: 0}, + }) +} + +func createStreamBuildInput(result *buildInput, context string) (*buildInput, error) { + if result.buildCtx != nil { + return result, errors.Errorf("stream is not supported for context: %s", context) + } + + var err error + if result.dockerfileCtx == nil { + result.dockerfileCtx, err = os.Open(result.dockerfilePath) + if err != nil { + return result, errors.Wrapf(err, "failed to open %s", result.dockerfilePath) + } + result.addCleanup(func() { result.dockerfileCtx.Close() }) + } + return result, nil +} + +type translatorFunc func(reference.NamedTagged) (reference.Canonical, error) + +// resolvedTag records the repository, tag, and resolved digest reference +// from a Dockerfile rewrite. +type resolvedTag struct { + digestRef reference.Canonical + tagRef reference.NamedTagged +} + +func updateBuildInputForContentTrust(ctx context.Context, dockerCli command.Cli, buildInput *buildInput) ([]*resolvedTag, error) { + if !command.IsTrusted() { + return nil, nil + } + translator := func(ref reference.NamedTagged) (reference.Canonical, error) { + return TrustedReference(ctx, dockerCli, ref, nil) + } + // if there is a tar wrapper, the dockerfile needs to be replaced inside it + if buildInput.buildCtx != nil { + // Wrap the tar archive to replace the Dockerfile entry with the rewritten + // Dockerfile which uses trusted pulls. + buildCtx, resolvedTags := rewriteDockerfileForContentTrust(buildInput.buildCtx, buildInput.dockerfilePath, translator) + buildInput.buildCtx = buildCtx + return resolvedTags, nil + } + + if buildInput.dockerfileCtx != nil { + // if there was not archive context still do the possible replacements in Dockerfile + newDockerfile, _, err := rewriteDockerfileFrom(buildInput.dockerfileCtx, translator) + if err != nil { + return nil, err + } + buildInput.dockerfileCtx = ioutil.NopCloser(bytes.NewBuffer(newDockerfile)) + // TODO: shouldn't this also return resolvedTags? + } + return nil, nil +} diff --git a/cli/command/image/build_session.go b/cli/command/image/build_session.go index 897d237cd699..1b576f98e4e0 100644 --- a/cli/command/image/build_session.go +++ b/cli/command/image/build_session.go @@ -17,10 +17,13 @@ import ( "github.com/docker/cli/cli/command/image/build" cliconfig "github.com/docker/cli/cli/config" "github.com/docker/docker/api/types/versions" + "github.com/docker/docker/client" "github.com/docker/docker/pkg/progress" "github.com/moby/buildkit/session" "github.com/moby/buildkit/session/filesync" "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "golang.org/x/net/context" "golang.org/x/time/rate" ) @@ -30,21 +33,36 @@ func isSessionSupported(dockerCli command.Cli) bool { return dockerCli.ServerInfo().HasExperimental && versions.GreaterThanOrEqualTo(dockerCli.Client().ClientVersion(), "1.31") } -func trySession(dockerCli command.Cli, contextDir string) (*session.Session, error) { +func newBuildContextSession(dockerCli command.Cli, contextDir string) (*session.Session, error) { var s *session.Session - if isSessionSupported(dockerCli) { - sharedKey, err := getBuildSharedKey(contextDir) - if err != nil { - return nil, errors.Wrap(err, "failed to get build shared key") - } - s, err = session.NewSession(filepath.Base(contextDir), sharedKey) - if err != nil { - return nil, errors.Wrap(err, "failed to create session") - } + if !isSessionSupported(dockerCli) { + return s, errors.New("stream is not supported") + } + sharedKey, err := getBuildSharedKey(contextDir) + if err != nil { + return nil, errors.Wrap(err, "failed to get build shared key") + } + s, err = session.NewSession(filepath.Base(contextDir), sharedKey) + if err != nil { + return nil, errors.Wrap(err, "failed to create session") } return s, nil } +func startSession(ctx context.Context, contextSession *session.Session, client client.APIClient, cancel func()) string { + if contextSession == nil { + return "" + } + go func() { + logrus.Debugf("running session: %v", contextSession.ID()) + if err := contextSession.Run(ctx, client.DialSession); err != nil { + logrus.Error(err) + cancel() // cancel progress context + } + }() + return contextSession.ID() +} + func addDirToSession(session *session.Session, contextDir string, progressOutput progress.Output, done chan error) error { excludes, err := build.ReadDockerignore(contextDir) if err != nil { From fd927e6d060735cf5184860da009caeff3621cbb Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 5 Jul 2017 12:18:24 -0400 Subject: [PATCH 5/6] Unit test for trusted tag. Signed-off-by: Daniel Nephin --- cli/command/image/trust.go | 12 +++--- cli/command/image/trust_test.go | 70 ++++++++++++++++++++++----------- 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index 7a70ef96c02c..39691454944f 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -344,15 +344,13 @@ func convertTarget(t client.Target) (target, error) { } // TagTrusted tags a trusted ref -// nolint: interfacer -func TagTrusted(ctx context.Context, cli command.Cli, trustedRef reference.Canonical, ref reference.NamedTagged) error { +func TagTrusted(ctx context.Context, cli command.Cli, fromRef reference.Reference, toRef reference.Reference) error { // Use familiar references when interacting with client and output - familiarRef := reference.FamiliarString(ref) - trustedFamiliarRef := reference.FamiliarString(trustedRef) + from := reference.FamiliarString(fromRef) + to := reference.FamiliarString(toRef) - fmt.Fprintf(cli.Err(), "Tagging %s as %s\n", trustedFamiliarRef, familiarRef) - - return cli.Client().ImageTag(ctx, trustedFamiliarRef, familiarRef) + fmt.Fprintf(cli.Err(), "Tagging %s as %s\n", from, to) + return cli.Client().ImageTag(ctx, from, to) } // AuthResolver returns an auth resolver function from a command.Cli diff --git a/cli/command/image/trust_test.go b/cli/command/image/trust_test.go index 6071ebdea522..23661ea513f6 100644 --- a/cli/command/image/trust_test.go +++ b/cli/command/image/trust_test.go @@ -1,11 +1,15 @@ package image import ( + "fmt" "io/ioutil" "os" "testing" "github.com/docker/cli/cli/trust" + "github.com/docker/cli/internal/test" + "github.com/docker/cli/internal/test/testutil" + "github.com/docker/distribution/reference" registrytypes "github.com/docker/docker/api/types/registry" "github.com/docker/docker/registry" "github.com/stretchr/testify/assert" @@ -13,6 +17,7 @@ import ( "github.com/theupdateframework/notary/client" "github.com/theupdateframework/notary/passphrase" "github.com/theupdateframework/notary/trustpinning" + "golang.org/x/net/context" ) func unsetENV() { @@ -20,46 +25,65 @@ func unsetENV() { os.Unsetenv("DOCKER_CONTENT_TRUST_SERVER") } -func TestENVTrustServer(t *testing.T) { +func TestTrustServerFromEnv(t *testing.T) { defer unsetENV() indexInfo := ®istrytypes.IndexInfo{Name: "testserver"} - if err := os.Setenv("DOCKER_CONTENT_TRUST_SERVER", "https://notary-test.com:5000"); err != nil { - t.Fatal("Failed to set ENV variable") - } - output, err := trust.Server(indexInfo) + require.NoError(t, os.Setenv("DOCKER_CONTENT_TRUST_SERVER", "https://notary-test.com:5000")) + expectedStr := "https://notary-test.com:5000" - if err != nil || output != expectedStr { - t.Fatalf("Expected server to be %s, got %s", expectedStr, output) - } + output, err := trust.Server(indexInfo) + require.NoError(t, err) + assert.Equal(t, expectedStr, output) } -func TestHTTPENVTrustServer(t *testing.T) { +func TestTrustServerNotHTTPS(t *testing.T) { defer unsetENV() indexInfo := ®istrytypes.IndexInfo{Name: "testserver"} - if err := os.Setenv("DOCKER_CONTENT_TRUST_SERVER", "http://notary-test.com:5000"); err != nil { - t.Fatal("Failed to set ENV variable") - } + require.NoError(t, os.Setenv("DOCKER_CONTENT_TRUST_SERVER", "http://notary-test.com:5000")) + _, err := trust.Server(indexInfo) - if err == nil { - t.Fatal("Expected error with invalid scheme") - } + testutil.ErrorContains(t, err, "valid https URL required for trust server") } -func TestOfficialTrustServer(t *testing.T) { +func TestTrustServerOfficial(t *testing.T) { indexInfo := ®istrytypes.IndexInfo{Name: "testserver", Official: true} + output, err := trust.Server(indexInfo) - if err != nil || output != registry.NotaryServer { - t.Fatalf("Expected server to be %s, got %s", registry.NotaryServer, output) - } + require.NoError(t, err) + assert.Equal(t, registry.NotaryServer, output) } -func TestNonOfficialTrustServer(t *testing.T) { +func TestTrustServerNonOfficial(t *testing.T) { indexInfo := ®istrytypes.IndexInfo{Name: "testserver", Official: false} - output, err := trust.Server(indexInfo) expectedStr := "https://" + indexInfo.Name - if err != nil || output != expectedStr { - t.Fatalf("Expected server to be %s, got %s", expectedStr, output) + + output, err := trust.Server(indexInfo) + require.NoError(t, err) + assert.Equal(t, expectedStr, output) +} + +func TestTagTrusted(t *testing.T) { + ctx := context.Background() + expectedFrom := "alpine@sha256:f02537b30c729bb1ee0c67c8bff3a28972bed0abbee8f5520421994c5247b896" + expectedTo := "example/app:v1" + fakeClient := &fakeClient{ + imageTagFunc: func(from string, to string) error { + assert.Equal(t, expectedFrom, from) + assert.Equal(t, expectedTo, to) + return nil + }, } + cli := test.NewFakeCli(fakeClient) + + canonical, err := reference.Parse("docker.io/library/alpine@sha256:f02537b30c729bb1ee0c67c8bff3a28972bed0abbee8f5520421994c5247b896") + require.NoError(t, err) + namedTag, err := reference.Parse("docker.io/example/app:v1") + require.NoError(t, err) + + err = TagTrusted(ctx, cli, canonical, namedTag) + require.NoError(t, err) + expectedOut := fmt.Sprintf("Tagging %s as %s\n", expectedFrom, expectedTo) + assert.Equal(t, expectedOut, cli.ErrBuffer().String()) } func TestAddTargetToAllSignableRolesError(t *testing.T) { From f1c2552c380d5f2eb10be5a663b1543f89027b02 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 6 Jul 2017 13:30:51 -0400 Subject: [PATCH 6/6] Cleanup buildBuffer so that it doesn't print io.Writer Also rename it to buildOutputBuffer Signed-off-by: Daniel Nephin --- cli/command/image/build.go | 81 +++++++++++------------------- cli/command/image/build_context.go | 4 +- 2 files changed, 32 insertions(+), 53 deletions(-) diff --git a/cli/command/image/build.go b/cli/command/image/build.go index 1d57209a883e..26881e09018f 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -168,7 +168,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { return err } - buildBuffer := newBuildBuffer(dockerCli.Out(), dockerCli.Err(), options.quiet) + buildBuffer := newBuildBuffer(dockerCli.Out(), options.quiet) buildInput, err := setupContextAndDockerfile(dockerCli, buildBuffer, options) if err != nil { return err @@ -198,14 +198,15 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { } } - progressOutput := newProgaressOutput(dockerCli.Out(), buildBuffer.progress) + progressOutput := newProgressOutput(dockerCli.Out(), buildBuffer.progress) var body io.Reader var remote string if contextSession != nil { - buf, err := setupContextStream(progressOutput, contextSession, buildBuffer, buildInput.contextDir) + buf, err := setupContextStream(contextSession, buildInput.contextDir, progressOutput, buildBuffer.output) if err != nil { return err } + buildBuffer.output = buf // TODO: move this to a method on bufferedWriter defer func() { select { @@ -257,7 +258,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { response, err := dockerCli.Client().ImageBuild(ctx, body, buildOptions) if err != nil { - buildBuffer.PrintErrorIfQuiet() + buildBuffer.PrintBuffers(dockerCli.Err()) cancel() return err } @@ -270,7 +271,8 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { warnBuildWindowsToLinux(dockerCli.Out(), response.OSType, options.quiet) - buildBuffer.PrintImageIDIfQuiet() + // Print the imageID if it's buffered (when options.quiet is set) + buildBuffer.PrintOutputBuffer(dockerCli.Out()) if err := idFile.Save(imageID); err != nil { return err @@ -288,21 +290,18 @@ func setupRequestBody(progressOutput progress.Output, buildCtx io.ReadCloser) io return progress.NewProgressReader(buildCtx, progressOutput, 0, "", "Sending build context to Docker daemon") } -func setupContextStream(progressOutput progress.Output, contextSession *session.Session, buildBuffer *buildBuffer, contextDir string) (*bufferedWriter, error) { +func setupContextStream(contextSession *session.Session, contextDir string, progressOutput progress.Output, out io.Writer) (*bufferedWriter, error) { syncDone := make(chan error) // used to signal first progress reporting completed. // progress would also send errors but don't need it here as errors // are handled by session.Run() and ImageBuild() if err := addDirToSession(contextSession, contextDir, progressOutput, syncDone); err != nil { return nil, err } - - buf := newBufferedWriter(syncDone, buildBuffer.output) - buildBuffer.SetOutput(buf) - return buf, nil + return newBufferedWriter(syncDone, out), nil } // Setup an upload progress bar -func newProgaressOutput(out *command.OutStream, progressWriter io.Writer) progress.Output { +func newProgressOutput(out *command.OutStream, progressWriter io.Writer) progress.Output { progressOutput := streamformatter.NewProgressOutput(progressWriter) if !out.IsTerminal() { progressOutput = &lastProgressOutput{output: progressOutput} @@ -379,62 +378,42 @@ func rewriteDockerfileForContentTrust(buildCtx io.ReadCloser, dockerfileName str }), resolvedTags } -type buildBuffer struct { - quiet bool - stderr io.Writer - stdout io.Writer +// buildOutputBuffer manages io.Writers used to print build progress and errors +type buildOutputBuffer struct { output io.Writer progress io.Writer } -func newBuildBuffer(out io.Writer, stderr io.Writer, quiet bool) *buildBuffer { - stdout := out +func newBuildBuffer(out io.Writer, quiet bool) *buildOutputBuffer { progress := out if quiet { - out = bytes.NewBuffer(nil) - progress = bytes.NewBuffer(nil) - } - return &buildBuffer{ - output: out, - progress: progress, - stdout: stdout, - quiet: quiet, - stderr: stderr, - } -} - -// PrintErrorIfQuiet to the stderr stream. If quiet is not set then the error -// was printed elsewhere, so do nothing. -func (bb *buildBuffer) PrintErrorIfQuiet() { - if bb.quiet { - fmt.Fprintf(bb.stderr, "%s%s", bb.progress, bb.output) + out = new(bytes.Buffer) + progress = new(bytes.Buffer) } + return &buildOutputBuffer{output: out, progress: progress} } -// PrintImageIDIfQuiet to stdout. If quiet is not set then the image ID is -// printed elsewhere, so do nothing. -func (bb *buildBuffer) PrintImageIDIfQuiet() { - if bb.quiet { - fmt.Fprintf(bb.stdout, "%s", bb.output) - } +// PrintBuffers to the writer +func (bb *buildOutputBuffer) PrintBuffers(writer io.Writer) { + bb.PrintProgressBuffer(writer) + bb.PrintOutputBuffer(writer) } -// PrintProgressOnError to stderr if quiet is set. If quiet is not set then -// the progress is printed elsewhere, so do nothing. -func (bb *buildBuffer) PrintProgressOnError() { - if bb.quiet { - fmt.Fprintln(bb.stderr, bb.progress) +// PrintOutputBuffer to the writer if output is a buffer +func (bb *buildOutputBuffer) PrintOutputBuffer(writer io.Writer) { + if buffer, ok := bb.output.(*bytes.Buffer); ok { + fmt.Fprintf(writer, buffer.String()) } } -// SetOutput to a new Writer. If quiet is set then do nothing. -func (bb *buildBuffer) SetOutput(out io.Writer) { - if !bb.quiet { - bb.output = out +// PrintProgressBuffer to the writer with a trailing newline if progress is a buffer +func (bb *buildOutputBuffer) PrintProgressBuffer(writer io.Writer) { + if buffer, ok := bb.progress.(*bytes.Buffer); ok { + fmt.Fprintln(writer, buffer.String()) } } -func streamResponse(dockerCli command.Cli, response types.ImageBuildResponse, buffer *buildBuffer) (string, error) { +func streamResponse(dockerCli command.Cli, response types.ImageBuildResponse, buffer *buildOutputBuffer) (string, error) { var imageID string aux := func(auxJSON *json.RawMessage) { var result types.BuildResult @@ -452,7 +431,7 @@ func streamResponse(dockerCli command.Cli, response types.ImageBuildResponse, bu if jerr.Code == 0 { jerr.Code = 1 } - buffer.PrintErrorIfQuiet() + buffer.PrintBuffers(dockerCli.Err()) return "", cli.StatusError{Status: jerr.Message, StatusCode: jerr.Code} } return "", err diff --git a/cli/command/image/build_context.go b/cli/command/image/build_context.go index 741a07c8ac8f..db53d43b99c6 100644 --- a/cli/command/image/build_context.go +++ b/cli/command/image/build_context.go @@ -34,7 +34,7 @@ func (bi *buildInput) addCleanup(fnc func()) { bi.cleanups = append([]func(){fnc}, bi.cleanups...) } -func setupContextAndDockerfile(dockerCli command.Cli, buildBuffer *buildBuffer, options buildOptions) (*buildInput, error) { +func setupContextAndDockerfile(dockerCli command.Cli, buildBuffer *buildOutputBuffer, options buildOptions) (*buildInput, error) { result := &buildInput{} if options.dockerfileFromStdin() { @@ -64,7 +64,7 @@ func setupContextAndDockerfile(dockerCli command.Cli, buildBuffer *buildBuffer, } if err != nil { - buildBuffer.PrintProgressOnError() + buildBuffer.PrintProgressBuffer(dockerCli.Err()) return result, errors.Errorf("unable to prepare context: %s", err) }