Refactor remote context parsing

Redefine a better interface for remote context dependency.

Separate Dockerfile build instruction from remote context.

Signed-off-by: Tonis Tiigi <[email protected]>

api/server/router/build/backend.go

@@ -16,5 +16,5 @@ type Backend interface {
 	// by the caller.
 	//
 	// TODO: make this return a reference instead of string
-	BuildFromContext(ctx context.Context, src io.ReadCloser, remote string, buildOptions *types.ImageBuildOptions, pg backend.ProgressWriter) (string, error)
+	BuildFromContext(ctx context.Context, src io.ReadCloser, buildOptions *types.ImageBuildOptions, pg backend.ProgressWriter) (string, error)
 }

api/server/router/build/build_routes.go

@@ -21,7 +21,7 @@ import (
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/streamformatter"
-	"github.com/docker/go-units"
+	units "github.com/docker/go-units"
 	"golang.org/x/net/context"
 )
 
@@ -57,6 +57,7 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui
 	options.SecurityOpt = r.Form["securityopt"]
 	options.Squash = httputils.BoolValue(r, "squash")
 	options.Target = r.FormValue("target")
+	options.RemoteContext = r.FormValue("remote")
 
 	if r.Form.Get("shmsize") != "" {
 		shmSize, err := strconv.ParseInt(r.Form.Get("shmsize"), 10, 64)
@@ -184,16 +185,14 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r *
 	}
 	buildOptions.AuthConfigs = authConfigs
 
-	remoteURL := r.FormValue("remote")
-
 	// Currently, only used if context is from a remote url.
 	// Look at code in DetectContextFromRemoteURL for more information.
 	createProgressReader := func(in io.ReadCloser) io.ReadCloser {
 		progressOutput := sf.NewProgressOutput(output, true)
 		if buildOptions.SuppressOutput {
 			progressOutput = sf.NewProgressOutput(notVerboseBuffer, true)
 		}
-		return progress.NewProgressReader(in, progressOutput, r.ContentLength, "Downloading context", remoteURL)
+		return progress.NewProgressReader(in, progressOutput, r.ContentLength, "Downloading context", buildOptions.RemoteContext)
 	}
 
 	out := io.Writer(output)
@@ -211,7 +210,7 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r *
 		ProgressReaderFunc: createProgressReader,
 	}
 
-	imgID, err := br.backend.BuildFromContext(ctx, r.Body, remoteURL, buildOptions, pg)
+	imgID, err := br.backend.BuildFromContext(ctx, r.Body, buildOptions, pg)
 	if err != nil {
 		return errf(err)
 	}

builder/builder.go

