From 82f0e1e5d8831939d4e0856017a03449cd44f580 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Wed, 23 May 2018 06:15:11 +0000 Subject: [PATCH] build: fix `-f` handling with buildkit This commit brings a more pedantic change in the following ambiguous case: cat Dockerfile | docker build -f otherDockerfile - The legacy builder does not error out and prefers the Dockerfile from stdin while the buildkit-based one errors out. Note that this is only in the case where stdin is a Dockerfile (not an archive) Signed-off-by: Tibor Vass --- cli/command/image/build_buildkit.go | 82 ++++++++++++++++------------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/cli/command/image/build_buildkit.go b/cli/command/image/build_buildkit.go index d678d78a3c..7f8666c070 100644 --- a/cli/command/image/build_buildkit.go +++ b/cli/command/image/build_buildkit.go @@ -31,6 +31,8 @@ import ( const uploadRequestRemote = "upload-request" +var errDockerfileConflict = errors.New("ambiguous Dockerfile source: both stdin and flag correspond to Dockerfiles") + func runBuildBuildKit(dockerCli command.Cli, options buildOptions) error { ctx := appcontext.Context() @@ -45,10 +47,14 @@ func runBuildBuildKit(dockerCli command.Cli, options buildOptions) error { buildID := stringid.GenerateRandomID() var ( - remote string - body io.Reader - dockerfileName = filepath.Base(options.dockerfileName) + remote string + body io.Reader + dockerfileName = options.dockerfileName + dockerfileReader io.ReadCloser + dockerfileDir string + contextDir string ) + switch { case options.contextFromStdin(): if options.dockerfileFromStdin() { @@ -62,46 +68,25 @@ func runBuildBuildKit(dockerCli command.Cli, options buildOptions) error { body = rc remote = uploadRequestRemote } else { - dockerfileDir, err := build.WriteTempDockerfile(rc) - if err != nil { - return err + if options.dockerfileName != "" { + return errDockerfileConflict } - defer os.RemoveAll(dockerfileDir) - emptyDir, _ := ioutil.TempDir("", "stupid-empty-dir") - defer os.RemoveAll(emptyDir) - s.Allow(filesync.NewFSSyncProvider([]filesync.SyncedDir{ - { - Name: "context", - Dir: emptyDir, - }, - { - Name: "dockerfile", - Dir: dockerfileDir, - }, - })) + dockerfileReader = rc remote = clientSessionRemote + // TODO: make fssync handle empty contextdir + contextDir, _ = ioutil.TempDir("", "empty-dir") + defer os.RemoveAll(contextDir) } case isLocalDir(options.context): - dockerfileDir := filepath.Dir(options.dockerfileName) + contextDir = options.context if options.dockerfileFromStdin() { - dockerfileDir, err = build.WriteTempDockerfile(os.Stdin) - if err != nil { - return err - } - defer os.RemoveAll(dockerfileDir) - dockerfileName = build.DefaultDockerfileName + dockerfileReader = os.Stdin + } else if options.dockerfileName != "" { + dockerfileName = filepath.Base(options.dockerfileName) + dockerfileDir = filepath.Dir(options.dockerfileName) + } else { + dockerfileDir = options.context } - s.Allow(filesync.NewFSSyncProvider([]filesync.SyncedDir{ - { - Name: "context", - Dir: options.context, - Map: resetUIDAndGID, - }, - { - Name: "dockerfile", - Dir: dockerfileDir, - }, - })) remote = clientSessionRemote case urlutil.IsGitURL(options.context): remote = options.context @@ -111,6 +96,29 @@ func runBuildBuildKit(dockerCli command.Cli, options buildOptions) error { return errors.Errorf("unable to prepare context: path %q not found", options.context) } + if dockerfileReader != nil { + dockerfileName = build.DefaultDockerfileName + dockerfileDir, err = build.WriteTempDockerfile(dockerfileReader) + if err != nil { + return err + } + defer os.RemoveAll(dockerfileDir) + } + + if dockerfileDir != "" { + s.Allow(filesync.NewFSSyncProvider([]filesync.SyncedDir{ + { + Name: "context", + Dir: contextDir, + Map: resetUIDAndGID, + }, + { + Name: "dockerfile", + Dir: dockerfileDir, + }, + })) + } + // statusContext, cancelStatus := context.WithCancel(ctx) // defer cancelStatus()