diff --git a/api/server/router/build/backend.go b/api/server/router/build/backend.go index 751999e637..5625f97157 100644 --- a/api/server/router/build/backend.go +++ b/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) } diff --git a/api/server/router/build/build_routes.go b/api/server/router/build/build_routes.go index ba86d80fbf..bbe312ef53 100644 --- a/api/server/router/build/build_routes.go +++ b/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,8 +185,6 @@ 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 { @@ -193,7 +192,7 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * 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) } diff --git a/builder/builder.go b/builder/builder.go index e6edc85c82..2e27b4a9b2 100644 --- a/builder/builder.go +++ b/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 diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 69d1e7df4b..8ae30b6393 100644 --- a/builder/dockerfile/builder.go +++ b/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 diff --git a/builder/dockerfile/evaluator_test.go b/builder/dockerfile/evaluator_test.go index 419fa0f9ff..b6a5ec59d3 100644 --- a/builder/dockerfile/evaluator_test.go +++ b/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), } diff --git a/builder/dockerfile/imagecontext.go b/builder/dockerfile/imagecontext.go index 9e35d28821..ab26782d0d 100644 --- a/builder/dockerfile/imagecontext.go +++ b/builder/dockerfile/imagecontext.go @@ -119,14 +119,14 @@ 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") } @@ -134,14 +134,14 @@ func (im *imageMount) context() (builder.Context, error) { 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 { diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 4ef7beb08c..e6dc40d173 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -8,7 +8,6 @@ import ( "encoding/hex" "fmt" "io" - "io/ioutil" "net/http" "net/url" "os" @@ -25,7 +24,7 @@ import ( "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/builder" "github.com/docker/docker/builder/dockerfile/parser" - "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/builder/remotecontext" "github.com/docker/docker/pkg/httputils" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/jsonmessage" @@ -33,7 +32,6 @@ import ( "github.com/docker/docker/pkg/streamformatter" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/system" - "github.com/docker/docker/pkg/tarsum" "github.com/docker/docker/pkg/urlutil" "github.com/pkg/errors" ) @@ -88,7 +86,9 @@ func (b *Builder) commit(id string, autoCmd strslice.StrSlice, comment string) e } type copyInfo struct { - builder.FileInfo + root string + path string + hash string decompress bool } @@ -109,19 +109,23 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD // the copy until we've looked at all src files var err error for _, orig := range args[0 : len(args)-1] { - var fi builder.FileInfo if urlutil.IsURL(orig) { if !allowRemote { return fmt.Errorf("Source can't be a URL for %s", cmdName) } - fi, err = b.download(orig) + remote, path, err := b.download(orig) + if err != nil { + return err + } + defer os.RemoveAll(remote.Root()) + h, err := remote.Hash(path) if err != nil { return err } - defer os.RemoveAll(filepath.Dir(fi.Path())) infos = append(infos, copyInfo{ - FileInfo: fi, - decompress: false, + root: remote.Root(), + path: path, + hash: h, }) continue } @@ -147,20 +151,15 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD var origPaths string if len(infos) == 1 { - fi := infos[0].FileInfo - origPaths = fi.Name() - if hfi, ok := fi.(builder.Hashed); ok { - srcHash = hfi.Hash() - } + info := infos[0] + origPaths = info.path + srcHash = info.hash } else { var hashs []string var origs []string for _, info := range infos { - fi := info.FileInfo - origs = append(origs, fi.Name()) - if hfi, ok := fi.(builder.Hashed); ok { - hashs = append(hashs, hfi.Hash()) - } + origs = append(origs, info.path) + hashs = append(hashs, info.hash) } hasher := sha256.New() hasher.Write([]byte(strings.Join(hashs, ","))) @@ -197,7 +196,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD } for _, info := range infos { - if err := b.docker.CopyOnBuild(container.ID, dest, info.FileInfo, info.decompress); err != nil { + if err := b.docker.CopyOnBuild(container.ID, dest, info.root, info.path, info.decompress); err != nil { return err } } @@ -205,7 +204,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD return b.commit(container.ID, cmd, comment) } -func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) { +func (b *Builder) download(srcURL string) (remote builder.Source, p string, err error) { // get filename from URL u, err := url.Parse(srcURL) if err != nil { @@ -248,17 +247,12 @@ func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) { progressOutput := stdoutFormatter.StreamFormatter.NewProgressOutput(stdoutFormatter.Writer, true) progressReader := progress.NewProgressReader(resp.Body, progressOutput, resp.ContentLength, "", "Downloading") // Download and dump result to tmp file + // TODO: add filehash directly if _, err = io.Copy(tmpFile, progressReader); err != nil { tmpFile.Close() return } fmt.Fprintln(b.Stdout) - // ignoring error because the file was already opened successfully - tmpFileSt, err := tmpFile.Stat() - if err != nil { - tmpFile.Close() - return - } // Set the mtime to the Last-Modified header value if present // Otherwise just remove atime and mtime @@ -279,21 +273,12 @@ func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) { return } - // Calc the checksum, even if we're using the cache - r, err := archive.Tar(tmpFileName, archive.Uncompressed) + lc, err := remotecontext.NewLazyContext(tmpDir) if err != nil { return } - tarSum, err := tarsum.NewTarSum(r, true, tarsum.Version1) - if err != nil { - return - } - if _, err = io.Copy(ioutil.Discard, tarSum); err != nil { - return - } - hash := tarSum.Sum(nil) - r.Close() - return &builder.HashedFileInfo{FileInfo: builder.PathFileInfo{FileInfo: tmpFileSt, FilePath: tmpFileName}, FileHash: hash}, nil + + return lc, filename, nil } var windowsBlacklist = map[string]bool{ @@ -328,37 +313,40 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression } origPath = strings.TrimPrefix(origPath, "."+string(os.PathSeparator)) - context := b.context + source := b.source var err error if imageSource != nil { - context, err = imageSource.context() + source, err = imageSource.context() if err != nil { return nil, err } } - if context == nil { + if source == nil { return nil, errors.Errorf("No context given. Impossible to use %s", cmdName) } // Deal with wildcards if allowWildcards && containsWildcards(origPath) { var copyInfos []copyInfo - if err := context.Walk("", func(path string, info builder.FileInfo, err error) error { + if err := filepath.Walk(source.Root(), func(path string, info os.FileInfo, err error) error { if err != nil { return err } - if info.Name() == "" { - // Why are we doing this check? + rel, err := remotecontext.Rel(source.Root(), path) + if err != nil { + return err + } + if rel == "." { return nil } - if match, _ := filepath.Match(origPath, path); !match { + if match, _ := filepath.Match(origPath, rel); !match { return nil } // Note we set allowWildcards to false in case the name has // a * in it - subInfos, err := b.calcCopyInfo(cmdName, path, allowLocalDecompression, false, imageSource) + subInfos, err := b.calcCopyInfo(cmdName, rel, allowLocalDecompression, false, imageSource) if err != nil { return err } @@ -371,38 +359,56 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression } // Must be a dir or a file - statPath, fi, err := context.Stat(origPath) + hash, err := source.Hash(origPath) if err != nil { return nil, err } - copyInfos := []copyInfo{{FileInfo: fi, decompress: allowLocalDecompression}} - - hfi, handleHash := fi.(builder.Hashed) - if !handleHash { - return copyInfos, nil + fi, err := remotecontext.StatAt(source, origPath) + if err != nil { + return nil, err } + + // TODO: remove, handle dirs in Hash() + copyInfos := []copyInfo{{root: source.Root(), path: origPath, hash: hash, decompress: allowLocalDecompression}} + if imageSource != nil { // fast-cache based on imageID if h, ok := b.imageContexts.getCache(imageSource.id, origPath); ok { - hfi.SetHash(h.(string)) + copyInfos[0].hash = h.(string) return copyInfos, nil } } // Deal with the single file case if !fi.IsDir() { - hfi.SetHash("file:" + hfi.Hash()) + copyInfos[0].hash = "file:" + copyInfos[0].hash return copyInfos, nil } + + fp, err := remotecontext.FullPath(source, origPath) + if err != nil { + return nil, err + } // Must be a dir var subfiles []string - err = context.Walk(statPath, func(path string, info builder.FileInfo, err error) error { + err = filepath.Walk(fp, func(path string, info os.FileInfo, err error) error { if err != nil { return err } + rel, err := remotecontext.Rel(source.Root(), path) + if err != nil { + return err + } + if rel == "." { + return nil + } + hash, err := source.Hash(rel) + if err != nil { + return nil + } // we already checked handleHash above - subfiles = append(subfiles, info.(builder.Hashed).Hash()) + subfiles = append(subfiles, hash) return nil }) if err != nil { @@ -412,9 +418,9 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression sort.Strings(subfiles) hasher := sha256.New() hasher.Write([]byte(strings.Join(subfiles, ","))) - hfi.SetHash("dir:" + hex.EncodeToString(hasher.Sum(nil))) + copyInfos[0].hash = "dir:" + hex.EncodeToString(hasher.Sum(nil)) if imageSource != nil { - b.imageContexts.setCache(imageSource.id, origPath, hfi.Hash()) + b.imageContexts.setCache(imageSource.id, origPath, copyInfos[0].hash) } return copyInfos, nil @@ -655,59 +661,3 @@ func (b *Builder) clearTmp() { fmt.Fprintf(b.Stdout, "Removing intermediate container %s\n", stringid.TruncateID(c)) } } - -// readAndParseDockerfile reads a Dockerfile from the current context. -func (b *Builder) readAndParseDockerfile() (*parser.Result, error) { - // If no -f was specified then look for 'Dockerfile'. If we can't find - // that then look for 'dockerfile'. If neither are found then default - // back to 'Dockerfile' and use that in the error message. - if b.options.Dockerfile == "" { - b.options.Dockerfile = builder.DefaultDockerfileName - if _, _, err := b.context.Stat(b.options.Dockerfile); os.IsNotExist(err) { - lowercase := strings.ToLower(b.options.Dockerfile) - if _, _, err := b.context.Stat(lowercase); err == nil { - b.options.Dockerfile = lowercase - } - } - } - - result, err := b.parseDockerfile() - if err != nil { - return nil, err - } - - // After the Dockerfile has been parsed, we need to check the .dockerignore - // file for either "Dockerfile" or ".dockerignore", and if either are - // present then erase them from the build context. These files should never - // have been sent from the client but we did send them to make sure that - // we had the Dockerfile to actually parse, and then we also need the - // .dockerignore file to know whether either file should be removed. - // Note that this assumes the Dockerfile has been read into memory and - // is now safe to be removed. - if dockerIgnore, ok := b.context.(builder.DockerIgnoreContext); ok { - dockerIgnore.Process([]string{b.options.Dockerfile}) - } - return result, nil -} - -func (b *Builder) parseDockerfile() (*parser.Result, error) { - f, err := b.context.Open(b.options.Dockerfile) - if err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("Cannot locate specified Dockerfile: %s", b.options.Dockerfile) - } - return nil, err - } - defer f.Close() - if f, ok := f.(*os.File); ok { - // ignoring error because Open already succeeded - fi, err := f.Stat() - if err != nil { - return nil, fmt.Errorf("Unexpected error reading Dockerfile: %v", err) - } - if fi.Size() == 0 { - return nil, fmt.Errorf("The Dockerfile (%s) cannot be empty", b.options.Dockerfile) - } - } - return parser.Parse(f) -} diff --git a/builder/dockerfile/internals_test.go b/builder/dockerfile/internals_test.go index b979401467..f5bee6da94 100644 --- a/builder/dockerfile/internals_test.go +++ b/builder/dockerfile/internals_test.go @@ -1,11 +1,12 @@ package dockerfile import ( + "context" "fmt" "testing" - "github.com/docker/docker/api/types" "github.com/docker/docker/builder" + "github.com/docker/docker/builder/remotecontext" "github.com/docker/docker/pkg/archive" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -17,7 +18,7 @@ func TestEmptyDockerfile(t *testing.T) { createTestTempFile(t, contextDir, builder.DefaultDockerfileName, "", 0777) - readAndCheckDockerfile(t, "emptyDockerfile", contextDir, "", "The Dockerfile (Dockerfile) cannot be empty") + readAndCheckDockerfile(t, "emptyDockerfile", contextDir, "", "the Dockerfile (Dockerfile) cannot be empty") } func TestSymlinkDockerfile(t *testing.T) { @@ -63,21 +64,10 @@ func readAndCheckDockerfile(t *testing.T, testName, contextDir, dockerfilePath, } }() - context, err := builder.MakeTarSumContext(tarStream) - require.NoError(t, err) - - defer func() { - if err = context.Close(); err != nil { - t.Fatalf("Error when closing tar context: %s", err) - } - }() - - options := &types.ImageBuildOptions{ - Dockerfile: dockerfilePath, + if dockerfilePath == "" { // handled in BuildWithContext + dockerfilePath = builder.DefaultDockerfileName } - b := &Builder{options: options, context: context} - - _, err = b.readAndParseDockerfile() + _, _, err = remotecontext.Detect(context.Background(), "", dockerfilePath, tarStream, nil) assert.EqualError(t, err, expectedError) } diff --git a/builder/dockerfile/mockbackend_test.go b/builder/dockerfile/mockbackend_test.go index d64125b082..d015372a16 100644 --- a/builder/dockerfile/mockbackend_test.go +++ b/builder/dockerfile/mockbackend_test.go @@ -69,7 +69,7 @@ func (m *MockBackend) ContainerCreateWorkdir(containerID string) error { return nil } -func (m *MockBackend) CopyOnBuild(containerID string, destPath string, src builder.FileInfo, decompress bool) error { +func (m *MockBackend) CopyOnBuild(containerID string, destPath string, srcRoot string, srcPath string, decompress bool) error { return nil } diff --git a/builder/dockerignore.go b/builder/dockerignore.go deleted file mode 100644 index 3da7913367..0000000000 --- a/builder/dockerignore.go +++ /dev/null @@ -1,48 +0,0 @@ -package builder - -import ( - "os" - - "github.com/docker/docker/builder/dockerignore" - "github.com/docker/docker/pkg/fileutils" -) - -// DockerIgnoreContext wraps a ModifiableContext to add a method -// for handling the .dockerignore file at the root of the context. -type DockerIgnoreContext struct { - ModifiableContext -} - -// Process reads the .dockerignore file at the root of the embedded context. -// If .dockerignore does not exist in the context, then nil is returned. -// -// It can take a list of files to be removed after .dockerignore is removed. -// This is used for server-side implementations of builders that need to send -// the .dockerignore file as well as the special files specified in filesToRemove, -// but expect them to be excluded from the context after they were processed. -// -// For example, server-side Dockerfile builders are expected to pass in the name -// of the Dockerfile to be removed after it was parsed. -// -// TODO: Don't require a ModifiableContext (use Context instead) and don't remove -// files, instead handle a list of files to be excluded from the context. -func (c DockerIgnoreContext) Process(filesToRemove []string) error { - f, err := c.Open(".dockerignore") - // Note that a missing .dockerignore file isn't treated as an error - if err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - excludes, _ := dockerignore.ReadAll(f) - f.Close() - filesToRemove = append([]string{".dockerignore"}, filesToRemove...) - for _, fileToRemove := range filesToRemove { - rm, _ := fileutils.Matches(fileToRemove, excludes) - if rm { - c.Remove(fileToRemove) - } - } - return nil -} diff --git a/builder/remotecontext/detect.go b/builder/remotecontext/detect.go new file mode 100644 index 0000000000..ef21de21d3 --- /dev/null +++ b/builder/remotecontext/detect.go @@ -0,0 +1,172 @@ +package remotecontext + +import ( + "bufio" + "context" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/Sirupsen/logrus" + "github.com/docker/docker/builder" + "github.com/docker/docker/builder/dockerfile/parser" + "github.com/docker/docker/builder/dockerignore" + "github.com/docker/docker/pkg/fileutils" + "github.com/docker/docker/pkg/httputils" + "github.com/docker/docker/pkg/symlink" + "github.com/docker/docker/pkg/urlutil" + "github.com/pkg/errors" +) + +// Detect returns a context and dockerfile from remote location or local +// archive. progressReader is only used if remoteURL is actually a URL +// (not empty, and not a Git endpoint). +func Detect(ctx context.Context, remoteURL string, dockerfilePath string, r io.ReadCloser, progressReader func(in io.ReadCloser) io.ReadCloser) (remote builder.Source, dockerfile *parser.Result, err error) { + switch { + case remoteURL == "": + remote, dockerfile, err = newArchiveRemote(r, dockerfilePath) + case urlutil.IsGitURL(remoteURL): + remote, dockerfile, err = newGitRemote(remoteURL, dockerfilePath) + case urlutil.IsURL(remoteURL): + remote, dockerfile, err = newURLRemote(remoteURL, dockerfilePath, progressReader) + default: + err = fmt.Errorf("remoteURL (%s) could not be recognized as URL", remoteURL) + } + return +} + +func newArchiveRemote(rc io.ReadCloser, dockerfilePath string) (builder.Source, *parser.Result, error) { + c, err := MakeTarSumContext(rc) + if err != nil { + return nil, nil, err + } + + return withDockerfileFromContext(c.(modifiableContext), dockerfilePath) +} + +func withDockerfileFromContext(c modifiableContext, dockerfilePath string) (builder.Source, *parser.Result, error) { + df, err := openAt(c, dockerfilePath) + if err != nil { + if os.IsNotExist(err) { + if dockerfilePath == builder.DefaultDockerfileName { + lowercase := strings.ToLower(dockerfilePath) + if _, err := StatAt(c, lowercase); err == nil { + return withDockerfileFromContext(c, lowercase) + } + } + return nil, nil, errors.Errorf("Cannot locate specified Dockerfile: %s", dockerfilePath) // backwards compatible error + } + c.Close() + return nil, nil, err + } + + res, err := readAndParseDockerfile(dockerfilePath, df) + if err != nil { + return nil, nil, err + } + + df.Close() + + if err := removeDockerfile(c, dockerfilePath); err != nil { + c.Close() + return nil, nil, err + } + + return c, res, nil +} + +func newGitRemote(gitURL string, dockerfilePath string) (builder.Source, *parser.Result, error) { + c, err := MakeGitContext(gitURL) // TODO: change this to NewLazyContext + if err != nil { + return nil, nil, err + } + return withDockerfileFromContext(c.(modifiableContext), dockerfilePath) +} + +func newURLRemote(url string, dockerfilePath string, progressReader func(in io.ReadCloser) io.ReadCloser) (builder.Source, *parser.Result, error) { + var dockerfile io.ReadCloser + dockerfileFoundErr := errors.New("found-dockerfile") + c, err := MakeRemoteContext(url, map[string]func(io.ReadCloser) (io.ReadCloser, error){ + httputils.MimeTypes.TextPlain: func(rc io.ReadCloser) (io.ReadCloser, error) { + dockerfile = rc + return nil, dockerfileFoundErr + }, + // fallback handler (tar context) + "": func(rc io.ReadCloser) (io.ReadCloser, error) { + return progressReader(rc), nil + }, + }) + if err != nil { + if err == dockerfileFoundErr { + res, err := parser.Parse(dockerfile) + if err != nil { + return nil, nil, err + } + return nil, res, nil + } + return nil, nil, err + } + + return withDockerfileFromContext(c.(modifiableContext), dockerfilePath) +} + +func removeDockerfile(c modifiableContext, filesToRemove ...string) error { + f, err := openAt(c, ".dockerignore") + // Note that a missing .dockerignore file isn't treated as an error + switch { + case os.IsNotExist(err): + return nil + case err != nil: + return err + } + excludes, err := dockerignore.ReadAll(f) + f.Close() + filesToRemove = append([]string{".dockerignore"}, filesToRemove...) + for _, fileToRemove := range filesToRemove { + if rm, _ := fileutils.Matches(fileToRemove, excludes); rm { + if err := c.Remove(fileToRemove); err != nil { + logrus.Errorf("failed to remove %s: %v", fileToRemove, err) + } + } + } + return nil +} + +func readAndParseDockerfile(name string, rc io.Reader) (*parser.Result, error) { + br := bufio.NewReader(rc) + if _, err := br.Peek(1); err != nil { + if err == io.EOF { + return nil, errors.Errorf("the Dockerfile (%s) cannot be empty", name) + } + return nil, errors.Wrap(err, "unexpected error reading Dockerfile") + } + return parser.Parse(br) +} + +func openAt(remote builder.Source, path string) (*os.File, error) { + fullPath, err := FullPath(remote, path) + if err != nil { + return nil, err + } + return os.Open(fullPath) +} + +// StatAt is a helper for calling Stat on a path from a source +func StatAt(remote builder.Source, path string) (os.FileInfo, error) { + fullPath, err := FullPath(remote, path) + if err != nil { + return nil, err + } + return os.Stat(fullPath) +} + +// FullPath is a helper for getting a full path for a path from a source +func FullPath(remote builder.Source, path string) (string, error) { + fullPath, err := symlink.FollowSymlinkInScope(filepath.Join(remote.Root(), path), remote.Root()) + if err != nil { + return "", fmt.Errorf("Forbidden path outside the build context: %s (%s)", path, fullPath) // backwards compat with old error + } + return fullPath, nil +} diff --git a/builder/dockerignore_test.go b/builder/remotecontext/detect_test.go similarity index 62% rename from builder/dockerignore_test.go rename to builder/remotecontext/detect_test.go index 3c0ceda4cf..6b47ac2274 100644 --- a/builder/dockerignore_test.go +++ b/builder/remotecontext/detect_test.go @@ -1,11 +1,21 @@ -package builder +package remotecontext import ( + "errors" "io/ioutil" "log" "os" + "path/filepath" "sort" "testing" + + "github.com/docker/docker/builder" +) + +const ( + dockerfileContents = "FROM busybox" + dockerignoreFilename = ".dockerignore" + testfileContents = "test" ) const shouldStayFilename = "should_stay" @@ -43,10 +53,9 @@ func checkDirectory(t *testing.T, dir string, expectedFiles []string) { } func executeProcess(t *testing.T, contextDir string) { - modifiableCtx := &tarSumContext{root: contextDir} - ctx := DockerIgnoreContext{ModifiableContext: modifiableCtx} + modifiableCtx := &stubRemote{root: contextDir} - err := ctx.Process([]string{DefaultDockerfileName}) + err := removeDockerfile(modifiableCtx, builder.DefaultDockerfileName) if err != nil { t.Fatalf("Error when executing Process: %s", err) @@ -59,7 +68,7 @@ func TestProcessShouldRemoveDockerfileDockerignore(t *testing.T) { createTestTempFile(t, contextDir, shouldStayFilename, testfileContents, 0777) createTestTempFile(t, contextDir, dockerignoreFilename, "Dockerfile\n.dockerignore", 0777) - createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777) + createTestTempFile(t, contextDir, builder.DefaultDockerfileName, dockerfileContents, 0777) executeProcess(t, contextDir) @@ -72,11 +81,11 @@ func TestProcessNoDockerignore(t *testing.T) { defer cleanup() createTestTempFile(t, contextDir, shouldStayFilename, testfileContents, 0777) - createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777) + createTestTempFile(t, contextDir, builder.DefaultDockerfileName, dockerfileContents, 0777) executeProcess(t, contextDir) - checkDirectory(t, contextDir, []string{shouldStayFilename, DefaultDockerfileName}) + checkDirectory(t, contextDir, []string{shouldStayFilename, builder.DefaultDockerfileName}) } @@ -85,11 +94,30 @@ func TestProcessShouldLeaveAllFiles(t *testing.T) { defer cleanup() createTestTempFile(t, contextDir, shouldStayFilename, testfileContents, 0777) - createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777) + createTestTempFile(t, contextDir, builder.DefaultDockerfileName, dockerfileContents, 0777) createTestTempFile(t, contextDir, dockerignoreFilename, "input1\ninput2", 0777) executeProcess(t, contextDir) - checkDirectory(t, contextDir, []string{shouldStayFilename, DefaultDockerfileName, dockerignoreFilename}) + checkDirectory(t, contextDir, []string{shouldStayFilename, builder.DefaultDockerfileName, dockerignoreFilename}) } + +// TODO: remove after moving to a separate pkg +type stubRemote struct { + root string +} + +func (r *stubRemote) Hash(path string) (string, error) { + return "", errors.New("not implemented") +} + +func (r *stubRemote) Root() string { + return r.root +} +func (r *stubRemote) Close() error { + return errors.New("not implemented") +} +func (r *stubRemote) Remove(p string) error { + return os.Remove(filepath.Join(r.root, p)) +} diff --git a/builder/git.go b/builder/remotecontext/git.go similarity index 79% rename from builder/git.go rename to builder/remotecontext/git.go index 74df244611..4204c9f7bb 100644 --- a/builder/git.go +++ b/builder/remotecontext/git.go @@ -1,14 +1,15 @@ -package builder +package remotecontext import ( "os" + "github.com/docker/docker/builder" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/gitutils" ) // MakeGitContext returns a Context from gitURL that is cloned in a temporary directory. -func MakeGitContext(gitURL string) (ModifiableContext, error) { +func MakeGitContext(gitURL string) (builder.Source, error) { root, err := gitutils.Clone(gitURL) if err != nil { return nil, err diff --git a/builder/remotecontext/lazycontext.go b/builder/remotecontext/lazycontext.go index 1f89c8d884..283d82a7b1 100644 --- a/builder/remotecontext/lazycontext.go +++ b/builder/remotecontext/lazycontext.go @@ -2,7 +2,6 @@ package remotecontext import ( "encoding/hex" - "io" "os" "path/filepath" "runtime" @@ -10,14 +9,13 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/pkg/pools" - "github.com/docker/docker/pkg/symlink" "github.com/pkg/errors" ) // NewLazyContext creates a new LazyContext. LazyContext defines a hashed build // context based on a root directory. Individual files are hashed first time // they are asked. It is not safe to call methods of LazyContext concurrently. -func NewLazyContext(root string) (builder.Context, error) { +func NewLazyContext(root string) (builder.Source, error) { return &lazyContext{ root: root, sums: make(map[string]string), @@ -29,86 +27,39 @@ type lazyContext struct { sums map[string]string } +func (c *lazyContext) Root() string { + return c.root +} + func (c *lazyContext) Close() error { return nil } -func (c *lazyContext) Open(path string) (io.ReadCloser, error) { - cleanPath, fullPath, err := c.normalize(path) +func (c *lazyContext) Hash(path string) (string, error) { + cleanPath, fullPath, err := normalize(path, c.root) if err != nil { - return nil, err + return "", err } - r, err := os.Open(fullPath) + fi, err := os.Lstat(fullPath) if err != nil { - return nil, errors.WithStack(convertPathError(err, cleanPath)) - } - return r, nil -} - -func (c *lazyContext) Stat(path string) (string, builder.FileInfo, error) { - // TODO: although stat returns builder.FileInfo it builder.Context actually requires Hashed - cleanPath, fullPath, err := c.normalize(path) - if err != nil { - return "", nil, err + return "", err } - st, err := os.Lstat(fullPath) + relPath, err := Rel(c.root, fullPath) if err != nil { - return "", nil, errors.WithStack(convertPathError(err, cleanPath)) - } - - relPath, err := rel(c.root, fullPath) - if err != nil { - return "", nil, errors.WithStack(convertPathError(err, cleanPath)) + return "", errors.WithStack(convertPathError(err, cleanPath)) } sum, ok := c.sums[relPath] if !ok { - sum, err = c.prepareHash(relPath, st) + sum, err = c.prepareHash(relPath, fi) if err != nil { - return "", nil, err + return "", err } } - fi := &builder.HashedFileInfo{ - builder.PathFileInfo{st, fullPath, filepath.Base(cleanPath)}, - sum, - } - return relPath, fi, nil -} - -func (c *lazyContext) Walk(root string, walkFn builder.WalkFunc) error { - _, fullPath, err := c.normalize(root) - if err != nil { - return err - } - return filepath.Walk(fullPath, func(fullPath string, fi os.FileInfo, err error) error { - relPath, err := rel(c.root, fullPath) - if err != nil { - return errors.WithStack(err) - } - if relPath == "." { - return nil - } - - sum, ok := c.sums[relPath] - if !ok { - sum, err = c.prepareHash(relPath, fi) - if err != nil { - return err - } - } - - hfi := &builder.HashedFileInfo{ - builder.PathFileInfo{FileInfo: fi, FilePath: fullPath}, - sum, - } - if err := walkFn(relPath, hfi, nil); err != nil { - return err - } - return nil - }) + return sum, nil } func (c *lazyContext) prepareHash(relPath string, fi os.FileInfo) (string, error) { @@ -132,25 +83,9 @@ func (c *lazyContext) prepareHash(relPath string, fi os.FileInfo) (string, error return sum, nil } -func (c *lazyContext) normalize(path string) (cleanPath, fullPath string, err error) { - // todo: combine these helpers with tarsum after they are moved to same package - cleanPath = filepath.Clean(string(os.PathSeparator) + path)[1:] - fullPath, err = symlink.FollowSymlinkInScope(filepath.Join(c.root, path), c.root) - if err != nil { - return "", "", errors.Wrapf(err, "forbidden path outside the build context: %s (%s)", path, fullPath) - } - return -} - -func convertPathError(err error, cleanpath string) error { - if err, ok := err.(*os.PathError); ok { - err.Path = cleanpath - return err - } - return err -} - -func rel(basepath, targpath string) (string, error) { +// Rel makes a path relative to base path. Same as `filepath.Rel` but can also +// handle UUID paths in windows. +func Rel(basepath, targpath string) (string, error) { // filepath.Rel can't handle UUID paths in windows if runtime.GOOS == "windows" { pfx := basepath + `\` diff --git a/builder/remote.go b/builder/remotecontext/remote.go similarity index 69% rename from builder/remote.go rename to builder/remotecontext/remote.go index b790301619..6be464fff1 100644 --- a/builder/remote.go +++ b/builder/remotecontext/remote.go @@ -1,4 +1,4 @@ -package builder +package remotecontext import ( "bytes" @@ -8,9 +8,8 @@ import ( "io/ioutil" "regexp" - "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/builder" "github.com/docker/docker/pkg/httputils" - "github.com/docker/docker/pkg/urlutil" ) // When downloading remote contexts, limit the amount (in bytes) @@ -30,7 +29,7 @@ var mimeRe = regexp.MustCompile(acceptableRemoteMIME) // If a match is found, then the body is sent to the contentType handler and a (potentially compressed) tar stream is expected // to be returned. If no match is found, it is assumed the body is a tar stream (compressed or not). // In either case, an (assumed) tar stream is passed to MakeTarSumContext whose result is returned. -func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io.ReadCloser) (io.ReadCloser, error)) (ModifiableContext, error) { +func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io.ReadCloser) (io.ReadCloser, error)) (builder.Source, error) { f, err := httputils.Download(remoteURL) if err != nil { return nil, fmt.Errorf("error downloading remote context %s: %v", remoteURL, err) @@ -67,46 +66,6 @@ func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io. return MakeTarSumContext(contextReader) } -// DetectContextFromRemoteURL returns a context and in certain cases the name of the dockerfile to be used -// irrespective of user input. -// progressReader is only used if remoteURL is actually a URL (not empty, and not a Git endpoint). -func DetectContextFromRemoteURL(r io.ReadCloser, remoteURL string, createProgressReader func(in io.ReadCloser) io.ReadCloser) (context ModifiableContext, dockerfileName string, err error) { - switch { - case remoteURL == "": - context, err = MakeTarSumContext(r) - case urlutil.IsGitURL(remoteURL): - context, err = MakeGitContext(remoteURL) - case urlutil.IsURL(remoteURL): - context, err = MakeRemoteContext(remoteURL, map[string]func(io.ReadCloser) (io.ReadCloser, error){ - httputils.MimeTypes.TextPlain: func(rc io.ReadCloser) (io.ReadCloser, error) { - dockerfile, err := ioutil.ReadAll(rc) - if err != nil { - return nil, err - } - - // dockerfileName is set to signal that the remote was interpreted as a single Dockerfile, in which case the caller - // should use dockerfileName as the new name for the Dockerfile, irrespective of any other user input. - dockerfileName = DefaultDockerfileName - - // TODO: return a context without tarsum - r, err := archive.Generate(dockerfileName, string(dockerfile)) - if err != nil { - return nil, err - } - - return ioutil.NopCloser(r), nil - }, - // fallback handler (tar context) - "": func(rc io.ReadCloser) (io.ReadCloser, error) { - return createProgressReader(rc), nil - }, - }) - default: - err = fmt.Errorf("remoteURL (%s) could not be recognized as URL", remoteURL) - } - return -} - // inspectResponse looks into the http response data at r to determine whether its // content-type is on the list of acceptable content types for remote build contexts. // This function returns: diff --git a/builder/remote_test.go b/builder/remotecontext/remote_test.go similarity index 91% rename from builder/remote_test.go rename to builder/remotecontext/remote_test.go index b44b59fbe5..af9db4b927 100644 --- a/builder/remote_test.go +++ b/builder/remotecontext/remote_test.go @@ -1,4 +1,4 @@ -package builder +package remotecontext import ( "bytes" @@ -9,6 +9,7 @@ import ( "net/url" "testing" + "github.com/docker/docker/builder" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/httputils" ) @@ -175,13 +176,13 @@ func TestMakeRemoteContext(t *testing.T) { contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") defer cleanup() - createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777) + createTestTempFile(t, contextDir, builder.DefaultDockerfileName, dockerfileContents, 0777) mux := http.NewServeMux() server := httptest.NewServer(mux) serverURL, _ := url.Parse(server.URL) - serverURL.Path = "/" + DefaultDockerfileName + serverURL.Path = "/" + builder.DefaultDockerfileName remoteURL := serverURL.String() mux.Handle("/", http.FileServer(http.Dir(contextDir))) @@ -193,7 +194,7 @@ func TestMakeRemoteContext(t *testing.T) { return nil, err } - r, err := archive.Generate(DefaultDockerfileName, string(dockerfile)) + r, err := archive.Generate(builder.DefaultDockerfileName, string(dockerfile)) if err != nil { return nil, err } @@ -221,13 +222,13 @@ func TestMakeRemoteContext(t *testing.T) { t.Fatalf("Size of file info sums should be 1, got: %d", fileInfoSums.Len()) } - fileInfo := fileInfoSums.GetFile(DefaultDockerfileName) + fileInfo := fileInfoSums.GetFile(builder.DefaultDockerfileName) if fileInfo == nil { - t.Fatalf("There should be file named %s in fileInfoSums", DefaultDockerfileName) + t.Fatalf("There should be file named %s in fileInfoSums", builder.DefaultDockerfileName) } if fileInfo.Pos() != 0 { - t.Fatalf("File %s should have position 0, got %d", DefaultDockerfileName, fileInfo.Pos()) + t.Fatalf("File %s should have position 0, got %d", builder.DefaultDockerfileName, fileInfo.Pos()) } } diff --git a/builder/tarsum.go b/builder/remotecontext/tarsum.go similarity index 54% rename from builder/tarsum.go rename to builder/remotecontext/tarsum.go index 77d3142e7b..dabf590e59 100644 --- a/builder/tarsum.go +++ b/builder/remotecontext/tarsum.go @@ -1,16 +1,17 @@ -package builder +package remotecontext import ( - "fmt" "io" "os" "path/filepath" + "github.com/docker/docker/builder" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/tarsum" + "github.com/pkg/errors" ) type tarSumContext struct { @@ -30,44 +31,11 @@ func convertPathError(err error, cleanpath string) error { return err } -func (c *tarSumContext) Open(path string) (io.ReadCloser, error) { - cleanpath, fullpath, err := c.normalize(path) - if err != nil { - return nil, err - } - r, err := os.Open(fullpath) - if err != nil { - return nil, convertPathError(err, cleanpath) - } - return r, nil -} - -func (c *tarSumContext) Stat(path string) (string, FileInfo, error) { - cleanpath, fullpath, err := c.normalize(path) - if err != nil { - return "", nil, err - } - - st, err := os.Lstat(fullpath) - if err != nil { - return "", nil, convertPathError(err, cleanpath) - } - - rel, err := filepath.Rel(c.root, fullpath) - if err != nil { - return "", nil, convertPathError(err, cleanpath) - } - - // We set sum to path by default for the case where GetFile returns nil. - // The usual case is if relative path is empty. - sum := path - // Use the checksum of the followed path(not the possible symlink) because - // this is the file that is actually copied. - if tsInfo := c.sums.GetFile(filepath.ToSlash(rel)); tsInfo != nil { - sum = tsInfo.Sum() - } - fi := &HashedFileInfo{PathFileInfo{st, fullpath, filepath.Base(cleanpath)}, sum} - return rel, fi, nil +type modifiableContext interface { + builder.Source + // Remove deletes the entry specified by `path`. + // It is usual for directory entries to delete all its subentries. + Remove(path string) error } // MakeTarSumContext returns a build Context from a tar stream. @@ -78,7 +46,7 @@ func (c *tarSumContext) Stat(path string) (string, FileInfo, error) { // all those sums then becomes the source of truth for all operations on this Context. // // Closing tarStream has to be done by the caller. -func MakeTarSumContext(tarStream io.Reader) (ModifiableContext, error) { +func MakeTarSumContext(tarStream io.Reader) (builder.Source, error) { root, err := ioutils.TempDir("", "docker-builder") if err != nil { return nil, err @@ -114,46 +82,47 @@ func MakeTarSumContext(tarStream io.Reader) (ModifiableContext, error) { return tsc, nil } -func (c *tarSumContext) normalize(path string) (cleanpath, fullpath string, err error) { - cleanpath = filepath.Clean(string(os.PathSeparator) + path)[1:] - fullpath, err = symlink.FollowSymlinkInScope(filepath.Join(c.root, path), c.root) - if err != nil { - return "", "", fmt.Errorf("Forbidden path outside the build context: %s (%s)", path, fullpath) - } - _, err = os.Lstat(fullpath) - if err != nil { - return "", "", convertPathError(err, path) - } - return -} - -func (c *tarSumContext) Walk(root string, walkFn WalkFunc) error { - root = filepath.Join(c.root, filepath.Join(string(filepath.Separator), root)) - return filepath.Walk(root, func(fullpath string, info os.FileInfo, err error) error { - rel, err := filepath.Rel(c.root, fullpath) - if err != nil { - return err - } - if rel == "." { - return nil - } - - sum := rel - if tsInfo := c.sums.GetFile(filepath.ToSlash(rel)); tsInfo != nil { - sum = tsInfo.Sum() - } - fi := &HashedFileInfo{PathFileInfo{FileInfo: info, FilePath: fullpath}, sum} - if err := walkFn(rel, fi, nil); err != nil { - return err - } - return nil - }) +func (c *tarSumContext) Root() string { + return c.root } func (c *tarSumContext) Remove(path string) error { - _, fullpath, err := c.normalize(path) + _, fullpath, err := normalize(path, c.root) if err != nil { return err } return os.RemoveAll(fullpath) } + +func (c *tarSumContext) Hash(path string) (string, error) { + cleanpath, fullpath, err := normalize(path, c.root) + if err != nil { + return "", err + } + + rel, err := filepath.Rel(c.root, fullpath) + if err != nil { + return "", convertPathError(err, cleanpath) + } + + // Use the checksum of the followed path(not the possible symlink) because + // this is the file that is actually copied. + if tsInfo := c.sums.GetFile(filepath.ToSlash(rel)); tsInfo != nil { + return tsInfo.Sum(), nil + } + // We set sum to path by default for the case where GetFile returns nil. + // The usual case is if relative path is empty. + return path, nil // backwards compat TODO: see if really needed +} + +func normalize(path, root string) (cleanPath, fullPath string, err error) { + cleanPath = filepath.Clean(string(os.PathSeparator) + path)[1:] + fullPath, err = symlink.FollowSymlinkInScope(filepath.Join(root, path), root) + if err != nil { + return "", "", errors.Wrapf(err, "forbidden path outside the build context: %s (%s)", path, cleanPath) + } + if _, err := os.Lstat(fullPath); err != nil { + return "", "", convertPathError(err, path) + } + return +} diff --git a/builder/remotecontext/tarsum_test.go b/builder/remotecontext/tarsum_test.go new file mode 100644 index 0000000000..abb64eeeb9 --- /dev/null +++ b/builder/remotecontext/tarsum_test.go @@ -0,0 +1,166 @@ +package remotecontext + +import ( + "io/ioutil" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/docker/docker/builder" + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/reexec" +) + +const ( + filename = "test" + contents = "contents test" +) + +func init() { + reexec.Init() +} + +func TestCloseRootDirectory(t *testing.T) { + contextDir, err := ioutil.TempDir("", "builder-tarsum-test") + + if err != nil { + t.Fatalf("Error with creating temporary directory: %s", err) + } + + tarsum := &tarSumContext{root: contextDir} + + err = tarsum.Close() + + if err != nil { + t.Fatalf("Error while executing Close: %s", err) + } + + _, err = os.Stat(contextDir) + + if !os.IsNotExist(err) { + t.Fatal("Directory should not exist at this point") + defer os.RemoveAll(contextDir) + } +} + +func TestHashFile(t *testing.T) { + contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") + defer cleanup() + + createTestTempFile(t, contextDir, filename, contents, 0777) + + tarSum := makeTestTarsumContext(t, contextDir) + + sum, err := tarSum.Hash(filename) + + if err != nil { + t.Fatalf("Error when executing Stat: %s", err) + } + + if len(sum) == 0 { + t.Fatalf("Hash returned empty sum") + } + + expected := "1149ab94af7be6cc1da1335e398f24ee1cf4926b720044d229969dfc248ae7ec" + if runtime.GOOS == "windows" { + expected = "55dfeb344351ab27f59aa60ebb0ed12025a2f2f4677bf77d26ea7a671274a9ca" + } + + if actual := sum; expected != actual { + t.Fatalf("invalid checksum. expected %s, got %s", expected, actual) + } +} + +func TestHashSubdir(t *testing.T) { + contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") + defer cleanup() + + contextSubdir := filepath.Join(contextDir, "builder-tarsum-test-subdir") + err := os.Mkdir(contextSubdir, 0700) + if err != nil { + t.Fatalf("Failed to make directory: %s", contextSubdir) + } + + testFilename := createTestTempFile(t, contextSubdir, filename, contents, 0777) + + tarSum := makeTestTarsumContext(t, contextDir) + + relativePath, err := filepath.Rel(contextDir, testFilename) + + if err != nil { + t.Fatalf("Error when getting relative path: %s", err) + } + + sum, err := tarSum.Hash(relativePath) + + if err != nil { + t.Fatalf("Error when executing Stat: %s", err) + } + + if len(sum) == 0 { + t.Fatalf("Hash returned empty sum") + } + + expected := "d7f8d6353dee4816f9134f4156bf6a9d470fdadfb5d89213721f7e86744a4e69" + if runtime.GOOS == "windows" { + expected = "74a3326b8e766ce63a8e5232f22e9dd895be647fb3ca7d337e5e0a9b3da8ef28" + } + + if actual := sum; expected != actual { + t.Fatalf("invalid checksum. expected %s, got %s", expected, actual) + } +} + +func TestStatNotExisting(t *testing.T) { + contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") + defer cleanup() + + tarSum := &tarSumContext{root: contextDir} + + _, err := tarSum.Hash("not-existing") + + if !os.IsNotExist(err) { + t.Fatalf("This file should not exist: %s", err) + } +} + +func TestRemoveDirectory(t *testing.T) { + contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") + defer cleanup() + + contextSubdir := createTestTempSubdir(t, contextDir, "builder-tarsum-test-subdir") + + relativePath, err := filepath.Rel(contextDir, contextSubdir) + + if err != nil { + t.Fatalf("Error when getting relative path: %s", err) + } + + tarSum := &tarSumContext{root: contextDir} + + err = tarSum.Remove(relativePath) + + if err != nil { + t.Fatalf("Error when executing Remove: %s", err) + } + + _, err = os.Stat(contextSubdir) + + if !os.IsNotExist(err) { + t.Fatal("Directory should not exist at this point") + } +} + +func makeTestTarsumContext(t *testing.T, dir string) builder.Source { + tarStream, err := archive.Tar(dir, archive.Uncompressed) + if err != nil { + t.Fatalf("error: %s", err) + } + defer tarStream.Close() + tarSum, err := MakeTarSumContext(tarStream) + if err != nil { + t.Fatalf("Error when executing MakeTarSumContext: %s", err) + } + return tarSum +} diff --git a/builder/utils_test.go b/builder/remotecontext/utils_test.go similarity index 92% rename from builder/utils_test.go rename to builder/remotecontext/utils_test.go index adc264539a..1e23ab4f73 100644 --- a/builder/utils_test.go +++ b/builder/remotecontext/utils_test.go @@ -1,4 +1,4 @@ -package builder +package remotecontext import ( "io/ioutil" @@ -7,12 +7,6 @@ import ( "testing" ) -const ( - dockerfileContents = "FROM busybox" - dockerignoreFilename = ".dockerignore" - testfileContents = "test" -) - // createTestTempDir creates a temporary directory for testing. // It returns the created path and a cleanup function which is meant to be used as deferred call. // When an error occurs, it terminates the test. diff --git a/builder/tarsum_test.go b/builder/tarsum_test.go deleted file mode 100644 index b3a0876b34..0000000000 --- a/builder/tarsum_test.go +++ /dev/null @@ -1,265 +0,0 @@ -package builder - -import ( - "bufio" - "bytes" - "io/ioutil" - "os" - "path/filepath" - "testing" - - "github.com/docker/docker/pkg/archive" - "github.com/docker/docker/pkg/reexec" -) - -const ( - filename = "test" - contents = "contents test" -) - -func init() { - reexec.Init() -} - -func TestCloseRootDirectory(t *testing.T) { - contextDir, err := ioutil.TempDir("", "builder-tarsum-test") - - if err != nil { - t.Fatalf("Error with creating temporary directory: %s", err) - } - - tarsum := &tarSumContext{root: contextDir} - - err = tarsum.Close() - - if err != nil { - t.Fatalf("Error while executing Close: %s", err) - } - - _, err = os.Stat(contextDir) - - if !os.IsNotExist(err) { - t.Fatal("Directory should not exist at this point") - defer os.RemoveAll(contextDir) - } -} - -func TestOpenFile(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - createTestTempFile(t, contextDir, filename, contents, 0777) - - tarSum := &tarSumContext{root: contextDir} - - file, err := tarSum.Open(filename) - - if err != nil { - t.Fatalf("Error when executing Open: %s", err) - } - - defer file.Close() - - scanner := bufio.NewScanner(file) - buff := bytes.NewBufferString("") - - for scanner.Scan() { - buff.WriteString(scanner.Text()) - } - - if contents != buff.String() { - t.Fatalf("Contents are not equal. Expected: %s, got: %s", contents, buff.String()) - } - -} - -func TestOpenNotExisting(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - tarSum := &tarSumContext{root: contextDir} - - file, err := tarSum.Open("not-existing") - - if file != nil { - t.Fatal("Opened file should be nil") - } - - if !os.IsNotExist(err) { - t.Fatalf("Error when executing Open: %s", err) - } -} - -func TestStatFile(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - testFilename := createTestTempFile(t, contextDir, filename, contents, 0777) - - tarSum := &tarSumContext{root: contextDir} - - relPath, fileInfo, err := tarSum.Stat(filename) - - if err != nil { - t.Fatalf("Error when executing Stat: %s", err) - } - - if relPath != filename { - t.Fatalf("Relative path should be equal to %s, got %s", filename, relPath) - } - - if fileInfo.Path() != testFilename { - t.Fatalf("Full path should be equal to %s, got %s", testFilename, fileInfo.Path()) - } -} - -func TestStatSubdir(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - contextSubdir := createTestTempSubdir(t, contextDir, "builder-tarsum-test-subdir") - - testFilename := createTestTempFile(t, contextSubdir, filename, contents, 0777) - - tarSum := &tarSumContext{root: contextDir} - - relativePath, err := filepath.Rel(contextDir, testFilename) - - if err != nil { - t.Fatalf("Error when getting relative path: %s", err) - } - - relPath, fileInfo, err := tarSum.Stat(relativePath) - - if err != nil { - t.Fatalf("Error when executing Stat: %s", err) - } - - if relPath != relativePath { - t.Fatalf("Relative path should be equal to %s, got %s", relativePath, relPath) - } - - if fileInfo.Path() != testFilename { - t.Fatalf("Full path should be equal to %s, got %s", testFilename, fileInfo.Path()) - } -} - -func TestStatNotExisting(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - tarSum := &tarSumContext{root: contextDir} - - relPath, fileInfo, err := tarSum.Stat("not-existing") - - if relPath != "" { - t.Fatal("Relative path should be nil") - } - - if fileInfo != nil { - t.Fatal("File info should be nil") - } - - if !os.IsNotExist(err) { - t.Fatalf("This file should not exist: %s", err) - } -} - -func TestRemoveDirectory(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - contextSubdir := createTestTempSubdir(t, contextDir, "builder-tarsum-test-subdir") - - relativePath, err := filepath.Rel(contextDir, contextSubdir) - - if err != nil { - t.Fatalf("Error when getting relative path: %s", err) - } - - tarSum := &tarSumContext{root: contextDir} - - err = tarSum.Remove(relativePath) - - if err != nil { - t.Fatalf("Error when executing Remove: %s", err) - } - - _, err = os.Stat(contextSubdir) - - if !os.IsNotExist(err) { - t.Fatal("Directory should not exist at this point") - } -} - -func TestMakeTarSumContext(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - createTestTempFile(t, contextDir, filename, contents, 0777) - - tarStream, err := archive.Tar(contextDir, archive.Uncompressed) - - if err != nil { - t.Fatalf("error: %s", err) - } - - defer tarStream.Close() - - tarSum, err := MakeTarSumContext(tarStream) - - if err != nil { - t.Fatalf("Error when executing MakeTarSumContext: %s", err) - } - - if tarSum == nil { - t.Fatal("Tar sum context should not be nil") - } -} - -func TestWalkWithoutError(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - contextSubdir := createTestTempSubdir(t, contextDir, "builder-tarsum-test-subdir") - - createTestTempFile(t, contextSubdir, filename, contents, 0777) - - tarSum := &tarSumContext{root: contextDir} - - walkFun := func(path string, fi FileInfo, err error) error { - return nil - } - - err := tarSum.Walk(contextSubdir, walkFun) - - if err != nil { - t.Fatalf("Error when executing Walk: %s", err) - } -} - -type WalkError struct { -} - -func (we WalkError) Error() string { - return "Error when executing Walk" -} - -func TestWalkWithError(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - contextSubdir := createTestTempSubdir(t, contextDir, "builder-tarsum-test-subdir") - - tarSum := &tarSumContext{root: contextDir} - - walkFun := func(path string, fi FileInfo, err error) error { - return WalkError{} - } - - err := tarSum.Walk(contextSubdir, walkFun) - - if err == nil { - t.Fatal("Error should not be nil") - } -} diff --git a/daemon/archive.go b/daemon/archive.go index f1018c7368..9b0a2f4a02 100644 --- a/daemon/archive.go +++ b/daemon/archive.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/docker/docker/api/types" - "github.com/docker/docker/builder" "github.com/docker/docker/container" "github.com/docker/docker/layer" "github.com/docker/docker/pkg/archive" @@ -15,6 +14,7 @@ import ( "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/stringid" + "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" "github.com/pkg/errors" ) @@ -369,8 +369,12 @@ func (daemon *Daemon) containerCopy(container *container.Container, resource str // specified by a container object. // TODO: make sure callers don't unnecessarily convert destPath with filepath.FromSlash (Copy does it already). // CopyOnBuild should take in abstract paths (with slashes) and the implementation should convert it to OS-specific paths. -func (daemon *Daemon) CopyOnBuild(cID string, destPath string, src builder.FileInfo, decompress bool) error { - srcPath := src.Path() +func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decompress bool) error { + fullSrcPath, err := symlink.FollowSymlinkInScope(filepath.Join(srcRoot, srcPath), srcRoot) + if err != nil { + return err + } + destExists := true destDir := false rootUID, rootGID := daemon.GetRemappedUIDGID() @@ -418,14 +422,19 @@ func (daemon *Daemon) CopyOnBuild(cID string, destPath string, src builder.FileI GIDMaps: gidMaps, } + src, err := os.Stat(fullSrcPath) + if err != nil { + return err + } + if src.IsDir() { // copy as directory - if err := archiver.CopyWithTar(srcPath, destPath); err != nil { + if err := archiver.CopyWithTar(fullSrcPath, destPath); err != nil { return err } - return fixPermissions(srcPath, destPath, rootUID, rootGID, destExists) + return fixPermissions(fullSrcPath, destPath, rootUID, rootGID, destExists) } - if decompress && archive.IsArchivePath(srcPath) { + if decompress && archive.IsArchivePath(fullSrcPath) { // Only try to untar if it is a file and that we've been told to decompress (when ADD-ing a remote file) // First try to unpack the source as an archive @@ -438,7 +447,7 @@ func (daemon *Daemon) CopyOnBuild(cID string, destPath string, src builder.FileI } // try to successfully untar the orig - err := archiver.UntarPath(srcPath, tarDest) + err := archiver.UntarPath(fullSrcPath, tarDest) /* if err != nil { logrus.Errorf("Couldn't untar to %s: %v", tarDest, err) @@ -449,17 +458,17 @@ func (daemon *Daemon) CopyOnBuild(cID string, destPath string, src builder.FileI // only needed for fixPermissions, but might as well put it before CopyFileWithTar if destDir || (destExists && destStat.IsDir()) { - destPath = filepath.Join(destPath, src.Name()) + destPath = filepath.Join(destPath, filepath.Base(srcPath)) } if err := idtools.MkdirAllNewAs(filepath.Dir(destPath), 0755, rootUID, rootGID); err != nil { return err } - if err := archiver.CopyFileWithTar(srcPath, destPath); err != nil { + if err := archiver.CopyFileWithTar(fullSrcPath, destPath); err != nil { return err } - return fixPermissions(srcPath, destPath, rootUID, rootGID, destExists) + return fixPermissions(fullSrcPath, destPath, rootUID, rootGID, destExists) } // MountImage returns mounted path with rootfs of an image. diff --git a/integration-cli/docker_api_build_test.go b/integration-cli/docker_api_build_test.go index f0c89dbd2a..feb20ee724 100644 --- a/integration-cli/docker_api_build_test.go +++ b/integration-cli/docker_api_build_test.go @@ -22,13 +22,11 @@ func (s *DockerSuite) TestBuildAPIDockerFileRemote(c *check.C) { var testD string if testEnv.DaemonPlatform() == "windows" { testD = `FROM busybox -COPY * /tmp/ RUN find / -name ba* RUN find /tmp/` } else { // -xdev is required because sysfs can cause EPERM testD = `FROM busybox -COPY * /tmp/ RUN find / -xdev -name ba* RUN find /tmp/` } @@ -45,7 +43,7 @@ RUN find /tmp/` // Make sure Dockerfile exists. // Make sure 'baz' doesn't exist ANYWHERE despite being mentioned in the URL out := string(buf) - c.Assert(out, checker.Contains, "/tmp/Dockerfile") + c.Assert(out, checker.Contains, "RUN find /tmp") c.Assert(out, checker.Not(checker.Contains), "baz") }