@@ -6,7 +6,6 @@ package builder
 
 import (
 	"io"
-	"os"
 	"time"
 
 	"github.com/docker/distribution/reference"
@@ -22,85 +21,17 @@ const (
 	DefaultDockerfileName string = "Dockerfile"
 )
 
-// Context represents a file system tree.
-type Context interface {
+// Source defines a location that can be used as a source for the ADD/COPY
+// instructions in the builder.
+type Source interface {
+	// Root returns root path for accessing source
+	Root() string
 	// Close allows to signal that the filesystem tree won't be used anymore.
 	// For Context implementations using a temporary directory, it is recommended to
 	// delete the temporary directory in Close().
 	Close() error
-	// Stat returns an entry corresponding to path if any.
-	// It is recommended to return an error if path was not found.
-	// If path is a symlink it also returns the path to the target file.
-	Stat(path string) (string, FileInfo, error)
-	// Open opens path from the context and returns a readable stream of it.
-	Open(path string) (io.ReadCloser, error)
-	// Walk walks the tree of the context with the function passed to it.
-	Walk(root string, walkFn WalkFunc) error
-}
-
-// WalkFunc is the type of the function called for each file or directory visited by Context.Walk().
-type WalkFunc func(path string, fi FileInfo, err error) error
-
-// ModifiableContext represents a modifiable Context.
-// TODO: remove this interface once we can get rid of Remove()
-type ModifiableContext interface {
-	Context
-	// Remove deletes the entry specified by `path`.
-	// It is usual for directory entries to delete all its subentries.
-	Remove(path string) error
-}
-
-// FileInfo extends os.FileInfo to allow retrieving an absolute path to the file.
-// TODO: remove this interface once pkg/archive exposes a walk function that Context can use.
-type FileInfo interface {
-	os.FileInfo
-	Path() string
-}
-
-// PathFileInfo is a convenience struct that implements the FileInfo interface.
-type PathFileInfo struct {
-	os.FileInfo
-	// FilePath holds the absolute path to the file.
-	FilePath string
-	// FileName holds the basename for the file.
-	FileName string
-}
-
-// Path returns the absolute path to the file.
-func (fi PathFileInfo) Path() string {
-	return fi.FilePath
-}
-
-// Name returns the basename of the file.
-func (fi PathFileInfo) Name() string {
-	if fi.FileName != "" {
-		return fi.FileName
-	}
-	return fi.FileInfo.Name()
-}
-
-// Hashed defines an extra method intended for implementations of os.FileInfo.
-type Hashed interface {
-	// Hash returns the hash of a file.
-	Hash() string
-	SetHash(string)
-}
-
-// HashedFileInfo is a convenient struct that augments FileInfo with a field.
-type HashedFileInfo struct {
-	FileInfo
-	// FileHash represents the hash of a file.
-	FileHash string
-}
-
-// Hash returns the hash of a file.
-func (fi HashedFileInfo) Hash() string {
-	return fi.FileHash
-}
-
-// SetHash sets the hash of a file.
-func (fi *HashedFileInfo) SetHash(h string) {
-	fi.FileHash = h
+	// Hash returns a checksum for a file
+	Hash(path string) (string, error)
 }
 
 // Backend abstracts calls to a Docker Daemon.
@@ -134,12 +65,9 @@ type Backend interface {
 
 	// ContainerCopy copies/extracts a source FileInfo to a destination path inside a container
 	// specified by a container object.
-	// TODO: make an Extract method instead of passing `decompress`
-	// TODO: do not pass a FileInfo, instead refactor the archive package to export a Walk function that can be used
-	// with Context.Walk
-	// ContainerCopy(name string, res string) (io.ReadCloser, error)
-	// TODO: use copyBackend api
-	CopyOnBuild(containerID string, destPath string, src FileInfo, decompress bool) error
+	// TODO: extract in the builder instead of passing `decompress`
+	// TODO: use containerd/fs.changestream instead as a source
+	CopyOnBuild(containerID string, destPath string, srcRoot string, srcPath string, decompress bool) error
 
 	// HasExperimental checks if the backend supports experimental features
 	HasExperimental() bool

builder/dockerfile/builder.go

@@ -17,6 +17,7 @@ import (
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/dockerfile/command"
 	"github.com/docker/docker/builder/dockerfile/parser"
+	"github.com/docker/docker/builder/remotecontext"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/pkg/errors"
@@ -48,7 +49,7 @@ type Builder struct {
 	Output io.Writer
 
 	docker    builder.Backend
-	context   builder.Context
+	source    builder.Source
 	clientCtx context.Context
 
 	runConfig     *container.Config // runconfig for cmd, run, entrypoint etc.
@@ -80,35 +81,37 @@ func NewBuildManager(b builder.Backend) (bm *BuildManager) {
 }
 
 // BuildFromContext builds a new image from a given context.
-func (bm *BuildManager) BuildFromContext(ctx context.Context, src io.ReadCloser, remote string, buildOptions *types.ImageBuildOptions, pg backend.ProgressWriter) (string, error) {
+func (bm *BuildManager) BuildFromContext(ctx context.Context, src io.ReadCloser, buildOptions *types.ImageBuildOptions, pg backend.ProgressWriter) (string, error) {
 	if buildOptions.Squash && !bm.backend.HasExperimental() {
 		return "", apierrors.NewBadRequestError(errors.New("squash is only supported with experimental mode"))
 	}
-	buildContext, dockerfileName, err := builder.DetectContextFromRemoteURL(src, remote, pg.ProgressReaderFunc)
+	if buildOptions.Dockerfile == "" {
+		buildOptions.Dockerfile = builder.DefaultDockerfileName
+	}
+
+	source, dockerfile, err := remotecontext.Detect(ctx, buildOptions.RemoteContext, buildOptions.Dockerfile, src, pg.ProgressReaderFunc)
 	if err != nil {
 		return "", err
 	}
-	defer func() {
-		if err := buildContext.Close(); err != nil {
-			logrus.Debugf("[BUILDER] failed to remove temporary context: %v", err)
-		}
-	}()
-
-	if len(dockerfileName) > 0 {
-		buildOptions.Dockerfile = dockerfileName
+	if source != nil {
+		defer func() {
+			if err := source.Close(); err != nil {
+				logrus.Debugf("[BUILDER] failed to remove temporary context: %v", err)
+			}
+		}()
 	}
-	b, err := NewBuilder(ctx, buildOptions, bm.backend, builder.DockerIgnoreContext{ModifiableContext: buildContext})
+	b, err := NewBuilder(ctx, buildOptions, bm.backend, source)
 	if err != nil {
 		return "", err
 	}
 	b.imageContexts.cache = bm.pathCache
-	return b.build(pg.StdoutFormatter, pg.StderrFormatter, pg.Output)
+	return b.build(dockerfile, pg.StdoutFormatter, pg.StderrFormatter, pg.Output)
 }
 
 // NewBuilder creates a new Dockerfile builder from an optional dockerfile and a Config.
 // If dockerfile is nil, the Dockerfile specified by Config.DockerfileName,
 // will be read from the Context passed to Build().
-func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, backend builder.Backend, buildContext builder.Context) (b *Builder, err error) {
+func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, backend builder.Backend, source builder.Source) (b *Builder, err error) {
 	if config == nil {
 		config = new(types.ImageBuildOptions)
 	}
@@ -118,7 +121,7 @@ func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, back
 		Stdout:        os.Stdout,
 		Stderr:        os.Stderr,
 		docker:        backend,
-		context:       buildContext,
+		source:        source,
 		runConfig:     new(container.Config),
 		tmpContainers: map[string]struct{}{},
 		buildArgs:     newBuildArgs(config.BuildArgs),
@@ -173,18 +176,13 @@ func sanitizeRepoAndTags(names []string) ([]reference.Named, error) {
 
 // build runs the Dockerfile builder from a context and a docker object that allows to make calls
 // to Docker.
-func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) {
+func (b *Builder) build(dockerfile *parser.Result, stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) {
 	defer b.imageContexts.unmount()
 
 	b.Stdout = stdout
 	b.Stderr = stderr
 	b.Output = out
 
-	dockerfile, err := b.readAndParseDockerfile()
-	if err != nil {
-		return "", err
-	}
-
 	repoAndTags, err := sanitizeRepoAndTags(b.options.Tags)
 	if err != nil {
 		return "", err

builder/dockerfile/evaluator_test.go

@@ -7,8 +7,8 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
-	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/dockerfile/parser"
+	"github.com/docker/docker/builder/remotecontext"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/reexec"
 )
@@ -158,7 +158,7 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) {
 		}
 	}()
 
-	context, err := builder.MakeTarSumContext(tarStream)
+	context, err := remotecontext.MakeTarSumContext(tarStream)
 
 	if err != nil {
 		t.Fatalf("Error when creating tar context: %s", err)
@@ -186,7 +186,7 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) {
 		runConfig: config,
 		options:   options,
 		Stdout:    ioutil.Discard,
-		context:   context,
+		source:    context,
 		buildArgs: newBuildArgs(options.BuildArgs),
 	}
 

builder/dockerfile/imagecontext.go

@@ -119,29 +119,29 @@ func (ic *imageContexts) setCache(id, path string, v interface{}) {
 // by an existing image
 type imageMount struct {
 	id        string
-	ctx       builder.Context
+	source    builder.Source
 	release   func() error
 	ic        *imageContexts
 	runConfig *container.Config
 }
 
-func (im *imageMount) context() (builder.Context, error) {
-	if im.ctx == nil {
+func (im *imageMount) context() (builder.Source, error) {
+	if im.source == nil {
 		if im.id == "" {
 			return nil, errors.Errorf("could not copy from empty context")
 		}
 		p, release, err := im.ic.b.docker.MountImage(im.id)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to mount %s", im.id)
 		}
-		ctx, err := remotecontext.NewLazyContext(p)
+		source, err := remotecontext.NewLazyContext(p)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to create lazycontext for %s", p)
 		}
 		im.release = release
-		im.ctx = ctx
+		im.source = source
 	}
-	return im.ctx, nil
+	return im.source, nil
 }
 
 func (im *imageMount) unmount() error {