Merge pull request #44191 from corhere/drop-containerfs-iface

Remove LCOW: pkg/containerfs: drop ContainerFS abstraction

builder/builder-next/adapters/snapshot/snapshot.go

@@ -291,7 +291,7 @@ func (s *snapshotter) Mounts(ctx context.Context, key string) (snapshot.Mountabl
 					return nil, nil, err
 				}
 				return []mount.Mount{{
-						Source:  rootfs.Path(),
+						Source:  rootfs,
 						Type:    "bind",
 						Options: []string{"rbind"},
 					}}, func() error {
@@ -312,7 +312,7 @@ func (s *snapshotter) Mounts(ctx context.Context, key string) (snapshot.Mountabl
 				return nil, nil, err
 			}
 			return []mount.Mount{{
-					Source:  rootfs.Path(),
+					Source:  rootfs,
 					Type:    "bind",
 					Options: []string{"rbind"},
 				}}, func() error {

builder/builder.go

@@ -14,7 +14,6 @@ import (
 	containerpkg "github.com/docker/docker/container"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
-	"github.com/docker/docker/pkg/containerfs"
 )
 
 const (
@@ -26,7 +25,7 @@ const (
 // instructions in the builder.
 type Source interface {
 	// Root returns root path for accessing source
-	Root() containerfs.ContainerFS
+	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().
@@ -110,6 +109,6 @@ type ROLayer interface {
 // RWLayer is active layer that can be read/modified
 type RWLayer interface {
 	Release() error
-	Root() containerfs.ContainerFS
+	Root() string
 	Commit() (ROLayer, error)
 }

builder/dockerfile/copy.go

@@ -1,7 +1,6 @@
 package dockerfile // import "github.com/docker/docker/builder/dockerfile"
 
 import (
-	"archive/tar"
 	"fmt"
 	"io"
 	"mime"
@@ -39,14 +38,14 @@ type pathCache interface {
 // copyInfo is a data object which stores the metadata about each source file in
 // a copyInstruction
 type copyInfo struct {
-	root         containerfs.ContainerFS
+	root         string
 	path         string
 	hash         string
 	noDecompress bool
 }
 
 func (c copyInfo) fullPath() (string, error) {
-	return c.root.ResolveScopedPath(c.path, true)
+	return containerfs.ResolveScopedPath(c.root, c.path)
 }
 
 func newCopyInfoFromSource(source builder.Source, path string, hash string) copyInfo {
@@ -160,7 +159,7 @@ func (o *copier) getCopyInfoForSourcePath(orig, dest string) ([]copyInfo, error)
 		}
 		path = unnamedFilename
 	}
-	o.tmpPaths = append(o.tmpPaths, remote.Root().Path())
+	o.tmpPaths = append(o.tmpPaths, remote.Root())
 
 	hash, err := remote.Hash(path)
 	ci := newCopyInfoFromSource(remote, path, hash)
@@ -203,7 +202,7 @@ func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo,
 
 		o.source, err = remotecontext.NewLazySource(rwLayer.Root())
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to create context for copy from %s", rwLayer.Root().Path())
+			return nil, errors.Wrapf(err, "failed to create context for copy from %s", rwLayer.Root())
 		}
 	}
 
@@ -260,7 +259,7 @@ func (o *copier) storeInPathCache(im *imageMount, path string, hash string) {
 func (o *copier) copyWithWildcards(origPath string) ([]copyInfo, error) {
 	root := o.source.Root()
 	var copyInfos []copyInfo
-	if err := root.Walk(root.Path(), func(path string, info os.FileInfo, err error) error {
+	if err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
@@ -272,7 +271,7 @@ func (o *copier) copyWithWildcards(origPath string) ([]copyInfo, error) {
 		if rel == "." {
 			return nil
 		}
-		if match, _ := root.Match(origPath, rel); !match {
+		if match, _ := filepath.Match(origPath, rel); !match {
 			return nil
 		}
 
@@ -318,7 +317,7 @@ func walkSource(source builder.Source, origPath string) ([]string, error) {
 	}
 	// Must be a dir
 	var subfiles []string
-	err = source.Root().Walk(fp, func(path string, info os.FileInfo, err error) error {
+	err = filepath.Walk(fp, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
@@ -443,19 +442,14 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b
 		return
 	}
 
-	lc, err := remotecontext.NewLazySource(containerfs.NewLocalContainerFS(tmpDir))
+	lc, err := remotecontext.NewLazySource(tmpDir)
 	return lc, filename, err
 }
 
 type copyFileOptions struct {
 	decompress bool
 	identity   *idtools.Identity
-	archiver   Archiver
-}
-
-type copyEndpoint struct {
-	driver containerfs.Driver
-	path   string
+	archiver   *archive.Archiver
 }
 
 func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions) error {
@@ -471,96 +465,77 @@ func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions)
 
 	archiver := options.archiver
 
-	srcEndpoint := &copyEndpoint{driver: source.root, path: srcPath}
-	destEndpoint := &copyEndpoint{driver: dest.root, path: destPath}
-
-	src, err := source.root.Stat(srcPath)
+	src, err := os.Stat(srcPath)
 	if err != nil {
 		return errors.Wrapf(err, "source path not found")
 	}
 	if src.IsDir() {
-		return copyDirectory(archiver, srcEndpoint, destEndpoint, options.identity)
+		return copyDirectory(archiver, srcPath, destPath, options.identity)
 	}
-	if options.decompress && isArchivePath(source.root, srcPath) && !source.noDecompress {
+	if options.decompress && archive.IsArchivePath(srcPath) && !source.noDecompress {
 		return archiver.UntarPath(srcPath, destPath)
 	}
 
-	destExistsAsDir, err := isExistingDirectory(destEndpoint)
+	destExistsAsDir, err := isExistingDirectory(destPath)
 	if err != nil {
 		return err
 	}
 	// dest.path must be used because destPath has already been cleaned of any
 	// trailing slash
-	if endsInSlash(dest.root, dest.path) || destExistsAsDir {
+	if endsInSlash(dest.path) || destExistsAsDir {
 		// source.path must be used to get the correct filename when the source
 		// is a symlink
-		destPath = dest.root.Join(destPath, source.root.Base(source.path))
-		destEndpoint = &copyEndpoint{driver: dest.root, path: destPath}
-	}
-	return copyFile(archiver, srcEndpoint, destEndpoint, options.identity)
-}
-
-func isArchivePath(driver containerfs.ContainerFS, path string) bool {
-	file, err := driver.Open(path)
-	if err != nil {
-		return false
-	}
-	defer file.Close()
-	rdr, err := archive.DecompressStream(file)
-	if err != nil {
-		return false
+		destPath = filepath.Join(destPath, filepath.Base(source.path))
 	}
-	r := tar.NewReader(rdr)
-	_, err = r.Next()
-	return err == nil
+	return copyFile(archiver, srcPath, destPath, options.identity)
 }
 
-func copyDirectory(archiver Archiver, source, dest *copyEndpoint, identity *idtools.Identity) error {
+func copyDirectory(archiver *archive.Archiver, source, dest string, identity *idtools.Identity) error {
 	destExists, err := isExistingDirectory(dest)
 	if err != nil {
 		return errors.Wrapf(err, "failed to query destination path")
 	}
 
-	if err := archiver.CopyWithTar(source.path, dest.path); err != nil {
+	if err := archiver.CopyWithTar(source, dest); err != nil {
 		return errors.Wrapf(err, "failed to copy directory")
 	}
 	if identity != nil {
-		return fixPermissions(source.path, dest.path, *identity, !destExists)
+		return fixPermissions(source, dest, *identity, !destExists)
 	}
 	return nil
 }
 
-func copyFile(archiver Archiver, source, dest *copyEndpoint, identity *idtools.Identity) error {
+func copyFile(archiver *archive.Archiver, source, dest string, identity *idtools.Identity) error {
 	if identity == nil {
 		// Use system.MkdirAll here, which is a custom version of os.MkdirAll
 		// modified for use on Windows to handle volume GUID paths. These paths
 		// are of the form \\?\Volume{<GUID>}\<path>. An example would be:
 		// \\?\Volume{dae8d3ac-b9a1-11e9-88eb-e8554b2ba1db}\bin\busybox.exe
-		if err := system.MkdirAll(filepath.Dir(dest.path), 0755); err != nil {
+		if err := system.MkdirAll(filepath.Dir(dest), 0755); err != nil {
 			return err
 		}
 	} else {
-		if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, *identity); err != nil {
+		if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest), 0755, *identity); err != nil {
 			return errors.Wrapf(err, "failed to create new directory")
 		}
 	}
 
-	if err := archiver.CopyFileWithTar(source.path, dest.path); err != nil {
+	if err := archiver.CopyFileWithTar(source, dest); err != nil {
 		return errors.Wrapf(err, "failed to copy file")
 	}
 	if identity != nil {
-		return fixPermissions(source.path, dest.path, *identity, false)
+		return fixPermissions(source, dest, *identity, false)
 	}
 	return nil
 }
 
-func endsInSlash(driver containerfs.Driver, path string) bool {
-	return strings.HasSuffix(path, string(driver.Separator()))
+func endsInSlash(path string) bool {
+	return strings.HasSuffix(path, string(filepath.Separator))
 }
 
 // isExistingDirectory returns true if the path exists and is a directory
-func isExistingDirectory(point *copyEndpoint) (bool, error) {
-	destStat, err := point.driver.Stat(point.path)
+func isExistingDirectory(path string) (bool, error) {
+	destStat, err := os.Stat(path)
 	switch {
 	case errors.Is(err, os.ErrNotExist):
 		return false, nil

builder/dockerfile/copy_test.go

@@ -4,7 +4,6 @@ import (
 	"net/http"
 	"testing"
 
-	"github.com/docker/docker/pkg/containerfs"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/fs"
@@ -39,7 +38,7 @@ func TestIsExistingDirectory(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
-		result, err := isExistingDirectory(&copyEndpoint{driver: containerfs.NewLocalDriver(), path: testcase.path})
+		result, err := isExistingDirectory(testcase.path)
 		if !assert.Check(t, err) {
 			continue
 		}

builder/dockerfile/copy_unix.go

@@ -9,7 +9,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/docker/docker/pkg/containerfs"
 	"github.com/docker/docker/pkg/idtools"
 )
 
@@ -19,8 +18,7 @@ func fixPermissions(source, destination string, identity idtools.Identity, overr
 		err           error
 	)
 	if !overrideSkip {
-		destEndpoint := &copyEndpoint{driver: containerfs.NewLocalDriver(), path: destination}
-		skipChownRoot, err = isExistingDirectory(destEndpoint)
+		skipChownRoot, err = isExistingDirectory(destination)
 		if err != nil {
 			return err
 		}

builder/dockerfile/internals.go

@@ -7,7 +7,6 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
-	"io"
 	"strings"
 
 	"github.com/docker/docker/api/types"
@@ -17,59 +16,15 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/chrootarchive"
-	"github.com/docker/docker/pkg/containerfs"
-	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/go-connections/nat"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
-// Archiver defines an interface for copying files from one destination to
-// another using Tar/Untar.
-type Archiver interface {
-	TarUntar(src, dst string) error
-	UntarPath(src, dst string) error
-	CopyWithTar(src, dst string) error
-	CopyFileWithTar(src, dst string) error
-	IdentityMapping() idtools.IdentityMapping
-}
-
-// The builder will use the following interfaces if the container fs implements
-// these for optimized copies to and from the container.
-type extractor interface {
-	ExtractArchive(src io.Reader, dst string, opts *archive.TarOptions) error
-}
-
-type archiver interface {
-	ArchivePath(src string, opts *archive.TarOptions) (io.ReadCloser, error)
-}
-
-// helper functions to get tar/untar func
-func untarFunc(i interface{}) containerfs.UntarFunc {
-	if ea, ok := i.(extractor); ok {
-		return ea.ExtractArchive
-	}
-	return chrootarchive.Untar
-}
-
-func tarFunc(i interface{}) containerfs.TarFunc {
-	if ap, ok := i.(archiver); ok {
-		return ap.ArchivePath
-	}
-	return archive.TarWithOptions
-}
-
-func (b *Builder) getArchiver(src, dst containerfs.Driver) Archiver {
-	t, u := tarFunc(src), untarFunc(dst)
-	return &containerfs.Archiver{
-		SrcDriver: src,
-		DstDriver: dst,
-		Tar:       t,
-		Untar:     u,
-		IDMapping: b.idMapping,
-	}
+func (b *Builder) getArchiver() *archive.Archiver {
+	return chrootarchive.NewArchiver(b.idMapping)
 }
 
 func (b *Builder) commit(dispatchState *dispatchState, comment string) error {
@@ -192,7 +147,7 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error {
 	// translated (if necessary because of user namespaces), and replace
 	// the root pair with the chown pair for copy operations
 	if inst.chownStr != "" {
-		identity, err = parseChownFlag(b, state, inst.chownStr, destInfo.root.Path(), b.idMapping)
+		identity, err = parseChownFlag(b, state, inst.chownStr, destInfo.root, b.idMapping)
 		if err != nil {
 			if b.options.Platform != "windows" {
 				return errors.Wrapf(err, "unable to convert uid/gid chown string to host mapping")
@@ -205,7 +160,7 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error {
 	for _, info := range inst.infos {
 		opts := copyFileOptions{
 			decompress: inst.allowLocalDecompression,
-			archiver:   b.getArchiver(info.root, destInfo.root),
+			archiver:   b.getArchiver(),
 		}
 		if !inst.preserveOwnership {
 			opts.identity = &identity

builder/dockerfile/internals_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/pkg/containerfs"
 	"github.com/docker/go-connections/nat"
 	"github.com/opencontainers/go-digest"
 	"gotest.tools/v3/assert"
@@ -183,8 +182,8 @@ func TestDeepCopyRunConfig(t *testing.T) {
 
 type MockRWLayer struct{}
 
-func (l *MockRWLayer) Release() error                { return nil }
-func (l *MockRWLayer) Root() containerfs.ContainerFS { return nil }
+func (l *MockRWLayer) Release() error { return nil }
+func (l *MockRWLayer) Root() string   { return "" }
 func (l *MockRWLayer) Commit() (builder.ROLayer, error) {
 	return &MockROLayer{
 		diffID: layer.DiffID(digest.Digest("sha256:1234")),